QEMU fw_cfg DMA interface

QEMU fw_cfg DMA interface

[Marc Marí]

Implementation of the FW CFG DMA interface.

When running a Linux guest on top of QEMU, using the -kernel options, this
is the timing improvement for x86:

QEMU commit 090d0bf and SeaBIOS commit 2fc20dc
QEMU startup time: .078
BIOS startup time: .060
Kernel setup time: .578
Total time: .716

QEMU with this patch series and SeaBIOS with this patch series
QEMU startup time: .080
BIOS startup time: .039
Kernel setup time: .002
Total time: .121

QEMU startup time is the time between the start and the first kvm_entry.
BIOS startup time is the time between the first kvm_entry and the start of
function do_boot, in SeaBIOS.
Kernel setup time is the time between the start of the function do_boot in
SeaBIOS and the jump to the Linux kernel.

As you can see, both the BIOS (because of ACPI tables and other configurations)
and the Linux kernel boot (because of the copy to memory) are greatly
improved with this new interface.

Also, this new interface is an addon to the old interface. Both interfaces
are compatible and interchangeable.

Changes from v1:
 - Take into account order of fields in the FWCfgDmaAccess structure
 - Check and change endianness of FWCfgDmaAccess fields
 - Change order of fields in the FWCfgDmaAccess structure
 - Add FW_CFG_DMA_CTL_SKIP feature for control field
 - Split FW_CFG_SIZE in QEMU
 - Make FW_CFG_ID a bitmap of features
 - Add 64 bit address support for the transfer. Trigger when writing the low
   address, and address is 0 by default and at the end of each transfer.
 - Align ports and addresses.
 - Preserve old fw_cfg_comb_valid behaviour in QEMU
 - Update documentation to reflect all these changes
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] QEMU fw_cfg DMA interface documentation

[Marc Marí]

Add fw_cfg DMA interface specfication in the fw_cfg documentation.

Signed-off-by: Marc Marí <markmb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/fw-cfg.txt | 51 +++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt b/Documentation/devicetree/bindings/arm/fw-cfg.txt
index 953fb64..766ddbe 100644
--- a/Documentation/devicetree/bindings/arm/fw-cfg.txt
+++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt
@@ -45,6 +45,53 @@ blob to be read from the data register has size 4, and it is to be interpreted
 as a uint32_t value in little endian byte order. The current value
 (corresponding to the above outer protocol) is zero.
 
+If bit 1 of the feature bitmap is set, the DMA interface is present. This
+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
+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.
+
 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.
@@ -56,6 +103,8 @@ 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.
+  * With DMA interface enabled: Bytes 0xc to 0x13 cover the DMA address
+    register.
   * Further registers may be appended to the region in case of future interface
     revisions / feature bits.
 
@@ -66,7 +115,7 @@ Example:
 	#address-cells = <0x2>;
 
 	fw-cfg@9020000 {
+		reg = <0x0 0x9020000 0x0 0x14>;
 		compatible = "qemu,fw-cfg-mmio";
-		reg = <0x0 0x9020000 0x0 0xa>;
 	};
 };
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Stefan Hajnoczi]

On Mon, Aug 31, 2015 at 10:11 AM, Marc Marí <markmb@redhat.com> 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 | 51 +++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt b/Documentation/devicetree/bindings/arm/fw-cfg.txt
> index 953fb64..766ddbe 100644
> --- a/Documentation/devicetree/bindings/arm/fw-cfg.txt
> +++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt
> @@ -45,6 +45,53 @@ blob to be read from the data register has size 4, and it is to be interpreted
>  as a uint32_t value in little endian byte order. The current value
>  (corresponding to the above outer protocol) is zero.
>
> +If bit 1 of the feature bitmap is set, the DMA interface is present. This
> +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
> +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;

I think including the selector field would be nice to avoid extra
register accesses, but I'm not that familiar with fw_cfg so maybe
there's a reason not to include that field.

> +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.

Minor clarification:
s/address/physical RAM address/

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Marc Marí]

On Wed, 2 Sep 2015 09:20:12 +0100
Stefan Hajnoczi <stefanha-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Aug 31, 2015 at 10:11 AM, Marc Marí <markmb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Add fw_cfg DMA interface specfication in the fw_cfg documentation.
> >
> > Signed-off-by: Marc Marí <markmb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/arm/fw-cfg.txt | 51
> > +++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1
> > deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt
> > b/Documentation/devicetree/bindings/arm/fw-cfg.txt index
> > 953fb64..766ddbe 100644 ---
> > a/Documentation/devicetree/bindings/arm/fw-cfg.txt +++
> > b/Documentation/devicetree/bindings/arm/fw-cfg.txt @@ -45,6 +45,53
> > @@ blob to be read from the data register has size 4, and it is to
> > be interpreted as a uint32_t value in little endian byte order. The
> > current value (corresponding to the above outer protocol) is zero.
> >
> > +If bit 1 of the feature bitmap is set, the DMA interface is
> > present. This +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 +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;
> 
> I think including the selector field would be nice to avoid extra
> register accesses, but I'm not that familiar with fw_cfg so maybe
> there's a reason not to include that field.

It's just simplicity. If you want to read a few times from the same
field (like in ACPI tables, read the data size and then the data), you
need a way to enable and disable the selector and manage the current
offset for that entry. This is already provided with the "old"
interface.

Moreover, if, for some reason, both systems are being used
simultaneously, some way of interacting both selectors and offsets is
needed.

I think the overhead of writing the selector apart is not that big,
compared to the trouble of adding a new one.

Thanks
Marc
 
> > +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.
> 
> Minor clarification:
> s/address/physical RAM address/
> 
> Reviewed-by: Stefan Hajnoczi <stefanha-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Gerd Hoffmann]

  Hi,

> It's just simplicity. If you want to read a few times from the same
> field (like in ACPI tables, read the data size and then the data), you
> need a way to enable and disable the selector and manage the current
> offset for that entry. This is already provided with the "old"
> interface.

Could be handled with a 'select' control bit.  Only when set select
entry and reset offset to zero.

Also: would it make sense to allow an *array* of FWCfgDmaAccess structs?
With a 'more' bit in control we could indicate that there are more
entries.  I'm not sure firmware would actually use that though ...

cheers,
  Gerd

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Laszlo Ersek]

On 09/07/15 13:08, Gerd Hoffmann wrote:
>   Hi,
> 
>> It's just simplicity. If you want to read a few times from the same
>> field (like in ACPI tables, read the data size and then the data), you
>> need a way to enable and disable the selector and manage the current
>> offset for that entry. This is already provided with the "old"
>> interface.
> 
> Could be handled with a 'select' control bit.  Only when set select
> entry and reset offset to zero.
> 
> Also: would it make sense to allow an *array* of FWCfgDmaAccess structs?
> With a 'more' bit in control we could indicate that there are more
> entries.  I'm not sure firmware would actually use that though ...

Seems far-fetched.

Thanks
Laszlo

> cheers,
>   Gerd
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Kevin O'Connor]

On Mon, Sep 07, 2015 at 01:08:29PM +0200, Gerd Hoffmann wrote:
> > It's just simplicity. If you want to read a few times from the same
> > field (like in ACPI tables, read the data size and then the data), you
> > need a way to enable and disable the selector and manage the current
> > offset for that entry. This is already provided with the "old"
> > interface.
> 
> Could be handled with a 'select' control bit.  Only when set select
> entry and reset offset to zero.

I think two features would help "round off" the new fw_cfg DMA
proposal: add a select bit as you describe (that uses the 16 most
significant bits of the "control" field for the "select entry" when
the bit is set), and define a static signature (eg, "QEMU CFG") when
reading the 64bit MMIO dma register.

Both are optional features that don't change the fundamental
interface; I was thinking of sending them as two patches on top of
Marc's next version of his patch series (if no one else gets to it
first).

-Kevin

Re: [PATCH v2] QEMU fw_cfg DMA interface documentation

[Marc Marí]

On Tue, 8 Sep 2015 12:46:08 -0400
"Kevin O'Connor" <kevin@koconnor.net> wrote:

> On Mon, Sep 07, 2015 at 01:08:29PM +0200, Gerd Hoffmann wrote:
> > > It's just simplicity. If you want to read a few times from the
> > > same field (like in ACPI tables, read the data size and then the
> > > data), you need a way to enable and disable the selector and
> > > manage the current offset for that entry. This is already
> > > provided with the "old" interface.
> > 
> > Could be handled with a 'select' control bit.  Only when set select
> > entry and reset offset to zero.
> 
> I think two features would help "round off" the new fw_cfg DMA
> proposal: add a select bit as you describe (that uses the 16 most
> significant bits of the "control" field for the "select entry" when
> the bit is set), and define a static signature (eg, "QEMU CFG") when
> reading the 64bit MMIO dma register.
> 
> Both are optional features that don't change the fundamental
> interface; I was thinking of sending them as two patches on top of
> Marc's next version of his patch series (if no one else gets to it
> first).
> 

As there will be (at least) another version, I can add those simple
changes in the next version.

Thanks
Marc