Re: [Qemu-devel] [PATCH 0/3] spapr: introduce a new sPAPRIrq backend

["Cédric Le Goater" <clg@kaod.org>]

On 09/10/2018 05:02 PM, Greg Kurz wrote:
> On Mon, 10 Sep 2018 13:02:19 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> Hello,
>>
>> This series adds a new sPAPRIrq backend increasing the number of
>> available IRQ numbers in pseries-3.1 machines. This change is an
>> opportunity to also fix the "ibm,pe-total-#msi" and remove the old
>> XICS_IRQS_SPAPR definition.
>>
> 
> This cleanup looks sane but I still have a concern with the semantics of
> "ibm,pe-total-#msi".
> 
> According to LoPAPR:
> 
> "ibm,pe-total-#msi"
> 
> property name defines the maximum number of Message Signaled Interrupts (MSI plus MSI-X) that are available
> to the PE below this device tree node. This number only indicates the number of available interrupts, not the num-
> ber assigned. The number assigned for an IOA may be obtained by Function 0 (Query only) of the ibm,change-msi
> RTAS call.
> prop-encoded-array: Maximum number of interrupts encoded as with encode-int.
> 
> IIUC, the PHB is given ibm,pe-total-#msi MSIs that it can assign to devices.
> 
> But we currently have only one global allocator in the machine, so having
> each PHB advertising the full range of the allocator still looks weird.

yes. Multiple PHBs share the same IRQ number space and in this
case the advertised number "ibm,pe-total-#msi" does not reflect 
the maximum number of allocatable interrupts per PHB.

The patch improves only the value for one PHB and, as of today, 
it is still wrong when Multiple PHBs are involved.

> Shouldn't this be divided by the number of PHBs ? Or should we have one
> separate allocator for each PHB ?

That would mean segmenting the IRQ number space and I am not 
fond of this solution because we have plenty of space to share:

	0xd00 MSIs

When we find a scenario reaching this limit, I think what we 
should do is to dynamically extend the IRQ number space in QEMU 
and in KVM. It should not be a problem.

We could also downsize "ibm,pe-total-#msi". It is quite big
today. some controllers do have a lot of IRQs but no more 
that a hundred. I might be wrong. 

C.

>> Thanks,
>>
>> C.
>>
>> Cédric Le Goater (3):
>>   spapr: introduce a spapr_irq class 'nr_msis' attribute
>>   spapr: increase the size of the IRQ number space
>>   spapr_pci: fix "ibm,pe-total-#msi" value
>>
>>  include/hw/ppc/spapr_irq.h |  2 ++
>>  include/hw/ppc/xics.h      |  2 --
>>  hw/ppc/spapr.c             |  1 +
>>  hw/ppc/spapr_irq.c         | 22 ++++++++++++++++++++--
>>  hw/ppc/spapr_pci.c         |  5 +++--
>>  5 files changed, 26 insertions(+), 6 deletions(-)
>>
>