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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
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 49bd1a41e7..a776998504 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -71,7 +71,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 };
@@ -125,6 +125,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
@@ -195,38 +198,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()) {
@@ -242,72 +243,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);
@@ -321,7 +301,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,
@@ -329,28 +309,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
On 8/22/2025 8:12 PM, 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
some nits below, otherwise
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.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 49bd1a41e7..a776998504 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -71,7 +71,7 @@
>
> #define XEN_IOAPIC_NUM_PIRQS 128ULL
>
> -#ifdef CONFIG_IDE_ISA
> +#ifdef CONFIG_ISAPC
This seems unrelated to this patch?
> 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 };
> @@ -125,6 +125,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;
Suggest adding
assert(pcmc->pci_enabled);
like what pc_q35_init() does.
> /*
> * Calculate ram split, for memory below and above 4G. It's a bit
> @@ -195,38 +198,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()) {
> @@ -242,72 +243,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);
> @@ -321,7 +301,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,
> @@ -329,28 +309,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);
>
On 26/08/2025 13:09, Xiaoyao Li wrote:
> On 8/22/2025 8:12 PM, 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>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> some nits below, otherwise
>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.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 49bd1a41e7..a776998504 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -71,7 +71,7 @@
>> #define XEN_IOAPIC_NUM_PIRQS 128ULL
>> -#ifdef CONFIG_IDE_ISA
>> +#ifdef CONFIG_ISAPC
>
> This seems unrelated to this patch?
Ooops I think this must have slipped in when working on an earlier
version of the series, but I believe it can just be removed.
>> 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 };
>> @@ -125,6 +125,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;
>
> Suggest adding
>
> assert(pcmc->pci_enabled);
>
> like what pc_q35_init() does.
Good idea! I'll add this into the next version.
>> /*
>> * Calculate ram split, for memory below and above 4G. It's a bit
>> @@ -195,38 +198,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()) {
>> @@ -242,72 +243,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);
>> @@ -321,7 +301,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,
>> @@ -329,28 +309,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);
ATB,
Mark.
© 2016 - 2025 Red Hat, Inc.