Re: [Qemu-devel] [PATCH v5 10/36] spapr/xive: introduce a XIVE interrupt controller

[Cédric Le Goater]

[ ... ]
 
>>> With that approach it might make sense to embed it
>>> here, rather than subclassing it 
>>
>> ah. why not indeed. I have to think about it. 
>>
>>> (the old composition vs. inheritance debate).
>>
>> he. but then the XiveRouter needs to become a QOM interface if we 
>> want to be able to define XIVE table accessors for sPAPRXive. See
>> the  spapr_xive_class_init() routine.
> 
> Erm.. I'm not really sure what you're getting at here.

if we compose a sPAPRXive object with a XiveSource object and a XiveRouter 
object, how will the  XiveRouter object access the XIVE internal tables 
which are in the sPAPRXive object ? 

Thinking of it, I am not sure a QOM interface would solve the problem now. 
So we are stuck with inheritance.

[ ... ]

>>>> +qemu_irq spapr_xive_qirq(sPAPRXive *xive, uint32_t lisn)
>>>> +{
>>>> +    XiveSource *xsrc = &xive->source;
>>>> +
>>>> +    if (lisn >= xive->nr_irqs) {
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    if (!(xive->eat[lisn].w & EAS_VALID)) {
>>>> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %x\n", lisn);
>>>
>>> I don't think this is a guest error - gettint the qirq by number
>>> should generally be something qemu code does.
>>
>> Even if the IRQ was not defined by the machine ? The EAS_VALID bit is
>> raised when the IRQ is enabled at the XIVE level, which means that the
>> IRQ number has been claimed by some device of the machine. You cannot
>> get a qirq by number for  some random IRQ number. Can you ?
> 
> Well, you shouldn't.  The point is that it is qemu code (specifically
> the machine setup stuff) that will be calling this, and it shouldn't
> be calling it with irq numbers that haven't been
> enabled/claimed/whatever.

so it should be an assert ? 

Thanks,

C.