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

[BALATON Zoltan via <qemu-devel@nongnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

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,
>>>
>>
>
>