[PATCH 0/5] hw/nvram/fw_cfg: Document and tighten up register layouts

[PATCH 0/5] hw/nvram/fw_cfg: Document and tighten up register layouts

[Peter Maydell]

We support the fw_cfg device on more architectures and machines that
we let on about in the documentation.  Luckily most of the new ones
(notably riscv and loongarch) have followed the straightforward layout
that the Arm virt board picked. Allowing every machine type to have
its own special-snowflake register layout has no particular benefits
and tends to result in guest OSes accumulating ifdef ladders to deal
with all the unnecessary variation.
    
This patchset:
 * updates the docs to present the arm/riscv/loongarch memory
   mapped layout as the "standard" layout to be used by all
   new architectures/machines in future
 * adds the layouts used by various existing boards that we
   didn't document (PA-RISC, SPARC, PPC and MIPS)
 * tightens up fw_cfg_init_mem_dma() and fw_cfg_init_io_dma()
   functions so that they don't provide flexibility to the
   caller to pick their own weird new layout. None of the
   callers were using this flexibility, so it's better to
   have fw_cfg_init_mem_dma() give the "standard MMIO" layout
   and fw_cfg_init_io_dma() give the x86 ioport layout.

thanks
-- PMM

Peter Maydell (5):
  docs/specs/fw_cfg: Document all architecture register layouts
  hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()
  hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports
  hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA
  hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()

 docs/specs/fw_cfg.rst     | 28 ++++++++++++++++---
 hw/arm/virt.c             |  2 +-
 hw/i386/fw_cfg.c          |  3 +-
 hw/i386/microvm.c         |  3 +-
 hw/i386/pc.c              |  3 +-
 hw/loongarch/fw_cfg.c     |  3 +-
 hw/nvram/fw_cfg.c         | 22 ++++++---------
 hw/riscv/virt.c           |  3 +-
 include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++----
 9 files changed, 92 insertions(+), 33 deletions(-)

-- 
2.43.0

[PATCH 1/5] docs/specs/fw_cfg: Document all architecture register layouts

[Peter Maydell]

We implement the fw_cfg device for more architectures and machines
that we let on about in our documentation.  Luckily most of the new
ones (notably riscv and loongarch) have followed the straightforward
layout that the Arm virt board picked.

Restructure the documentation to present this as the "standard"
layout, followed by the other layouts used by various other boards
for historical reasons.  This adds PA-RISC, SPARC, PPC and MIPS.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/specs/fw_cfg.rst | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/docs/specs/fw_cfg.rst b/docs/specs/fw_cfg.rst
index 31ae31576b..7e2fe0851d 100644
--- a/docs/specs/fw_cfg.rst
+++ b/docs/specs/fw_cfg.rst
@@ -84,15 +84,35 @@ increasing address order, similar to memcpy().
 Register Locations
 ------------------
 
+For a memory-mapped fw_cfg device, the standard register layout is:
+
+    * base address : Data Register (64 bit)
+    * base address + 8 : Selector Register (16 bit)
+    * base address + 16 : DMA Address Register (64 bit)
+
+Some architectures or machines have a different layout for historical reasons:
+
 x86, x86_64
     * Selector Register IOport: 0x510
     * Data Register IOport:     0x511
     * DMA Address IOport:       0x514
 
-Arm
-    * Selector Register address: Base + 8 (2 bytes)
-    * Data Register address:     Base + 0 (8 bytes)
-    * DMA Address address:       Base + 16 (8 bytes)
+PA-RISC:
+    * base address : Selector Register (16 bit)
+    * base address + 4 : Data Register (8 bit)
+
+32-bit SPARC, PPC ``g3beige``, ``mac99``, ``prep``:
+   * base address : Selector Register (16 bit)
+   * base address + 2 : Data Register (8 bit)
+
+64-bit SPARC:
+    * base address : Selector Register (16 bit)
+    * base address + 1 : Data Register (8 bit)
+
+MIPS ``loongson3-virt`` machine:
+    * base address : Selector Register (16 bit)
+    * base address + 8 : Data Register (64 bit)
+
 
 ACPI Interface
 --------------
-- 
2.43.0

[PATCH 2/5] hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()

[Peter Maydell]

Currently fw_cfg_init_mem_dma() allows the caller to customize the
register layout, by specifying separately the offsets for control,
data and DMA registers, plus the width of the data register.

In practice, all the boards using this function specify the same
standard layout: "base + 8, base, 8, base + 16", meaning that the
data register is 8 bytes and the registers are data at offset 0,
control/selector at offset 8, and DMA at offset 16.

Allowing every board to be different is gratuitous and useless
variation which leads to code in guest OSes having architecture
ifdeffery to cope with it.  Avoid potentially introducing any more of
this by removing all the arguments from fw_cfg_init_mem_dma(), so
that the callers only specify the base address.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/virt.c             |  2 +-
 hw/loongarch/fw_cfg.c     |  3 +--
 hw/nvram/fw_cfg.c         | 10 ++++------
 hw/riscv/virt.c           |  3 +--
 include/hw/nvram/fw_cfg.h | 23 ++++++++++++++++++++---
 5 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b090233893..2cd1c639f5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1941,7 +1941,7 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
     FWCfgState *fw_cfg;
     char *nodename;
 
-    fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16, as);
+    fw_cfg = fw_cfg_init_mem_dma(base, as);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
 
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
diff --git a/hw/loongarch/fw_cfg.c b/hw/loongarch/fw_cfg.c
index d2a79efbf7..4c976ce1e5 100644
--- a/hw/loongarch/fw_cfg.c
+++ b/hw/loongarch/fw_cfg.c
@@ -23,8 +23,7 @@ FWCfgState *virt_fw_cfg_init(ram_addr_t ram_size, MachineState *ms)
     int max_cpus = ms->smp.max_cpus;
     int smp_cpus = ms->smp.cpus;
 
-    fw_cfg = fw_cfg_init_mem_dma(VIRT_FWCFG_BASE + 8, VIRT_FWCFG_BASE, 8,
-                                 VIRT_FWCFG_BASE + 16, &address_space_memory);
+    fw_cfg = fw_cfg_init_mem_dma(VIRT_FWCFG_BASE, &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 1d7d835421..59cf92293c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1088,13 +1088,11 @@ static FWCfgState *fw_cfg_init_mem_internal(hwaddr ctl_addr,
     return s;
 }
 
-FWCfgState *fw_cfg_init_mem_dma(hwaddr ctl_addr,
-                                hwaddr data_addr, uint32_t data_width,
-                                hwaddr dma_addr, AddressSpace *dma_as)
+FWCfgState *fw_cfg_init_mem_dma(hwaddr base_addr, AddressSpace *dma_as)
 {
-    assert(dma_addr && dma_as);
-    return fw_cfg_init_mem_internal(ctl_addr, data_addr, data_width,
-                                    dma_addr, dma_as);
+    assert(dma_as);
+    return fw_cfg_init_mem_internal(base_addr + 8, base_addr, 8,
+                                    base_addr + 16, dma_as);
 }
 
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ce64eaaef7..a10840a81d 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1266,8 +1266,7 @@ static FWCfgState *create_fw_cfg(const MachineState *ms, hwaddr base)
 {
     FWCfgState *fw_cfg;
 
-    fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16,
-                                 &address_space_memory);
+    fw_cfg = fw_cfg_init_mem_dma(base, &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
 
     return fw_cfg;
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 56f17a0bdc..45a3747908 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -309,9 +309,26 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
                                 AddressSpace *dma_as);
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
                                   unsigned data_width);
-FWCfgState *fw_cfg_init_mem_dma(hwaddr ctl_addr,
-                                hwaddr data_addr, uint32_t data_width,
-                                hwaddr dma_addr, AddressSpace *dma_as);
+/**
+ * fw_cfg_init_mem_dma:
+ * @base_addr: address to map the device at
+ * @as: the device will do DMA to/from this AddressSpace
+ *
+ * Create and map a fw_cfg device at the specified base address.
+ *
+ * This always creates a device with DMA support, and the "standard"
+ * register layout:
+ *  - offset 0 : data, 64 bits
+ *  - offset 8 : selector, 16 bits
+ *  - offset 16 : DMA address, 64 bits
+ *
+ * The device will be created, configured and realized, and its
+ * memory regions for the registers will be mapped at the specified
+ * address.
+ *
+ * Returns the device object.
+ */
+FWCfgState *fw_cfg_init_mem_dma(hwaddr base_addr, AddressSpace *dma_as);
 
 FWCfgState *fw_cfg_find(void);
 bool fw_cfg_dma_enabled(void *opaque);
-- 
2.43.0

[PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Peter Maydell]

The fw_cfg_init_io_dma() function allows the caller to specify the
base port number of the selector/data register and the base port
number of the DMA address register separately. No caller actually
uses this: they all pass in base + 4 for the dma_iobase.

To reduce the risk of unnecessary variation in what different x86
machine types use as their fw_cfg register layout, remove the
dma_iobase argument from fw_cfg_init_io_dma(), and have the function
always use the same "DMA port is base port + 4" layout.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/i386/fw_cfg.c          |  3 +--
 hw/i386/microvm.c         |  3 +--
 hw/i386/pc.c              |  3 +--
 hw/nvram/fw_cfg.c         |  8 ++++----
 include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 2876490f06..d422302c1c 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -127,8 +127,7 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
     const CPUArchIdList *cpus = mc->possible_cpu_arch_ids(ms);
     int nb_numa_nodes = ms->numa_state->num_nodes;
 
-    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
-                                &address_space_memory);
+    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
 
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 779741ec76..e7adab7d2e 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -320,8 +320,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
         e820_add_entry(0x100000000ULL, x86ms->above_4g_mem_size, E820_RAM);
     }
 
-    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
-                                &address_space_memory);
+    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, &address_space_memory);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, machine->smp.max_cpus);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2ecad3c503..f9d8990d1d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -568,8 +568,7 @@ void xen_load_linux(PCMachineState *pcms)
 
     assert(MACHINE(pcms)->kernel_filename != NULL);
 
-    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
-                                &address_space_memory);
+    fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
     rom_set_fw(fw_cfg);
 
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 59cf92293c..f68191553b 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1019,15 +1019,14 @@ static void fw_cfg_common_realize(DeviceState *dev, Error **errp)
     qemu_add_machine_init_done_notifier(&s->machine_ready);
 }
 
-FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
-                                AddressSpace *dma_as)
+FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, AddressSpace *dma_as)
 {
     DeviceState *dev;
     SysBusDevice *sbd;
     FWCfgIoState *ios;
     FWCfgState *s;
     MemoryRegion *iomem = get_system_io();
-    bool dma_requested = dma_iobase && dma_as;
+    bool dma_requested = dma_as;
 
     dev = qdev_new(TYPE_FW_CFG_IO);
     if (!dma_requested) {
@@ -1048,7 +1047,8 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
         /* 64 bits for the address field */
         s->dma_as = dma_as;
         s->dma_addr = 0;
-        memory_region_add_subregion(iomem, dma_iobase, &s->dma_iomem);
+        /* DMA register ioport is always at base + 4 */
+        memory_region_add_subregion(iomem, iobase + 4, &s->dma_iomem);
     }
 
     return s;
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 45a3747908..be3fb5f8aa 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
                                     Object *parent, const char *part,
                                     const char *filename, Error **errp);
 
-FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
-                                AddressSpace *dma_as);
+/**
+ * fw_cfg_init_io_dma:
+ * @iobase: x86 port number which is the base of the fw_cfg port range
+ * @dma_as: the device will do DMA to/from this AddressSpace
+ *
+ * Create a fw_cfg device and map it into the specified I/O port range.
+ *
+ * This creates a device with the x86 PC standard port I/O layout:
+ * - Selector Register IOport: @iobase
+ * - Data Register IOport:     @iobase + 1
+ * - DMA Address IOport:       @iobase + 4
+ *
+ * Returns the device object.
+ */
+FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, AddressSpace *dma_as);
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
                                   unsigned data_width);
 /**
-- 
2.43.0

[PATCH 4/5] hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA

[Peter Maydell]

Currently fw_cfg_init_io_dma() allows the caller to pass a NULL
dma_as argument, which causes it to create a fw_cfg without
the DMA port or DMA support. None of the callers use this
capability: they all pass &address_space_memory.

We don't really want to leave the door open for some future x86
machine type which doesn't support DMA for the fw_cfg device, so
remove this, and instead make the function assert that it has a
non-NULL dma_as argument, like fw_cfg_init_mem_dma().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/nvram/fw_cfg.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index f68191553b..a9d45adb2d 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1026,12 +1026,10 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, AddressSpace *dma_as)
     FWCfgIoState *ios;
     FWCfgState *s;
     MemoryRegion *iomem = get_system_io();
-    bool dma_requested = dma_as;
+
+    assert(dma_as);
 
     dev = qdev_new(TYPE_FW_CFG_IO);
-    if (!dma_requested) {
-        qdev_prop_set_bit(dev, "dma_enabled", false);
-    }
 
     object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
                               OBJECT(dev));
-- 
2.43.0

[PATCH 5/5] hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()

[Peter Maydell]

The last few commits have added doc comments for all the fw_cfg_init*
functions except for fw_cfg_init_mem_nodma().  Fill in the gap by
adding a doc comment for it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/nvram/fw_cfg.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index be3fb5f8aa..b75858025f 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -320,6 +320,24 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
  * Returns the device object.
  */
 FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, AddressSpace *dma_as);
+
+/**
+ * fw_cfg_init_mem_nodma:
+ *
+ * @ctl_addr: address of the selector register
+ * @data_addr: address of the data address
+ * @data_width: width of the data register in bytes
+ *
+ * Create a fw_cfg device without DMA support, and map its
+ * registers at the specified addresses.
+ *
+ * Do not use this function in code for a board type that didn't
+ * already support the fw_cfg device. All new board types should
+ * include DMA support and use the standard register layout -- use
+ * fw_cfg_init_mem_dma() instead.
+ *
+ * Returns the device object.
+ */
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
                                   unsigned data_width);
 /**
-- 
2.43.0

Re: [PATCH 0/5] hw/nvram/fw_cfg: Document and tighten up register layouts

[Richard Henderson]

On 5/29/26 10:46, Peter Maydell wrote:
> Peter Maydell (5):
>    docs/specs/fw_cfg: Document all architecture register layouts
>    hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()
>    hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports
>    hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA
>    hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 5/5] hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()

[Philippe Mathieu-Daudé]

On 29/5/26 19:46, Peter Maydell wrote:
> The last few commits have added doc comments for all the fw_cfg_init*
> functions except for fw_cfg_init_mem_nodma().  Fill in the gap by
> adding a doc comment for it.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/nvram/fw_cfg.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>

Re: [PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Philippe Mathieu-Daudé]

On 29/5/26 19:46, Peter Maydell wrote:
> The fw_cfg_init_io_dma() function allows the caller to specify the
> base port number of the selector/data register and the base port
> number of the DMA address register separately. No caller actually
> uses this: they all pass in base + 4 for the dma_iobase.
> 
> To reduce the risk of unnecessary variation in what different x86
> machine types use as their fw_cfg register layout, remove the
> dma_iobase argument from fw_cfg_init_io_dma(), and have the function
> always use the same "DMA port is base port + 4" layout.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/i386/fw_cfg.c          |  3 +--
>   hw/i386/microvm.c         |  3 +--
>   hw/i386/pc.c              |  3 +--
>   hw/nvram/fw_cfg.c         |  8 ++++----
>   include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
>   5 files changed, 22 insertions(+), 12 deletions(-)


> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 45a3747908..be3fb5f8aa 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
>                                       Object *parent, const char *part,
>                                       const char *filename, Error **errp);
>   
> -FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
> -                                AddressSpace *dma_as);
> +/**
> + * fw_cfg_init_io_dma:
> + * @iobase: x86 port number which is the base of the fw_cfg port range

s/x86/ISA/?

Anyway,
Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>

> + * @dma_as: the device will do DMA to/from this AddressSpace
> + *
> + * Create a fw_cfg device and map it into the specified I/O port range.
> + *
> + * This creates a device with the x86 PC standard port I/O layout:
> + * - Selector Register IOport: @iobase
> + * - Data Register IOport:     @iobase + 1
> + * - DMA Address IOport:       @iobase + 4
> + *
> + * Returns the device object.
> + */
> +FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, AddressSpace *dma_as);
>   FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
>                                     unsigned data_width);
>   /**

Re: [PATCH 1/5] docs/specs/fw_cfg: Document all architecture register layouts

[Philippe Mathieu-Daudé]

Hi Peter,

On 29/5/26 19:46, Peter Maydell wrote:
> We implement the fw_cfg device for more architectures and machines
> that we let on about in our documentation.  Luckily most of the new
> ones (notably riscv and loongarch) have followed the straightforward
> layout that the Arm virt board picked.
> 
> Restructure the documentation to present this as the "standard"
> layout, followed by the other layouts used by various other boards
> for historical reasons.  This adds PA-RISC, SPARC, PPC and MIPS.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   docs/specs/fw_cfg.rst | 28 ++++++++++++++++++++++++----
>   1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/specs/fw_cfg.rst b/docs/specs/fw_cfg.rst
> index 31ae31576b..7e2fe0851d 100644
> --- a/docs/specs/fw_cfg.rst
> +++ b/docs/specs/fw_cfg.rst
> @@ -84,15 +84,35 @@ increasing address order, similar to memcpy().
>   Register Locations
>   ------------------
>   
> +For a memory-mapped fw_cfg device, the standard register layout is:
> +
> +    * base address : Data Register (64 bit)
> +    * base address + 8 : Selector Register (16 bit)
> +    * base address + 16 : DMA Address Register (64 bit)
> +
> +Some architectures or machines have a different layout for historical reasons:
> +
>   x86, x86_64
>       * Selector Register IOport: 0x510
>       * Data Register IOport:     0x511
>       * DMA Address IOport:       0x514
> +64-bit SPARC:
> +    * base address : Selector Register (16 bit)
> +    * base address + 1 : Data Register (8 bit)

IIUC they have both the same base address.

Re: [PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Peter Maydell]

On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
>
> On 29/5/26 19:46, Peter Maydell wrote:
> > The fw_cfg_init_io_dma() function allows the caller to specify the
> > base port number of the selector/data register and the base port
> > number of the DMA address register separately. No caller actually
> > uses this: they all pass in base + 4 for the dma_iobase.
> >
> > To reduce the risk of unnecessary variation in what different x86
> > machine types use as their fw_cfg register layout, remove the
> > dma_iobase argument from fw_cfg_init_io_dma(), and have the function
> > always use the same "DMA port is base port + 4" layout.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/i386/fw_cfg.c          |  3 +--
> >   hw/i386/microvm.c         |  3 +--
> >   hw/i386/pc.c              |  3 +--
> >   hw/nvram/fw_cfg.c         |  8 ++++----
> >   include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
> >   5 files changed, 22 insertions(+), 12 deletions(-)
>
>
> > diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> > index 45a3747908..be3fb5f8aa 100644
> > --- a/include/hw/nvram/fw_cfg.h
> > +++ b/include/hw/nvram/fw_cfg.h
> > @@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
> >                                       Object *parent, const char *part,
> >                                       const char *filename, Error **errp);
> >
> > -FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
> > -                                AddressSpace *dma_as);
> > +/**
> > + * fw_cfg_init_io_dma:
> > + * @iobase: x86 port number which is the base of the fw_cfg port range
>
> s/x86/ISA/?

This is specifically x86, because we use this where we're
wiring fw_cfg up to the x86 in/out port instructions. There's
no ISA-bus version of fw_cfg. Anything other than x86 should
be using the memory-mapped fw_cfg.

-- PMM

Re: [PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Philippe Mathieu-Daudé]

On 8/6/26 11:12, Peter Maydell wrote:
> On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
>>
>> On 29/5/26 19:46, Peter Maydell wrote:
>>> The fw_cfg_init_io_dma() function allows the caller to specify the
>>> base port number of the selector/data register and the base port
>>> number of the DMA address register separately. No caller actually
>>> uses this: they all pass in base + 4 for the dma_iobase.
>>>
>>> To reduce the risk of unnecessary variation in what different x86
>>> machine types use as their fw_cfg register layout, remove the
>>> dma_iobase argument from fw_cfg_init_io_dma(), and have the function
>>> always use the same "DMA port is base port + 4" layout.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>    hw/i386/fw_cfg.c          |  3 +--
>>>    hw/i386/microvm.c         |  3 +--
>>>    hw/i386/pc.c              |  3 +--
>>>    hw/nvram/fw_cfg.c         |  8 ++++----
>>>    include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
>>>    5 files changed, 22 insertions(+), 12 deletions(-)
>>
>>
>>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
>>> index 45a3747908..be3fb5f8aa 100644
>>> --- a/include/hw/nvram/fw_cfg.h
>>> +++ b/include/hw/nvram/fw_cfg.h
>>> @@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
>>>                                        Object *parent, const char *part,
>>>                                        const char *filename, Error **errp);
>>>
>>> -FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
>>> -                                AddressSpace *dma_as);
>>> +/**
>>> + * fw_cfg_init_io_dma:
>>> + * @iobase: x86 port number which is the base of the fw_cfg port range
>>
>> s/x86/ISA/?
> 
> This is specifically x86, because we use this where we're
> wiring fw_cfg up to the x86 in/out port instructions. There's
> no ISA-bus version of fw_cfg. Anything other than x86 should
> be using the memory-mapped fw_cfg.

But this "x86 port number" is really a address on the ISA I/O bus.

Maybe make explicit we want this method restricted to x86 instead?

"ISA port number which is the base of the fw_cfg port range (restricted 
to x86 targets)."

Regards,

Phil.

Re: [PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Peter Maydell]

On Mon, 8 Jun 2026 at 17:02, Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
>
> On 8/6/26 11:12, Peter Maydell wrote:
> > On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
> >>
> >> On 29/5/26 19:46, Peter Maydell wrote:
> >>> The fw_cfg_init_io_dma() function allows the caller to specify the
> >>> base port number of the selector/data register and the base port
> >>> number of the DMA address register separately. No caller actually
> >>> uses this: they all pass in base + 4 for the dma_iobase.
> >>>
> >>> To reduce the risk of unnecessary variation in what different x86
> >>> machine types use as their fw_cfg register layout, remove the
> >>> dma_iobase argument from fw_cfg_init_io_dma(), and have the function
> >>> always use the same "DMA port is base port + 4" layout.
> >>>
> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>> ---
> >>>    hw/i386/fw_cfg.c          |  3 +--
> >>>    hw/i386/microvm.c         |  3 +--
> >>>    hw/i386/pc.c              |  3 +--
> >>>    hw/nvram/fw_cfg.c         |  8 ++++----
> >>>    include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
> >>>    5 files changed, 22 insertions(+), 12 deletions(-)
> >>
> >>
> >>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> >>> index 45a3747908..be3fb5f8aa 100644
> >>> --- a/include/hw/nvram/fw_cfg.h
> >>> +++ b/include/hw/nvram/fw_cfg.h
> >>> @@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
> >>>                                        Object *parent, const char *part,
> >>>                                        const char *filename, Error **errp);
> >>>
> >>> -FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
> >>> -                                AddressSpace *dma_as);
> >>> +/**
> >>> + * fw_cfg_init_io_dma:
> >>> + * @iobase: x86 port number which is the base of the fw_cfg port range
> >>
> >> s/x86/ISA/?
> >
> > This is specifically x86, because we use this where we're
> > wiring fw_cfg up to the x86 in/out port instructions. There's
> > no ISA-bus version of fw_cfg. Anything other than x86 should
> > be using the memory-mapped fw_cfg.
>
> But this "x86 port number" is really a address on the ISA I/O bus.
>
> Maybe make explicit we want this method restricted to x86 instead?

I do, which is why I say "x86 port number".

> "ISA port number which is the base of the fw_cfg port range (restricted
> to x86 targets)."

This seems more confusing to me because it brings ISA into it.
We don't care about ISA or not ISA, we only care that this is
an x86 I/O port. It happens that on the PC the x86 I/O ports
can be handled by ISA cards, but it's not relevant here.
In particular, the fw_cfg device is not an ISA device.

thanks
-- PMM

Re: [PATCH 3/5] hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports

[Philippe Mathieu-Daudé]

On 8/6/26 18:11, Peter Maydell wrote:
> On Mon, 8 Jun 2026 at 17:02, Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
>>
>> On 8/6/26 11:12, Peter Maydell wrote:
>>> On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
>>>>
>>>> On 29/5/26 19:46, Peter Maydell wrote:
>>>>> The fw_cfg_init_io_dma() function allows the caller to specify the
>>>>> base port number of the selector/data register and the base port
>>>>> number of the DMA address register separately. No caller actually
>>>>> uses this: they all pass in base + 4 for the dma_iobase.
>>>>>
>>>>> To reduce the risk of unnecessary variation in what different x86
>>>>> machine types use as their fw_cfg register layout, remove the
>>>>> dma_iobase argument from fw_cfg_init_io_dma(), and have the function
>>>>> always use the same "DMA port is base port + 4" layout.
>>>>>
>>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>> ---
>>>>>     hw/i386/fw_cfg.c          |  3 +--
>>>>>     hw/i386/microvm.c         |  3 +--
>>>>>     hw/i386/pc.c              |  3 +--
>>>>>     hw/nvram/fw_cfg.c         |  8 ++++----
>>>>>     include/hw/nvram/fw_cfg.h | 17 +++++++++++++++--
>>>>>     5 files changed, 22 insertions(+), 12 deletions(-)
>>>>
>>>>
>>>>> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
>>>>> index 45a3747908..be3fb5f8aa 100644
>>>>> --- a/include/hw/nvram/fw_cfg.h
>>>>> +++ b/include/hw/nvram/fw_cfg.h
>>>>> @@ -305,8 +305,21 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
>>>>>                                         Object *parent, const char *part,
>>>>>                                         const char *filename, Error **errp);
>>>>>
>>>>> -FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
>>>>> -                                AddressSpace *dma_as);
>>>>> +/**
>>>>> + * fw_cfg_init_io_dma:
>>>>> + * @iobase: x86 port number which is the base of the fw_cfg port range
>>>>
>>>> s/x86/ISA/?
>>>
>>> This is specifically x86, because we use this where we're
>>> wiring fw_cfg up to the x86 in/out port instructions. There's
>>> no ISA-bus version of fw_cfg. Anything other than x86 should
>>> be using the memory-mapped fw_cfg.
>>
>> But this "x86 port number" is really a address on the ISA I/O bus.
>>
>> Maybe make explicit we want this method restricted to x86 instead?
> 
> I do, which is why I say "x86 port number".
> 
>> "ISA port number which is the base of the fw_cfg port range (restricted
>> to x86 targets)."
> 
> This seems more confusing to me because it brings ISA into it.
> We don't care about ISA or not ISA, we only care that this is
> an x86 I/O port. It happens that on the PC the x86 I/O ports
> can be handled by ISA cards, but it's not relevant here.
> In particular, the fw_cfg device is not an ISA device.

Fine.

Re: [PATCH 1/5] docs/specs/fw_cfg: Document all architecture register layouts

[Peter Maydell]

On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
>
> Hi Peter,
>
> On 29/5/26 19:46, Peter Maydell wrote:
> > We implement the fw_cfg device for more architectures and machines
> > that we let on about in our documentation.  Luckily most of the new
> > ones (notably riscv and loongarch) have followed the straightforward
> > layout that the Arm virt board picked.
> >
> > Restructure the documentation to present this as the "standard"
> > layout, followed by the other layouts used by various other boards
> > for historical reasons.  This adds PA-RISC, SPARC, PPC and MIPS.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   docs/specs/fw_cfg.rst | 28 ++++++++++++++++++++++++----
> >   1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/docs/specs/fw_cfg.rst b/docs/specs/fw_cfg.rst
> > index 31ae31576b..7e2fe0851d 100644
> > --- a/docs/specs/fw_cfg.rst
> > +++ b/docs/specs/fw_cfg.rst
> > @@ -84,15 +84,35 @@ increasing address order, similar to memcpy().
> >   Register Locations
> >   ------------------
> >
> > +For a memory-mapped fw_cfg device, the standard register layout is:
> > +
> > +    * base address : Data Register (64 bit)
> > +    * base address + 8 : Selector Register (16 bit)
> > +    * base address + 16 : DMA Address Register (64 bit)
> > +
> > +Some architectures or machines have a different layout for historical reasons:
> > +
> >   x86, x86_64
> >       * Selector Register IOport: 0x510
> >       * Data Register IOport:     0x511
> >       * DMA Address IOport:       0x514
> > +64-bit SPARC:
> > +    * base address : Selector Register (16 bit)
> > +    * base address + 1 : Data Register (8 bit)
>
> IIUC they have both the same base address.

This one's confusing, because the code for both sparc64 and x86
fw_cfg is more lax than what we document.

The sun4u code does:
    qdev_prop_set_bit(dev, "dma_enabled", false);
and:
    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
                                &FW_CFG_IO(dev)->comb_iomem);

so it uses the fw_cfg's comb_iomem MemoryRegion. This is the
same thing the x86 ioport version uses (except that for sparc64 we
disable the DMA register), so it's the same layout, with
the selector register at the base address and the data register
nominally at an offset 1 greater than that.

The implementation in fw_cfg.c (fw_cfg_comb_mem_ops) doesn't actually
check the address, only the size of data written, and dispatches like this:
 - 1 byte writes are ignored (they used to be data writes in ancient QEMU)
 - 2 byte writes are selector register writes
 - 1 byte reads are data register reads
 - everything else is rejected by the .valid.accepts function
and you can use either baseaddr or baseaddr+1 for any of that.

What I opted to document here is (a) the same thing we already
document for the x86 ioport interface, which uses comb_iomem
and so also is not actually checking the address values, and
(b) what the Linux driver in drivers/firmware/qemu_fw_cfg.c uses:

# elif (defined(CONFIG_X86) || defined(CONFIG_SPARC64)) /* x86, sun4u */
#  define FW_CFG_CTRL_OFF 0x00
#  define FW_CFG_DATA_OFF 0x01
#  define FW_CFG_DMA_OFF 0x04
# else

We could document the lax conditions that our implementation
actually enforces for both sparc and x86, I guess, but they
felt to me more like an accident of implementation. What do
you think?

thanks
-- PMM

Re: [PATCH 1/5] docs/specs/fw_cfg: Document all architecture register layouts

[Philippe Mathieu-Daudé]

On 16/6/26 11:29, Peter Maydell wrote:
> On Wed, 3 Jun 2026 at 08:08, Philippe Mathieu-Daudé <philmd@mailo.com> wrote:
>>
>> Hi Peter,
>>
>> On 29/5/26 19:46, Peter Maydell wrote:
>>> We implement the fw_cfg device for more architectures and machines
>>> that we let on about in our documentation.  Luckily most of the new
>>> ones (notably riscv and loongarch) have followed the straightforward
>>> layout that the Arm virt board picked.
>>>
>>> Restructure the documentation to present this as the "standard"
>>> layout, followed by the other layouts used by various other boards
>>> for historical reasons.  This adds PA-RISC, SPARC, PPC and MIPS.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>    docs/specs/fw_cfg.rst | 28 ++++++++++++++++++++++++----
>>>    1 file changed, 24 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/docs/specs/fw_cfg.rst b/docs/specs/fw_cfg.rst
>>> index 31ae31576b..7e2fe0851d 100644
>>> --- a/docs/specs/fw_cfg.rst
>>> +++ b/docs/specs/fw_cfg.rst
>>> @@ -84,15 +84,35 @@ increasing address order, similar to memcpy().
>>>    Register Locations
>>>    ------------------
>>>
>>> +For a memory-mapped fw_cfg device, the standard register layout is:
>>> +
>>> +    * base address : Data Register (64 bit)
>>> +    * base address + 8 : Selector Register (16 bit)
>>> +    * base address + 16 : DMA Address Register (64 bit)
>>> +
>>> +Some architectures or machines have a different layout for historical reasons:
>>> +
>>>    x86, x86_64
>>>        * Selector Register IOport: 0x510
>>>        * Data Register IOport:     0x511
>>>        * DMA Address IOport:       0x514
>>> +64-bit SPARC:
>>> +    * base address : Selector Register (16 bit)
>>> +    * base address + 1 : Data Register (8 bit)
>>
>> IIUC they have both the same base address.
> 
> This one's confusing, because the code for both sparc64 and x86
> fw_cfg is more lax than what we document.
> 
> The sun4u code does:
>      qdev_prop_set_bit(dev, "dma_enabled", false);
> and:
>      memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>                                  &FW_CFG_IO(dev)->comb_iomem);
> 
> so it uses the fw_cfg's comb_iomem MemoryRegion. This is the
> same thing the x86 ioport version uses (except that for sparc64 we
> disable the DMA register), so it's the same layout, with
> the selector register at the base address and the data register
> nominally at an offset 1 greater than that.
> 
> The implementation in fw_cfg.c (fw_cfg_comb_mem_ops) doesn't actually
> check the address, only the size of data written, and dispatches like this:
>   - 1 byte writes are ignored (they used to be data writes in ancient QEMU)
>   - 2 byte writes are selector register writes
>   - 1 byte reads are data register reads
>   - everything else is rejected by the .valid.accepts function
> and you can use either baseaddr or baseaddr+1 for any of that.
> 
> What I opted to document here is (a) the same thing we already
> document for the x86 ioport interface, which uses comb_iomem
> and so also is not actually checking the address values, and
> (b) what the Linux driver in drivers/firmware/qemu_fw_cfg.c uses:
> 
> # elif (defined(CONFIG_X86) || defined(CONFIG_SPARC64)) /* x86, sun4u */
> #  define FW_CFG_CTRL_OFF 0x00
> #  define FW_CFG_DATA_OFF 0x01
> #  define FW_CFG_DMA_OFF 0x04
> # else
> 
> We could document the lax conditions that our implementation
> actually enforces for both sparc and x86, I guess, but they
> felt to me more like an accident of implementation. What do
> you think?

Looking at it as an accident of the implementation, we are good
indeed. Thanks for the clarification.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

> 
> thanks
> -- PMM
> 

Re: [PATCH 2/5] hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()

[Philippe Mathieu-Daudé]

On 29/5/26 19:46, Peter Maydell wrote:
> Currently fw_cfg_init_mem_dma() allows the caller to customize the
> register layout, by specifying separately the offsets for control,
> data and DMA registers, plus the width of the data register.
> 
> In practice, all the boards using this function specify the same
> standard layout: "base + 8, base, 8, base + 16", meaning that the
> data register is 8 bytes and the registers are data at offset 0,
> control/selector at offset 8, and DMA at offset 16.
> 
> Allowing every board to be different is gratuitous and useless
> variation which leads to code in guest OSes having architecture
> ifdeffery to cope with it.  Avoid potentially introducing any more of
> this by removing all the arguments from fw_cfg_init_mem_dma(), so
> that the callers only specify the base address.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/virt.c             |  2 +-
>   hw/loongarch/fw_cfg.c     |  3 +--
>   hw/nvram/fw_cfg.c         | 10 ++++------
>   hw/riscv/virt.c           |  3 +--
>   include/hw/nvram/fw_cfg.h | 23 ++++++++++++++++++++---
>   5 files changed, 27 insertions(+), 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 4/5] hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA

[Philippe Mathieu-Daudé]

On 29/5/26 19:46, Peter Maydell wrote:
> Currently fw_cfg_init_io_dma() allows the caller to pass a NULL
> dma_as argument, which causes it to create a fw_cfg without
> the DMA port or DMA support. None of the callers use this
> capability: they all pass &address_space_memory.
> 
> We don't really want to leave the door open for some future x86
> machine type which doesn't support DMA for the fw_cfg device, so
> remove this, and instead make the function assert that it has a
> non-NULL dma_as argument, like fw_cfg_init_mem_dma().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/nvram/fw_cfg.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH 0/5] hw/nvram/fw_cfg: Document and tighten up register layouts

[Peter Maydell]

These have all now been reviewed. Since fw_cfg is marked as
Orphaned in MAINTAINERS, I'm going to take the series via
target-arm.next. Let me know if anybody would prefer something else.

thanks
-- PMM

On Fri, 29 May 2026 at 18:46, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> We support the fw_cfg device on more architectures and machines that
> we let on about in the documentation.  Luckily most of the new ones
> (notably riscv and loongarch) have followed the straightforward layout
> that the Arm virt board picked. Allowing every machine type to have
> its own special-snowflake register layout has no particular benefits
> and tends to result in guest OSes accumulating ifdef ladders to deal
> with all the unnecessary variation.
>
> This patchset:
>  * updates the docs to present the arm/riscv/loongarch memory
>    mapped layout as the "standard" layout to be used by all
>    new architectures/machines in future
>  * adds the layouts used by various existing boards that we
>    didn't document (PA-RISC, SPARC, PPC and MIPS)
>  * tightens up fw_cfg_init_mem_dma() and fw_cfg_init_io_dma()
>    functions so that they don't provide flexibility to the
>    caller to pick their own weird new layout. None of the
>    callers were using this flexibility, so it's better to
>    have fw_cfg_init_mem_dma() give the "standard MMIO" layout
>    and fw_cfg_init_io_dma() give the x86 ioport layout.
>
> thanks
> -- PMM
>
> Peter Maydell (5):
>   docs/specs/fw_cfg: Document all architecture register layouts
>   hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()
>   hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports
>   hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA
>   hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()
>
>  docs/specs/fw_cfg.rst     | 28 ++++++++++++++++---
>  hw/arm/virt.c             |  2 +-
>  hw/i386/fw_cfg.c          |  3 +-
>  hw/i386/microvm.c         |  3 +-
>  hw/i386/pc.c              |  3 +-
>  hw/loongarch/fw_cfg.c     |  3 +-
>  hw/nvram/fw_cfg.c         | 22 ++++++---------
>  hw/riscv/virt.c           |  3 +-
>  include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++----
>  9 files changed, 92 insertions(+), 33 deletions(-)

Re: [PATCH 0/5] hw/nvram/fw_cfg: Document and tighten up register layouts

[Michael S. Tsirkin]

On Fri, Jul 03, 2026 at 10:41:32AM +0100, Peter Maydell wrote:
> These have all now been reviewed. Since fw_cfg is marked as
> Orphaned in MAINTAINERS,

I don't think fw cfg is good as orphaned.
Any takers? If not I guess I will have to add myself there.

> I'm going to take the series via
> target-arm.next. Let me know if anybody would prefer something else.
> 
> thanks
> -- PMM

Pls go ahead:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> On Fri, 29 May 2026 at 18:46, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > We support the fw_cfg device on more architectures and machines that
> > we let on about in the documentation.  Luckily most of the new ones
> > (notably riscv and loongarch) have followed the straightforward layout
> > that the Arm virt board picked. Allowing every machine type to have
> > its own special-snowflake register layout has no particular benefits
> > and tends to result in guest OSes accumulating ifdef ladders to deal
> > with all the unnecessary variation.
> >
> > This patchset:
> >  * updates the docs to present the arm/riscv/loongarch memory
> >    mapped layout as the "standard" layout to be used by all
> >    new architectures/machines in future
> >  * adds the layouts used by various existing boards that we
> >    didn't document (PA-RISC, SPARC, PPC and MIPS)
> >  * tightens up fw_cfg_init_mem_dma() and fw_cfg_init_io_dma()
> >    functions so that they don't provide flexibility to the
> >    caller to pick their own weird new layout. None of the
> >    callers were using this flexibility, so it's better to
> >    have fw_cfg_init_mem_dma() give the "standard MMIO" layout
> >    and fw_cfg_init_io_dma() give the x86 ioport layout.
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (5):
> >   docs/specs/fw_cfg: Document all architecture register layouts
> >   hw/nvram/fw_cfg: Enforce standard layout for fw_cfg_init_mem_dma()
> >   hw/nvram/fw_cfg: Enforce standard layout for x86 fw_cfg I/O ports
> >   hw/nvram/fw_cfg: Remove support for I/O port fw_cfg without DMA
> >   hw/nvram/fw_cfg: Document fw_cfg_init_mem_nodma()
> >
> >  docs/specs/fw_cfg.rst     | 28 ++++++++++++++++---
> >  hw/arm/virt.c             |  2 +-
> >  hw/i386/fw_cfg.c          |  3 +-
> >  hw/i386/microvm.c         |  3 +-
> >  hw/i386/pc.c              |  3 +-
> >  hw/loongarch/fw_cfg.c     |  3 +-
> >  hw/nvram/fw_cfg.c         | 22 ++++++---------
> >  hw/riscv/virt.c           |  3 +-
> >  include/hw/nvram/fw_cfg.h | 58 +++++++++++++++++++++++++++++++++++----
> >  9 files changed, 92 insertions(+), 33 deletions(-)