Re: [Qemu-devel] [PATCH v2 2/5] fw_cfg DMA interface documentation

[Peter Maydell]

On 31 August 2015 at 10:10, Marc Marí <address@hidden> wrote:
> Add fw_cfg DMA interface specification in the documentation.
>
> Based on Gerd Hoffman's initial implementation.
>
> Signed-off-by: Marc Marí <address@hidden>
> ---
>  docs/specs/fw_cfg.txt | 68 
> ++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 5bc7b96..06302f6 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -76,6 +76,13 @@ increasing address order, similar to memcpy().
>
>  Selector Register IOport: 0x510
>  Data Register IOport:     0x511
> +DMA Address IOport:       0x514
> +
> +=== ARM Register Locations ===
> +
> +Selector Register address: 0x09020000
> +Data Register address:     0x09020008
> +DMA Address address:       0x0902000c

These addresses shouldn't be documented -- the correct API is that the guest
needs to find the base address of the fw_cfg device via device tree
or ACPI table. You can document the layout of the registers within the
device, obviously (ie +0, +4, +8).

>  == Firmware Configuration Items ==
>
> @@ -86,11 +93,12 @@ by selecting the "signature" item using key 0x0000 
> (FW_CFG_SIGNATURE),
>  and reading four bytes from the data register. If the fw_cfg device is
>  present, the four bytes read will contain the characters "QEMU".
>
> -=== Revision (Key 0x0001, FW_CFG_ID) ===
> +=== Revision / feature bitmap (Key 0x0001, FW_CFG_ID) ===
>
> -A 32-bit little-endian unsigned int, this item is used as an interface
> -revision number, and is currently set to 1 by QEMU when fw_cfg is
> -initialized.
> +A 32-bit little-endian unsigned int, this item is used to check for enabled
> +features.
> + - Bit 0: traditional interface. Always set.
> + - Bit 1: DMA interface.
>
>  === File Directory (Key 0x0019, FW_CFG_FILE_DIR) ===
>
> @@ -132,6 +140,58 @@ Selector Reg.    Range Usage
>  In practice, the number of allowed firmware configuration items is given
>  by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h).
>
> += Guest-side DMA Interface =
> +
> +If bit 1 of the feature bitmap is set, the DMA interface is present. This 
> does
> +not replace the existing fw_cfg interface, it is an add-on. This interface
> +can be used through the 64-bit wide address register.
> +
> +The address register, as the selector register, is in little-endian format
> +when using IOports, and in big-endian format when using MMIO. The value for
> +the register is 0 at startup and after an operation. A write to the lower
> +half triggers an operation. This means, that operations with 32-bit addresses

Delete this comma.

> +can be triggered with just one write, whereas operations with 64-bit 
> addresses
> +can be triggered with one 64-bit write or two 32-bit writes, starting with 
> the
> +higher part.
> +
> +In this register, a physical RAM address to a FWCfgDmaAccess structure should

"the physical address of a FWCfgDmaAccess structure in RAM"

> +be written. This is the format of the FWCfgDmaAccess structure:
> +
> +typedef struct FWCfgDmaAccess {
> +    uint32_t control;
> +    uint32_t length;
> +    uint64_t address;
> +} FWCfgDmaAccess;
> +
> +The fields of the structure are in big endian mode, and the field at the 
> lowest
> +address is the "control" field.
> +
> +The "control" field has the following bits:
> + - Bit 0: Error
> + - Bit 1: Read
> + - Bit 2: Skip
> +
> +When an operation is triggered, if the "control" field has bit 1 set, a read
> +operation will be performed. "length" bytes for the current selector and
> +offset will be copied into the address specified by the "address" field.
> +
> +If the control field has only bit 2 set, a skip operation will be perfomed.
> +The offset for the current selector will be advanced "length" bytes.

The implication here is that the operation completes before the
guest write to the address register returns,

> +To check result, read the "control" field:
> +   error bit set        ->  something went wrong.
> +   all bits cleared     ->  transfer finished successfully.
> +   otherwise            ->  transfer still in progress (doesn't happen
> +                            today due to implementation not being async,
> +                            but may in the future).

Is there much point in having an async transfer interface which
requires the guest to busy-wait polling the control field?

> +
> +Target address goes up and transfer length goes down as the transfer happens,
> +so after a successful transfer the length field is zero and the address field
> +points right after the memory block written.
> +
> +If a partial transfer happened before an error occured the address and

"occurred".

> +length registers indicate how much data has been transfered successfully.

"transferred".

> +
>  = Host-side API =
>
>  The following functions are available to the QEMU programmer for adding

thanks
-- PMM