[PATCH-for-11.0 v6 00/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[PATCH-for-11.0 v6 00/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Philippe Mathieu-Daudé]

Hi Zhao,

This is my answer to this comment of yours:

 > Although other callers of fw_cfg_init_io_dma() besides x86 also pass
 > DMA arguments to create DMA-enabled FwCfgIoState, the "dma_enabled"
 > property of FwCfgIoState cannot yet be removed, because Sun4u and Sun4v
 > still create DMA-disabled FwCfgIoState (bypass fw_cfg_init_io_dma()) in
 > sun4uv_init() (hw/sparc64/sun4u.c).
 >
 > Maybe reusing fw_cfg_init_io_dma() for them would be a better choice, or
 > adding fw_cfg_init_io_nodma(). However, before that, first simplify the
 > handling of FwCfgState in x86.
 >
 > Considering that FwCfgIoState in x86 enables DMA by default, remove the
 > handling for DMA-disabled cases and replace DMA checks with assertions
 > to ensure that the default DMA-enabled setting is not broken.

My series is to apply just after this patch of your series:

  [PATCH v5 10/28] hw/mips/loongson3_virt: Prefer using fw_cfg_init_mem_nodma()

Regards,

Phil.

Based-on: <20251202162835.3227894-11-zhao1.liu@intel.com>

Philippe Mathieu-Daudé (12):
  hw/ppc/mac: Use fw_cfg_init_mem_nodma()
  hw/sparc/sun4m: Use fw_cfg_init_mem_nodma()
  hw/nvram/fw_cfg: Factor fw_cfg_init_mem_internal() out
  hw/nvram/fw_cfg: Rename fw_cfg_init_mem_wide() ->
    fw_cfg_init_mem_dma()
  hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()
  hw/nvram/fw_cfg: Factor fw_cfg_init_io_internal() out
  hw/nvram/fw_cfg: Add fw_cfg_init_io_nodma() helper
  hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()
  hw/nvram/fw_cfg: Remove fw_cfg_io_properties::dma_enabled
  hw/i386/x86: Remove X86MachineClass::fwcfg_dma_enabled field
  hw/i386/pc: Remove multiboot.bin
  hw/i386: Remove linuxboot.bin

Zhao Liu (1):
  hw/i386: Assume fw_cfg DMA is always enabled

 include/hw/i386/x86.h             |   2 -
 include/hw/nvram/fw_cfg.h         |  11 +-
 pc-bios/optionrom/optionrom.h     |   4 -
 hw/arm/virt.c                     |   2 +-
 hw/i386/fw_cfg.c                  |  19 +--
 hw/i386/microvm.c                 |   6 +-
 hw/i386/multiboot.c               |   7 +-
 hw/i386/pc.c                      |   7 +-
 hw/i386/x86-common.c              |   7 +-
 hw/i386/x86.c                     |   2 -
 hw/loongarch/fw_cfg.c             |   4 +-
 hw/nvram/fw_cfg.c                 |  61 ++++----
 hw/ppc/mac_newworld.c             |  11 +-
 hw/ppc/mac_oldworld.c             |  11 +-
 hw/riscv/virt.c                   |   4 +-
 hw/sparc/sun4m.c                  |  12 +-
 hw/sparc64/sun4u.c                |   9 +-
 pc-bios/meson.build               |   2 -
 pc-bios/multiboot.bin             | Bin 1024 -> 0 bytes
 pc-bios/optionrom/Makefile        |   2 +-
 pc-bios/optionrom/linuxboot.S     | 195 -------------------------
 pc-bios/optionrom/multiboot.S     | 232 -----------------------------
 pc-bios/optionrom/multiboot_dma.S | 234 +++++++++++++++++++++++++++++-
 23 files changed, 302 insertions(+), 542 deletions(-)
 delete mode 100644 pc-bios/multiboot.bin
 delete mode 100644 pc-bios/optionrom/linuxboot.S
 delete mode 100644 pc-bios/optionrom/multiboot.S

-- 
2.51.0

[PATCH-for-11.0 v6 01/13] hw/ppc/mac: Use fw_cfg_init_mem_nodma()

[Philippe Mathieu-Daudé]

Use fw_cfg_init_mem_nodma() instead of open-coding it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/mac_newworld.c | 11 +----------
 hw/ppc/mac_oldworld.c | 11 +----------
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 951de4bae4b..89cd1ed0311 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -453,16 +453,7 @@ static void ppc_core99_init(MachineState *machine)
     pmac_format_nvram_partition(nvr, MACIO_NVRAM_SIZE);
     /* No PCI init: the BIOS will do it */
 
-    dev = qdev_new(TYPE_FW_CFG_MEM);
-    fw_cfg = FW_CFG(dev);
-    qdev_prop_set_uint32(dev, "data_width", 1);
-    qdev_prop_set_bit(dev, "dma_enabled", false);
-    object_property_add_child(OBJECT(machine), TYPE_FW_CFG, OBJECT(fw_cfg));
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, CFG_ADDR);
-    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
-
+    fw_cfg = fw_cfg_init_mem_nodma(CFG_ADDR, CFG_ADDR + 2, 1);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)machine->smp.max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index cd2bb46442f..25fb8ed070d 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -294,16 +294,7 @@ static void ppc_heathrow_init(MachineState *machine)
 
     /* No PCI init: the BIOS will do it */
 
-    dev = qdev_new(TYPE_FW_CFG_MEM);
-    fw_cfg = FW_CFG(dev);
-    qdev_prop_set_uint32(dev, "data_width", 1);
-    qdev_prop_set_bit(dev, "dma_enabled", false);
-    object_property_add_child(OBJECT(machine), TYPE_FW_CFG, OBJECT(fw_cfg));
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, CFG_ADDR);
-    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
-
+    fw_cfg = fw_cfg_init_mem_nodma(CFG_ADDR, CFG_ADDR + 2, 1);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)machine->smp.max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
-- 
2.51.0

[PATCH-for-11.0 v6 02/13] hw/sparc/sun4m: Use fw_cfg_init_mem_nodma()

[Philippe Mathieu-Daudé]

Use fw_cfg_init_mem_nodma() instead of open-coding it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sparc/sun4m.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 53d7ae08ae9..85c8d727594 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -1051,17 +1051,7 @@ static void sun4m_hw_init(MachineState *machine)
         ecc_init(hwdef->ecc_base, slavio_irq[28],
                  hwdef->ecc_version);
 
-    dev = qdev_new(TYPE_FW_CFG_MEM);
-    fw_cfg = FW_CFG(dev);
-    qdev_prop_set_uint32(dev, "data_width", 1);
-    qdev_prop_set_bit(dev, "dma_enabled", false);
-    object_property_add_child(OBJECT(qdev_get_machine()), TYPE_FW_CFG,
-                              OBJECT(fw_cfg));
-    s = SYS_BUS_DEVICE(dev);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, CFG_ADDR);
-    sysbus_mmio_map(s, 1, CFG_ADDR + 2);
-
+    fw_cfg = fw_cfg_init_mem_nodma(CFG_ADDR, CFG_ADDR + 2, 1);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     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)machine->ram_size);
-- 
2.51.0

[PATCH-for-11.0 v6 03/13] hw/nvram/fw_cfg: Factor fw_cfg_init_mem_internal() out

[Philippe Mathieu-Daudé]

Factor fw_cfg_init_mem_internal() out of fw_cfg_init_mem_wide().
In fw_cfg_init_mem_wide(), assert DMA arguments are provided.
Callers without DMA have to use the fw_cfg_init_mem() helper.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/nvram/fw_cfg.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 2b8715679fe..c65deeb7c38 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1054,9 +1054,9 @@ FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
     return s;
 }
 
-FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
-                                 hwaddr data_addr, uint32_t data_width,
-                                 hwaddr dma_addr, AddressSpace *dma_as)
+static FWCfgState *fw_cfg_init_mem_internal(hwaddr ctl_addr,
+                                            hwaddr data_addr, uint32_t data_width,
+                                            hwaddr dma_addr, AddressSpace *dma_as)
 {
     DeviceState *dev;
     SysBusDevice *sbd;
@@ -1088,10 +1088,19 @@ FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
     return s;
 }
 
+FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
+                                 hwaddr data_addr, uint32_t data_width,
+                                 hwaddr dma_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);
+}
+
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
                                   unsigned data_width)
 {
-    return fw_cfg_init_mem_wide(ctl_addr, data_addr, data_width, 0, NULL);
+    return fw_cfg_init_mem_internal(ctl_addr, data_addr, data_width, 0, NULL);
 }
 
 
-- 
2.51.0

[PATCH-for-11.0 v6 04/13] hw/nvram/fw_cfg: Rename fw_cfg_init_mem_wide() -> fw_cfg_init_mem_dma()

[Philippe Mathieu-Daudé]

"wide" in fw_cfg_init_mem_wide() means "DMA support".
Rename for clarity.

Suggested-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/nvram/fw_cfg.h | 6 +++---
 hw/arm/virt.c             | 2 +-
 hw/loongarch/fw_cfg.c     | 4 ++--
 hw/nvram/fw_cfg.c         | 6 +++---
 hw/riscv/virt.c           | 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index d5161a79436..c4c49886754 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -309,9 +309,9 @@ 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_wide(hwaddr ctl_addr,
-                                 hwaddr data_addr, uint32_t data_width,
-                                 hwaddr dma_addr, AddressSpace *dma_as);
+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_find(void);
 bool fw_cfg_dma_enabled(void *opaque);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 25fb2bab568..23d88e2fd01 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1412,7 +1412,7 @@ static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
     FWCfgState *fw_cfg;
     char *nodename;
 
-    fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
+    fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16, 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 493563669e5..d2a79efbf76 100644
--- a/hw/loongarch/fw_cfg.c
+++ b/hw/loongarch/fw_cfg.c
@@ -23,8 +23,8 @@ 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_wide(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 + 8, VIRT_FWCFG_BASE, 8,
+                                 VIRT_FWCFG_BASE + 16, &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 c65deeb7c38..3f0d337eb9c 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1088,9 +1088,9 @@ static FWCfgState *fw_cfg_init_mem_internal(hwaddr ctl_addr,
     return s;
 }
 
-FWCfgState *fw_cfg_init_mem_wide(hwaddr ctl_addr,
-                                 hwaddr data_addr, uint32_t data_width,
-                                 hwaddr dma_addr, AddressSpace *dma_as)
+FWCfgState *fw_cfg_init_mem_dma(hwaddr ctl_addr,
+                                hwaddr data_addr, uint32_t data_width,
+                                hwaddr dma_addr, AddressSpace *dma_as)
 {
     assert(dma_addr && dma_as);
     return fw_cfg_init_mem_internal(ctl_addr, data_addr, data_width,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 17909206c7e..bfbb28f5bd2 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1274,8 +1274,8 @@ static FWCfgState *create_fw_cfg(const MachineState *ms, hwaddr base)
 {
     FWCfgState *fw_cfg;
 
-    fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16,
-                                  &address_space_memory);
+    fw_cfg = fw_cfg_init_mem_dma(base + 8, base, 8, base + 16,
+                                 &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)ms->smp.cpus);
 
     return fw_cfg;
-- 
2.51.0

[PATCH-for-11.0 v6 05/13] hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()

[Philippe Mathieu-Daudé]

To allow callers to use I/O MemoryRegion different than the
global get_system_io(), pass it as argument.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/nvram/fw_cfg.h | 4 ++--
 hw/i386/fw_cfg.c          | 3 ++-
 hw/i386/microvm.c         | 3 ++-
 hw/i386/pc.c              | 3 ++-
 hw/nvram/fw_cfg.c         | 5 ++---
 5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index c4c49886754..7348cbfbc34 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -305,8 +305,8 @@ 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);
+FWCfgState *fw_cfg_init_io_dma(MemoryRegion *io, 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,
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 5c0bcd5f8a9..498da9ec69c 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -127,7 +127,8 @@ 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,
+    fw_cfg = fw_cfg_init_io_dma(get_system_io(),
+                                FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
                                 &address_space_memory);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, boot_cpus);
 
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 94d22a232ac..69f04d74a15 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -289,6 +289,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
     X86MachineState *x86ms = X86_MACHINE(mms);
     MemoryRegion *ram_below_4g, *ram_above_4g;
     MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *io_memory = get_system_io();
     FWCfgState *fw_cfg;
     ram_addr_t lowmem = 0xc0000000; /* 3G */
     int i;
@@ -319,7 +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,
+    fw_cfg = fw_cfg_init_io_dma(io_memory, FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
                                 &address_space_memory);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, machine->smp.cpus);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2b8d3982c4a..5c52079be31 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -659,7 +659,8 @@ 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,
+    fw_cfg = fw_cfg_init_io_dma(get_system_io(),
+                                FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
                                 &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 3f0d337eb9c..34d7d107678 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1019,14 +1019,13 @@ 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(MemoryRegion *iomem, uint32_t iobase,
+                               uint32_t dma_iobase, AddressSpace *dma_as)
 {
     DeviceState *dev;
     SysBusDevice *sbd;
     FWCfgIoState *ios;
     FWCfgState *s;
-    MemoryRegion *iomem = get_system_io();
     bool dma_requested = dma_iobase && dma_as;
 
     dev = qdev_new(TYPE_FW_CFG_IO);
-- 
2.51.0

[PATCH-for-11.0 v6 06/13] hw/nvram/fw_cfg: Factor fw_cfg_init_io_internal() out

[Philippe Mathieu-Daudé]

Factor fw_cfg_init_io_internal() out of fw_cfg_init_io_dma().
In fw_cfg_init_io_dma(), assert DMA arguments are provided.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/nvram/fw_cfg.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 34d7d107678..2699e593860 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1019,8 +1019,9 @@ 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(MemoryRegion *iomem, uint32_t iobase,
-                               uint32_t dma_iobase, AddressSpace *dma_as)
+static FWCfgState *fw_cfg_init_io_internal(MemoryRegion *iomem,
+                                           uint32_t iobase, uint32_t dma_iobase,
+                                           AddressSpace *dma_as)
 {
     DeviceState *dev;
     SysBusDevice *sbd;
@@ -1053,6 +1054,13 @@ FWCfgState *fw_cfg_init_io_dma(MemoryRegion *iomem, uint32_t iobase,
     return s;
 }
 
+FWCfgState *fw_cfg_init_io_dma(MemoryRegion *iomem, uint32_t iobase,
+                               uint32_t dma_iobase, AddressSpace *dma_as)
+{
+    assert(dma_iobase);
+    return fw_cfg_init_io_internal(iomem, iobase, dma_iobase, dma_as);
+}
+
 static FWCfgState *fw_cfg_init_mem_internal(hwaddr ctl_addr,
                                             hwaddr data_addr, uint32_t data_width,
                                             hwaddr dma_addr, AddressSpace *dma_as)
-- 
2.51.0

[PATCH-for-11.0 v6 07/13] hw/nvram/fw_cfg: Add fw_cfg_init_io_nodma() helper

[Philippe Mathieu-Daudé]

Calling fw_cfg_init_io_nodma(...) is more explicit
than fw_cfg_init_io_dma(..., 0, NULL).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/nvram/fw_cfg.h | 1 +
 hw/nvram/fw_cfg.c         | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 7348cbfbc34..a2b8f7fa864 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -305,6 +305,7 @@ bool fw_cfg_add_file_from_generator(FWCfgState *s,
                                     Object *parent, const char *part,
                                     const char *filename, Error **errp);
 
+FWCfgState *fw_cfg_init_io_nodma(MemoryRegion *io, uint32_t iobase);
 FWCfgState *fw_cfg_init_io_dma(MemoryRegion *io, uint32_t iobase,
                                uint32_t dma_iobase, AddressSpace *dma_as);
 FWCfgState *fw_cfg_init_mem_nodma(hwaddr ctl_addr, hwaddr data_addr,
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 2699e593860..079c28f9212 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1054,6 +1054,11 @@ static FWCfgState *fw_cfg_init_io_internal(MemoryRegion *iomem,
     return s;
 }
 
+FWCfgState *fw_cfg_init_io_nodma(MemoryRegion *iomem, uint32_t iobase)
+{
+    return fw_cfg_init_io_internal(iomem, iobase, 0, NULL);
+}
+
 FWCfgState *fw_cfg_init_io_dma(MemoryRegion *iomem, uint32_t iobase,
                                uint32_t dma_iobase, AddressSpace *dma_as)
 {
-- 
2.51.0

[PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Philippe Mathieu-Daudé]

Use fw_cfg_init_io_nodma() instead of open-coding it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sparc64/sun4u.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 82c3e7c855b..6dc9f64b74d 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -683,14 +683,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                            graphic_width, graphic_height, graphic_depth,
                            (uint8_t *)&macaddr);
 
-    dev = qdev_new(TYPE_FW_CFG_IO);
-    qdev_prop_set_bit(dev, "dma_enabled", false);
-    object_property_add_child(OBJECT(ebus), TYPE_FW_CFG, OBJECT(dev));
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
-                                &FW_CFG_IO(dev)->comb_iomem);
-
-    fw_cfg = FW_CFG(dev);
+    fw_cfg = fw_cfg_init_io_nodma(pci_address_space_io(ebus), BIOS_CFG_IOPORT);
     fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)machine->smp.cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)machine->smp.max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)machine->ram_size);
-- 
2.51.0

[PATCH-for-11.0 v6 09/13] hw/nvram/fw_cfg: Remove fw_cfg_io_properties::dma_enabled

[Philippe Mathieu-Daudé]

Now than all calls to fw_cfg_init_io_dma() pass DMA arguments,
the 'dma_enabled' of the TYPE_FW_CFG_IO type is not used anymore.
Remove it, simplifying fw_cfg_init_io_dma() and fw_cfg_io_realize().

Note, we can not remove the equivalent in fw_cfg_mem_properties[]
because it is still used in HPPA and MIPS Loongson3 machines:

  $ git grep -w fw_cfg_init_mem_nodma
  hw/hppa/machine.c:204:    fw_cfg = fw_cfg_init_mem_nodma(addr, addr + 4, 1);
  hw/mips/loongson3_virt.c:289:    fw_cfg = fw_cfg_init_mem_nodma(cfg_addr, cfg_addr + 8, 8);

'linuxboot.bin' isn't used anymore, we'll remove it in the
next commit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/fw_cfg.c  | 15 +++++++--------
 hw/nvram/fw_cfg.c | 26 ++++++++------------------
 2 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index 498da9ec69c..bf38338fb47 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -216,18 +216,17 @@ void fw_cfg_build_feature_control(MachineState *ms, FWCfgState *fw_cfg)
 #ifdef CONFIG_ACPI
 void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg)
 {
+    uint8_t io_size;
+    Aml *dev = aml_device("FWCF");
+    Aml *crs = aml_resource_template();
+
     /*
      * when using port i/o, the 8-bit data register *always* overlaps
      * with half of the 16-bit control register. Hence, the total size
-     * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
-     * DMA control register is located at FW_CFG_DMA_IO_BASE + 4
+     * of the i/o region used is FW_CFG_CTL_SIZE; And the DMA control
+     * register is located at FW_CFG_DMA_IO_BASE + 4
      */
-    Object *obj = OBJECT(fw_cfg);
-    uint8_t io_size = object_property_get_bool(obj, "dma_enabled", NULL) ?
-        ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
-        FW_CFG_CTL_SIZE;
-    Aml *dev = aml_device("FWCF");
-    Aml *crs = aml_resource_template();
+    io_size = ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t);
 
     aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
 
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 079c28f9212..e0a140d01c0 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1027,12 +1027,9 @@ static FWCfgState *fw_cfg_init_io_internal(MemoryRegion *iomem,
     SysBusDevice *sbd;
     FWCfgIoState *ios;
     FWCfgState *s;
-    bool dma_requested = dma_iobase && dma_as;
 
+    assert(dma_iobase);
     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));
@@ -1043,13 +1040,10 @@ static FWCfgState *fw_cfg_init_io_internal(MemoryRegion *iomem,
     memory_region_add_subregion(iomem, iobase, &ios->comb_iomem);
 
     s = FW_CFG(dev);
-
-    if (s->dma_enabled) {
-        /* 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);
-    }
+    /* 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);
 
     return s;
 }
@@ -1198,8 +1192,6 @@ static void fw_cfg_file_slots_allocate(FWCfgState *s, Error **errp)
 }
 
 static const Property fw_cfg_io_properties[] = {
-    DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
-                     true),
     DEFINE_PROP_UINT16("x-file-slots", FWCfgIoState, parent_obj.file_slots,
                        FW_CFG_FILE_SLOTS_DFLT),
 };
@@ -1220,11 +1212,9 @@ static void fw_cfg_io_realize(DeviceState *dev, Error **errp)
     memory_region_init_io(&s->comb_iomem, OBJECT(s), &fw_cfg_comb_mem_ops,
                           FW_CFG(s), "fwcfg", FW_CFG_CTL_SIZE);
 
-    if (FW_CFG(s)->dma_enabled) {
-        memory_region_init_io(&FW_CFG(s)->dma_iomem, OBJECT(s),
-                              &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
-                              sizeof(dma_addr_t));
-    }
+    memory_region_init_io(&FW_CFG(s)->dma_iomem, OBJECT(s),
+                          &fw_cfg_dma_mem_ops, FW_CFG(s), "fwcfg.dma",
+                          sizeof(dma_addr_t));
 
     fw_cfg_common_realize(dev, errp);
 }
-- 
2.51.0

[PATCH-for-11.0 v6 10/13] hw/i386/x86: Remove X86MachineClass::fwcfg_dma_enabled field

[Philippe Mathieu-Daudé]

The X86MachineClass::fwcfg_dma_enabled boolean was only used
by the pc-q35-2.6 and pc-i440fx-2.6 machines, which got
removed. Remove it and simplify.

'multiboot.bin' isn't used anymore, we'll remove it in the
next commit.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/hw/i386/x86.h | 2 --
 hw/i386/microvm.c     | 3 ---
 hw/i386/multiboot.c   | 7 +------
 hw/i386/x86-common.c  | 3 +--
 hw/i386/x86.c         | 2 --
 5 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 8755cad50a3..201eee80eb7 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -30,8 +30,6 @@
 struct X86MachineClass {
     MachineClass parent;
 
-    /* use DMA capable linuxboot option rom */
-    bool fwcfg_dma_enabled;
     /* CPU and apic information: */
     bool apic_xrupt_override;
 };
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 69f04d74a15..8521df83584 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -641,7 +641,6 @@ GlobalProperty microvm_properties[] = {
 
 static void microvm_class_init(ObjectClass *oc, const void *data)
 {
-    X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
     MicrovmMachineClass *mmc = MICROVM_MACHINE_CLASS(oc);
     MachineClass *mc = MACHINE_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
@@ -675,8 +674,6 @@ static void microvm_class_init(ObjectClass *oc, const void *data)
     hc->unplug_request = microvm_device_unplug_request_cb;
     hc->unplug = microvm_device_unplug_cb;
 
-    x86mc->fwcfg_dma_enabled = true;
-
     object_class_property_add(oc, MICROVM_MACHINE_RTC, "OnOffAuto",
                               microvm_machine_get_rtc,
                               microvm_machine_set_rtc,
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 78690781b74..3b993126edb 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -153,7 +153,6 @@ int load_multiboot(X86MachineState *x86ms,
                    int kernel_file_size,
                    uint8_t *header)
 {
-    bool multiboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
     int i, is_multiboot = 0;
     uint32_t flags = 0;
     uint32_t mh_entry_addr;
@@ -402,11 +401,7 @@ int load_multiboot(X86MachineState *x86ms,
     fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
                      sizeof(bootinfo));
 
-    if (multiboot_dma_enabled) {
-        option_rom[nb_option_roms].name = "multiboot_dma.bin";
-    } else {
-        option_rom[nb_option_roms].name = "multiboot.bin";
-    }
+    option_rom[nb_option_roms].name = "multiboot_dma.bin";
     option_rom[nb_option_roms].bootindex = 0;
     nb_option_roms++;
 
diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index 60b7ab80433..1ee55382dab 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -645,7 +645,6 @@ void x86_load_linux(X86MachineState *x86ms,
                     int acpi_data_size,
                     bool pvh_enabled)
 {
-    bool linuxboot_dma_enabled = X86_MACHINE_GET_CLASS(x86ms)->fwcfg_dma_enabled;
     uint16_t protocol;
     int setup_size, kernel_size, cmdline_size;
     int dtb_size, setup_data_offset;
@@ -1004,7 +1003,7 @@ void x86_load_linux(X86MachineState *x86ms,
 
     option_rom[nb_option_roms].bootindex = 0;
     option_rom[nb_option_roms].name = "linuxboot.bin";
-    if (linuxboot_dma_enabled && fw_cfg_dma_enabled(fw_cfg)) {
+    if (fw_cfg_dma_enabled(fw_cfg)) {
         option_rom[nb_option_roms].name = "linuxboot_dma.bin";
     }
     nb_option_roms++;
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index f80533df1c5..dbf104d60af 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -375,14 +375,12 @@ static void x86_machine_initfn(Object *obj)
 static void x86_machine_class_init(ObjectClass *oc, const void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
-    X86MachineClass *x86mc = X86_MACHINE_CLASS(oc);
     NMIClass *nc = NMI_CLASS(oc);
 
     mc->cpu_index_to_instance_props = x86_cpu_index_to_props;
     mc->get_default_cpu_node_id = x86_get_default_cpu_node_id;
     mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids;
     mc->kvm_type = x86_kvm_type;
-    x86mc->fwcfg_dma_enabled = true;
     nc->nmi_monitor_handler = x86_nmi;
 
     object_class_property_add(oc, X86_MACHINE_SMM, "OnOffAuto",
-- 
2.51.0

[PATCH-for-11.0 v6 11/13] hw/i386/pc: Remove multiboot.bin

[Philippe Mathieu-Daudé]

All PC machines now use the multiboot_dma.bin binary,
we can remove the non-DMA version (multiboot.bin).

This doesn't change multiboot_dma binary file.

Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 pc-bios/optionrom/optionrom.h     |   4 -
 hw/i386/pc.c                      |   1 -
 pc-bios/meson.build               |   1 -
 pc-bios/multiboot.bin             | Bin 1024 -> 0 bytes
 pc-bios/optionrom/Makefile        |   2 +-
 pc-bios/optionrom/multiboot.S     | 232 -----------------------------
 pc-bios/optionrom/multiboot_dma.S | 234 +++++++++++++++++++++++++++++-
 7 files changed, 233 insertions(+), 241 deletions(-)
 delete mode 100644 pc-bios/multiboot.bin
 delete mode 100644 pc-bios/optionrom/multiboot.S

diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
index 7bcdf0eeb24..2e6e2493f83 100644
--- a/pc-bios/optionrom/optionrom.h
+++ b/pc-bios/optionrom/optionrom.h
@@ -117,16 +117,12 @@
  *
  * Clobbers: %eax, %edx, %es, %ecx, %edi and adresses %esp-20 to %esp
  */
-#ifdef USE_FW_CFG_DMA
 #define read_fw_blob_dma(var)                           \
         read_fw         var ## _SIZE;                   \
         mov             %eax, %ecx;                     \
         read_fw         var ## _ADDR;                   \
         mov             %eax, %edi ;                    \
         read_fw_dma     var ## _DATA, %ecx, %edi
-#else
-#define read_fw_blob_dma(var) read_fw_blob(var)
-#endif
 
 #define read_fw_blob_pre(var)                           \
         read_fw         var ## _SIZE;                   \
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5c52079be31..edba8e4b97d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -670,7 +670,6 @@ void xen_load_linux(PCMachineState *pcms)
         assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
                !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
                !strcmp(option_rom[i].name, "pvh.bin") ||
-               !strcmp(option_rom[i].name, "multiboot.bin") ||
                !strcmp(option_rom[i].name, "multiboot_dma.bin"));
         rom_add_option(option_rom[i].name, option_rom[i].bootindex);
     }
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 9260aaad78e..efe45c16705 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -62,7 +62,6 @@ blobs = [
   'efi-e1000e.rom',
   'efi-vmxnet3.rom',
   'qemu-nsis.bmp',
-  'multiboot.bin',
   'multiboot_dma.bin',
   'linuxboot.bin',
   'linuxboot_dma.bin',
diff --git a/pc-bios/multiboot.bin b/pc-bios/multiboot.bin
deleted file mode 100644
index e772713c95749bee82c20002b50ec6d05b2d4987..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 1024
zcmeHFF-Tic6utlZQ$OjD#Hxcx2u0GNQv6GySOkZR(ulaX<>%N!Y#>cWhY}nf36J7X
zN(%*X6NHY>xcqO11dG^02a8L@B~ihln|%1*|7(haWa`)l@80w7;U4Ziyv0rZ8{K-w
zX(Ib3tLZ)RgZ}w1ei{~QEqQq9q1J-iHc<Nk_t=0qf%XfPZVHw2DY#ujc2P|}*P%6X
z5J{pPlX4<yP&N5pXK;s@UhB~&!E&UdqEwGZF6xQMIcu9YL#zeSRCoLGt{Nh+08t>}
z{m%E-b32A??+@XljSYirtWObwngi<ymS1Ta*dFGMp;8@=_3Z52!v09{UU~`QnFsB_
z7BiEC)uZxH%SW9kPWCicN{_iUjmk<~D^H|R%@|m9%43Woc(Pkeq%n{&8ND5Z*tPt#
zJua+xXAQhN4K(1MMs0{u_6sp>l#NmvPZ7KC<lsLdQgMFC@A6POvMoDMgW=M+K;T<o
zTkpnNq6x*`vM0CGE>t40l-CQ}*)y<y--c)(x}o&1TMzwX+ToGS@VER4zR&s70fl+(
qI)9<-H_-!n#lLJmGq*^~<$US&%R-@)$`@YPx#A6#|L`9<;9UXPG71m?

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 30d07026c79..1183ef88922 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -2,7 +2,7 @@ include config.mak
 SRC_DIR := $(TOPSRC_DIR)/pc-bios/optionrom
 VPATH = $(SRC_DIR)
 
-all: multiboot.bin multiboot_dma.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
+all: multiboot_dma.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
 # Dummy command so that make thinks it has done something
 	@true
 
diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S
deleted file mode 100644
index c95e35c9cb6..00000000000
--- a/pc-bios/optionrom/multiboot.S
+++ /dev/null
@@ -1,232 +0,0 @@
-/*
- * Multiboot Option ROM
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright Novell Inc, 2009
- *   Authors: Alexander Graf <agraf@suse.de>
- */
-
-#include "optionrom.h"
-
-#define BOOT_ROM_PRODUCT "multiboot loader"
-
-#define MULTIBOOT_MAGIC		0x2badb002
-
-#define GS_PROT_JUMP		0
-#define GS_GDT_DESC		6
-
-
-BOOT_ROM_START
-
-run_multiboot:
-
-	cli
-	cld
-
-	mov		%cs, %eax
-	shl		$0x4, %eax
-
-	/* set up a long jump descriptor that is PC relative */
-
-	/* move stack memory to %gs */
-	mov		%ss, %ecx
-	shl		$0x4, %ecx
-	mov		%esp, %ebx
-	add		%ebx, %ecx
-	sub		$0x20, %ecx
-	sub		$0x30, %esp
-	shr		$0x4, %ecx
-	mov		%cx, %gs
-
-	/* now push the indirect jump descriptor there */
-	mov		(prot_jump), %ebx
-	add		%eax, %ebx
-	movl		%ebx, %gs:GS_PROT_JUMP
-	mov		$8, %bx
-	movw		%bx, %gs:GS_PROT_JUMP + 4
-
-	/* fix the gdt descriptor to be PC relative */
-	movw		(gdt_desc), %bx
-	movw		%bx, %gs:GS_GDT_DESC
-	movl		(gdt_desc+2), %ebx
-	add		%eax, %ebx
-	movl		%ebx, %gs:GS_GDT_DESC + 2
-
-	xor		%eax, %eax
-	mov		%eax, %es
-
-	/* Read the bootinfo struct into RAM */
-	read_fw_blob_dma(FW_CFG_INITRD)
-
-	/* FS = bootinfo_struct */
-	read_fw		FW_CFG_INITRD_ADDR
-	shr		$4, %eax
-	mov		%ax, %fs
-
-	/* Account for the EBDA in the multiboot structure's e801
-	 * map.
-	 */
-	int		$0x12
-	cwtl
-	movl		%eax, %fs:4
-
-	/* ES = mmap_addr */
-	mov 		%fs:48, %eax
-	shr		$4, %eax
-	mov		%ax, %es
-
-	/* Initialize multiboot mmap structs using int 0x15(e820) */
-	xor		%ebx, %ebx
-	/* Start storing mmap data at %es:0 */
-	xor		%edi, %edi
-
-mmap_loop:
-	/* The multiboot entry size has offset -4, so leave some space */
-	add		$4, %di
-	/* entry size (mmap struct) & max buffer size (int15) */
-	movl		$20, %ecx
-	/* e820 */
-	movl		$0x0000e820, %eax
-	/* 'SMAP' magic */
-	movl		$0x534d4150, %edx
-	int		$0x15
-
-mmap_check_entry:
-	/* Error or last entry already done? */
-	jb		mmap_done
-
-mmap_store_entry:
-	/* store entry size */
-	/* old as(1) doesn't like this insn so emit the bytes instead:
-	movl		%ecx, %es:-4(%edi)
-	*/
-	.dc.b		0x26,0x67,0x66,0x89,0x4f,0xfc
-
-	/* %edi += entry_size, store as mbs_mmap_length */
-	add		%ecx, %edi
-	movw		%di, %fs:0x2c
-
-	/* Continuation value 0 means last entry */
-	test		%ebx, %ebx
-	jnz		mmap_loop
-
-mmap_done:
-	/* Calculate upper_mem field: The amount of memory between 1 MB and
-	   the first upper memory hole. Get it from the mmap. */
-	xor		%di, %di
-	mov		$0x100000, %edx
-upper_mem_entry:
-	cmp		%fs:0x2c, %di
-	je		upper_mem_done
-	add		$4, %di
-
-	/* Skip if type != 1 */
-	cmpl		$1, %es:16(%di)
-	jne		upper_mem_next
-
-	/* Skip if > 4 GB */
-	movl		%es:4(%di), %eax
-	test		%eax, %eax
-	jnz		upper_mem_next
-
-	/* Check for contiguous extension (base <= %edx < base + length) */
-	movl		%es:(%di), %eax
-	cmp		%eax, %edx
-	jb		upper_mem_next
-	addl		%es:8(%di), %eax
-	cmp		%eax, %edx
-	jae		upper_mem_next
-
-	/* If so, update %edx, and restart the search (mmap isn't ordered) */
-	mov		%eax, %edx
-	xor		%di, %di
-	jmp		upper_mem_entry
-
-upper_mem_next:
-	addl		%es:-4(%di), %edi
-	jmp		upper_mem_entry
-
-upper_mem_done:
-	sub		$0x100000, %edx
-	shr		$10, %edx
-	mov		%edx, %fs:0x8
-
-real_to_prot:
-	/* Load the GDT before going into protected mode */
-lgdt:
-	data32 lgdt	%gs:GS_GDT_DESC
-
-	/* get us to protected mode now */
-	movl		$1, %eax
-	movl		%eax, %cr0
-
-	/* the LJMP sets CS for us and gets us to 32-bit */
-ljmp:
-	data32 ljmp	*%gs:GS_PROT_JUMP
-
-prot_mode:
-.code32
-
-	/* initialize all other segments */
-	movl		$0x10, %eax
-	movl		%eax, %ss
-	movl		%eax, %ds
-	movl		%eax, %es
-	movl		%eax, %fs
-	movl		%eax, %gs
-
-	/* Read the kernel and modules into RAM */
-	read_fw_blob_dma(FW_CFG_KERNEL)
-
-	/* Jump off to the kernel */
-	read_fw		FW_CFG_KERNEL_ENTRY
-	mov		%eax, %ecx
-
-	/* EBX contains a pointer to the bootinfo struct */
-	read_fw		FW_CFG_INITRD_ADDR
-	movl		%eax, %ebx
-
-	/* EAX has to contain the magic */
-	movl		$MULTIBOOT_MAGIC, %eax
-ljmp2:
-	jmp		*%ecx
-
-/* Variables */
-.align 4, 0
-prot_jump:	.long prot_mode
-		.short 8
-
-.align 8, 0
-gdt:
-	/* 0x00 */
-.byte	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-
-	/* 0x08: code segment (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00
-
-	/* 0x10: data segment (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00
-
-	/* 0x18: code segment (base=0, limit=0x0ffff, type=16bit code exec/read/conf, DPL=0, 1b) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00
-
-	/* 0x20: data segment (base=0, limit=0x0ffff, type=16bit data read/write, DPL=0, 1b) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00
-
-gdt_desc:
-.short	(5 * 8) - 1
-.long	gdt
-
-BOOT_ROM_END
diff --git a/pc-bios/optionrom/multiboot_dma.S b/pc-bios/optionrom/multiboot_dma.S
index d809af3e23f..c95e35c9cb6 100644
--- a/pc-bios/optionrom/multiboot_dma.S
+++ b/pc-bios/optionrom/multiboot_dma.S
@@ -1,2 +1,232 @@
-#define USE_FW_CFG_DMA 1
-#include "multiboot.S"
+/*
+ * Multiboot Option ROM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * Copyright Novell Inc, 2009
+ *   Authors: Alexander Graf <agraf@suse.de>
+ */
+
+#include "optionrom.h"
+
+#define BOOT_ROM_PRODUCT "multiboot loader"
+
+#define MULTIBOOT_MAGIC		0x2badb002
+
+#define GS_PROT_JUMP		0
+#define GS_GDT_DESC		6
+
+
+BOOT_ROM_START
+
+run_multiboot:
+
+	cli
+	cld
+
+	mov		%cs, %eax
+	shl		$0x4, %eax
+
+	/* set up a long jump descriptor that is PC relative */
+
+	/* move stack memory to %gs */
+	mov		%ss, %ecx
+	shl		$0x4, %ecx
+	mov		%esp, %ebx
+	add		%ebx, %ecx
+	sub		$0x20, %ecx
+	sub		$0x30, %esp
+	shr		$0x4, %ecx
+	mov		%cx, %gs
+
+	/* now push the indirect jump descriptor there */
+	mov		(prot_jump), %ebx
+	add		%eax, %ebx
+	movl		%ebx, %gs:GS_PROT_JUMP
+	mov		$8, %bx
+	movw		%bx, %gs:GS_PROT_JUMP + 4
+
+	/* fix the gdt descriptor to be PC relative */
+	movw		(gdt_desc), %bx
+	movw		%bx, %gs:GS_GDT_DESC
+	movl		(gdt_desc+2), %ebx
+	add		%eax, %ebx
+	movl		%ebx, %gs:GS_GDT_DESC + 2
+
+	xor		%eax, %eax
+	mov		%eax, %es
+
+	/* Read the bootinfo struct into RAM */
+	read_fw_blob_dma(FW_CFG_INITRD)
+
+	/* FS = bootinfo_struct */
+	read_fw		FW_CFG_INITRD_ADDR
+	shr		$4, %eax
+	mov		%ax, %fs
+
+	/* Account for the EBDA in the multiboot structure's e801
+	 * map.
+	 */
+	int		$0x12
+	cwtl
+	movl		%eax, %fs:4
+
+	/* ES = mmap_addr */
+	mov 		%fs:48, %eax
+	shr		$4, %eax
+	mov		%ax, %es
+
+	/* Initialize multiboot mmap structs using int 0x15(e820) */
+	xor		%ebx, %ebx
+	/* Start storing mmap data at %es:0 */
+	xor		%edi, %edi
+
+mmap_loop:
+	/* The multiboot entry size has offset -4, so leave some space */
+	add		$4, %di
+	/* entry size (mmap struct) & max buffer size (int15) */
+	movl		$20, %ecx
+	/* e820 */
+	movl		$0x0000e820, %eax
+	/* 'SMAP' magic */
+	movl		$0x534d4150, %edx
+	int		$0x15
+
+mmap_check_entry:
+	/* Error or last entry already done? */
+	jb		mmap_done
+
+mmap_store_entry:
+	/* store entry size */
+	/* old as(1) doesn't like this insn so emit the bytes instead:
+	movl		%ecx, %es:-4(%edi)
+	*/
+	.dc.b		0x26,0x67,0x66,0x89,0x4f,0xfc
+
+	/* %edi += entry_size, store as mbs_mmap_length */
+	add		%ecx, %edi
+	movw		%di, %fs:0x2c
+
+	/* Continuation value 0 means last entry */
+	test		%ebx, %ebx
+	jnz		mmap_loop
+
+mmap_done:
+	/* Calculate upper_mem field: The amount of memory between 1 MB and
+	   the first upper memory hole. Get it from the mmap. */
+	xor		%di, %di
+	mov		$0x100000, %edx
+upper_mem_entry:
+	cmp		%fs:0x2c, %di
+	je		upper_mem_done
+	add		$4, %di
+
+	/* Skip if type != 1 */
+	cmpl		$1, %es:16(%di)
+	jne		upper_mem_next
+
+	/* Skip if > 4 GB */
+	movl		%es:4(%di), %eax
+	test		%eax, %eax
+	jnz		upper_mem_next
+
+	/* Check for contiguous extension (base <= %edx < base + length) */
+	movl		%es:(%di), %eax
+	cmp		%eax, %edx
+	jb		upper_mem_next
+	addl		%es:8(%di), %eax
+	cmp		%eax, %edx
+	jae		upper_mem_next
+
+	/* If so, update %edx, and restart the search (mmap isn't ordered) */
+	mov		%eax, %edx
+	xor		%di, %di
+	jmp		upper_mem_entry
+
+upper_mem_next:
+	addl		%es:-4(%di), %edi
+	jmp		upper_mem_entry
+
+upper_mem_done:
+	sub		$0x100000, %edx
+	shr		$10, %edx
+	mov		%edx, %fs:0x8
+
+real_to_prot:
+	/* Load the GDT before going into protected mode */
+lgdt:
+	data32 lgdt	%gs:GS_GDT_DESC
+
+	/* get us to protected mode now */
+	movl		$1, %eax
+	movl		%eax, %cr0
+
+	/* the LJMP sets CS for us and gets us to 32-bit */
+ljmp:
+	data32 ljmp	*%gs:GS_PROT_JUMP
+
+prot_mode:
+.code32
+
+	/* initialize all other segments */
+	movl		$0x10, %eax
+	movl		%eax, %ss
+	movl		%eax, %ds
+	movl		%eax, %es
+	movl		%eax, %fs
+	movl		%eax, %gs
+
+	/* Read the kernel and modules into RAM */
+	read_fw_blob_dma(FW_CFG_KERNEL)
+
+	/* Jump off to the kernel */
+	read_fw		FW_CFG_KERNEL_ENTRY
+	mov		%eax, %ecx
+
+	/* EBX contains a pointer to the bootinfo struct */
+	read_fw		FW_CFG_INITRD_ADDR
+	movl		%eax, %ebx
+
+	/* EAX has to contain the magic */
+	movl		$MULTIBOOT_MAGIC, %eax
+ljmp2:
+	jmp		*%ecx
+
+/* Variables */
+.align 4, 0
+prot_jump:	.long prot_mode
+		.short 8
+
+.align 8, 0
+gdt:
+	/* 0x00 */
+.byte	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+
+	/* 0x08: code segment (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00
+
+	/* 0x10: data segment (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00
+
+	/* 0x18: code segment (base=0, limit=0x0ffff, type=16bit code exec/read/conf, DPL=0, 1b) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9e, 0x00, 0x00
+
+	/* 0x20: data segment (base=0, limit=0x0ffff, type=16bit data read/write, DPL=0, 1b) */
+.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0x00, 0x00
+
+gdt_desc:
+.short	(5 * 8) - 1
+.long	gdt
+
+BOOT_ROM_END
-- 
2.51.0

[PATCH-for-11.0 v6 12/13] hw/i386: Assume fw_cfg DMA is always enabled

[Philippe Mathieu-Daudé]

From: Zhao Liu <zhao1.liu@intel.com>

Now all calls of x86 machines to fw_cfg_init_io_dma() pass DMA
arguments, so the FWCfgState (FWCfgIoState) created by x86 machines
enables DMA by default.

Then 'linuxboot.bin' isn't used anymore, and it will be removed in the
next commit.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.c     | 1 +
 hw/i386/x86-common.c | 6 ++----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index bf38338fb47..528862d2e33 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -226,6 +226,7 @@ void fw_cfg_add_acpi_dsdt(Aml *scope, FWCfgState *fw_cfg)
      * of the i/o region used is FW_CFG_CTL_SIZE; And the DMA control
      * register is located at FW_CFG_DMA_IO_BASE + 4
      */
+    assert(fw_cfg_dma_enabled(fw_cfg));
     io_size = ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t);
 
     aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
diff --git a/hw/i386/x86-common.c b/hw/i386/x86-common.c
index 1ee55382dab..e8dc4d903bd 100644
--- a/hw/i386/x86-common.c
+++ b/hw/i386/x86-common.c
@@ -1002,10 +1002,8 @@ void x86_load_linux(X86MachineState *x86ms,
     }
 
     option_rom[nb_option_roms].bootindex = 0;
-    option_rom[nb_option_roms].name = "linuxboot.bin";
-    if (fw_cfg_dma_enabled(fw_cfg)) {
-        option_rom[nb_option_roms].name = "linuxboot_dma.bin";
-    }
+    assert(fw_cfg_dma_enabled(fw_cfg));
+    option_rom[nb_option_roms].name = "linuxboot_dma.bin";
     nb_option_roms++;
 }
 
-- 
2.51.0

[PATCH-for-11.0 v6 13/13] hw/i386: Remove linuxboot.bin

[Philippe Mathieu-Daudé]

All machines now use the linuxboot_dma.bin binary, so it's safe to
remove the non-DMA version (linuxboot.bin).

Suggested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/i386/pc.c                  |   3 +-
 pc-bios/meson.build           |   1 -
 pc-bios/optionrom/Makefile    |   2 +-
 pc-bios/optionrom/linuxboot.S | 195 ----------------------------------
 4 files changed, 2 insertions(+), 199 deletions(-)
 delete mode 100644 pc-bios/optionrom/linuxboot.S

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index edba8e4b97d..08aa28e6a8d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -667,8 +667,7 @@ void xen_load_linux(PCMachineState *pcms)
 
     x86_load_linux(x86ms, fw_cfg, PC_FW_DATA, pcmc->pvh_enabled);
     for (i = 0; i < nb_option_roms; i++) {
-        assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
-               !strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
+        assert(!strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
                !strcmp(option_rom[i].name, "pvh.bin") ||
                !strcmp(option_rom[i].name, "multiboot_dma.bin"));
         rom_add_option(option_rom[i].name, option_rom[i].bootindex);
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index efe45c16705..2f470ed1294 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -63,7 +63,6 @@ blobs = [
   'efi-vmxnet3.rom',
   'qemu-nsis.bmp',
   'multiboot_dma.bin',
-  'linuxboot.bin',
   'linuxboot_dma.bin',
   'kvmvapic.bin',
   'pvh.bin',
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 1183ef88922..e694c7aac00 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -2,7 +2,7 @@ include config.mak
 SRC_DIR := $(TOPSRC_DIR)/pc-bios/optionrom
 VPATH = $(SRC_DIR)
 
-all: multiboot_dma.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
+all: multiboot_dma.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
 # Dummy command so that make thinks it has done something
 	@true
 
diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S
deleted file mode 100644
index ba821ab922d..00000000000
--- a/pc-bios/optionrom/linuxboot.S
+++ /dev/null
@@ -1,195 +0,0 @@
-/*
- * Linux Boot Option ROM
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see <http://www.gnu.org/licenses/>.
- *
- * Copyright Novell Inc, 2009
- *   Authors: Alexander Graf <agraf@suse.de>
- *
- * Based on code in hw/pc.c.
- */
-
-#include "optionrom.h"
-
-#define BOOT_ROM_PRODUCT "Linux loader"
-
-BOOT_ROM_START
-
-run_linuxboot:
-
-	cli
-	cld
-
-	jmp		copy_kernel
-boot_kernel:
-
-	read_fw		FW_CFG_SETUP_ADDR
-
-	mov		%eax, %ebx
-	shr		$4, %ebx
-
-	/* All segments contain real_addr */
-	mov		%bx, %ds
-	mov		%bx, %es
-	mov		%bx, %fs
-	mov		%bx, %gs
-	mov		%bx, %ss
-
-	/* CX = CS we want to jump to */
-	add		$0x20, %bx
-	mov		%bx, %cx
-
-	/* SP = cmdline_addr-real_addr-16 */
-	read_fw		FW_CFG_CMDLINE_ADDR
-	mov		%eax, %ebx
-	read_fw		FW_CFG_SETUP_ADDR
-	sub		%eax, %ebx
-	sub		$16, %ebx
-	mov		%ebx, %esp
-
-	/* Build indirect lret descriptor */
-	pushw		%cx		/* CS */
-	xor		%ax, %ax
-	pushw		%ax		/* IP = 0 */
-
-	/* Clear registers */
-	xor		%eax, %eax
-	xor		%ebx, %ebx
-	xor		%ecx, %ecx
-	xor		%edx, %edx
-	xor		%edi, %edi
-	xor		%ebp, %ebp
-
-	/* Jump to Linux */
-	lret
-
-
-copy_kernel:
-	/* Read info block in low memory (0x10000 or 0x90000) */
-	read_fw		FW_CFG_SETUP_ADDR
-	shr		$4, %eax
-	mov		%eax, %es
-	xor		%edi, %edi
-	read_fw_blob_addr32_edi(FW_CFG_SETUP)
-
-	cmpw            $0x203, %es:0x206      // if protocol >= 0x203
-	jae             1f                     // have initrd_max
-	movl            $0x37ffffff, %es:0x22c // else assume 0x37ffffff
-1:
-
-	/* Check if using kernel-specified initrd address */
-	read_fw		FW_CFG_INITRD_ADDR
-	mov		%eax, %edi             // (load_kernel wants it in %edi)
-	read_fw		FW_CFG_INITRD_SIZE     // find end of initrd
-	add		%edi, %eax
-	xor		%es:0x22c, %eax        // if it matches es:0x22c
-	and		$-4096, %eax           // (apart from padding for page)
-	jz		load_kernel            // then initrd is not at top
-					       // of memory
-
-	/* pc.c placed the initrd at end of memory.  Compute a better
-	 * initrd address based on e801 data.
-	 */
-	mov		$0xe801, %ax
-	xor		%cx, %cx
-	xor		%dx, %dx
-	int		$0x15
-
-	/* Output could be in AX/BX or CX/DX */
-	or		%cx, %cx
-	jnz		1f
-	or		%dx, %dx
-	jnz		1f
-	mov		%ax, %cx
-	mov		%bx, %dx
-1:
-
-	or		%dx, %dx
-	jnz		2f
-	addw		$1024, %cx            /* add 1 MB */
-	movzwl		%cx, %edi
-	shll		$10, %edi             /* convert to bytes */
-	jmp		3f
-
-2:
-	addw		$16777216 >> 16, %dx  /* add 16 MB */
-	movzwl		%dx, %edi
-	shll		$16, %edi             /* convert to bytes */
-
-3:
-	read_fw         FW_CFG_INITRD_SIZE
-	subl            %eax, %edi
-	andl            $-4096, %edi          /* EDI = start of initrd */
-	movl		%edi, %es:0x218       /* put it in the header */
-
-load_kernel:
-	/* We need to load the kernel into memory we can't access in 16 bit
-	   mode, so let's get into 32 bit mode, write the kernel and jump
-	   back again. */
-
-	/* Reserve space on the stack for our GDT descriptor. */
-	mov             %esp, %ebp
-	sub             $16, %esp
-
-	/* Now create the GDT descriptor */
-	movw		$((3 * 8) - 1), -16(%bp)
-	mov		%cs, %eax
-	movzwl		%ax, %eax
-	shl		$4, %eax
-	addl		$gdt, %eax
-	movl		%eax, -14(%bp)
-
-	/* And load the GDT */
-	data32 lgdt	-16(%bp)
-	mov		%ebp, %esp
-
-	/* Get us to protected mode now */
-	mov		$1, %eax
-	mov		%eax, %cr0
-
-	/* So we can set ES to a 32-bit segment */
-	mov		$0x10, %eax
-	mov		%eax, %es
-
-	/* We're now running in 16-bit CS, but 32-bit ES! */
-
-	/* Load kernel and initrd */
-	read_fw_blob_addr32_edi(FW_CFG_INITRD)
-	read_fw_blob_addr32(FW_CFG_KERNEL)
-	read_fw_blob_addr32(FW_CFG_CMDLINE)
-
-	/* And now jump into Linux! */
-	mov		$0, %eax
-	mov		%eax, %cr0
-
-	/* ES = CS */
-	mov		%cs, %ax
-	mov		%ax, %es
-
-	jmp		boot_kernel
-
-/* Variables */
-
-.align 4, 0
-gdt:
-	/* 0x00 */
-.byte	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-
-	/* 0x08: code segment (base=0, limit=0xfffff, type=32bit code exec/read, DPL=0, 4k) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x9a, 0xcf, 0x00
-
-	/* 0x10: data segment (base=0, limit=0xfffff, type=32bit data read/write, DPL=0, 4k) */
-.byte	0xff, 0xff, 0x00, 0x00, 0x00, 0x92, 0xcf, 0x00
-
-BOOT_ROM_END
-- 
2.51.0

Re: [PATCH-for-11.0 v6 00/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Philippe Mathieu-Daudé]

New (unreviewed) patches: 1, 2, 5-8

On 3/12/25 07:09, Philippe Mathieu-Daudé wrote:
> Hi Zhao,
> 
> This is my answer to this comment of yours:
> 
>   > Although other callers of fw_cfg_init_io_dma() besides x86 also pass
>   > DMA arguments to create DMA-enabled FwCfgIoState, the "dma_enabled"
>   > property of FwCfgIoState cannot yet be removed, because Sun4u and Sun4v
>   > still create DMA-disabled FwCfgIoState (bypass fw_cfg_init_io_dma()) in
>   > sun4uv_init() (hw/sparc64/sun4u.c).
>   >
>   > Maybe reusing fw_cfg_init_io_dma() for them would be a better choice, or
>   > adding fw_cfg_init_io_nodma(). However, before that, first simplify the
>   > handling of FwCfgState in x86.
>   >
>   > Considering that FwCfgIoState in x86 enables DMA by default, remove the
>   > handling for DMA-disabled cases and replace DMA checks with assertions
>   > to ensure that the default DMA-enabled setting is not broken.
> 
> My series is to apply just after this patch of your series:
> 
>    [PATCH v5 10/28] hw/mips/loongson3_virt: Prefer using fw_cfg_init_mem_nodma()
> 
> Regards,
> 
> Phil.
> 
> Based-on: <20251202162835.3227894-11-zhao1.liu@intel.com>
> 
> Philippe Mathieu-Daudé (12):
>    hw/ppc/mac: Use fw_cfg_init_mem_nodma()
>    hw/sparc/sun4m: Use fw_cfg_init_mem_nodma()
>    hw/nvram/fw_cfg: Factor fw_cfg_init_mem_internal() out
>    hw/nvram/fw_cfg: Rename fw_cfg_init_mem_wide() ->
>      fw_cfg_init_mem_dma()
>    hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()
>    hw/nvram/fw_cfg: Factor fw_cfg_init_io_internal() out
>    hw/nvram/fw_cfg: Add fw_cfg_init_io_nodma() helper
>    hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()
>    hw/nvram/fw_cfg: Remove fw_cfg_io_properties::dma_enabled
>    hw/i386/x86: Remove X86MachineClass::fwcfg_dma_enabled field
>    hw/i386/pc: Remove multiboot.bin
>    hw/i386: Remove linuxboot.bin

Re: [PATCH-for-11.0 v6 12/13] hw/i386: Assume fw_cfg DMA is always enabled

[Philippe Mathieu-Daudé]

On 3/12/25 07:09, Philippe Mathieu-Daudé wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Now all calls of x86 machines to fw_cfg_init_io_dma() pass DMA
> arguments, so the FWCfgState (FWCfgIoState) created by x86 machines
> enables DMA by default.
> 
> Then 'linuxboot.bin' isn't used anymore, and it will be removed in the
> next commit.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/fw_cfg.c     | 1 +
>   hw/i386/x86-common.c | 6 ++----
>   2 files changed, 3 insertions(+), 4 deletions(-)

Re: [PATCH-for-11.0 v6 01/13] hw/ppc/mac: Use fw_cfg_init_mem_nodma()

[Zhao Liu]

On Wed, Dec 03, 2025 at 07:09:29AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  3 Dec 2025 07:09:29 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-11.0 v6 01/13] hw/ppc/mac: Use fw_cfg_init_mem_nodma()
> X-Mailer: git-send-email 2.51.0
> 
> Use fw_cfg_init_mem_nodma() instead of open-coding it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/ppc/mac_newworld.c | 11 +----------
>  hw/ppc/mac_oldworld.c | 11 +----------
>  2 files changed, 2 insertions(+), 20 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH-for-11.0 v6 02/13] hw/sparc/sun4m: Use fw_cfg_init_mem_nodma()

[Zhao Liu]

On Wed, Dec 03, 2025 at 07:09:30AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  3 Dec 2025 07:09:30 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-11.0 v6 02/13] hw/sparc/sun4m: Use
>  fw_cfg_init_mem_nodma()
> X-Mailer: git-send-email 2.51.0
> 
> Use fw_cfg_init_mem_nodma() instead of open-coding it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/sparc/sun4m.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH-for-11.0 v6 05/13] hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()

[Zhao Liu]

On Wed, Dec 03, 2025 at 07:09:33AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  3 Dec 2025 07:09:33 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-11.0 v6 05/13] hw/nvram/fw_cfg: Propagate I/O
>  MemoryRegion to fw_cfg_init_io_dma()
> X-Mailer: git-send-email 2.51.0
> 
> To allow callers to use I/O MemoryRegion different than the
> global get_system_io(), pass it as argument.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/nvram/fw_cfg.h | 4 ++--
>  hw/i386/fw_cfg.c          | 3 ++-
>  hw/i386/microvm.c         | 3 ++-
>  hw/i386/pc.c              | 3 ++-
>  hw/nvram/fw_cfg.c         | 5 ++---
>  5 files changed, 10 insertions(+), 8 deletions(-)

...

> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 94d22a232ac..69f04d74a15 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -289,6 +289,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
>      X86MachineState *x86ms = X86_MACHINE(mms);
>      MemoryRegion *ram_below_4g, *ram_above_4g;
>      MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *io_memory = get_system_io();
>      FWCfgState *fw_cfg;
>      ram_addr_t lowmem = 0xc0000000; /* 3G */
>      int i;
> @@ -319,7 +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,
> +    fw_cfg = fw_cfg_init_io_dma(io_memory, FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,

It seems here we can use get_system_io() directly :).

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

>                                  &address_space_memory);
>  
>      fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, machine->smp.cpus);

Re: [PATCH-for-11.0 v6 06/13] hw/nvram/fw_cfg: Factor fw_cfg_init_io_internal() out

[Zhao Liu]

On Wed, Dec 03, 2025 at 07:09:34AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  3 Dec 2025 07:09:34 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-11.0 v6 06/13] hw/nvram/fw_cfg: Factor
>  fw_cfg_init_io_internal() out
> X-Mailer: git-send-email 2.51.0
> 
> Factor fw_cfg_init_io_internal() out of fw_cfg_init_io_dma().
> In fw_cfg_init_io_dma(), assert DMA arguments are provided.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/nvram/fw_cfg.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH-for-11.0 v6 07/13] hw/nvram/fw_cfg: Add fw_cfg_init_io_nodma() helper

[Zhao Liu]

On Wed, Dec 03, 2025 at 07:09:35AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  3 Dec 2025 07:09:35 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH-for-11.0 v6 07/13] hw/nvram/fw_cfg: Add
>  fw_cfg_init_io_nodma() helper
> X-Mailer: git-send-email 2.51.0
> 
> Calling fw_cfg_init_io_nodma(...) is more explicit
> than fw_cfg_init_io_dma(..., 0, NULL).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/hw/nvram/fw_cfg.h | 1 +
>  hw/nvram/fw_cfg.c         | 5 +++++
>  2 files changed, 6 insertions(+)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Zhao Liu]

Hi Philippe,

> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 82c3e7c855b..6dc9f64b74d 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -683,14 +683,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                             graphic_width, graphic_height, graphic_depth,
>                             (uint8_t *)&macaddr);
>  
> -    dev = qdev_new(TYPE_FW_CFG_IO);
> -    qdev_prop_set_bit(dev, "dma_enabled", false);
> -    object_property_add_child(OBJECT(ebus), TYPE_FW_CFG, OBJECT(dev));

There's another difference: fw_cfg_init_io_nodma() uses `machine` as the
parent and here sun4uv uses `ebus`.

I think maybe one reason to use `ebus` is because...

> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
... because the parent region is managed by ebus.

Perhaps we should add another argument: Object *parent?

> -                                &FW_CFG_IO(dev)->comb_iomem);
> -

Thanks,
Zhao

Re: [PATCH-for-11.0 v6 05/13] hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()

[Philippe Mathieu-Daudé]

On 3/12/25 16:39, Zhao Liu wrote:
> On Wed, Dec 03, 2025 at 07:09:33AM +0100, Philippe Mathieu-Daudé wrote:
>> Date: Wed,  3 Dec 2025 07:09:33 +0100
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Subject: [PATCH-for-11.0 v6 05/13] hw/nvram/fw_cfg: Propagate I/O
>>   MemoryRegion to fw_cfg_init_io_dma()
>> X-Mailer: git-send-email 2.51.0
>>
>> To allow callers to use I/O MemoryRegion different than the
>> global get_system_io(), pass it as argument.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/nvram/fw_cfg.h | 4 ++--
>>   hw/i386/fw_cfg.c          | 3 ++-
>>   hw/i386/microvm.c         | 3 ++-
>>   hw/i386/pc.c              | 3 ++-
>>   hw/nvram/fw_cfg.c         | 5 ++---
>>   5 files changed, 10 insertions(+), 8 deletions(-)
> 
> ...
> 
>> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
>> index 94d22a232ac..69f04d74a15 100644
>> --- a/hw/i386/microvm.c
>> +++ b/hw/i386/microvm.c
>> @@ -289,6 +289,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
>>       X86MachineState *x86ms = X86_MACHINE(mms);
>>       MemoryRegion *ram_below_4g, *ram_above_4g;
>>       MemoryRegion *system_memory = get_system_memory();
>> +    MemoryRegion *io_memory = get_system_io();
>>       FWCfgState *fw_cfg;
>>       ram_addr_t lowmem = 0xc0000000; /* 3G */
>>       int i;
>> @@ -319,7 +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,
>> +    fw_cfg = fw_cfg_init_io_dma(io_memory, FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4,
> 
> It seems here we can use get_system_io() directly :).

I tried to follow the function style (see local system_memory
declaration); I don't mind ;)

> 
> Otherwise,
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Thanks!

> 
>>                                   &address_space_memory);
>>   
>>       fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, machine->smp.cpus);

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Philippe Mathieu-Daudé]

On 3/12/25 16:51, Zhao Liu wrote:
> Hi Philippe,
> 
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 82c3e7c855b..6dc9f64b74d 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -683,14 +683,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>                              graphic_width, graphic_height, graphic_depth,
>>                              (uint8_t *)&macaddr);
>>   
>> -    dev = qdev_new(TYPE_FW_CFG_IO);
>> -    qdev_prop_set_bit(dev, "dma_enabled", false);
>> -    object_property_add_child(OBJECT(ebus), TYPE_FW_CFG, OBJECT(dev));
> 
> There's another difference: fw_cfg_init_io_nodma() uses `machine` as the
> parent and here sun4uv uses `ebus`.

Ah yeah I wanted to comment it but forgot :facepalm:

> 
> I think maybe one reason to use `ebus` is because...
> 
>> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> ... because the parent region is managed by ebus.
> 
> Perhaps we should add another argument: Object *parent?

I thought about it but don't think so, all instances but this one use
the machine container.

I'll improve the description.

> 
>> -                                &FW_CFG_IO(dev)->comb_iomem);
>> -
> 
> Thanks,
> Zhao
> 

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Mark Cave-Ayland]

On 03/12/2025 17:14, Philippe Mathieu-Daudé wrote:

> On 3/12/25 16:51, Zhao Liu wrote:
>> Hi Philippe,
>>
>>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>>> index 82c3e7c855b..6dc9f64b74d 100644
>>> --- a/hw/sparc64/sun4u.c
>>> +++ b/hw/sparc64/sun4u.c
>>> @@ -683,14 +683,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>>                              graphic_width, graphic_height, graphic_depth,
>>>                              (uint8_t *)&macaddr);
>>> -    dev = qdev_new(TYPE_FW_CFG_IO);
>>> -    qdev_prop_set_bit(dev, "dma_enabled", false);
>>> -    object_property_add_child(OBJECT(ebus), TYPE_FW_CFG, OBJECT(dev));
>>
>> There's another difference: fw_cfg_init_io_nodma() uses `machine` as the
>> parent and here sun4uv uses `ebus`.
> 
> Ah yeah I wanted to comment it but forgot :facepalm:
> 
>>
>> I think maybe one reason to use `ebus` is because...
>>
>>> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
>> ... because the parent region is managed by ebus.
>>
>> Perhaps we should add another argument: Object *parent?
> 
> I thought about it but don't think so, all instances but this one use
> the machine container.
> 
> I'll improve the description.

The reason that the fw_cfg device lives under ebus on sun4u is because the ebus 
device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped into I/O 
address space along with other ISA devices. I'm not sure that setting the parent to 
the machine is the right thing to do here.


ATB,

Mark.

Re: [PATCH-for-11.0 v6 00/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Michael S. Tsirkin]

On Wed, Dec 03, 2025 at 07:09:28AM +0100, Philippe Mathieu-Daudé wrote:
> Hi Zhao,
> 
> This is my answer to this comment of yours:
> 
>  > Although other callers of fw_cfg_init_io_dma() besides x86 also pass
>  > DMA arguments to create DMA-enabled FwCfgIoState, the "dma_enabled"
>  > property of FwCfgIoState cannot yet be removed, because Sun4u and Sun4v
>  > still create DMA-disabled FwCfgIoState (bypass fw_cfg_init_io_dma()) in
>  > sun4uv_init() (hw/sparc64/sun4u.c).
>  >
>  > Maybe reusing fw_cfg_init_io_dma() for them would be a better choice, or
>  > adding fw_cfg_init_io_nodma(). However, before that, first simplify the
>  > handling of FwCfgState in x86.
>  >
>  > Considering that FwCfgIoState in x86 enables DMA by default, remove the
>  > handling for DMA-disabled cases and replace DMA checks with assertions
>  > to ensure that the default DMA-enabled setting is not broken.
> 
> My series is to apply just after this patch of your series:
> 
>   [PATCH v5 10/28] hw/mips/loongson3_virt: Prefer using fw_cfg_init_mem_nodma()
> 
> Regards,
> 
> Phil.


pc/fwcfg things:

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


> Based-on: <20251202162835.3227894-11-zhao1.liu@intel.com>
> 
> Philippe Mathieu-Daudé (12):
>   hw/ppc/mac: Use fw_cfg_init_mem_nodma()
>   hw/sparc/sun4m: Use fw_cfg_init_mem_nodma()
>   hw/nvram/fw_cfg: Factor fw_cfg_init_mem_internal() out
>   hw/nvram/fw_cfg: Rename fw_cfg_init_mem_wide() ->
>     fw_cfg_init_mem_dma()
>   hw/nvram/fw_cfg: Propagate I/O MemoryRegion to fw_cfg_init_io_dma()
>   hw/nvram/fw_cfg: Factor fw_cfg_init_io_internal() out
>   hw/nvram/fw_cfg: Add fw_cfg_init_io_nodma() helper
>   hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()
>   hw/nvram/fw_cfg: Remove fw_cfg_io_properties::dma_enabled
>   hw/i386/x86: Remove X86MachineClass::fwcfg_dma_enabled field
>   hw/i386/pc: Remove multiboot.bin
>   hw/i386: Remove linuxboot.bin
> 
> Zhao Liu (1):
>   hw/i386: Assume fw_cfg DMA is always enabled
> 
>  include/hw/i386/x86.h             |   2 -
>  include/hw/nvram/fw_cfg.h         |  11 +-
>  pc-bios/optionrom/optionrom.h     |   4 -
>  hw/arm/virt.c                     |   2 +-
>  hw/i386/fw_cfg.c                  |  19 +--
>  hw/i386/microvm.c                 |   6 +-
>  hw/i386/multiboot.c               |   7 +-
>  hw/i386/pc.c                      |   7 +-
>  hw/i386/x86-common.c              |   7 +-
>  hw/i386/x86.c                     |   2 -
>  hw/loongarch/fw_cfg.c             |   4 +-
>  hw/nvram/fw_cfg.c                 |  61 ++++----
>  hw/ppc/mac_newworld.c             |  11 +-
>  hw/ppc/mac_oldworld.c             |  11 +-
>  hw/riscv/virt.c                   |   4 +-
>  hw/sparc/sun4m.c                  |  12 +-
>  hw/sparc64/sun4u.c                |   9 +-
>  pc-bios/meson.build               |   2 -
>  pc-bios/multiboot.bin             | Bin 1024 -> 0 bytes
>  pc-bios/optionrom/Makefile        |   2 +-
>  pc-bios/optionrom/linuxboot.S     | 195 -------------------------
>  pc-bios/optionrom/multiboot.S     | 232 -----------------------------
>  pc-bios/optionrom/multiboot_dma.S | 234 +++++++++++++++++++++++++++++-
>  23 files changed, 302 insertions(+), 542 deletions(-)
>  delete mode 100644 pc-bios/multiboot.bin
>  delete mode 100644 pc-bios/optionrom/linuxboot.S
>  delete mode 100644 pc-bios/optionrom/multiboot.S
> 
> -- 
> 2.51.0
> 
> 

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Zhao Liu]

> > > I think maybe one reason to use `ebus` is because...
> > > 
> > > > -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > > > -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
> > >                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > ... because the parent region is managed by ebus.
> > > 
> > > Perhaps we should add another argument: Object *parent?
> > 
> > I thought about it but don't think so, all instances but this one use
> > the machine container.
> > 
> > I'll improve the description.
> 
> The reason that the fw_cfg device lives under ebus on sun4u is because the
> ebus device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped
> into I/O address space along with other ISA devices. I'm not sure that
> setting the parent to the machine is the right thing to do here.

Thanks Philippe and Mark, IIUC, ebus doesn't have the hotplug use case,
so it lives long enough like the sun4u/v machine, therefore replacing
the parent object "ebus" with machine is safe.

And it might be better to explicitly set ebus as not supporting hotplug
(via dc->hotpluggable = false).

Adding a "parent" argument to the generic interface fw_cfg_init_io_nodma()
doesn't seem necessary at the moment, since using the default machine as
parent seems enough to meet all current needs in QEMU.

What do you think?

Regards,
Zhao

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Mark Cave-Ayland]

On 18/12/2025 08:52, Zhao Liu wrote:

>>>> I think maybe one reason to use `ebus` is because...
>>>>
>>>>> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>>>> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>>>>                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> ... because the parent region is managed by ebus.
>>>>
>>>> Perhaps we should add another argument: Object *parent?
>>>
>>> I thought about it but don't think so, all instances but this one use
>>> the machine container.
>>>
>>> I'll improve the description.
>>
>> The reason that the fw_cfg device lives under ebus on sun4u is because the
>> ebus device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped
>> into I/O address space along with other ISA devices. I'm not sure that
>> setting the parent to the machine is the right thing to do here.
> 
> Thanks Philippe and Mark, IIUC, ebus doesn't have the hotplug use case,
> so it lives long enough like the sun4u/v machine, therefore replacing
> the parent object "ebus" with machine is safe.

It's safe, but it still doesn't make sense for sun4u/v because there is no 
machine-level I/O address space as per x86. It really does exist as a separate legacy 
bus under a PCI bridge.

> And it might be better to explicitly set ebus as not supporting hotplug
> (via dc->hotpluggable = false).

That's correct, ebus does not support hotplug.

> Adding a "parent" argument to the generic interface fw_cfg_init_io_nodma()
> doesn't seem necessary at the moment, since using the default machine as
> parent seems enough to meet all current needs in QEMU.
> 
> What do you think?

My preference would be add to add the parent argument as it's easy to do, and doesn't 
attempt to enforce x86-type constraints on other architectures.


ATB,

Mark.

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Philippe Mathieu-Daudé]

On 18/12/25 09:52, Zhao Liu wrote:
>>>> I think maybe one reason to use `ebus` is because...
>>>>
>>>>> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>>>> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>>>>                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> ... because the parent region is managed by ebus.
>>>>
>>>> Perhaps we should add another argument: Object *parent?
>>>
>>> I thought about it but don't think so, all instances but this one use
>>> the machine container.
>>>
>>> I'll improve the description.
>>
>> The reason that the fw_cfg device lives under ebus on sun4u is because the
>> ebus device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped
>> into I/O address space along with other ISA devices. I'm not sure that
>> setting the parent to the machine is the right thing to do here.
> 
> Thanks Philippe and Mark, IIUC, ebus doesn't have the hotplug use case,
> so it lives long enough like the sun4u/v machine, therefore replacing
> the parent object "ebus" with machine is safe.
> 
> And it might be better to explicitly set ebus as not supporting hotplug
> (via dc->hotpluggable = false).
> 
> Adding a "parent" argument to the generic interface fw_cfg_init_io_nodma()
> doesn't seem necessary at the moment, since using the default machine as
> parent seems enough to meet all current needs in QEMU.
> 
> What do you think?

fw_cfg is per guest, and there can only be once instance of it; so IMHO it
makes sense to use the machine as parent. I should have posted a better
commit description upfront, sorry.

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Zhao Liu]

> > > The reason that the fw_cfg device lives under ebus on sun4u is because the
> > > ebus device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped
> > > into I/O address space along with other ISA devices. I'm not sure that
> > > setting the parent to the machine is the right thing to do here.
> > 
> > Thanks Philippe and Mark, IIUC, ebus doesn't have the hotplug use case,
> > so it lives long enough like the sun4u/v machine, therefore replacing
> > the parent object "ebus" with machine is safe.
> 
> It's safe, but it still doesn't make sense for sun4u/v because there is no
> machine-level I/O address space as per x86. It really does exist as a
> separate legacy bus under a PCI bridge.

I tend to agree with Philippe's perspective - since it's per-Guest,
setting the parent as machine seems appropriate.

> That's correct, ebus does not support hotplug.
> 
> > Adding a "parent" argument to the generic interface fw_cfg_init_io_nodma()
> > doesn't seem necessary at the moment, since using the default machine as
> > parent seems enough to meet all current needs in QEMU.
> > 
> > What do you think?
> 
> My preference would be add to add the parent argument as it's easy to do,
> and doesn't attempt to enforce x86-type constraints on other architectures.

I think it's not a x86-specific issue. The parent parameter's role is
lifecycle management - if we allowed custom parents, we'd have to
account for various complications that could result from early parent
destruction, so it's not much easy IMO.

Thanks,
Zhao

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Zhao Liu]

> fw_cfg is per guest, and there can only be once instance of it; so IMHO it
> makes sense to use the machine as parent. I should have posted a better
> commit description upfront, sorry.

You're welcome. It looks like most issues with the another series for
removing v2.6 & 2.7 pc have been resolved, so I'm trying to help clarify
this part to move things forward a bit.

Regards,
Zhao

Re: [PATCH-for-11.0 v6 08/13] hw/sparc64/sun4u: Use fw_cfg_init_io_nodma()

[Mark Cave-Ayland]

On 18/12/2025 11:22, Philippe Mathieu-Daudé wrote:

> On 18/12/25 09:52, Zhao Liu wrote:
>>>>> I think maybe one reason to use `ebus` is because...
>>>>>
>>>>>> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>>>>> -    memory_region_add_subregion(pci_address_space_io(ebus), BIOS_CFG_IOPORT,
>>>>>                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>> ... because the parent region is managed by ebus.
>>>>>
>>>>> Perhaps we should add another argument: Object *parent?
>>>>
>>>> I thought about it but don't think so, all instances but this one use
>>>> the machine container.
>>>>
>>>> I'll improve the description.
>>>
>>> The reason that the fw_cfg device lives under ebus on sun4u is because the
>>> ebus device is effectively a PCI-ISA bridge, and the fw_cfg port is mapped
>>> into I/O address space along with other ISA devices. I'm not sure that
>>> setting the parent to the machine is the right thing to do here.
>>
>> Thanks Philippe and Mark, IIUC, ebus doesn't have the hotplug use case,
>> so it lives long enough like the sun4u/v machine, therefore replacing
>> the parent object "ebus" with machine is safe.
>>
>> And it might be better to explicitly set ebus as not supporting hotplug
>> (via dc->hotpluggable = false).
>>
>> Adding a "parent" argument to the generic interface fw_cfg_init_io_nodma()
>> doesn't seem necessary at the moment, since using the default machine as
>> parent seems enough to meet all current needs in QEMU.
>>
>> What do you think?
> 
> fw_cfg is per guest, and there can only be once instance of it; so IMHO it
> makes sense to use the machine as parent. I should have posted a better
> commit description upfront, sorry.

That's true, but the I/O is mapped to the ISA bus and so if the ISA bus does not 
exist, it is impossible to access the device even if it exists on the machine.

Following on from this: at some point should we not be aiming for a qdev bus to have 
its devices as QOM children? Assigning fw_cfg directly to the machine would also be 
wrong in this scenario.


ATB,

Mark.