Re: [PATCH v6 03/10] i386/pc: pass pci_hole64_size to pc_memory_init()

[Joao Martins]

On 7/9/22 21:51, B wrote:
> Am 1. Juli 2022 16:10:07 UTC schrieb Joao Martins <joao.m.martins@oracle.com>:
>> Use the pre-initialized pci-host qdev and fetch the
>> pci-hole64-size into pc_memory_init() newly added argument.
>> piix needs a bit of care given all the !pci_enabled()
>> and that the pci_hole64_size is private to i440fx.
> 
> It exposes this value as the property PCI_HOST_PROP_PCI_HOLE64_SIZE. 

Indeed.

> Reusing it allows for not touching i440fx in this patch at all.
> 
> For code symmetry reasons the analogue property could be used for Q35 as well.
> 
Presumably you want me to change into below while deleting 
i440fx_pci_hole64_size()
from this patch (see snip below). IMHO, gotta say that in q35 the code symmetry
doesn't buy much readability here, albeit it does remove any need for that other
helper in i440fx.

@Igor let me know if you agree with the change and whether I can keep the 
Reviewed-by.

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 504ddd0deece..cc0855066d06 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -167,7 +167,9 @@ static void pc_init1(MachineState *machine,
         memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
         rom_memory = pci_memory;
         i440fx_host = qdev_new(host_type);
-        hole64_size = i440fx_pci_hole64_size(i440fx_host);
+        hole64_size = object_property_get_uint(OBJECT(i440fx_host),
+                                               PCI_HOST_PROP_PCI_HOLE64_SIZE,
+                                               &error_abort);
     } else {
         pci_memory = NULL;
         rom_memory = system_memory;
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4b747c59c19a..466f3ef3c918 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -208,7 +208,9 @@ static void pc_q35_init(MachineState *machine)
     q35_host = Q35_HOST_DEVICE(qdev_new(TYPE_Q35_HOST_DEVICE));

     if (pcmc->pci_enabled) {
-        pci_hole64_size = q35_host->mch.pci_hole64_size;
+        pci_hole64_size = object_property_get_uint(OBJECT(q35_host),
+                                                   
PCI_HOST_PROP_PCI_HOLE64_SIZE,
+                                                   &error_abort);
     }

     /* allocate ram and load rom/bios */
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 15680da7d709..d5426ef4a53c 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -237,13 +237,6 @@ static void i440fx_realize(PCIDevice *dev, Error **errp)
     }
 }

-uint64_t i440fx_pci_hole64_size(DeviceState *i440fx_dev)
-{
-        I440FXState *i440fx = I440FX_PCI_HOST_BRIDGE(i440fx_dev);
-
-        return i440fx->pci_hole64_size;
-}
-
 PCIBus *i440fx_init(const char *pci_type,
                     DeviceState *dev,
                     MemoryRegion *address_space_mem,