| Message ID | 1438858878-29450-3-git-send-email-markmb@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 06, 2015 at 01:01:15PM +0200, Marc Marí wrote: > Add fw_cfg DMA interface specification in the documentation. > > Based on Gerd Hoffman's initial implementation. > > Signed-off-by: Marc Marí <markmb@redhat.com> > --- > docs/specs/fw_cfg.txt | 42 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt > index 5bc7b96..dc8051e 100644 > --- a/docs/specs/fw_cfg.txt > +++ b/docs/specs/fw_cfg.txt > @@ -76,6 +76,7 @@ increasing address order, similar to memcpy(). > > Selector Register IOport: 0x510 > Data Register IOport: 0x511 > +DMA Address IOport: 0x512 > > == Firmware Configuration Items == > > @@ -89,8 +90,9 @@ present, the four bytes read will contain the characters "QEMU". > === Revision (Key 0x0001, FW_CFG_ID) === > > A 32-bit little-endian unsigned int, this item is used as an interface > -revision number, and is currently set to 1 by QEMU when fw_cfg is > -initialized. > +revision number. If it is set to 1, the interface is the traditional > +selector / data interface. If it is set to 2, the DMA extension is > +also present. > > === File Directory (Key 0x0019, FW_CFG_FILE_DIR) === > > @@ -132,6 +134,42 @@ Selector Reg. Range Usage > In practice, the number of allowed firmware configuration items is given > by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). > > += Guest-side DMA Interface = > + > +For revision value 2, the DMA interface is present. This does not replace > +the existing fw_cfg interface, it is an add-on. > + > +When this interface is enabled the DMA Address register can be used to > +write the address of a FWCfgDmaAccess structure: > + > +typedef struct FWCfgDmaAccess { > + uint64_t address; > + uint32_t length; > + uint32_t control; > +} FWCfgDmaAccess; > + > +If "control" has the bit 1 set (FW_CFG_DMA_CTL_READ), a read operation is > +performed on the selected entry. "length" bytes of data in that fw_cfg > +entry are copied to the address specified by "address". > + > +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. Thanks! I have a few suggestions on the interface: - I think it would be better to place the 'control' field first (ie, control/length/address instead of address/length/control). Placing the 'control' field first makes potential future extensions easier - that is, it would make it easier for future control bits to change the layout of the struct. - It would be good to add a 'select' field to the struct. I think this could be done by using a 'u16 control; u16 select' instead of the current 'u32 control'. It's common for guests to select a fw_cfg entry and read its entire contents - with the 'select' field in the struct this could be done with a single guest/host fault. A new control bit could be added (eg, FW_CFG_DMA_CTL_SELECT) to determine whether or not the select field is used - iff the bit is set then the given fw_cfg entry is selected and the read position is reset to the start prior to the DMA data copy. - Instead of using address==0 to check for skip, I think a new control bit would be a little more friendly. That is, one could add a new control bit FW_CFG_DMA_CTL_COPY, and iff it is set then do the DMA style copy to the 'address' field. If it is not set, then 'address' is ignored and effectively the read position is just incremented. -Kevin
On Thu, 6 Aug 2015 10:20:38 -0400 "Kevin O'Connor" <kevin@koconnor.net> wrote: > On Thu, Aug 06, 2015 at 01:01:15PM +0200, Marc Marí wrote: > > Add fw_cfg DMA interface specification in the documentation. > > > > Based on Gerd Hoffman's initial implementation. > > > > Signed-off-by: Marc Marí <markmb@redhat.com> > > --- > > docs/specs/fw_cfg.txt | 42 > > ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 > > insertions(+), 2 deletions(-) > > > > diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt > > index 5bc7b96..dc8051e 100644 > > --- a/docs/specs/fw_cfg.txt > > +++ b/docs/specs/fw_cfg.txt > > @@ -76,6 +76,7 @@ increasing address order, similar to memcpy(). > > > > Selector Register IOport: 0x510 > > Data Register IOport: 0x511 > > +DMA Address IOport: 0x512 > > > > == Firmware Configuration Items == > > > > @@ -89,8 +90,9 @@ present, the four bytes read will contain the > > characters "QEMU". === Revision (Key 0x0001, FW_CFG_ID) === > > > > A 32-bit little-endian unsigned int, this item is used as an > > interface -revision number, and is currently set to 1 by QEMU when > > fw_cfg is -initialized. > > +revision number. If it is set to 1, the interface is the > > traditional +selector / data interface. If it is set to 2, the DMA > > extension is +also present. > > > > === File Directory (Key 0x0019, FW_CFG_FILE_DIR) === > > > > @@ -132,6 +134,42 @@ Selector Reg. Range Usage > > In practice, the number of allowed firmware configuration items is > > given by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). > > > > += Guest-side DMA Interface = > > + > > +For revision value 2, the DMA interface is present. This does not > > replace +the existing fw_cfg interface, it is an add-on. > > + > > +When this interface is enabled the DMA Address register can be > > used to +write the address of a FWCfgDmaAccess structure: > > + > > +typedef struct FWCfgDmaAccess { > > + uint64_t address; > > + uint32_t length; > > + uint32_t control; > > +} FWCfgDmaAccess; > > + > > +If "control" has the bit 1 set (FW_CFG_DMA_CTL_READ), a read > > operation is +performed on the selected entry. "length" bytes of > > data in that fw_cfg +entry are copied to the address specified by > > "address". + > > +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. > > Thanks! > > I have a few suggestions on the interface: > > - I think it would be better to place the 'control' field first (ie, > control/length/address instead of address/length/control). Placing > the 'control' field first makes potential future extensions easier - > that is, it would make it easier for future control bits to change > the layout of the struct. > > - It would be good to add a 'select' field to the struct. I think > this could be done by using a 'u16 control; u16 select' instead of > the current 'u32 control'. It's common for guests to select a > fw_cfg entry and read its entire contents - with the 'select' field > in the struct this could be done with a single guest/host fault. A > new control bit could be added (eg, FW_CFG_DMA_CTL_SELECT) to > determine whether or not the select field is used - iff the bit is > set then the given fw_cfg entry is selected and the read position is > reset to the start prior to the DMA data copy. (Problems with email, I got this delivered today) I don't think there's much problem in using the original fw cfg select field. Adding a new select field will add complexity to the guest (when use one select field or the other), and the host (how the two select fields interact with each other). I think this part is good enough as it is now. Thanks Marc
diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt index 5bc7b96..dc8051e 100644 --- a/docs/specs/fw_cfg.txt +++ b/docs/specs/fw_cfg.txt @@ -76,6 +76,7 @@ increasing address order, similar to memcpy(). Selector Register IOport: 0x510 Data Register IOport: 0x511 +DMA Address IOport: 0x512 == Firmware Configuration Items == @@ -89,8 +90,9 @@ present, the four bytes read will contain the characters "QEMU". === Revision (Key 0x0001, FW_CFG_ID) === A 32-bit little-endian unsigned int, this item is used as an interface -revision number, and is currently set to 1 by QEMU when fw_cfg is -initialized. +revision number. If it is set to 1, the interface is the traditional +selector / data interface. If it is set to 2, the DMA extension is +also present. === File Directory (Key 0x0019, FW_CFG_FILE_DIR) === @@ -132,6 +134,42 @@ Selector Reg. Range Usage In practice, the number of allowed firmware configuration items is given by the value of FW_CFG_MAX_ENTRY (see fw_cfg.h). += Guest-side DMA Interface = + +For revision value 2, the DMA interface is present. This does not replace +the existing fw_cfg interface, it is an add-on. + +When this interface is enabled the DMA Address register can be used to +write the address of a FWCfgDmaAccess structure: + +typedef struct FWCfgDmaAccess { + uint64_t address; + uint32_t length; + uint32_t control; +} FWCfgDmaAccess; + +If "control" has the bit 1 set (FW_CFG_DMA_CTL_READ), a read operation is +performed on the selected entry. "length" bytes of data in that fw_cfg +entry are copied to the address specified by "address". + +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: + 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 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. + = Host-side API = The following functions are available to the QEMU programmer for adding
Add fw_cfg DMA interface specification in the documentation. Based on Gerd Hoffman's initial implementation. Signed-off-by: Marc Marí <markmb@redhat.com> --- docs/specs/fw_cfg.txt | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-)