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

[Cédric Le Goater]

On 11/23/2017 12:12 PM, David Gibson wrote:
> 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.

OK.

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

I agree.
 
>> 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.

No problem. I am also trying to cover too many things at the same 
time. XIVE is a big piece enough. 

I think the XIVE patchset is stabilizing. So I will it send today 
and see how it is perceived. But let's keep in mind that we have 
some more needs for the PHBs and their IRQs placement.

Thanks,

C.