Re: [Qemu-devel] [PATCH for-2.12 v3 08/11] spapr: introduce a XICSFabric irq_is_lsi() operation

[David Gibson]

On Fri, Nov 17, 2017 at 08:23:00AM +0100, Cédric Le Goater wrote:
> On 11/17/2017 05:54 AM, David Gibson wrote:
> > On Fri, Nov 10, 2017 at 03:20:14PM +0000, Cédric Le Goater wrote:
> >> It will be used later on to distinguish the allocation of an LSI
> >> interrupt from an MSI and also to reduce the use of the ICSIRQState
> >> array of the ICSState object, which is on our way to introduce XIVE.
> >>
> >> The 'irq' parameter continues to refer to the global IRQ number space.
> >>
> >> On PowerNV, only the PSI controller interrupts are handled and they
> >> are all LSIs.
> >>
> >> Signed-off-by: Cédric Le Goater <address@hidden>
> > 
> > !?! AFAICT this is a step backwards.  The users of ics_is_lsi() here
> > are in the xics code.  So they already have the right information
> > locally in the ICSState object.  Why on earth would you indirect
> > through the fabric.
> 
> because I am trying to get rid of the fact that the current ICS 
> allocator handles two things at the same time : IRQ allocation 
> and IRQ type. And that's a problem because the ICSState object 
> is just used everywhere in the code.

That's a good goal, but I don't think this is the right way there.
One of the reasons that conflation of allocation and typing is bad is
that the typing is essentially local information to the PIC itself,
whereas the allocation is a machine concern.

By using a XICSFabric hook you're essentially wiring the irq typing
through the machine, which again seems like a step in the wrong
direction.

> OK, So that's is not to your liking, I will come up with some 
> other solution. Thanks for looking.

Sorry if I was a bit harsh; quite a few unrelated things have been
frustrating me, which has made me grumpy.

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

signature.asc
Description: PGP signature