qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kevin O'Connor
Subject: Re: [Qemu-devel] [PATCH v2 2/5] fw_cfg DMA interface documentation
Date: Mon, 31 Aug 2015 11:36:17 -0400
User-agent: Mutt/1.5.23 (2014-03-12)

On Mon, Aug 31, 2015 at 11:10:14AM +0200, Marc Marí wrote:
> Add fw_cfg DMA interface specification in the documentation.
> 
> Based on Gerd Hoffman's initial implementation.

Thanks for working on this.

[...]
> += 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
> +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
> +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.
> +
> +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).
> +
> +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
> +length registers indicate how much data has been transfered successfully.

If the interface will support asynchronous transfers, then it is
necessary to document which field QEMU will update last.  That way,
the guest will know which particular field to wait on.  It should
probably also be documented that that particular field must have
native alignment (eg, alignment of 4 for a 32bit field).

The reason this is important, is because of a subtle race condition.
For example, if QEMU updates "length" and then "control", but the
guest spins on "length" then it is possible for the guest to exit its
spin loop and use the memory containing "control" for something else
before QEMU finishes updating "control".  This would result in
corruption of that memory when QEMU later overwrites it.

My suggestion would be for QEMU to only ever update "control" and not
modify any other parts of the FWCfgDmaAccess struct.  This makes the
interface and documentation simpler and it sidesteps this issue.

-Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]