| Message ID | 1497772934-15561-6-git-send-email-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jun 18, 2017 at 09:02:14AM +0100, Mark Cave-Ayland wrote: > By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility > for the internal MemoryRegion fields to be mapped by name for boards that wish > to wire up the fw_cfg device themselves. > > An additional minor tidy-up is that the FWCfgEntry typedef is moved from the > struct definitions in fw_cfg.h to typedefs.h along with the others. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/nvram/fw_cfg.c | 55 ------------------------------------------ > include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++++++++++++ > include/qemu/typedefs.h | 1 + > 3 files changed, 59 insertions(+), 55 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index df99903..00771c9 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -40,14 +40,6 @@ > #define FW_CFG_NAME "fw_cfg" > #define FW_CFG_PATH "/machine/" FW_CFG_NAME > > -#define TYPE_FW_CFG "fw_cfg" > -#define TYPE_FW_CFG_IO "fw_cfg_io" > -#define TYPE_FW_CFG_MEM "fw_cfg_mem" > - > -#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) > -#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) > -#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) > - > /* FW_CFG_VERSION bits */ > #define FW_CFG_VERSION 0x01 > #define FW_CFG_VERSION_DMA 0x02 > @@ -61,53 +53,6 @@ > > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ > > -typedef struct FWCfgEntry { > - uint32_t len; > - bool allow_write; > - uint8_t *data; > - void *callback_opaque; > - FWCfgReadCallback read_callback; > -} FWCfgEntry; This still doesn't seem to do what Laszlo requested which is to keep as many types and macros as possible in fw_cfg.c, only put typedefs in fw_cfg.h. > - > -struct FWCfgState { > - /*< private >*/ > - SysBusDevice parent_obj; > - /*< public >*/ > - > - uint16_t file_slots; > - FWCfgEntry *entries[2]; > - int *entry_order; > - FWCfgFiles *files; > - uint16_t cur_entry; > - uint32_t cur_offset; > - Notifier machine_ready; > - > - int fw_cfg_order_override; > - > - bool dma_enabled; > - dma_addr_t dma_addr; > - AddressSpace *dma_as; > - MemoryRegion dma_iomem; > -}; > - > -struct FWCfgIoState { > - /*< private >*/ > - FWCfgState parent_obj; > - /*< public >*/ > - > - MemoryRegion comb_iomem; > -}; > - > -struct FWCfgMemState { > - /*< private >*/ > - FWCfgState parent_obj; > - /*< public >*/ > - > - MemoryRegion ctl_iomem, data_iomem; > - uint32_t data_width; > - MemoryRegionOps wide_data_ops; > -}; > - > #define JPG_FILE 0 > #define BMP_FILE 1 > > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index b980cba..b0511b9 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -1,8 +1,19 @@ > #ifndef FW_CFG_H > #define FW_CFG_H > > +#include "qemu/typedefs.h" > #include "exec/hwaddr.h" > #include "hw/nvram/fw_cfg_keys.h" > +#include "hw/sysbus.h" > +#include "sysemu/dma.h" > + > +#define TYPE_FW_CFG "fw_cfg" > +#define TYPE_FW_CFG_IO "fw_cfg_io" > +#define TYPE_FW_CFG_MEM "fw_cfg_mem" > + > +#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) > +#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) > +#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) > > typedef struct FWCfgFile { > uint32_t size; /* file size */ > @@ -35,6 +46,53 @@ typedef struct FWCfgDmaAccess { > > typedef void (*FWCfgReadCallback)(void *opaque); > > +struct FWCfgEntry { > + uint32_t len; > + bool allow_write; > + uint8_t *data; > + void *callback_opaque; > + FWCfgReadCallback read_callback; > +}; > + > +struct FWCfgState { > + /*< private >*/ > + SysBusDevice parent_obj; > + /*< public >*/ > + > + uint16_t file_slots; > + FWCfgEntry *entries[2]; > + int *entry_order; > + FWCfgFiles *files; > + uint16_t cur_entry; > + uint32_t cur_offset; > + Notifier machine_ready; > + > + int fw_cfg_order_override; > + > + bool dma_enabled; > + dma_addr_t dma_addr; > + AddressSpace *dma_as; > + MemoryRegion dma_iomem; > +}; > + > +struct FWCfgIoState { > + /*< private >*/ > + FWCfgState parent_obj; > + /*< public >*/ > + > + MemoryRegion comb_iomem; > +}; > + > +struct FWCfgMemState { > + /*< private >*/ > + FWCfgState parent_obj; > + /*< public >*/ > + > + MemoryRegion ctl_iomem, data_iomem; > + uint32_t data_width; > + MemoryRegionOps wide_data_ops; > +}; > + > /** > * fw_cfg_add_bytes: > * @s: fw_cfg device being modified > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > index f745d5f..2db2918 100644 > --- a/include/qemu/typedefs.h > +++ b/include/qemu/typedefs.h > @@ -30,6 +30,7 @@ typedef struct DisplaySurface DisplaySurface; > typedef struct DriveInfo DriveInfo; > typedef struct Error Error; > typedef struct EventNotifier EventNotifier; > +typedef struct FWCfgEntry FWCfgEntry; > typedef struct FWCfgIoState FWCfgIoState; > typedef struct FWCfgMemState FWCfgMemState; > typedef struct FWCfgState FWCfgState; > -- > 1.7.10.4
On 06/18/17 22:23, Michael S. Tsirkin wrote: > On Sun, Jun 18, 2017 at 09:02:14AM +0100, Mark Cave-Ayland wrote: >> By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility >> for the internal MemoryRegion fields to be mapped by name for boards that wish >> to wire up the fw_cfg device themselves. >> >> An additional minor tidy-up is that the FWCfgEntry typedef is moved from the >> struct definitions in fw_cfg.h to typedefs.h along with the others. I think this paragraph should be dropped. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/nvram/fw_cfg.c | 55 ------------------------------------------ >> include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++++++++++++ >> include/qemu/typedefs.h | 1 + >> 3 files changed, 59 insertions(+), 55 deletions(-) >> >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >> index df99903..00771c9 100644 >> --- a/hw/nvram/fw_cfg.c >> +++ b/hw/nvram/fw_cfg.c >> @@ -40,14 +40,6 @@ >> #define FW_CFG_NAME "fw_cfg" >> #define FW_CFG_PATH "/machine/" FW_CFG_NAME >> >> -#define TYPE_FW_CFG "fw_cfg" >> -#define TYPE_FW_CFG_IO "fw_cfg_io" >> -#define TYPE_FW_CFG_MEM "fw_cfg_mem" >> - >> -#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) >> -#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) >> -#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) >> - >> /* FW_CFG_VERSION bits */ >> #define FW_CFG_VERSION 0x01 >> #define FW_CFG_VERSION_DMA 0x02 >> @@ -61,53 +53,6 @@ >> >> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ >> >> -typedef struct FWCfgEntry { >> - uint32_t len; >> - bool allow_write; >> - uint8_t *data; >> - void *callback_opaque; >> - FWCfgReadCallback read_callback; >> -} FWCfgEntry; > > This still doesn't seem to do what Laszlo requested which is to keep > as many types and macros as possible in fw_cfg.c, only put typedefs in > fw_cfg.h. Sort of; what's missing from this version (for me anyway) is that the internals of FWCfgEntry should remain in the C file, because we never depend on those fields -- or the size of that structure -- externally. I'm OK with the rest. Mark, can you please squash the following diff into this patch -- this is what would implement my request (2) in <https://www.mail-archive.com/qemu-devel@nongnu.org/msg458313.html>: > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h > index b0511b9a9d77..b77ea48abb1d 100644 > --- a/include/hw/nvram/fw_cfg.h > +++ b/include/hw/nvram/fw_cfg.h > @@ -46,14 +46,6 @@ typedef struct FWCfgDmaAccess { > > typedef void (*FWCfgReadCallback)(void *opaque); > > -struct FWCfgEntry { > - uint32_t len; > - bool allow_write; > - uint8_t *data; > - void *callback_opaque; > - FWCfgReadCallback read_callback; > -}; > - > struct FWCfgState { > /*< private >*/ > SysBusDevice parent_obj; > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 00771c98505c..9b0aaa21a202 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -53,6 +53,14 @@ > > #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ > > +struct FWCfgEntry { > + uint32_t len; > + bool allow_write; > + uint8_t *data; > + void *callback_opaque; > + FWCfgReadCallback read_callback; > +}; > + > #define JPG_FILE 0 > #define BMP_FILE 1 > As I wrote in the msg linked above, 'FWCfgEntry need not be moved from the C file to the header file [...] it's fine to leave FWCfgEntry an incomplete type in "fw_cfg.h"'. With the above two changes (commit message and code update) squashed into patch #5: Reviewed-by: Laszlo Ersek <lersek@redhat.com> and also for the series: Tested-by: Laszlo Ersek <lersek@redhat.com> (I tested the IO mapped variant with OVMF, exercising fw_cfg DMA and fw_cfg write, and the MMIO mapped variant with ArmVirtQemu (aka "AAVMF"), exercising fw_cfg DMA.) Thanks Laszlo
On 18/06/17 21:23, Michael S. Tsirkin wrote: > On Sun, Jun 18, 2017 at 09:02:14AM +0100, Mark Cave-Ayland wrote: >> By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility >> for the internal MemoryRegion fields to be mapped by name for boards that wish >> to wire up the fw_cfg device themselves. >> >> An additional minor tidy-up is that the FWCfgEntry typedef is moved from the >> struct definitions in fw_cfg.h to typedefs.h along with the others. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/nvram/fw_cfg.c | 55 ------------------------------------------ >> include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++++++++++++ >> include/qemu/typedefs.h | 1 + >> 3 files changed, 59 insertions(+), 55 deletions(-) >> >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >> index df99903..00771c9 100644 >> --- a/hw/nvram/fw_cfg.c >> +++ b/hw/nvram/fw_cfg.c >> @@ -40,14 +40,6 @@ >> #define FW_CFG_NAME "fw_cfg" >> #define FW_CFG_PATH "/machine/" FW_CFG_NAME >> >> -#define TYPE_FW_CFG "fw_cfg" >> -#define TYPE_FW_CFG_IO "fw_cfg_io" >> -#define TYPE_FW_CFG_MEM "fw_cfg_mem" >> - >> -#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) >> -#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) >> -#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) >> - >> /* FW_CFG_VERSION bits */ >> #define FW_CFG_VERSION 0x01 >> #define FW_CFG_VERSION_DMA 0x02 >> @@ -61,53 +53,6 @@ >> >> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ >> >> -typedef struct FWCfgEntry { >> - uint32_t len; >> - bool allow_write; >> - uint8_t *data; >> - void *callback_opaque; >> - FWCfgReadCallback read_callback; >> -} FWCfgEntry; > > This still doesn't seem to do what Laszlo requested which is to keep as > many types and macros as possible in fw_cfg.c, only put typedefs in > fw_cfg.h. Yes, that's my fault as I misinterpreted Laszlo's comment related to the typedef. Fortunately I see from Laszlo's follow-up email that the fix is fairly trivial. ATB, Mark.
On 19/06/17 09:57, Laszlo Ersek wrote: > On 06/18/17 22:23, Michael S. Tsirkin wrote: >> On Sun, Jun 18, 2017 at 09:02:14AM +0100, Mark Cave-Ayland wrote: >>> By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility >>> for the internal MemoryRegion fields to be mapped by name for boards that wish >>> to wire up the fw_cfg device themselves. >>> >>> An additional minor tidy-up is that the FWCfgEntry typedef is moved from the >>> struct definitions in fw_cfg.h to typedefs.h along with the others. > > I think this paragraph should be dropped. No problem, I'll do that for the follow-up v6 patchset. >>> >>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> --- >>> hw/nvram/fw_cfg.c | 55 ------------------------------------------ >>> include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++++++++++++ >>> include/qemu/typedefs.h | 1 + >>> 3 files changed, 59 insertions(+), 55 deletions(-) >>> >>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >>> index df99903..00771c9 100644 >>> --- a/hw/nvram/fw_cfg.c >>> +++ b/hw/nvram/fw_cfg.c >>> @@ -40,14 +40,6 @@ >>> #define FW_CFG_NAME "fw_cfg" >>> #define FW_CFG_PATH "/machine/" FW_CFG_NAME >>> >>> -#define TYPE_FW_CFG "fw_cfg" >>> -#define TYPE_FW_CFG_IO "fw_cfg_io" >>> -#define TYPE_FW_CFG_MEM "fw_cfg_mem" >>> - >>> -#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) >>> -#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) >>> -#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) >>> - >>> /* FW_CFG_VERSION bits */ >>> #define FW_CFG_VERSION 0x01 >>> #define FW_CFG_VERSION_DMA 0x02 >>> @@ -61,53 +53,6 @@ >>> >>> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ >>> >>> -typedef struct FWCfgEntry { >>> - uint32_t len; >>> - bool allow_write; >>> - uint8_t *data; >>> - void *callback_opaque; >>> - FWCfgReadCallback read_callback; >>> -} FWCfgEntry; >> >> This still doesn't seem to do what Laszlo requested which is to keep >> as many types and macros as possible in fw_cfg.c, only put typedefs in >> fw_cfg.h. > > Sort of; what's missing from this version (for me anyway) is that the > internals of FWCfgEntry should remain in the C file, because we never > depend on those fields -- or the size of that structure -- externally. > I'm OK with the rest. > > Mark, can you please squash the following diff into this patch -- this > is what would implement my request (2) in > <https://www.mail-archive.com/qemu-devel@nongnu.org/msg458313.html>: > >> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h >> index b0511b9a9d77..b77ea48abb1d 100644 >> --- a/include/hw/nvram/fw_cfg.h >> +++ b/include/hw/nvram/fw_cfg.h >> @@ -46,14 +46,6 @@ typedef struct FWCfgDmaAccess { >> >> typedef void (*FWCfgReadCallback)(void *opaque); >> >> -struct FWCfgEntry { >> - uint32_t len; >> - bool allow_write; >> - uint8_t *data; >> - void *callback_opaque; >> - FWCfgReadCallback read_callback; >> -}; >> - >> struct FWCfgState { >> /*< private >*/ >> SysBusDevice parent_obj; >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >> index 00771c98505c..9b0aaa21a202 100644 >> --- a/hw/nvram/fw_cfg.c >> +++ b/hw/nvram/fw_cfg.c >> @@ -53,6 +53,14 @@ >> >> #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ >> >> +struct FWCfgEntry { >> + uint32_t len; >> + bool allow_write; >> + uint8_t *data; >> + void *callback_opaque; >> + FWCfgReadCallback read_callback; >> +}; >> + >> #define JPG_FILE 0 >> #define BMP_FILE 1 >> > > As I wrote in the msg linked above, 'FWCfgEntry need not be moved from > the C file to the header file [...] it's fine to leave FWCfgEntry an > incomplete type in "fw_cfg.h"'. Yes, that's totally my fault as I misinterpreted your comment from your previous email - sorry about that. > With the above two changes (commit message and code update) squashed > into patch #5: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > and also for the series: > > Tested-by: Laszlo Ersek <lersek@redhat.com> > > (I tested the IO mapped variant with OVMF, exercising fw_cfg DMA and > fw_cfg write, and the MMIO mapped variant with ArmVirtQemu (aka > "AAVMF"), exercising fw_cfg DMA.) Fantastic! It's good to know that the changes don't cause any regressions for both DMA operations and MMIO fw_cfg instances. I'll squash your diff into patch 5, update the tags and then re-post a v6 shortly. Assuming that this is the final revision, who is the right person to accept the patchset into their queue for merge upstream? ATB, Mark.
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index df99903..00771c9 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -40,14 +40,6 @@ #define FW_CFG_NAME "fw_cfg" #define FW_CFG_PATH "/machine/" FW_CFG_NAME -#define TYPE_FW_CFG "fw_cfg" -#define TYPE_FW_CFG_IO "fw_cfg_io" -#define TYPE_FW_CFG_MEM "fw_cfg_mem" - -#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) -#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) -#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) - /* FW_CFG_VERSION bits */ #define FW_CFG_VERSION 0x01 #define FW_CFG_VERSION_DMA 0x02 @@ -61,53 +53,6 @@ #define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */ -typedef struct FWCfgEntry { - uint32_t len; - bool allow_write; - uint8_t *data; - void *callback_opaque; - FWCfgReadCallback read_callback; -} FWCfgEntry; - -struct FWCfgState { - /*< private >*/ - SysBusDevice parent_obj; - /*< public >*/ - - uint16_t file_slots; - FWCfgEntry *entries[2]; - int *entry_order; - FWCfgFiles *files; - uint16_t cur_entry; - uint32_t cur_offset; - Notifier machine_ready; - - int fw_cfg_order_override; - - bool dma_enabled; - dma_addr_t dma_addr; - AddressSpace *dma_as; - MemoryRegion dma_iomem; -}; - -struct FWCfgIoState { - /*< private >*/ - FWCfgState parent_obj; - /*< public >*/ - - MemoryRegion comb_iomem; -}; - -struct FWCfgMemState { - /*< private >*/ - FWCfgState parent_obj; - /*< public >*/ - - MemoryRegion ctl_iomem, data_iomem; - uint32_t data_width; - MemoryRegionOps wide_data_ops; -}; - #define JPG_FILE 0 #define BMP_FILE 1 diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h index b980cba..b0511b9 100644 --- a/include/hw/nvram/fw_cfg.h +++ b/include/hw/nvram/fw_cfg.h @@ -1,8 +1,19 @@ #ifndef FW_CFG_H #define FW_CFG_H +#include "qemu/typedefs.h" #include "exec/hwaddr.h" #include "hw/nvram/fw_cfg_keys.h" +#include "hw/sysbus.h" +#include "sysemu/dma.h" + +#define TYPE_FW_CFG "fw_cfg" +#define TYPE_FW_CFG_IO "fw_cfg_io" +#define TYPE_FW_CFG_MEM "fw_cfg_mem" + +#define FW_CFG(obj) OBJECT_CHECK(FWCfgState, (obj), TYPE_FW_CFG) +#define FW_CFG_IO(obj) OBJECT_CHECK(FWCfgIoState, (obj), TYPE_FW_CFG_IO) +#define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM) typedef struct FWCfgFile { uint32_t size; /* file size */ @@ -35,6 +46,53 @@ typedef struct FWCfgDmaAccess { typedef void (*FWCfgReadCallback)(void *opaque); +struct FWCfgEntry { + uint32_t len; + bool allow_write; + uint8_t *data; + void *callback_opaque; + FWCfgReadCallback read_callback; +}; + +struct FWCfgState { + /*< private >*/ + SysBusDevice parent_obj; + /*< public >*/ + + uint16_t file_slots; + FWCfgEntry *entries[2]; + int *entry_order; + FWCfgFiles *files; + uint16_t cur_entry; + uint32_t cur_offset; + Notifier machine_ready; + + int fw_cfg_order_override; + + bool dma_enabled; + dma_addr_t dma_addr; + AddressSpace *dma_as; + MemoryRegion dma_iomem; +}; + +struct FWCfgIoState { + /*< private >*/ + FWCfgState parent_obj; + /*< public >*/ + + MemoryRegion comb_iomem; +}; + +struct FWCfgMemState { + /*< private >*/ + FWCfgState parent_obj; + /*< public >*/ + + MemoryRegion ctl_iomem, data_iomem; + uint32_t data_width; + MemoryRegionOps wide_data_ops; +}; + /** * fw_cfg_add_bytes: * @s: fw_cfg device being modified diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index f745d5f..2db2918 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -30,6 +30,7 @@ typedef struct DisplaySurface DisplaySurface; typedef struct DriveInfo DriveInfo; typedef struct Error Error; typedef struct EventNotifier EventNotifier; +typedef struct FWCfgEntry FWCfgEntry; typedef struct FWCfgIoState FWCfgIoState; typedef struct FWCfgMemState FWCfgMemState; typedef struct FWCfgState FWCfgState;
By exposing FWCfgIoState and FWCfgMemState internals we allow the possibility for the internal MemoryRegion fields to be mapped by name for boards that wish to wire up the fw_cfg device themselves. An additional minor tidy-up is that the FWCfgEntry typedef is moved from the struct definitions in fw_cfg.h to typedefs.h along with the others. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/nvram/fw_cfg.c | 55 ------------------------------------------ include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++++++++++++ include/qemu/typedefs.h | 1 + 3 files changed, 59 insertions(+), 55 deletions(-)