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

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, Nov 29, 2018 at 03:37:12PM +0100, Cédric Le Goater wrote:
> [ ... ]
>  
> >>> 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.

Uh.. true.  There are ways aroud it, but it gets a bit complicated.

> [ ... ]
> 
> >>>> +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 ?

Yes.
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]