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;
-
-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 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 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.
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.
© 2016 - 2026 Red Hat, Inc.