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

[Greg Kurz <groug@kaod.org>]

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

On Tue, 11 Sep 2018 11:52:46 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

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

Fair enough. Thanks for the clarification.

Cheers,

--
Greg

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