Re: [Qemu-devel] [RFC PATCH v2 05/21] ppc/xive: allocate IRQ numbers for the IPIs

[Cédric Le Goater]

On 09/19/2017 04:45 AM, David Gibson wrote:
> On Mon, Sep 11, 2017 at 07:12:19PM +0200, Cédric Le Goater wrote:
>> The number of IPIs is deduced from the max number of CPUs the guest
>> supports and the IRQ numbers for the IPIs are allocated from the top
>> of the IRQ number space to reduce conflict with other IRQ numbers
>> allocated by the devices.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> This is more ick associated with implementing XIVE in terms of XICS.
> We shouldn't need to "allocate" IRQs for the IPIs - they should just
> be a fixed set.  

They are allocated at the right beginning so we can consider them
fixed I suppose. 

> And we certainly shouldn't need to set the XICS irq type for XIVE irqs.

This is because, in this patchset, XIVE and XICS use the same IRQ 
allocator which happens to be the ICSIRQState array of XICS. yes, 
this is ugly but we are identifying the different constraints. 

We should be doing the same with a common interrupt source, that is
to allocate some IRQ numbers for the IPIs. 

C.


>> ---
>>  hw/intc/spapr_xive.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 1681affb0848..52c32f588d6d 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -58,6 +58,7 @@ static void spapr_xive_realize(DeviceState *dev, Error 
>> **errp)
>>      sPAPRXive *xive = SPAPR_XIVE(dev);
>>      Object *obj;
>>      Error *err = NULL;
>> +    int i;
>>  
>>      if (!xive->nr_targets) {
>>          error_setg(errp, "Number of interrupt targets needs to be greater 
>> 0");
>> @@ -80,6 +81,11 @@ static void spapr_xive_realize(DeviceState *dev, Error 
>> **errp)
>>  
>>      xive->ics = ICS_BASE(obj);
>>  
>> +    /* Allocate the last IRQ numbers for the IPIs */
>> +    for (i = xive->nr_irqs - xive->nr_targets; i < xive->nr_irqs; i++) {
>> +        ics_set_irq_type(xive->ics, i, false);
>> +    }
>> +
>>      /* Allocate SBEs (State Bit Entry). 2 bits, so 4 entries per byte */
>>      xive->sbe_size = DIV_ROUND_UP(xive->nr_irqs, 4);
>>      xive->sbe = g_malloc0(xive->sbe_size);
>