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

[David Gibson]

On Mon, Sep 10, 2018 at 07:24:47PM +0200, Cédric Le Goater wrote:
> On 09/10/2018 05:02 PM, Greg Kurz wrote:
> > On Mon, 10 Sep 2018 13:02:19 +0200
> > Cédric Le Goater <address@hidden> 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.

Yeah, this is my take as well.  The spec sort of implies, but doesn't
explicitly state a per-PHB allocation of MSIs.  But there's not really
a good reason to partition the irqs that way.  So it may be a bit fast
and loose w.r.t. PAPR, but as long as we have plenty of MSIs available
I think using a shared pool is unlikely to cause problems in practice.

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