Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[BALATON Zoltan]

On Mon, 12 Oct 2020, David Gibson wrote:
> On Mon, Oct 12, 2020 at 08:21:41AM +0200, Philippe Mathieu-Daudé wrote:
> > On 10/12/20 12:34 AM, David Gibson wrote:
> > > On Sun, Oct 11, 2020 at 09:03:32PM +0200, Philippe Mathieu-Daudé wrote:
> > > > The Grackle PCI host model expects the interrupt controller
> > > > being set, but does not verify it is present. Add a check to
> > > > help developers using this model.
> > >
> > > I don't think thaqt's very likely, but, sure, applied to ppc-for-5.2
> >
> > Do you want I correct the description as:
> > "The Grackle PCI host model expects the interrupt controller
> > being set, but does not verify it is present.
> > A developer basing its implementation on the Grackle model
> > might hit this problem. Add a check to help future developers
> > using this model as reference."?
>
> No, it's fine.  All I was saying is that the chances of anyone using
> Grackle in future seem very low to me.

So maybe an assert instead of a user visible error would be enough?

Regards,
BALATON Zoltan

> > > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > > ---
> > > >   hw/pci-host/grackle.c | 4 ++++
> > > >   1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
> > > > index 57c29b20afb..20361d215ca 100644
> > > > --- a/hw/pci-host/grackle.c
> > > > +++ b/hw/pci-host/grackle.c
> > > > @@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error **errp)
> > > >       GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
> > > >       PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> > > > +    if (!s->pic) {
> > > > +        error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not set");
> > > > +        return;
> > > > +    }
> > > >       phb->bus = pci_register_root_bus(dev, NULL,
> > > >                                        pci_grackle_set_irq,
> > > >                                        pci_grackle_map_irq,