On 01/09/2025 11:43, Peter Maydell wrote:

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://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_r_20250828111057.468712-2D16-2Dmark.caveayland-40nutanix.com&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=j27iua4jCqP3vgs27zBndXoJkryZlZLXIKTEEzLwm_3CiXikGXPBG0bXR6OX5MxL&s=EzEQ_YRd0ipeIVCSEOqy2TMkBTRS59USrtlOTA35vC8&e=
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

Thanks Peter, I've just sent a patch to do this.


ATB,

Mark.


Reply via email to