linux-kernel - Re: [PATCH] QEMU fw_cfg DMA interface documentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJSP0QULqrSNAtEDXZ5dMVvp+LqLkYdcAq9rqJqTVzedvfC5tQ@mail.gmail.com>
Date:	Thu, 6 Aug 2015 13:12:19 +0100
From:	Stefan Hajnoczi <stefanha@...il.com>
To:	Marc Marí <markmb@...hat.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Drew <drjones@...hat.com>, "Kevin O'Connor" <kevin@...onnor.net>,
	Gerd Hoffmann <kraxel@...hat.com>, Laszlo <lersek@...hat.com>
Subject: Re: [PATCH] QEMU fw_cfg DMA interface documentation

On Thu, Aug 6, 2015 at 12:03 PM, Marc Marí <markmb@...hat.com> wrote:
> Add fw_cfg DMA interface specfication in the fw_cfg documentation.
>
> Signed-off-by: Marc Marí <markmb@...hat.com>
> ---
>  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/