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

[Richard Henderson <richard.henderson@linaro.org>]

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~