* [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:46 ` Philippe Mathieu-Daudé
2025-07-10 8:52 ` [PATCH v4 02/18] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
` (16 subsequent siblings)
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
possible to specify any CPU via -cpu on the command line, it makes no
sense to allow modern 64-bit CPUs to be used.
Restrict the isapc machine to the available 32-bit CPUs, taking care to
handle the case where if a user inadvertently uses -cpu max then the "best"
32-bit CPU is used (in this case the pentium3).
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ea7572e783..67c52d79b2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
#ifdef CONFIG_ISAPC
static void pc_init_isa(MachineState *machine)
{
+ /*
+ * There is a small chance that someone unintentionally passes "-cpu max"
+ * for the isapc machine, which will provide a much more modern 32-bit
+ * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
+ * been specified, choose the "best" 32-bit cpu possible which we consider
+ * be the pentium3 (deliberately choosing an Intel CPU given that the
+ * default 486 CPU for the isapc machine is also an Intel CPU).
+ */
+ if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
+ machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
+ }
+
pc_init1(machine, NULL);
}
#endif
@@ -786,7 +798,19 @@ DEFINE_I440FX_MACHINE(2, 6);
#ifdef CONFIG_ISAPC
static void isapc_machine_options(MachineClass *m)
{
+ static const char * const valid_cpu_types[] = {
+ X86_CPU_TYPE_NAME("486"),
+ X86_CPU_TYPE_NAME("athlon"),
+ X86_CPU_TYPE_NAME("kvm32"),
+ X86_CPU_TYPE_NAME("pentium"),
+ X86_CPU_TYPE_NAME("pentium2"),
+ X86_CPU_TYPE_NAME("pentium3"),
+ X86_CPU_TYPE_NAME("qemu32"),
+ X86_CPU_TYPE_NAME("max"),
+ NULL
+ };
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
m->desc = "ISA-only PC";
m->max_cpus = 1;
m->option_rom_has_mr = true;
@@ -799,6 +823,7 @@ static void isapc_machine_options(MachineClass *m)
pcmc->has_reserved_memory = false;
m->default_nic = "ne2k_isa";
m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+ m->valid_cpu_types = valid_cpu_types;
m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
2025-07-10 8:52 ` [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
@ 2025-07-10 10:46 ` Philippe Mathieu-Daudé
2025-07-10 15:28 ` Mark Cave-Ayland
0 siblings, 1 reply; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-10 10:46 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 10:52, Mark Cave-Ayland wrote:
> The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
> possible to specify any CPU via -cpu on the command line, it makes no
> sense to allow modern 64-bit CPUs to be used.
>
> Restrict the isapc machine to the available 32-bit CPUs, taking care to
> handle the case where if a user inadvertently uses -cpu max then the "best"
> 32-bit CPU is used (in this case the pentium3).
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
> hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ea7572e783..67c52d79b2 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
> #ifdef CONFIG_ISAPC
> static void pc_init_isa(MachineState *machine)
> {
> + /*
> + * There is a small chance that someone unintentionally passes "-cpu max"
> + * for the isapc machine, which will provide a much more modern 32-bit
> + * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
> + * been specified, choose the "best" 32-bit cpu possible which we consider
> + * be the pentium3 (deliberately choosing an Intel CPU given that the
> + * default 486 CPU for the isapc machine is also an Intel CPU).
> + */
> + if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
> + machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
Please warn() the user, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> + }
> +
> pc_init1(machine, NULL);
> }
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs
2025-07-10 10:46 ` Philippe Mathieu-Daudé
@ 2025-07-10 15:28 ` Mark Cave-Ayland
0 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 15:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, pbonzini, mst, marcel.apfelbaum,
eduardo, imammedo, qemu-devel
On 10/07/2025 11:46, Philippe Mathieu-Daudé wrote:
> On 10/7/25 10:52, Mark Cave-Ayland wrote:
>> The isapc machine represents a legacy ISA PC with a 486 CPU. Whilst it is
>> possible to specify any CPU via -cpu on the command line, it makes no
>> sense to allow modern 64-bit CPUs to be used.
>>
>> Restrict the isapc machine to the available 32-bit CPUs, taking care to
>> handle the case where if a user inadvertently uses -cpu max then the
>> "best"
>> 32-bit CPU is used (in this case the pentium3).
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>> hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index ea7572e783..67c52d79b2 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -418,6 +418,18 @@ static void pc_set_south_bridge(Object *obj, int
>> value, Error **errp)
>> #ifdef CONFIG_ISAPC
>> static void pc_init_isa(MachineState *machine)
>> {
>> + /*
>> + * There is a small chance that someone unintentionally passes "-
>> cpu max"
>> + * for the isapc machine, which will provide a much more modern
>> 32-bit
>> + * CPU than would be expected for an ISA-era PC. If the "max" cpu
>> type has
>> + * been specified, choose the "best" 32-bit cpu possible which we
>> consider
>> + * be the pentium3 (deliberately choosing an Intel CPU given that
>> the
>> + * default 486 CPU for the isapc machine is also an Intel CPU).
>> + */
>> + if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
>> + machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
>
> Please warn() the user, otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> + }
>> +
>> pc_init1(machine, NULL);
>> }
Thanks! That sounds reasonable - I can add that in v5.
ATB,
Mark.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 02/18] hw/i386/pc_piix.c: remove include for loader.h
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 03/18] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init() Mark Cave-Ayland
` (15 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
This header is not required since the loader functionality is handled separately
by pc_memory_init() in pc.c.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 67c52d79b2..f66eaeb910 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -28,7 +28,6 @@
#include "qemu/units.h"
#include "hw/char/parallel-isa.h"
#include "hw/dma/i8257.h"
-#include "hw/loader.h"
#include "hw/i386/x86.h"
#include "hw/i386/pc.h"
#include "hw/i386/apic.h"
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 03/18] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 01/18] hw/i386/pc_piix.c: restrict isapc machine to 32-bit CPUs Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 02/18] hw/i386/pc_piix.c: remove include for loader.h Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 04/18] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa() Mark Cave-Ayland
` (14 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
This is to prepare for splitting the isapc machine into its own separate file.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 261 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 260 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f66eaeb910..0928b905d5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -417,6 +417,87 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
#ifdef CONFIG_ISAPC
static void pc_init_isa(MachineState *machine)
{
+ const char *pci_type = NULL;
+ PCMachineState *pcms = PC_MACHINE(machine);
+ PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+ X86MachineState *x86ms = X86_MACHINE(machine);
+ MemoryRegion *system_memory = get_system_memory();
+ MemoryRegion *system_io = get_system_io();
+ Object *phb = NULL;
+ ISABus *isa_bus;
+ Object *piix4_pm = NULL;
+ qemu_irq smi_irq;
+ GSIState *gsi_state;
+ MemoryRegion *ram_memory;
+ MemoryRegion *pci_memory = NULL;
+ MemoryRegion *rom_memory = system_memory;
+ ram_addr_t lowmem;
+ uint64_t hole64_size = 0;
+
+ /*
+ * Calculate ram split, for memory below and above 4G. It's a bit
+ * complicated for backward compatibility reasons ...
+ *
+ * - Traditional split is 3.5G (lowmem = 0xe0000000). This is the
+ * default value for max_ram_below_4g now.
+ *
+ * - Then, to gigabyte align the memory, we move the split to 3G
+ * (lowmem = 0xc0000000). But only in case we have to split in
+ * the first place, i.e. ram_size is larger than (traditional)
+ * lowmem. And for new machine types (gigabyte_align = true)
+ * only, for live migration compatibility reasons.
+ *
+ * - Next the max-ram-below-4g option was added, which allowed to
+ * reduce lowmem to a smaller value, to allow a larger PCI I/O
+ * window below 4G. qemu doesn't enforce gigabyte alignment here,
+ * but prints a warning.
+ *
+ * - Finally max-ram-below-4g got updated to also allow raising lowmem,
+ * so legacy non-PAE guests can get as much memory as possible in
+ * the 32bit address space below 4G.
+ *
+ * - Note that Xen has its own ram setup code in xen_ram_init(),
+ * called via xen_hvm_init_pc().
+ *
+ * Examples:
+ * qemu -M pc-1.7 -m 4G (old default) -> 3584M low, 512M high
+ * qemu -M pc -m 4G (new default) -> 3072M low, 1024M high
+ * qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high
+ * qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M)
+ */
+ if (xen_enabled()) {
+ xen_hvm_init_pc(pcms, &ram_memory);
+ } else {
+ ram_memory = machine->ram;
+ if (!pcms->max_ram_below_4g) {
+ pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+ }
+ lowmem = pcms->max_ram_below_4g;
+ if (machine->ram_size >= pcms->max_ram_below_4g) {
+ if (pcmc->gigabyte_align) {
+ if (lowmem > 0xc0000000) {
+ lowmem = 0xc0000000;
+ }
+ if (lowmem & (1 * GiB - 1)) {
+ warn_report("Large machine and max_ram_below_4g "
+ "(%" PRIu64 ") not a multiple of 1G; "
+ "possible bad performance.",
+ pcms->max_ram_below_4g);
+ }
+ }
+ }
+
+ if (machine->ram_size >= lowmem) {
+ x86ms->above_4g_mem_size = machine->ram_size - lowmem;
+ x86ms->below_4g_mem_size = lowmem;
+ } else {
+ x86ms->above_4g_mem_size = 0;
+ x86ms->below_4g_mem_size = machine->ram_size;
+ }
+ }
+
+ pc_machine_init_sgx_epc(pcms);
+
/*
* There is a small chance that someone unintentionally passes "-cpu max"
* for the isapc machine, which will provide a much more modern 32-bit
@@ -429,7 +510,185 @@ static void pc_init_isa(MachineState *machine)
machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
}
- pc_init1(machine, NULL);
+ x86_cpus_init(x86ms, pcmc->default_cpu_version);
+
+ if (kvm_enabled()) {
+ kvmclock_create(pcmc->kvmclock_create_always);
+ }
+
+ if (pcmc->pci_enabled) {
+ pci_memory = g_new(MemoryRegion, 1);
+ memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
+ rom_memory = pci_memory;
+
+ phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
+ object_property_add_child(OBJECT(machine), "i440fx", phb);
+ object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+ OBJECT(ram_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+ OBJECT(pci_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+ OBJECT(system_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+ OBJECT(system_io), &error_fatal);
+ object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+ x86ms->below_4g_mem_size, &error_fatal);
+ object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+ x86ms->above_4g_mem_size, &error_fatal);
+ object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
+ &error_fatal);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+ pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
+ pci_bus_map_irqs(pcms->pcibus,
+ xen_enabled() ? xen_pci_slot_get_pirq
+ : pc_pci_slot_get_pirq);
+
+ hole64_size = object_property_get_uint(phb,
+ PCI_HOST_PROP_PCI_HOLE64_SIZE,
+ &error_abort);
+ }
+
+ /* allocate ram and load rom/bios */
+ if (!xen_enabled()) {
+ pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
+ } else {
+ assert(machine->ram_size == x86ms->below_4g_mem_size +
+ x86ms->above_4g_mem_size);
+
+ pc_system_flash_cleanup_unused(pcms);
+ if (machine->kernel_filename != NULL) {
+ /* For xen HVM direct kernel boot, load linux here */
+ xen_load_linux(pcms);
+ }
+ }
+
+ gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+
+ if (pcmc->pci_enabled) {
+ PCIDevice *pci_dev;
+ DeviceState *dev;
+ size_t i;
+
+ pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
+ object_property_set_bool(OBJECT(pci_dev), "has-usb",
+ machine_usb(machine), &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+ x86_machine_is_acpi_enabled(x86ms),
+ &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
+ &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
+ &error_abort);
+ qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+ object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+ x86_machine_is_smm_enabled(x86ms),
+ &error_abort);
+ dev = DEVICE(pci_dev);
+ for (i = 0; i < ISA_NUM_IRQS; i++) {
+ qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
+ }
+ pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
+
+ if (xen_enabled()) {
+ pci_device_set_intx_routing_notifier(
+ pci_dev, piix_intx_routing_notifier_xen);
+
+ /*
+ * Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI.
+ */
+ pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
+ XEN_IOAPIC_NUM_PIRQS);
+ }
+
+ isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
+ x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+ "rtc"));
+ piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+ dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+ pci_ide_create_devs(PCI_DEVICE(dev));
+ pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
+ pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
+ } else {
+ isa_bus = isa_bus_new(NULL, system_memory, system_io,
+ &error_abort);
+ isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+
+ x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+ qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+ isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+
+ i8257_dma_init(OBJECT(machine), isa_bus, 0);
+ pcms->hpet_enabled = false;
+ }
+
+ if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+ pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+ }
+
+ if (phb) {
+ ioapic_init_gsi(gsi_state, phb);
+ }
+
+ if (tcg_enabled()) {
+ x86_register_ferr_irq(x86ms->gsi[13]);
+ }
+
+ pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+
+ /* init basic PC hardware */
+ pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
+ !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
+
+ pc_nic_init(pcmc, isa_bus, pcms->pcibus);
+
+#ifdef CONFIG_IDE_ISA
+ if (!pcmc->pci_enabled) {
+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+ int i;
+
+ ide_drive_get(hd, ARRAY_SIZE(hd));
+ for (i = 0; i < MAX_IDE_BUS; i++) {
+ ISADevice *dev;
+ char busname[] = "ide.0";
+ dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+ ide_irq[i],
+ hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+ /*
+ * The ide bus name is ide.0 for the first bus and ide.1 for the
+ * second one.
+ */
+ busname[4] = '0' + i;
+ pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
+ }
+ }
+#endif
+
+ if (piix4_pm) {
+ smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
+
+ qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
+ pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
+ /* TODO: Populate SPD eeprom data. */
+ smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
+
+ object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
+ TYPE_HOTPLUG_HANDLER,
+ (Object **)&x86ms->acpi_dev,
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_STRONG);
+ object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
+ piix4_pm, &error_abort);
+ }
+
+ if (machine->nvdimms_state->is_enabled) {
+ nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
+ x86_nvdimm_acpi_dsmio,
+ x86ms->fw_cfg, OBJECT(pcms));
+ }
}
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 04/18] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (2 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 03/18] hw/i386/pc_piix.c: duplicate pc_init1() into pc_isa_init() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 05/18] hw/i386/pc_piix.c: remove SMI and piix4_pm " Mark Cave-Ayland
` (13 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
PCI code will never be used for an isapc machine.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 105 ++++------------------------------------------
1 file changed, 8 insertions(+), 97 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 0928b905d5..548e81cf1b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -417,19 +417,16 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
#ifdef CONFIG_ISAPC
static void pc_init_isa(MachineState *machine)
{
- const char *pci_type = NULL;
PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
X86MachineState *x86ms = X86_MACHINE(machine);
MemoryRegion *system_memory = get_system_memory();
MemoryRegion *system_io = get_system_io();
- Object *phb = NULL;
ISABus *isa_bus;
Object *piix4_pm = NULL;
qemu_irq smi_irq;
GSIState *gsi_state;
MemoryRegion *ram_memory;
- MemoryRegion *pci_memory = NULL;
MemoryRegion *rom_memory = system_memory;
ram_addr_t lowmem;
uint64_t hole64_size = 0;
@@ -516,39 +513,6 @@ static void pc_init_isa(MachineState *machine)
kvmclock_create(pcmc->kvmclock_create_always);
}
- if (pcmc->pci_enabled) {
- pci_memory = g_new(MemoryRegion, 1);
- memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
- rom_memory = pci_memory;
-
- phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
- object_property_add_child(OBJECT(machine), "i440fx", phb);
- object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
- OBJECT(ram_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
- OBJECT(pci_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
- OBJECT(system_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
- OBJECT(system_io), &error_fatal);
- object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
- x86ms->below_4g_mem_size, &error_fatal);
- object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
- x86ms->above_4g_mem_size, &error_fatal);
- object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
- &error_fatal);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
-
- pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
- pci_bus_map_irqs(pcms->pcibus,
- xen_enabled() ? xen_pci_slot_get_pirq
- : pc_pci_slot_get_pirq);
-
- hole64_size = object_property_get_uint(phb,
- PCI_HOST_PROP_PCI_HOLE64_SIZE,
- &error_abort);
- }
-
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
@@ -565,74 +529,21 @@ static void pc_init_isa(MachineState *machine)
gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
- if (pcmc->pci_enabled) {
- PCIDevice *pci_dev;
- DeviceState *dev;
- size_t i;
-
- pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
- object_property_set_bool(OBJECT(pci_dev), "has-usb",
- machine_usb(machine), &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-acpi",
- x86_machine_is_acpi_enabled(x86ms),
- &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
- &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
- &error_abort);
- qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
- object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
- x86_machine_is_smm_enabled(x86ms),
- &error_abort);
- dev = DEVICE(pci_dev);
- for (i = 0; i < ISA_NUM_IRQS; i++) {
- qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
- }
- pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
-
- if (xen_enabled()) {
- pci_device_set_intx_routing_notifier(
- pci_dev, piix_intx_routing_notifier_xen);
-
- /*
- * Xen supports additional interrupt routes from the PCI devices to
- * the IOAPIC: the four pins of each PCI device on the bus are also
- * connected to the IOAPIC directly.
- * These additional routes can be discovered through ACPI.
- */
- pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
- XEN_IOAPIC_NUM_PIRQS);
- }
-
- isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
- x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
- "rtc"));
- piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
- dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
- pci_ide_create_devs(PCI_DEVICE(dev));
- pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
- pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
- } else {
- isa_bus = isa_bus_new(NULL, system_memory, system_io,
- &error_abort);
- isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+ isa_bus = isa_bus_new(NULL, system_memory, system_io,
+ &error_abort);
+ isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
- x86ms->rtc = isa_new(TYPE_MC146818_RTC);
- qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
- isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+ x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+ qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+ isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
- i8257_dma_init(OBJECT(machine), isa_bus, 0);
- pcms->hpet_enabled = false;
- }
+ i8257_dma_init(OBJECT(machine), isa_bus, 0);
+ pcms->hpet_enabled = false;
if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
pc_i8259_create(isa_bus, gsi_state->i8259_irq);
}
- if (phb) {
- ioapic_init_gsi(gsi_state, phb);
- }
-
if (tcg_enabled()) {
x86_register_ferr_irq(x86ms->gsi[13]);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 05/18] hw/i386/pc_piix.c: remove SMI and piix4_pm initialisation from pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (3 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 04/18] hw/i386/pc_piix.c: remove pcmc->pci_enabled dependent initialisation from pc_init_isa() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 06/18] hw/i386/pc_piix.c: remove SGX " Mark Cave-Ayland
` (12 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
These are based upon the PIIX4 PCI chipset and so can never be used on an isapc machine.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 548e81cf1b..cf451d1cb5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -423,8 +423,6 @@ static void pc_init_isa(MachineState *machine)
MemoryRegion *system_memory = get_system_memory();
MemoryRegion *system_io = get_system_io();
ISABus *isa_bus;
- Object *piix4_pm = NULL;
- qemu_irq smi_irq;
GSIState *gsi_state;
MemoryRegion *ram_memory;
MemoryRegion *rom_memory = system_memory;
@@ -578,23 +576,6 @@ static void pc_init_isa(MachineState *machine)
}
#endif
- if (piix4_pm) {
- smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
-
- qdev_connect_gpio_out_named(DEVICE(piix4_pm), "smi-irq", 0, smi_irq);
- pcms->smbus = I2C_BUS(qdev_get_child_bus(DEVICE(piix4_pm), "i2c"));
- /* TODO: Populate SPD eeprom data. */
- smbus_eeprom_init(pcms->smbus, 8, NULL, 0);
-
- object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
- TYPE_HOTPLUG_HANDLER,
- (Object **)&x86ms->acpi_dev,
- object_property_allow_set_link,
- OBJ_PROP_LINK_STRONG);
- object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
- piix4_pm, &error_abort);
- }
-
if (machine->nvdimms_state->is_enabled) {
nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
x86_nvdimm_acpi_dsmio,
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 06/18] hw/i386/pc_piix.c: remove SGX initialisation from pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (4 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 05/18] hw/i386/pc_piix.c: remove SMI and piix4_pm " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 07/18] hw/i386/pc_piix.c: remove nvdimm " Mark Cave-Ayland
` (11 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
The Intel SGX instructions only exist on recent CPUs and so would never be available
on a CPU from the pre-PCI era.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cf451d1cb5..cbe4b33ba4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -491,8 +491,6 @@ static void pc_init_isa(MachineState *machine)
}
}
- pc_machine_init_sgx_epc(pcms);
-
/*
* There is a small chance that someone unintentionally passes "-cpu max"
* for the isapc machine, which will provide a much more modern 32-bit
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 07/18] hw/i386/pc_piix.c: remove nvdimm initialisation from pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (5 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 06/18] hw/i386/pc_piix.c: remove SGX " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 08/18] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
` (10 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
NVDIMMs cannot be used by PCs from a pre-PCI era.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cbe4b33ba4..2c508f9db6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -573,12 +573,6 @@ static void pc_init_isa(MachineState *machine)
}
}
#endif
-
- if (machine->nvdimms_state->is_enabled) {
- nvdimm_init_acpi_state(machine->nvdimms_state, system_io,
- x86_nvdimm_acpi_dsmio,
- x86ms->fw_cfg, OBJECT(pcms));
- }
}
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 08/18] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (6 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 07/18] hw/i386/pc_piix.c: remove nvdimm " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 09/18] hw/i386/pc_piix.c: hardcode hole64_size to 0 " Mark Cave-Ayland
` (9 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
All isapc machines must have 32-bit CPUs and so the RAM split logic can be hardcoded
accordingly.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 58 ++++-------------------------------------------
1 file changed, 4 insertions(+), 54 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 2c508f9db6..80e4ed8c87 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,69 +426,19 @@ static void pc_init_isa(MachineState *machine)
GSIState *gsi_state;
MemoryRegion *ram_memory;
MemoryRegion *rom_memory = system_memory;
- ram_addr_t lowmem;
uint64_t hole64_size = 0;
/*
- * Calculate ram split, for memory below and above 4G. It's a bit
- * complicated for backward compatibility reasons ...
- *
- * - Traditional split is 3.5G (lowmem = 0xe0000000). This is the
- * default value for max_ram_below_4g now.
- *
- * - Then, to gigabyte align the memory, we move the split to 3G
- * (lowmem = 0xc0000000). But only in case we have to split in
- * the first place, i.e. ram_size is larger than (traditional)
- * lowmem. And for new machine types (gigabyte_align = true)
- * only, for live migration compatibility reasons.
- *
- * - Next the max-ram-below-4g option was added, which allowed to
- * reduce lowmem to a smaller value, to allow a larger PCI I/O
- * window below 4G. qemu doesn't enforce gigabyte alignment here,
- * but prints a warning.
- *
- * - Finally max-ram-below-4g got updated to also allow raising lowmem,
- * so legacy non-PAE guests can get as much memory as possible in
- * the 32bit address space below 4G.
- *
- * - Note that Xen has its own ram setup code in xen_ram_init(),
- * called via xen_hvm_init_pc().
- *
- * Examples:
- * qemu -M pc-1.7 -m 4G (old default) -> 3584M low, 512M high
- * qemu -M pc -m 4G (new default) -> 3072M low, 1024M high
- * qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high
- * qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M)
+ * There is no RAM split for the isapc machine
*/
if (xen_enabled()) {
xen_hvm_init_pc(pcms, &ram_memory);
} else {
ram_memory = machine->ram;
- if (!pcms->max_ram_below_4g) {
- pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
- }
- lowmem = pcms->max_ram_below_4g;
- if (machine->ram_size >= pcms->max_ram_below_4g) {
- if (pcmc->gigabyte_align) {
- if (lowmem > 0xc0000000) {
- lowmem = 0xc0000000;
- }
- if (lowmem & (1 * GiB - 1)) {
- warn_report("Large machine and max_ram_below_4g "
- "(%" PRIu64 ") not a multiple of 1G; "
- "possible bad performance.",
- pcms->max_ram_below_4g);
- }
- }
- }
- if (machine->ram_size >= lowmem) {
- x86ms->above_4g_mem_size = machine->ram_size - lowmem;
- x86ms->below_4g_mem_size = lowmem;
- } else {
- x86ms->above_4g_mem_size = 0;
- x86ms->below_4g_mem_size = machine->ram_size;
- }
+ pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+ x86ms->above_4g_mem_size = 0;
+ x86ms->below_4g_mem_size = machine->ram_size;
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 09/18] hw/i386/pc_piix.c: hardcode hole64_size to 0 in pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (7 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 08/18] hw/i386/pc_piix.c: simplify RAM size logic in pc_init_isa() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 10/18] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa() Mark Cave-Ayland
` (8 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
All isapc machines must have 32-bit CPUs and have no PCI 64-bit hole so it can be
hardcoded to 0.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 80e4ed8c87..48d0b2f5b5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,7 +426,6 @@ static void pc_init_isa(MachineState *machine)
GSIState *gsi_state;
MemoryRegion *ram_memory;
MemoryRegion *rom_memory = system_memory;
- uint64_t hole64_size = 0;
/*
* There is no RAM split for the isapc machine
@@ -461,7 +460,7 @@ static void pc_init_isa(MachineState *machine)
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
+ pc_memory_init(pcms, system_memory, rom_memory, 0);
} else {
assert(machine->ram_size == x86ms->below_4g_mem_size +
x86ms->above_4g_mem_size);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 10/18] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (8 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 09/18] hw/i386/pc_piix.c: hardcode hole64_size to 0 " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:47 ` Philippe Mathieu-Daudé
2025-07-10 8:52 ` [PATCH v4 11/18] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa() Mark Cave-Ayland
` (7 subsequent siblings)
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
This function contains 'assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled)' and so we can
safely assume that it should never be used for the isapc machine.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 48d0b2f5b5..f09ec48c0c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -465,7 +465,6 @@ static void pc_init_isa(MachineState *machine)
assert(machine->ram_size == x86ms->below_4g_mem_size +
x86ms->above_4g_mem_size);
- pc_system_flash_cleanup_unused(pcms);
if (machine->kernel_filename != NULL) {
/* For xen HVM direct kernel boot, load linux here */
xen_load_linux(pcms);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 11/18] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (9 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 10/18] hw/i386/pc_piix.c: remove pc_system_flash_cleanup_unused() from pc_init_isa() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 12/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false " Mark Cave-Ayland
` (6 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
By definition an isapc machine must always use ISA IDE drives so ensure that they
are always enabled. At the same time also remove the surrounding CONFIG_IDE_ISA
define since it will be enabled via the ISAPC Kconfig.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 35 +++++++++++++++--------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f09ec48c0c..9832b5d6c4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -426,6 +426,8 @@ static void pc_init_isa(MachineState *machine)
GSIState *gsi_state;
MemoryRegion *ram_memory;
MemoryRegion *rom_memory = system_memory;
+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+ int i;
/*
* There is no RAM split for the isapc machine
@@ -500,27 +502,20 @@ static void pc_init_isa(MachineState *machine)
pc_nic_init(pcmc, isa_bus, pcms->pcibus);
-#ifdef CONFIG_IDE_ISA
- if (!pcmc->pci_enabled) {
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
- int i;
-
- ide_drive_get(hd, ARRAY_SIZE(hd));
- for (i = 0; i < MAX_IDE_BUS; i++) {
- ISADevice *dev;
- char busname[] = "ide.0";
- dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
- ide_irq[i],
- hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
- /*
- * The ide bus name is ide.0 for the first bus and ide.1 for the
- * second one.
- */
- busname[4] = '0' + i;
- pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
- }
+ ide_drive_get(hd, ARRAY_SIZE(hd));
+ for (i = 0; i < MAX_IDE_BUS; i++) {
+ ISADevice *dev;
+ char busname[] = "ide.0";
+ dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+ ide_irq[i],
+ hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+ /*
+ * The ide bus name is ide.0 for the first bus and ide.1 for the
+ * second one.
+ */
+ busname[4] = '0' + i;
+ pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
}
-#endif
}
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 12/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false in pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (10 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 11/18] hw/i386/pc_piix.c: always initialise ISA IDE drives in pc_init_isa() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 13/18] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL " Mark Cave-Ayland
` (5 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
By definition PCI can never be enabled on an isapc machine so hardcode the relevant values
set via pcmc->pci_enabled.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9832b5d6c4..7b38e37d7b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -473,7 +473,7 @@ static void pc_init_isa(MachineState *machine)
}
}
- gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
+ gsi_state = pc_gsi_create(&x86ms->gsi, false);
isa_bus = isa_bus_new(NULL, system_memory, system_io,
&error_abort);
@@ -494,7 +494,7 @@ static void pc_init_isa(MachineState *machine)
x86_register_ferr_irq(x86ms->gsi[13]);
}
- pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+ pc_vga_init(isa_bus, NULL);
/* init basic PC hardware */
pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 13/18] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL in pc_init_isa()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (11 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 12/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always false " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 14/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
` (4 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
By definition PCI can never be enabled on an isapc machine so hardcode the PCIBus argument
of pc_nic_init() to NULL.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/i386/pc_piix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7b38e37d7b..70eaf5ed48 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -500,7 +500,7 @@ static void pc_init_isa(MachineState *machine)
pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
!MACHINE_CLASS(pcmc)->no_floppy, 0x4);
- pc_nic_init(pcmc, isa_bus, pcms->pcibus);
+ pc_nic_init(pcmc, isa_bus, NULL);
ide_drive_get(hd, ARRAY_SIZE(hd));
for (i = 0; i < MAX_IDE_BUS; i++) {
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 14/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1()
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (12 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 13/18] hw/i386/pc_piix.c: hardcode pcms->pci_bus to NULL " Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:48 ` Philippe Mathieu-Daudé
2025-07-10 8:52 ` [PATCH v4 15/18] hw/i386: move isapc machine to separate isapc.c file Mark Cave-Ayland
` (3 subsequent siblings)
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
PCI is always enabled on the pc-i440fx machine so hardcode the relevant logic
in pc_init1().
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 194 ++++++++++++++++++----------------------------
1 file changed, 76 insertions(+), 118 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 70eaf5ed48..cac016df22 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,7 +70,7 @@
#define XEN_IOAPIC_NUM_PIRQS 128ULL
-#ifdef CONFIG_IDE_ISA
+#ifdef CONFIG_ISAPC
static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
@@ -117,6 +117,9 @@ static void pc_init1(MachineState *machine, const char *pci_type)
MemoryRegion *rom_memory = system_memory;
ram_addr_t lowmem;
uint64_t hole64_size = 0;
+ PCIDevice *pci_dev;
+ DeviceState *dev;
+ size_t i;
/*
* Calculate ram split, for memory below and above 4G. It's a bit
@@ -187,38 +190,36 @@ static void pc_init1(MachineState *machine, const char *pci_type)
kvmclock_create(pcmc->kvmclock_create_always);
}
- if (pcmc->pci_enabled) {
- pci_memory = g_new(MemoryRegion, 1);
- memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
- rom_memory = pci_memory;
-
- phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
- object_property_add_child(OBJECT(machine), "i440fx", phb);
- object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
- OBJECT(ram_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
- OBJECT(pci_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
- OBJECT(system_memory), &error_fatal);
- object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
- OBJECT(system_io), &error_fatal);
- object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
- x86ms->below_4g_mem_size, &error_fatal);
- object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
- x86ms->above_4g_mem_size, &error_fatal);
- object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
- &error_fatal);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
-
- pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
- pci_bus_map_irqs(pcms->pcibus,
- xen_enabled() ? xen_pci_slot_get_pirq
- : pc_pci_slot_get_pirq);
-
- hole64_size = object_property_get_uint(phb,
- PCI_HOST_PROP_PCI_HOLE64_SIZE,
- &error_abort);
- }
+ pci_memory = g_new(MemoryRegion, 1);
+ memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
+ rom_memory = pci_memory;
+
+ phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
+ object_property_add_child(OBJECT(machine), "i440fx", phb);
+ object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
+ OBJECT(ram_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_PCI_MEM,
+ OBJECT(pci_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_SYSTEM_MEM,
+ OBJECT(system_memory), &error_fatal);
+ object_property_set_link(phb, PCI_HOST_PROP_IO_MEM,
+ OBJECT(system_io), &error_fatal);
+ object_property_set_uint(phb, PCI_HOST_BELOW_4G_MEM_SIZE,
+ x86ms->below_4g_mem_size, &error_fatal);
+ object_property_set_uint(phb, PCI_HOST_ABOVE_4G_MEM_SIZE,
+ x86ms->above_4g_mem_size, &error_fatal);
+ object_property_set_str(phb, I440FX_HOST_PROP_PCI_TYPE, pci_type,
+ &error_fatal);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(phb), &error_fatal);
+
+ pcms->pcibus = PCI_BUS(qdev_get_child_bus(DEVICE(phb), "pci.0"));
+ pci_bus_map_irqs(pcms->pcibus,
+ xen_enabled() ? xen_pci_slot_get_pirq
+ : pc_pci_slot_get_pirq);
+
+ hole64_size = object_property_get_uint(phb,
+ PCI_HOST_PROP_PCI_HOLE64_SIZE,
+ &error_abort);
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
@@ -234,72 +235,51 @@ static void pc_init1(MachineState *machine, const char *pci_type)
}
}
- gsi_state = pc_gsi_create(&x86ms->gsi, pcmc->pci_enabled);
-
- if (pcmc->pci_enabled) {
- PCIDevice *pci_dev;
- DeviceState *dev;
- size_t i;
-
- pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
- object_property_set_bool(OBJECT(pci_dev), "has-usb",
- machine_usb(machine), &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-acpi",
- x86_machine_is_acpi_enabled(x86ms),
- &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
- &error_abort);
- object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
- &error_abort);
- qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
- object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
- x86_machine_is_smm_enabled(x86ms),
- &error_abort);
- dev = DEVICE(pci_dev);
- for (i = 0; i < ISA_NUM_IRQS; i++) {
- qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
- }
- pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
-
- if (xen_enabled()) {
- pci_device_set_intx_routing_notifier(
- pci_dev, piix_intx_routing_notifier_xen);
-
- /*
- * Xen supports additional interrupt routes from the PCI devices to
- * the IOAPIC: the four pins of each PCI device on the bus are also
- * connected to the IOAPIC directly.
- * These additional routes can be discovered through ACPI.
- */
- pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
- XEN_IOAPIC_NUM_PIRQS);
- }
+ gsi_state = pc_gsi_create(&x86ms->gsi, true);
+
+ pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
+ object_property_set_bool(OBJECT(pci_dev), "has-usb",
+ machine_usb(machine), &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-acpi",
+ x86_machine_is_acpi_enabled(x86ms),
+ &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
+ &error_abort);
+ object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
+ &error_abort);
+ qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
+ object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
+ x86_machine_is_smm_enabled(x86ms),
+ &error_abort);
+ dev = DEVICE(pci_dev);
+ for (i = 0; i < ISA_NUM_IRQS; i++) {
+ qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
+ }
+ pci_realize_and_unref(pci_dev, pcms->pcibus, &error_fatal);
- isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
- x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
- "rtc"));
- piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
- dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
- pci_ide_create_devs(PCI_DEVICE(dev));
- pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
- pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
- } else {
- uint32_t irq;
+ if (xen_enabled()) {
+ pci_device_set_intx_routing_notifier(
+ pci_dev, piix_intx_routing_notifier_xen);
- isa_bus = isa_bus_new(NULL, system_memory, system_io,
- &error_abort);
- isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+ /*
+ * Xen supports additional interrupt routes from the PCI devices to
+ * the IOAPIC: the four pins of each PCI device on the bus are also
+ * connected to the IOAPIC directly.
+ * These additional routes can be discovered through ACPI.
+ */
+ pci_bus_irqs(pcms->pcibus, xen_intx_set_irq, pci_dev,
+ XEN_IOAPIC_NUM_PIRQS);
+ }
- x86ms->rtc = isa_new(TYPE_MC146818_RTC);
- qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
- isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
- irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
- &error_fatal);
- isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
+ isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(pci_dev), "isa.0"));
+ x86ms->rtc = ISA_DEVICE(object_resolve_path_component(OBJECT(pci_dev),
+ "rtc"));
+ piix4_pm = object_resolve_path_component(OBJECT(pci_dev), "pm");
+ dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));
+ pci_ide_create_devs(PCI_DEVICE(dev));
+ pcms->idebus[0] = qdev_get_child_bus(dev, "ide.0");
+ pcms->idebus[1] = qdev_get_child_bus(dev, "ide.1");
- i8257_dma_init(OBJECT(machine), isa_bus, 0);
- pcms->hpet_enabled = false;
- }
if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
pc_i8259_create(isa_bus, gsi_state->i8259_irq);
@@ -313,7 +293,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
x86_register_ferr_irq(x86ms->gsi[13]);
}
- pc_vga_init(isa_bus, pcmc->pci_enabled ? pcms->pcibus : NULL);
+ pc_vga_init(isa_bus, pcms->pcibus);
/* init basic PC hardware */
pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
@@ -321,28 +301,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
pc_nic_init(pcmc, isa_bus, pcms->pcibus);
-#ifdef CONFIG_IDE_ISA
- if (!pcmc->pci_enabled) {
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
- int i;
-
- ide_drive_get(hd, ARRAY_SIZE(hd));
- for (i = 0; i < MAX_IDE_BUS; i++) {
- ISADevice *dev;
- char busname[] = "ide.0";
- dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
- ide_irq[i],
- hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
- /*
- * The ide bus name is ide.0 for the first bus and ide.1 for the
- * second one.
- */
- busname[4] = '0' + i;
- pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
- }
- }
-#endif
-
if (piix4_pm) {
smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 14/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1()
2025-07-10 8:52 ` [PATCH v4 14/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
@ 2025-07-10 10:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-10 10:48 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 10:52, Mark Cave-Ayland wrote:
> PCI is always enabled on the pc-i440fx machine so hardcode the relevant logic
> in pc_init1().
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
> hw/i386/pc_piix.c | 194 ++++++++++++++++++----------------------------
> 1 file changed, 76 insertions(+), 118 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 15/18] hw/i386: move isapc machine to separate isapc.c file
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (13 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 14/18] hw/i386/pc_piix.c: assume pcmc->pci_enabled is always true in pc_init1() Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 16/18] hw/i386/pc_piix.c: remove unused headers after isapc machine split Mark Cave-Ayland
` (2 subsequent siblings)
17 siblings, 0 replies; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
Now that pc_init_isa() is independent of any PCI initialisation, move it into a
separate isapc.c file. This enables us to finally fix the dependency of ISAPC on
I440FX in hw/i386/Kconfig.
Note that as part of the move to a separate file we can see that the licence text
is a verbatim copy of the MIT licence. The text originates from commit 1df912cf9e
("VL license of the day is MIT/BSD") so we can be sure that this was the original
intent. As a consequence we can update the file header to use a SPDX tag as per
the current project contribution guidelines.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
Reviewed-by: Bernhard Beschow <shentey@gmail.com>
---
hw/i386/Kconfig | 3 -
hw/i386/isapc.c | 168 ++++++++++++++++++++++++++++++++++++++++++++
hw/i386/meson.build | 1 +
hw/i386/pc_piix.c | 148 --------------------------------------
4 files changed, 169 insertions(+), 151 deletions(-)
create mode 100644 hw/i386/isapc.c
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 14d23e27b5..8ffcc9f7aa 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -96,9 +96,6 @@ config ISAPC
select ISA_BUS
select PC
select IDE_ISA
- # FIXME: it is in the same file as i440fx, and does not compile
- # if separated
- depends on I440FX
config Q35
bool
diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
new file mode 100644
index 0000000000..bb22083821
--- /dev/null
+++ b/hw/i386/isapc.c
@@ -0,0 +1,168 @@
+/*
+ * QEMU PC System Emulator
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/char/parallel-isa.h"
+#include "hw/dma/i8257.h"
+#include "hw/i386/pc.h"
+#include "hw/ide/isa.h"
+#include "hw/ide/ide-bus.h"
+#include "system/kvm.h"
+#include "hw/i386/kvm/clock.h"
+#include "hw/xen/xen-x86.h"
+#include "system/xen.h"
+#include "hw/rtc/mc146818rtc.h"
+#include "target/i386/cpu.h"
+
+static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
+static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
+static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
+
+
+static void pc_init_isa(MachineState *machine)
+{
+ PCMachineState *pcms = PC_MACHINE(machine);
+ PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+ X86MachineState *x86ms = X86_MACHINE(machine);
+ MemoryRegion *system_memory = get_system_memory();
+ MemoryRegion *system_io = get_system_io();
+ ISABus *isa_bus;
+ GSIState *gsi_state;
+ MemoryRegion *ram_memory;
+ MemoryRegion *rom_memory = system_memory;
+ DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+ uint32_t irq;
+ int i;
+
+ /*
+ * There is no RAM split for the isapc machine
+ */
+ if (xen_enabled()) {
+ xen_hvm_init_pc(pcms, &ram_memory);
+ } else {
+ ram_memory = machine->ram;
+
+ pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
+ x86ms->above_4g_mem_size = 0;
+ x86ms->below_4g_mem_size = machine->ram_size;
+ }
+
+ /*
+ * There is a small chance that someone unintentionally passes "-cpu max"
+ * for the isapc machine, which will provide a much more modern 32-bit
+ * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
+ * been specified, choose the "best" 32-bit cpu possible which we consider
+ * be the pentium3 (deliberately choosing an Intel CPU given that the
+ * default 486 CPU for the isapc machine is also an Intel CPU).
+ */
+ if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
+ machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
+ }
+
+ x86_cpus_init(x86ms, pcmc->default_cpu_version);
+
+ if (kvm_enabled()) {
+ kvmclock_create(pcmc->kvmclock_create_always);
+ }
+
+ /* allocate ram and load rom/bios */
+ if (!xen_enabled()) {
+ pc_memory_init(pcms, system_memory, rom_memory, 0);
+ } else {
+ assert(machine->ram_size == x86ms->below_4g_mem_size +
+ x86ms->above_4g_mem_size);
+
+ if (machine->kernel_filename != NULL) {
+ /* For xen HVM direct kernel boot, load linux here */
+ xen_load_linux(pcms);
+ }
+ }
+
+ gsi_state = pc_gsi_create(&x86ms->gsi, false);
+
+ isa_bus = isa_bus_new(NULL, system_memory, system_io,
+ &error_abort);
+ isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
+
+ x86ms->rtc = isa_new(TYPE_MC146818_RTC);
+ qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
+ isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
+ irq = object_property_get_uint(OBJECT(x86ms->rtc), "irq",
+ &error_fatal);
+ isa_connect_gpio_out(ISA_DEVICE(x86ms->rtc), 0, irq);
+
+ i8257_dma_init(OBJECT(machine), isa_bus, 0);
+ pcms->hpet_enabled = false;
+
+ if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+ pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+ }
+
+ if (tcg_enabled()) {
+ x86_register_ferr_irq(x86ms->gsi[13]);
+ }
+
+ pc_vga_init(isa_bus, NULL);
+
+ /* init basic PC hardware */
+ pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
+ !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
+
+ pc_nic_init(pcmc, isa_bus, NULL);
+
+ ide_drive_get(hd, ARRAY_SIZE(hd));
+ for (i = 0; i < MAX_IDE_BUS; i++) {
+ ISADevice *dev;
+ char busname[] = "ide.0";
+ dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
+ ide_irq[i],
+ hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
+ /*
+ * The ide bus name is ide.0 for the first bus and ide.1 for the
+ * second one.
+ */
+ busname[4] = '0' + i;
+ pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
+ }
+}
+
+static void isapc_machine_options(MachineClass *m)
+{
+ static const char * const valid_cpu_types[] = {
+ X86_CPU_TYPE_NAME("486"),
+ X86_CPU_TYPE_NAME("athlon"),
+ X86_CPU_TYPE_NAME("kvm32"),
+ X86_CPU_TYPE_NAME("pentium"),
+ X86_CPU_TYPE_NAME("pentium2"),
+ X86_CPU_TYPE_NAME("pentium3"),
+ X86_CPU_TYPE_NAME("qemu32"),
+ X86_CPU_TYPE_NAME("max"),
+ NULL
+ };
+ PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
+
+ m->desc = "ISA-only PC";
+ m->max_cpus = 1;
+ m->option_rom_has_mr = true;
+ m->rom_file_has_mr = false;
+ pcmc->pci_enabled = false;
+ pcmc->has_acpi_build = false;
+ pcmc->smbios_defaults = false;
+ pcmc->gigabyte_align = false;
+ pcmc->smbios_legacy_mode = true;
+ pcmc->has_reserved_memory = false;
+ m->default_nic = "ne2k_isa";
+ m->default_cpu_type = X86_CPU_TYPE_NAME("486");
+ m->valid_cpu_types = valid_cpu_types;
+ m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
+ m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
+}
+
+DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
+ isapc_machine_options);
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 7896f348cf..436b3ce52d 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -14,6 +14,7 @@ i386_ss.add(when: 'CONFIG_X86_IOMMU', if_true: files('x86-iommu.c'),
i386_ss.add(when: 'CONFIG_AMD_IOMMU', if_true: files('amd_iommu.c'),
if_false: files('amd_iommu-stub.c'))
i386_ss.add(when: 'CONFIG_I440FX', if_true: files('pc_piix.c'))
+i386_ss.add(when: 'CONFIG_ISAPC', if_true: files('isapc.c'))
i386_ss.add(when: 'CONFIG_MICROVM', if_true: files('x86-common.c', 'microvm.c', 'acpi-microvm.c', 'microvm-dt.c'))
i386_ss.add(when: 'CONFIG_NITRO_ENCLAVE', if_true: files('nitro_enclave.c'))
i386_ss.add(when: 'CONFIG_Q35', if_true: files('pc_q35.c'))
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index cac016df22..b04c683e4e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,12 +70,6 @@
#define XEN_IOAPIC_NUM_PIRQS 128ULL
-#ifdef CONFIG_ISAPC
-static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
-static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
-static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
-#endif
-
/*
* Return the global irq number corresponding to a given device irq
* pin. We could also use the bus number to have a more precise mapping.
@@ -372,111 +366,6 @@ static void pc_set_south_bridge(Object *obj, int value, Error **errp)
pcms->south_bridge = PCSouthBridgeOption_lookup.array[value];
}
-#ifdef CONFIG_ISAPC
-static void pc_init_isa(MachineState *machine)
-{
- PCMachineState *pcms = PC_MACHINE(machine);
- PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
- X86MachineState *x86ms = X86_MACHINE(machine);
- MemoryRegion *system_memory = get_system_memory();
- MemoryRegion *system_io = get_system_io();
- ISABus *isa_bus;
- GSIState *gsi_state;
- MemoryRegion *ram_memory;
- MemoryRegion *rom_memory = system_memory;
- DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
- int i;
-
- /*
- * There is no RAM split for the isapc machine
- */
- if (xen_enabled()) {
- xen_hvm_init_pc(pcms, &ram_memory);
- } else {
- ram_memory = machine->ram;
-
- pcms->max_ram_below_4g = 0xe0000000; /* default: 3.5G */
- x86ms->above_4g_mem_size = 0;
- x86ms->below_4g_mem_size = machine->ram_size;
- }
-
- /*
- * There is a small chance that someone unintentionally passes "-cpu max"
- * for the isapc machine, which will provide a much more modern 32-bit
- * CPU than would be expected for an ISA-era PC. If the "max" cpu type has
- * been specified, choose the "best" 32-bit cpu possible which we consider
- * be the pentium3 (deliberately choosing an Intel CPU given that the
- * default 486 CPU for the isapc machine is also an Intel CPU).
- */
- if (!strcmp(machine->cpu_type, X86_CPU_TYPE_NAME("max"))) {
- machine->cpu_type = X86_CPU_TYPE_NAME("pentium3");
- }
-
- x86_cpus_init(x86ms, pcmc->default_cpu_version);
-
- if (kvm_enabled()) {
- kvmclock_create(pcmc->kvmclock_create_always);
- }
-
- /* allocate ram and load rom/bios */
- if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory, rom_memory, 0);
- } else {
- assert(machine->ram_size == x86ms->below_4g_mem_size +
- x86ms->above_4g_mem_size);
-
- if (machine->kernel_filename != NULL) {
- /* For xen HVM direct kernel boot, load linux here */
- xen_load_linux(pcms);
- }
- }
-
- gsi_state = pc_gsi_create(&x86ms->gsi, false);
-
- isa_bus = isa_bus_new(NULL, system_memory, system_io,
- &error_abort);
- isa_bus_register_input_irqs(isa_bus, x86ms->gsi);
-
- x86ms->rtc = isa_new(TYPE_MC146818_RTC);
- qdev_prop_set_int32(DEVICE(x86ms->rtc), "base_year", 2000);
- isa_realize_and_unref(x86ms->rtc, isa_bus, &error_fatal);
-
- i8257_dma_init(OBJECT(machine), isa_bus, 0);
- pcms->hpet_enabled = false;
-
- if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
- pc_i8259_create(isa_bus, gsi_state->i8259_irq);
- }
-
- if (tcg_enabled()) {
- x86_register_ferr_irq(x86ms->gsi[13]);
- }
-
- pc_vga_init(isa_bus, NULL);
-
- /* init basic PC hardware */
- pc_basic_device_init(pcms, isa_bus, x86ms->gsi, x86ms->rtc,
- !MACHINE_CLASS(pcmc)->no_floppy, 0x4);
-
- pc_nic_init(pcmc, isa_bus, NULL);
-
- ide_drive_get(hd, ARRAY_SIZE(hd));
- for (i = 0; i < MAX_IDE_BUS; i++) {
- ISADevice *dev;
- char busname[] = "ide.0";
- dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
- ide_irq[i],
- hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 1]);
- /*
- * The ide bus name is ide.0 for the first bus and ide.1 for the
- * second one.
- */
- busname[4] = '0' + i;
- pcms->idebus[i] = qdev_get_child_bus(DEVICE(dev), busname);
- }
-}
-#endif
-
#ifdef CONFIG_XEN
static void pc_xen_hvm_init_pci(MachineState *machine)
{
@@ -838,43 +727,6 @@ static void pc_i440fx_machine_2_6_options(MachineClass *m)
DEFINE_I440FX_MACHINE(2, 6);
-#ifdef CONFIG_ISAPC
-static void isapc_machine_options(MachineClass *m)
-{
- static const char * const valid_cpu_types[] = {
- X86_CPU_TYPE_NAME("486"),
- X86_CPU_TYPE_NAME("athlon"),
- X86_CPU_TYPE_NAME("kvm32"),
- X86_CPU_TYPE_NAME("pentium"),
- X86_CPU_TYPE_NAME("pentium2"),
- X86_CPU_TYPE_NAME("pentium3"),
- X86_CPU_TYPE_NAME("qemu32"),
- X86_CPU_TYPE_NAME("max"),
- NULL
- };
- PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-
- m->desc = "ISA-only PC";
- m->max_cpus = 1;
- m->option_rom_has_mr = true;
- m->rom_file_has_mr = false;
- pcmc->pci_enabled = false;
- pcmc->has_acpi_build = false;
- pcmc->smbios_defaults = false;
- pcmc->gigabyte_align = false;
- pcmc->smbios_legacy_mode = true;
- pcmc->has_reserved_memory = false;
- m->default_nic = "ne2k_isa";
- m->default_cpu_type = X86_CPU_TYPE_NAME("486");
- m->valid_cpu_types = valid_cpu_types;
- m->no_floppy = !module_object_class_by_name(TYPE_ISA_FDC);
- m->no_parallel = !module_object_class_by_name(TYPE_ISA_PARALLEL);
-}
-
-DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
- isapc_machine_options);
-#endif
-
#ifdef CONFIG_XEN
static void xenfv_machine_4_2_options(MachineClass *m)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 16/18] hw/i386/pc_piix.c: remove unused headers after isapc machine split
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (14 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 15/18] hw/i386: move isapc machine to separate isapc.c file Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:50 ` Philippe Mathieu-Daudé
2025-07-10 8:52 ` [PATCH v4 17/18] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
2025-07-10 8:52 ` [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
The headers for isapc-only devices can be removed from pc_piix.c since they are
no longer used by the i440fx-pc machine.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b04c683e4e..ed5a313500 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -27,19 +27,16 @@
#include "qemu/units.h"
#include "hw/char/parallel-isa.h"
-#include "hw/dma/i8257.h"
#include "hw/i386/x86.h"
#include "hw/i386/pc.h"
#include "hw/i386/apic.h"
#include "hw/pci-host/i440fx.h"
-#include "hw/rtc/mc146818rtc.h"
#include "hw/southbridge/piix.h"
#include "hw/display/ramfb.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_ids.h"
#include "hw/usb.h"
#include "net/net.h"
-#include "hw/ide/isa.h"
#include "hw/ide/pci.h"
#include "hw/irq.h"
#include "system/kvm.h"
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v4 17/18] hw/i386/pc_piix.c: replace rom_memory with pci_memory
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (15 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 16/18] hw/i386/pc_piix.c: remove unused headers after isapc machine split Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:49 ` Philippe Mathieu-Daudé
2025-07-10 8:52 ` [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
Now that we can guarantee the i440fx-pc machine will always have a PCI bus, any
instances of rom_memory can be replaced by pci_memory and rom_memory removed
completely.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/pc_piix.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ed5a313500..ede7b68946 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -105,7 +105,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
GSIState *gsi_state;
MemoryRegion *ram_memory;
MemoryRegion *pci_memory = NULL;
- MemoryRegion *rom_memory = system_memory;
ram_addr_t lowmem;
uint64_t hole64_size = 0;
PCIDevice *pci_dev;
@@ -183,7 +182,6 @@ static void pc_init1(MachineState *machine, const char *pci_type)
pci_memory = g_new(MemoryRegion, 1);
memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
- rom_memory = pci_memory;
phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
object_property_add_child(OBJECT(machine), "i440fx", phb);
@@ -214,7 +212,7 @@ static void pc_init1(MachineState *machine, const char *pci_type)
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
+ pc_memory_init(pcms, system_memory, pci_memory, hole64_size);
} else {
assert(machine->ram_size == x86ms->below_4g_mem_size +
x86ms->above_4g_mem_size);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 17/18] hw/i386/pc_piix.c: replace rom_memory with pci_memory
2025-07-10 8:52 ` [PATCH v4 17/18] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
@ 2025-07-10 10:49 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-10 10:49 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 10:52, Mark Cave-Ayland wrote:
> Now that we can guarantee the i440fx-pc machine will always have a PCI bus, any
> instances of rom_memory can be replaced by pci_memory and rom_memory removed
> completely.
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
> hw/i386/pc_piix.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory
2025-07-10 8:52 [PATCH v4 00/18] hw/i386: separate isapc out from pc_piix Mark Cave-Ayland
` (16 preceding siblings ...)
2025-07-10 8:52 ` [PATCH v4 17/18] hw/i386/pc_piix.c: replace rom_memory with pci_memory Mark Cave-Ayland
@ 2025-07-10 8:52 ` Mark Cave-Ayland
2025-07-10 10:53 ` Philippe Mathieu-Daudé
17 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 8:52 UTC (permalink / raw)
To: pbonzini, mst, marcel.apfelbaum, eduardo, imammedo, qemu-devel
Now that we can guarantee the isapc machine will never have a PCI bus, any
instances of rom_memory can be replaced by system_memory and rom_memory
removed completely.
Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
---
hw/i386/isapc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
index bb22083821..27c075b5f3 100644
--- a/hw/i386/isapc.c
+++ b/hw/i386/isapc.c
@@ -35,7 +35,6 @@ static void pc_init_isa(MachineState *machine)
ISABus *isa_bus;
GSIState *gsi_state;
MemoryRegion *ram_memory;
- MemoryRegion *rom_memory = system_memory;
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
uint32_t irq;
int i;
@@ -73,7 +72,7 @@ static void pc_init_isa(MachineState *machine)
/* allocate ram and load rom/bios */
if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory, rom_memory, 0);
+ pc_memory_init(pcms, system_memory, system_memory, 0);
} else {
assert(machine->ram_size == x86ms->below_4g_mem_size +
x86ms->above_4g_mem_size);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory
2025-07-10 8:52 ` [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory Mark Cave-Ayland
@ 2025-07-10 10:53 ` Philippe Mathieu-Daudé
2025-07-10 11:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-10 10:53 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 10:52, Mark Cave-Ayland wrote:
> Now that we can guarantee the isapc machine will never have a PCI bus, any
> instances of rom_memory can be replaced by system_memory and rom_memory
> removed completely.
>
> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> ---
> hw/i386/isapc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
> index bb22083821..27c075b5f3 100644
> --- a/hw/i386/isapc.c
> +++ b/hw/i386/isapc.c
> @@ -35,7 +35,6 @@ static void pc_init_isa(MachineState *machine)
> ISABus *isa_bus;
> GSIState *gsi_state;
> MemoryRegion *ram_memory;
> - MemoryRegion *rom_memory = system_memory;
> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> uint32_t irq;
> int i;
> @@ -73,7 +72,7 @@ static void pc_init_isa(MachineState *machine)
>
> /* allocate ram and load rom/bios */
> if (!xen_enabled()) {
> - pc_memory_init(pcms, system_memory, rom_memory, 0);
> + pc_memory_init(pcms, system_memory, system_memory, 0);
I'd prefer just call here:
x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true);
and in pc_system_firmware_init(): assert(pcmc->pci_enabled).
WDYT?
> } else {
> assert(machine->ram_size == x86ms->below_4g_mem_size +
> x86ms->above_4g_mem_size);
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory
2025-07-10 10:53 ` Philippe Mathieu-Daudé
@ 2025-07-10 11:05 ` Philippe Mathieu-Daudé
2025-07-10 15:35 ` Mark Cave-Ayland
0 siblings, 1 reply; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-10 11:05 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 12:53, Philippe Mathieu-Daudé wrote:
> On 10/7/25 10:52, Mark Cave-Ayland wrote:
>> Now that we can guarantee the isapc machine will never have a PCI bus,
>> any
>> instances of rom_memory can be replaced by system_memory and rom_memory
>> removed completely.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>> ---
>> hw/i386/isapc.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>> index bb22083821..27c075b5f3 100644
>> --- a/hw/i386/isapc.c
>> +++ b/hw/i386/isapc.c
>> @@ -35,7 +35,6 @@ static void pc_init_isa(MachineState *machine)
>> ISABus *isa_bus;
>> GSIState *gsi_state;
>> MemoryRegion *ram_memory;
>> - MemoryRegion *rom_memory = system_memory;
>> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>> uint32_t irq;
>> int i;
>> @@ -73,7 +72,7 @@ static void pc_init_isa(MachineState *machine)
>> /* allocate ram and load rom/bios */
>> if (!xen_enabled()) {
>> - pc_memory_init(pcms, system_memory, rom_memory, 0);
>> + pc_memory_init(pcms, system_memory, system_memory, 0);
>
> I'd prefer just call here:
>
> x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true);
>
> and in pc_system_firmware_init(): assert(pcmc->pci_enabled).
>
> WDYT?
What I have in mind (untested):
-- >8 --
diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
index 27c075b5f32..a7c2146916c 100644
--- a/hw/i386/isapc.c
+++ b/hw/i386/isapc.c
@@ -74,3 +74,4 @@ static void pc_init_isa(MachineState *machine)
if (!xen_enabled()) {
- pc_memory_init(pcms, system_memory, system_memory, 0);
+ pc_memory_init(pcms, system_memory, NULL, 0);
+ x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", system_memory,
true);
} else {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b2116335752..2952d3ee4ff 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -811,3 +811,3 @@ void pc_memory_init(PCMachineState *pcms,
MemoryRegion *system_memory,
- MemoryRegion *rom_memory,
+ MemoryRegion *pci_memory,
uint64_t pci_hole64_size)
@@ -826,2 +826,3 @@ void pc_memory_init(PCMachineState *pcms,
+ assert(pcmc->pci_enabled ^ !!pci_memory);
assert(machine->ram_size == x86ms->below_4g_mem_size +
@@ -955,3 +956,5 @@ void pc_memory_init(PCMachineState *pcms,
/* Initialize PC system firmware */
- pc_system_firmware_init(pcms, rom_memory);
+ if (pcmc->pci_enabled) {
+ pc_system_firmware_init(pcms, pci_memory);
+ }
@@ -969,3 +972,3 @@ void pc_memory_init(PCMachineState *pcms,
}
- memory_region_add_subregion_overlap(rom_memory,
+ memory_region_add_subregion_overlap(pci_memory,
PC_ROM_MIN_VGA,
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 821396c16e9..0c29e4188fc 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -221,6 +221,3 @@ void pc_system_firmware_init(PCMachineState *pcms,
- if (!pcmc->pci_enabled) {
- x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true);
- return;
- }
+ assert(pcmc->pci_enabled);
---
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory
2025-07-10 11:05 ` Philippe Mathieu-Daudé
@ 2025-07-10 15:35 ` Mark Cave-Ayland
2025-07-11 10:28 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 29+ messages in thread
From: Mark Cave-Ayland @ 2025-07-10 15:35 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, pbonzini, mst, marcel.apfelbaum,
eduardo, imammedo, qemu-devel
On 10/07/2025 12:05, Philippe Mathieu-Daudé wrote:
> On 10/7/25 12:53, Philippe Mathieu-Daudé wrote:
>> On 10/7/25 10:52, Mark Cave-Ayland wrote:
>>> Now that we can guarantee the isapc machine will never have a PCI
>>> bus, any
>>> instances of rom_memory can be replaced by system_memory and rom_memory
>>> removed completely.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>>> ---
>>> hw/i386/isapc.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>>> index bb22083821..27c075b5f3 100644
>>> --- a/hw/i386/isapc.c
>>> +++ b/hw/i386/isapc.c
>>> @@ -35,7 +35,6 @@ static void pc_init_isa(MachineState *machine)
>>> ISABus *isa_bus;
>>> GSIState *gsi_state;
>>> MemoryRegion *ram_memory;
>>> - MemoryRegion *rom_memory = system_memory;
>>> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>>> uint32_t irq;
>>> int i;
>>> @@ -73,7 +72,7 @@ static void pc_init_isa(MachineState *machine)
>>> /* allocate ram and load rom/bios */
>>> if (!xen_enabled()) {
>>> - pc_memory_init(pcms, system_memory, rom_memory, 0);
>>> + pc_memory_init(pcms, system_memory, system_memory, 0);
>>
>> I'd prefer just call here:
>>
>> x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true);
>>
>> and in pc_system_firmware_init(): assert(pcmc->pci_enabled).
>>
>> WDYT?
>
> What I have in mind (untested):
>
> -- >8 --
> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
> index 27c075b5f32..a7c2146916c 100644
> --- a/hw/i386/isapc.c
> +++ b/hw/i386/isapc.c
> @@ -74,3 +74,4 @@ static void pc_init_isa(MachineState *machine)
> if (!xen_enabled()) {
> - pc_memory_init(pcms, system_memory, system_memory, 0);
> + pc_memory_init(pcms, system_memory, NULL, 0);
> + x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", system_memory,
> true);
> } else {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b2116335752..2952d3ee4ff 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -811,3 +811,3 @@ void pc_memory_init(PCMachineState *pcms,
> MemoryRegion *system_memory,
> - MemoryRegion *rom_memory,
> + MemoryRegion *pci_memory,
> uint64_t pci_hole64_size)
> @@ -826,2 +826,3 @@ void pc_memory_init(PCMachineState *pcms,
>
> + assert(pcmc->pci_enabled ^ !!pci_memory);
> assert(machine->ram_size == x86ms->below_4g_mem_size +
> @@ -955,3 +956,5 @@ void pc_memory_init(PCMachineState *pcms,
> /* Initialize PC system firmware */
> - pc_system_firmware_init(pcms, rom_memory);
> + if (pcmc->pci_enabled) {
> + pc_system_firmware_init(pcms, pci_memory);
> + }
>
> @@ -969,3 +972,3 @@ void pc_memory_init(PCMachineState *pcms,
> }
> - memory_region_add_subregion_overlap(rom_memory,
> + memory_region_add_subregion_overlap(pci_memory,
> PC_ROM_MIN_VGA,
> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
> index 821396c16e9..0c29e4188fc 100644
> --- a/hw/i386/pc_sysfw.c
> +++ b/hw/i386/pc_sysfw.c
> @@ -221,6 +221,3 @@ void pc_system_firmware_init(PCMachineState *pcms,
>
> - if (!pcmc->pci_enabled) {
> - x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory,
> true);
> - return;
> - }
> + assert(pcmc->pci_enabled);
I think that's a good idea, however the original aim of this series was
just to do the basic split and tidy-up work (hopefully in time for 10.1).
There is certainly more tidy-up that is possible w.r.t. pc.c, but I
didn't want to start unraveling that thread right now for fear of this
series getting too large :/
ATB,
Mark.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v4 18/18] hw/i386/isapc.c: replace rom_memory with system_memory
2025-07-10 15:35 ` Mark Cave-Ayland
@ 2025-07-11 10:28 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 29+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-07-11 10:28 UTC (permalink / raw)
To: Mark Cave-Ayland, pbonzini, mst, marcel.apfelbaum, eduardo,
imammedo, qemu-devel
On 10/7/25 17:35, Mark Cave-Ayland wrote:
> On 10/07/2025 12:05, Philippe Mathieu-Daudé wrote:
>
>> On 10/7/25 12:53, Philippe Mathieu-Daudé wrote:
>>> On 10/7/25 10:52, Mark Cave-Ayland wrote:
>>>> Now that we can guarantee the isapc machine will never have a PCI
>>>> bus, any
>>>> instances of rom_memory can be replaced by system_memory and rom_memory
>>>> removed completely.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
>>>> ---
>>>> hw/i386/isapc.c | 3 +--
>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>>>> index bb22083821..27c075b5f3 100644
>>>> --- a/hw/i386/isapc.c
>>>> +++ b/hw/i386/isapc.c
>>>> @@ -35,7 +35,6 @@ static void pc_init_isa(MachineState *machine)
>>>> ISABus *isa_bus;
>>>> GSIState *gsi_state;
>>>> MemoryRegion *ram_memory;
>>>> - MemoryRegion *rom_memory = system_memory;
>>>> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
>>>> uint32_t irq;
>>>> int i;
>>>> @@ -73,7 +72,7 @@ static void pc_init_isa(MachineState *machine)
>>>> /* allocate ram and load rom/bios */
>>>> if (!xen_enabled()) {
>>>> - pc_memory_init(pcms, system_memory, rom_memory, 0);
>>>> + pc_memory_init(pcms, system_memory, system_memory, 0);
>>>
>>> I'd prefer just call here:
>>>
>>> x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory, true);
>>>
>>> and in pc_system_firmware_init(): assert(pcmc->pci_enabled).
>>>
>>> WDYT?
>>
>> What I have in mind (untested):
>>
>> -- >8 --
>> diff --git a/hw/i386/isapc.c b/hw/i386/isapc.c
>> index 27c075b5f32..a7c2146916c 100644
>> --- a/hw/i386/isapc.c
>> +++ b/hw/i386/isapc.c
>> @@ -74,3 +74,4 @@ static void pc_init_isa(MachineState *machine)
>> if (!xen_enabled()) {
>> - pc_memory_init(pcms, system_memory, system_memory, 0);
>> + pc_memory_init(pcms, system_memory, NULL, 0);
>> + x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin",
>> system_memory, true);
>> } else {
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index b2116335752..2952d3ee4ff 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -811,3 +811,3 @@ void pc_memory_init(PCMachineState *pcms,
>> MemoryRegion *system_memory,
>> - MemoryRegion *rom_memory,
>> + MemoryRegion *pci_memory,
>> uint64_t pci_hole64_size)
>> @@ -826,2 +826,3 @@ void pc_memory_init(PCMachineState *pcms,
>>
>> + assert(pcmc->pci_enabled ^ !!pci_memory);
>> assert(machine->ram_size == x86ms->below_4g_mem_size +
>> @@ -955,3 +956,5 @@ void pc_memory_init(PCMachineState *pcms,
>> /* Initialize PC system firmware */
>> - pc_system_firmware_init(pcms, rom_memory);
>> + if (pcmc->pci_enabled) {
>> + pc_system_firmware_init(pcms, pci_memory);
>> + }
>>
>> @@ -969,3 +972,3 @@ void pc_memory_init(PCMachineState *pcms,
>> }
>> - memory_region_add_subregion_overlap(rom_memory,
>> + memory_region_add_subregion_overlap(pci_memory,
>> PC_ROM_MIN_VGA,
>> diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>> index 821396c16e9..0c29e4188fc 100644
>> --- a/hw/i386/pc_sysfw.c
>> +++ b/hw/i386/pc_sysfw.c
>> @@ -221,6 +221,3 @@ void pc_system_firmware_init(PCMachineState *pcms,
>>
>> - if (!pcmc->pci_enabled) {
>> - x86_bios_rom_init(X86_MACHINE(pcms), "bios.bin", rom_memory,
>> true);
>> - return;
>> - }
>> + assert(pcmc->pci_enabled);
>
> I think that's a good idea, however the original aim of this series was
> just to do the basic split and tidy-up work (hopefully in time for 10.1).
>
> There is certainly more tidy-up that is possible w.r.t. pc.c, but I
> didn't want to start unraveling that thread right now for fear of this
> series getting too large :/
Fair enough.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 29+ messages in thread