Re: [PATCH] QEMU fw_cfg DMA interface documentation

[Stefan Hajnoczi]

On Thu, Aug 6, 2015 at 12:03 PM, Marc Marà <markmb@xxxxxxxxxx> wrote:
> Add fw_cfg DMA interface specfication in the fw_cfg documentation.
>
> Signed-off-by: Marc Marà <markmb@xxxxxxxxxx>
> ---
>  Documentation/devicetree/bindings/arm/fw-cfg.txt | 36 ++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt b/Documentation/devicetree/bindings/arm/fw-cfg.txt
> index 953fb64..c880eec 100644
> --- a/Documentation/devicetree/bindings/arm/fw-cfg.txt
> +++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt
> @@ -49,6 +49,41 @@ The guest kernel is not expected to use these registers (although it is

Please update the "=== Revision (Key 0x0001, FW_CFG_ID) ===" section
to say that currently the revision is 2.

>  certainly allowed to); the device tree bindings are documented here because
>  this is where device tree bindings reside in general.
>
> +Starting from revision 2, a DMA interface has also been added. This can be used
> +through a write-only, 64-bit wide address register.
> +
> +In this register, a pointer to a FWCfgDmaAccess structure can be written, in

s/pointer/physical RAM address/ is clearer

> +big endian mode. This is the format of the FWCfgDmaAccess structure:

Please be explicit about the *order* of 32-bit writes to the 64-bit
DMA register.

Big-endian only defines the layout of bits but it doesn't say in which
order the two 32-bit sub-registers need to be written.

> +typedef struct FWCfgDmaAccess {
> +    uint64_t address;
> +    uint32_t length;
> +    uint32_t control;
> +} FWCfgDmaAccess;
> +
> +Once the address to this structure has been written, an DMA operation is
> +started. If the "control" field has value 2, a read operation will be performed.
> +"length" bytes for the current selector and offset will be mapped into the
> +address specified by the "address" field.

"mapped" might be confusing.  "Copied" or "DMAed" is clearer.

> +If the field "address" has value 0, the read is considered a skip, and
> +the data is not copied anywhere, but the offset is still incremented.
> +
> +To check result, read the control register:

FWCfgDmaAccess.control is not a register, it's a field.

s/register/field/

> +   error bit set     ->  something went wrong.

Which bit number is the error bit?

> +   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).
> +
> +Target address goes up and transfer length goes down as the transfer
> +happens, so after a successful transfer the length register is zero
> +and the address register points right after the memory block written.

s/register/field/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/