On Sat, 30 Aug 2025 at 16:57, Paolo Bonzini <[email protected]> wrote:
>
> From: Mark Cave-Ayland <[email protected]>
>
> PCI is always enabled on the pc-i440fx machine so hardcode the relevant logic
> in pc_init1(). Add an assert() to ensure that this is always the case at
> runtime as already done in pc_q35_init().
>
> Signed-off-by: Mark Cave-Ayland <[email protected]>
> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
> Reviewed-by: Xiaoyao Li <[email protected]>
> Link:
> https://lore.kernel.org/r/[email protected]
> Signed-off-by: Paolo Bonzini <[email protected]>
Hi; Coverity points out (CID 1620557) that this change means
that an if() check later on in the function is no longer needed:
> @@ -195,38 +200,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;
> + 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);
> + phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
phb is now guaranteed not to be NULL (when before it might have
been NULL), so this bit of code near the bottom of pc_init1():
if (phb) {
ioapic_init_gsi(gsi_state, phb);
}
can be simplified to unconditionally call ioapic_init_gsi().
Would you mind sending a followup patch to clean that up?
thanks
-- PMM
