Message ID | 1444316621-21863-1-git-send-email-markmb@redhat.com |
---|---|
State | Under Review, archived |
Headers | show |
On 10/08/15 17:03, 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 | 52 +++++++++++++++++++++++- > 1 file changed, 51 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt b/Documentation/devicetree/bindings/arm/fw-cfg.txt > index 953fb64..0633aad 100644 > --- a/Documentation/devicetree/bindings/arm/fw-cfg.txt > +++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt > @@ -38,6 +38,9 @@ The presence of the registers can be verified by selecting the "signature" blob > with key 0x0000, and reading four bytes from the data register. The returned > signature is "QEMU". > > +If the DMA interface is available, then reading the DMA Address Register > +returns 0x51454d5520434647 ("QEMU CFG" in big-endian format). > + marking this for a later argument: (*) > The outermost protocol (involving the write / read sequences of the control and > data registers) is expected to be versioned, and/or described by feature bits. > The interface revision / feature bitmap can be retrieved with key 0x0001. The > @@ -45,6 +48,51 @@ 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. Please sync this language with the "least significant half" / "most significant half" wording that I requested after your QEMU [PATCH v4 2/7] fw_cfg DMA interface documentation and that you implemented in your present QEMU [PATCH v5 2/6] fw_cfg DMA interface documentation > + > +In this register, the physical address of a FWCfgDmaAccess structure in RAM > +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 > + - Bit 3: Select. The upper 16 bits are the selected index. > + > +When an operation is triggered, if the "control" field has bit 3 set, the > +upper 16 bits are interpreted as an index of a firmware configuration item. > +This has the same effect as writing the selector register. > + > +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 > +physical RAM address specified by the "address" field. > + > +If the "control" field has bit 2 set (and not bit 1), a skip operation will be > +performed. The offset for the current selector will be advanced "length" bytes. > + > +To check the 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). > + > 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. I guess this is all coming verbatim from the QEMU spec. I think that's okay, it doesn't speak about selector values etc, only about transport. I think it's okay (and consistent with the current text) to have all the details here. > @@ -56,6 +104,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 0x10 to 0x17 cover the DMA address > + register. > * Further registers may be appended to the region in case of future interface > revisions / feature bits. > > @@ -66,7 +116,7 @@ Example: > #address-cells = <0x2>; > > fw-cfg@9020000 { > + reg = <0x0 0x9020000 0x0 0x18>; > compatible = "qemu,fw-cfg-mmio"; > - reg = <0x0 0x9020000 0x0 0xa>; > }; > }; > Please make this a bit more precise. As already mentioned by Peter (in another part of the discussion), you can't read the DMA address register until you know that it exists. Therefore the part I marked with (*) above is only usable in the ioport-mapped, x86 case for feature detection. On ARM, what we key off of primarily is the *size* of the register block, and only if the DMA address register fits in there, can we read the feature bitmap (for double-checking), and/or read a signature from the DMA address register. Therefore, I think the example device tree snippet is okay, but the hunk above it should sound like: * Bytes 0x10 to 0x17, if they exist in the region -- and the feature bitmap confirms the presence of the DMA interface -- cover the DMA address register. The AAVMF client code does just this. (I.e., checks the size of the region first, then reads the feature bitmap *without* DMA, then performs further reads with DMA.) Thanks Laszlo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt b/Documentation/devicetree/bindings/arm/fw-cfg.txt index 953fb64..0633aad 100644 --- a/Documentation/devicetree/bindings/arm/fw-cfg.txt +++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt @@ -38,6 +38,9 @@ The presence of the registers can be verified by selecting the "signature" blob with key 0x0000, and reading four bytes from the data register. The returned signature is "QEMU". +If the DMA interface is available, then reading the DMA Address Register +returns 0x51454d5520434647 ("QEMU CFG" in big-endian format). + The outermost protocol (involving the write / read sequences of the control and data registers) is expected to be versioned, and/or described by feature bits. The interface revision / feature bitmap can be retrieved with key 0x0001. The @@ -45,6 +48,51 @@ 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, the physical address of a FWCfgDmaAccess structure in RAM +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 + - Bit 3: Select. The upper 16 bits are the selected index. + +When an operation is triggered, if the "control" field has bit 3 set, the +upper 16 bits are interpreted as an index of a firmware configuration item. +This has the same effect as writing the selector register. + +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 +physical RAM address specified by the "address" field. + +If the "control" field has bit 2 set (and not bit 1), a skip operation will be +performed. The offset for the current selector will be advanced "length" bytes. + +To check the 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). + 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 +104,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 0x10 to 0x17 cover the DMA address + register. * Further registers may be appended to the region in case of future interface revisions / feature bits. @@ -66,7 +116,7 @@ Example: #address-cells = <0x2>; fw-cfg@9020000 { + reg = <0x0 0x9020000 0x0 0x18>; compatible = "qemu,fw-cfg-mmio"; - reg = <0x0 0x9020000 0x0 0xa>; }; };
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 | 52 +++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)