diff mbox series

[4/8] hw/pci-host/bonito: Set reference using object_property_add_const_link()

Message ID 20230105130710.49264-5-philmd@linaro.org
State New
Headers show
Series hw/pci-host/bonito: Housekeeping | expand

Commit Message

Philippe Mathieu-Daudé Jan. 5, 2023, 1:07 p.m. UTC 
A QOM object shouldn't poke at another object internals.

Here the PCI host bridge instantiates its PCI function #0
and sets a reference to itself (so the function can access
the bridge fields).

Pass this reference with object_property_add_const_link(),
since the reference won't change during the object lifetime.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/bonito.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Richard Henderson Jan. 6, 2023, 6:49 p.m. UTC | #1 
On 1/5/23 05:07, Philippe Mathieu-Daudé wrote:
> A QOM object shouldn't poke at another object internals.
> 
> Here the PCI host bridge instantiates its PCI function #0
> and sets a reference to itself (so the function can access
> the bridge fields).
> 
> Pass this reference with object_property_add_const_link(),
> since the reference won't change during the object lifetime.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci-host/bonito.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 80ec424f86..d881c85509 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -656,10 +656,17 @@ static void bonito_host_realize(DeviceState *dev, Error **errp)
>   static void bonito_pci_realize(PCIDevice *dev, Error **errp)
>   {
>       PCIBonitoState *s = PCI_BONITO(dev);
> -    SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
> -    PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
> -    BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
>       MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
> +    SysBusDevice *sysbus;
> +    PCIHostState *phb;
> +    BonitoState *bs;
> +    Object *obj;
> +
> +    obj = object_property_get_link(OBJECT(dev), "host-bridge", &error_abort);
> +    s->pcihost = BONITO_PCI_HOST_BRIDGE(obj);
> +    sysbus = SYS_BUS_DEVICE(obj);
> +    phb = PCI_HOST_BRIDGE(obj);
> +    bs = BONITO_PCI_HOST_BRIDGE(obj);

It would be nice to re-order these so that you only perform the dynamic cast once:

     s->pcihost = bs;

Regardless,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
diff mbox series

Patch

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 80ec424f86..d881c85509 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -656,10 +656,17 @@  static void bonito_host_realize(DeviceState *dev, Error **errp)
 static void bonito_pci_realize(PCIDevice *dev, Error **errp)
 {
     PCIBonitoState *s = PCI_BONITO(dev);
-    SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
-    PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
-    BonitoState *bs = BONITO_PCI_HOST_BRIDGE(s->pcihost);
     MemoryRegion *pcimem_alias = g_new(MemoryRegion, 1);
+    SysBusDevice *sysbus;
+    PCIHostState *phb;
+    BonitoState *bs;
+    Object *obj;
+
+    obj = object_property_get_link(OBJECT(dev), "host-bridge", &error_abort);
+    s->pcihost = BONITO_PCI_HOST_BRIDGE(obj);
+    sysbus = SYS_BUS_DEVICE(obj);
+    phb = PCI_HOST_BRIDGE(obj);
+    bs = BONITO_PCI_HOST_BRIDGE(obj);
 
     /*
      * Bonito North Bridge, built on FPGA,
@@ -745,7 +752,6 @@  PCIBus *bonito_init(qemu_irq *pic)
     DeviceState *dev;
     BonitoState *pcihost;
     PCIHostState *phb;
-    PCIBonitoState *s;
     PCIDevice *d;
 
     dev = qdev_new(TYPE_BONITO_PCI_HOST_BRIDGE);
@@ -755,10 +761,9 @@  PCIBus *bonito_init(qemu_irq *pic)
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
     d = pci_new(PCI_DEVFN(0, 0), TYPE_PCI_BONITO);
-    s = PCI_BONITO(d);
-    s->pcihost = pcihost;
-    pcihost->pci_dev = s;
+    object_property_add_const_link(OBJECT(d), "host-bridge", OBJECT(dev));
     pci_realize_and_unref(d, phb->bus, &error_fatal);
+    pcihost->pci_dev = PCI_BONITO(d);
 
     return phb->bus;
 }