Re: [Qemu-ppc] [PATCH 3/3] spapr: introduce a fixed IRQ number space

[Cédric Le Goater]

[ ... ]

>>> This is 4 irqs per PHB, hence up to 32 PHBs. Cool, we're currently
>>> limited to 31 PHBs.
>>>   
>>>> +#define SPAPR_IRQ_MSI        0x1100  /* Offset of the dynamic range 
>>>> covered  
>>>
>>> We only support dynamic MSIs with PCI, maybe rename to SPAPR_IRQ_PCI_MSI ?  
>>
>> hmm, no. We could have CAPI devices there. remember ? ;)
>>
> 
> Well... OpenCAPI devices are exposed to the OS as PCI devices, so I'm not
> sure we need a CAPI specific range.

yes. so this range is common to all devices doing dynamic allocation
of IRQs. How should we call it ? 

>>>> +                                      * by the bitmap allocator */  
>>>
>>> The range size is hence 1k (XICS_IRQS_SPAPR) for the time being.  
>>
>> in fact we could this bogus limit and use spapr->irq_map_nr now.
>>
> 
> "we could *missing verb* this bogus limit"... so I'm not sure to
> understand...

oups. We could use spapr->irq_map_nr instead of XICS_IRQS_SPAPR when
defining : 

    _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", XICS_IRQS_SPAPR));

in spapr_pci.c

[ ... ]

>>>> +        if (spapr->xics_legacy) {
>>>> +            dev->irq = spapr_irq_findone(spapr, &local_err);
>>>> +            if (local_err) {
>>>> +                error_propagate(errp, local_err);
>>>> +                return;
>>>> +            }
>>>> +        } else {
>>>> +            dev->irq = SPAPR_IRQ_VIO + vio_index++;  
>>>
>>> This can overlap the next range if we have more than 64 VIO devices...  
>>
>> yes. but claim() should fail.
>>
> 
> Hmm... I have the impression claim() only fails if:
> - irq < ics->offset (ie, XICS_IRQ_BASE == 4096)
> - or irq >= ics->offset + ics->nr_irqs (ie, XICS_IRQS_SPAPR == 1024)
> - or irq is already in use
> 
> I can't find code that would prevent dev->irq to reach SPAPR_IRQ_MSI.

Ah yes. It can overlap. 

My previous proposal took care of overlaps but something simpler was 
requested. That's how I understand it at least. We can introduce 
a maximum for the VIO range or live with it. 

C.