Re: [Qemu-devel] [PATCH v4 2/7] fw_cfg DMA interface documentation

[Laszlo Ersek <lersek@redhat.com>]

On 10/01/15 14:16, Marc Marí wrote:
> Add fw_cfg DMA interface specification in the documentation.
> 
> Based on Gerd Hoffman's initial implementation.
> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/specs/fw_cfg.txt | 65 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 5bc7b96..2d6b2da 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: Base + 8 (2 bytes)
> +Data Register address:     Base + 0 (8 bytes)
> +DMA Address address:       Base + 16 (8 bytes)
>  
>  == 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,55 @@ 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 is in big-endian format. The value for the register is 0
> +at startup and after an operation. A write to the lower half triggers an

suggest "least significant half (at offset 4)" in place of "lower half"

> +operation. This means that operations with 32-bit addresses 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.

suggest "most significant half (at offset 0)" in place of "higher part".

With those changes:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> +
> +In this register, the physical address of a FWCfgDmaAccess structure in RAM
> +should 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
> + - Bit 3: Select. The upper 16 bits are the selected index.
> +
> +When an operation is triggered, if the "control" field has bit 3 set, the
> +upper 16 bits are interpreted as an index of a firmware configuration item.
> +This has the same effect as writing the selector register.
> +
> +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
> +physical RAM address specified by the "address" field.
> +
> +If the "control" field has bit 2 set (and not bit 1), a skip operation will be
> +performed. The offset for the current selector will be advanced "length" bytes.
> +
> +To check the 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).
> +
>  = Host-side API =
>  
>  The following functions are available to the QEMU programmer for adding
>