Date: Thu, 6 Aug 2015 16:08:49 +0200
From: Andrew Jones <drjones@redhat.com>
To: Marc =?iso-8859-1?Q?Mar=ED?= <markmb@redhat.com>
Cc: linux-kernel@vger.kernel.org, Stefan Hajnoczi <stefanha@gmail.com>,
        "Kevin O'Connor" <kevin@koconnor.net>,
        Gerd Hoffmann <kraxel@redhat.com>, Laszlo <lersek@redhat.com>
Subject: Re: [PATCH] QEMU fw_cfg DMA interface documentation
Message-ID: <20150806140849.GD3083@hawk.localdomain>
References: <1438858816-29385-1-git-send-email-markmb@redhat.com>
 <1438858987-29566-1-git-send-email-markmb@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1438858987-29566-1-git-send-email-markmb@redhat.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3826
Lines: 91

On Thu, Aug 06, 2015 at 01:03:07PM +0200, Marc Mar? wrote:
> Add fw_cfg DMA interface specfication in the fw_cfg documentation.
> 
> Signed-off-by: Marc Mar? <markmb@redhat.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
>  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
> +big endian mode. This is the format of the FWCfgDmaAccess structure:
s/mode/format/
> +
> +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.
> +
> +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.

So we can't DMA to address == 0? That might not generally be a good
idea, but why limit ourselves? Can't we add another control input for skip
instead? Actually, what inputs are accepted now? READ == 2, WRITE? ??

> +
> +To check result, read the control register:
> +   error bit set     ->  something went wrong.
echo Stefan's which bit question, and also what types of errors? If
there are many, then how about an error bit, plus field of bits for
an error code?

> +   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).
I don't think we need to point out that this isn't implemented yet, but
may be in the future. If that's how it may work, then let's just document
it. And why not specify an in-progress bit?

> +
> +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.
> +
> +If a partial transfer happened before an error occured the address and
> +length registers indicate how much data has been transfered
> +successfully.
> +
>  Required properties:
>  
>  - compatible: "qemu,fw-cfg-mmio".
> @@ -56,6 +91,7 @@ Required properties:
>  - reg: the MMIO region used by the device.
>    * Bytes 0x0 to 0x7 cover the data register.
>    * Bytes 0x8 to 0x9 cover the selector register.
> +  * From revision 2: Bytes 0xa to 0x11 cover the DMA address register.

I think we should naturally align the register, i.e we can reserve bytes
0xa - 0xf, and then put the DMA register at 0x10.

>    * Further registers may be appended to the region in case of future interface
>      revisions / feature bits.
>  
> -- 
> 2.4.3
> 

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