Re: [Qemu-devel] [PATCH 09/25] spapr: introduce handlers for XIVE interrupt sources

[Cédric Le Goater]

On 11/28/2017 05:45 AM, David Gibson wrote:
> On Thu, Nov 23, 2017 at 02:29:39PM +0100, Cédric Le Goater wrote:
>> These are very similar to the XICS handlers in a simpler form. They make
>> use of a status array for the LSI interrupts. The spapr_xive_irq() routine
>> in charge of triggering the CPU interrupt line will be filled later on.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
> 
> Is the status word you add here architected as part of the XIVE spec,
> or purely internal / implementation specific?

this is the model.

> 
>> ---
>>  hw/intc/spapr_xive.c        | 55 
>> +++++++++++++++++++++++++++++++++++++++++++--
>>  include/hw/ppc/spapr_xive.h | 14 +++++++++++-
>>  2 files changed, 66 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index b2fc3007c85f..66c533fb1d78 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -26,6 +26,47 @@
>>  
>>  #include "xive-internal.h"
>>  
>> +static void spapr_xive_irq(sPAPRXive *xive, int lisn)
>> +{
>> +
>> +}
>> +
>> +/*
>> + * XIVE Interrupt Source
>> + */
>> +static void spapr_xive_source_set_irq_msi(sPAPRXive *xive, int lisn, int 
>> val)
>> +{
>> +    if (val) {
>> +        spapr_xive_irq(xive, lisn);
>> +    }
>> +}
>> +
>> +static void spapr_xive_source_set_irq_lsi(sPAPRXive *xive, int lisn, int 
>> val)
>> +{
>> +    if (val) {
>> +        xive->status[lisn] |= XIVE_STATUS_ASSERTED;
>> +    } else {
>> +        xive->status[lisn] &= ~XIVE_STATUS_ASSERTED;
>> +    }
>> +
>> +    if (xive->status[lisn] & XIVE_STATUS_ASSERTED &&
>> +        !(xive->status[lisn] & XIVE_STATUS_SENT)) {
>> +        xive->status[lisn] |= XIVE_STATUS_SENT;
>> +        spapr_xive_irq(xive, lisn);
>> +    }
>> +}
>> +
>> +static void spapr_xive_source_set_irq(void *opaque, int lisn, int val)
>> +{
>> +    sPAPRXive *xive = SPAPR_XIVE(opaque);
>> +
>> +    if (spapr_xive_irq_is_lsi(xive, lisn)) {
>> +        spapr_xive_source_set_irq_lsi(xive, lisn, val);
>> +    } else {
>> +        spapr_xive_source_set_irq_msi(xive, lisn, val);
>> +    }
>> +}
>> +
>>  /*
>>   * Main XIVE object
>>   */
>> @@ -41,7 +82,8 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor 
>> *mon)
>>              continue;
>>          }
>>  
>> -        monitor_printf(mon, "  %4x %s %08x %08x\n", i,
>> +        monitor_printf(mon, "  %4x %s %s %08x %08x\n", i,
>> +                       spapr_xive_irq_is_lsi(xive, i) ? "LSI" : "MSI",
>>                         ive->w & IVE_MASKED ? "M" : " ",
>>                         (int) GETFIELD(IVE_EQ_INDEX, ive->w),
>>                         (int) GETFIELD(IVE_EQ_DATA, ive->w));
>> @@ -53,6 +95,8 @@ void spapr_xive_reset(void *dev)
>>      sPAPRXive *xive = SPAPR_XIVE(dev);
>>      int i;
>>  
>> +    /* Do not clear IRQs status */
>> +
>>      /* Mask all valid IVEs in the IRQ number space. */
>>      for (i = 0; i < xive->nr_irqs; i++) {
>>          XiveIVE *ive = &xive->ivt[i];
>> @@ -71,6 +115,11 @@ static void spapr_xive_realize(DeviceState *dev, Error 
>> **errp)
>>          return;
>>      }
>>  
>> +    /* QEMU IRQs */
>> +    xive->qirqs = qemu_allocate_irqs(spapr_xive_source_set_irq, xive,
>> +                                     xive->nr_irqs);
>> +    xive->status = g_malloc0(xive->nr_irqs);
>> +
>>      /* Allocate the IVT (Interrupt Virtualization Table) */
>>      xive->ivt = g_malloc0(xive->nr_irqs * sizeof(XiveIVE));
>>  
>> @@ -102,6 +151,7 @@ static const VMStateDescription vmstate_spapr_xive = {
>>          VMSTATE_UINT32_EQUAL(nr_irqs, sPAPRXive, NULL),
>>          VMSTATE_STRUCT_VARRAY_UINT32_ALLOC(ivt, sPAPRXive, nr_irqs, 1,
>>                                             vmstate_spapr_xive_ive, XiveIVE),
>> +        VMSTATE_VBUFFER_UINT32(status, sPAPRXive, 1, NULL, nr_irqs),
>>          VMSTATE_END_OF_LIST()
>>      },
>>  };
>> @@ -140,7 +190,7 @@ XiveIVE *spapr_xive_get_ive(sPAPRXive *xive, uint32_t 
>> lisn)
>>      return lisn < xive->nr_irqs ? &xive->ivt[lisn] : NULL;
>>  }
>>  
>> -bool spapr_xive_irq_set(sPAPRXive *xive, uint32_t lisn)
>> +bool spapr_xive_irq_set(sPAPRXive *xive, uint32_t lisn, bool lsi)
>>  {
>>      XiveIVE *ive = spapr_xive_get_ive(xive, lisn);
>>  
>> @@ -149,6 +199,7 @@ bool spapr_xive_irq_set(sPAPRXive *xive, uint32_t lisn)
>>      }
>>  
>>      ive->w |= IVE_VALID;
>> +    xive->status[lisn] |= lsi ? XIVE_STATUS_LSI : 0;
> 
> How does a hardware XIVE know which irqs are LSI and which are MSI?

AFAICT, it doesn't. LSI events are configured as the other XIVE interrupts. 
The level is converted in the P bit and the Q bit should always be zero.
So I should be able to simplify the proposed model which still is mimicking 
XICS  ... I will take a look at it. 

There are a sort of special degenerated LSIs but these are for bringup.

Thanks,

C. 

>>      return true;
>>  }
>>  
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index 795b3f4ded7c..6a799cdaba66 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -33,11 +33,23 @@ struct sPAPRXive {
>>      /* Properties */
>>      uint32_t     nr_irqs;
>>  
>> +     /* IRQ */
>> +    qemu_irq     *qirqs;
>> +#define XIVE_STATUS_LSI                0x1
>> +#define XIVE_STATUS_ASSERTED           0x2
>> +#define XIVE_STATUS_SENT               0x4
>> +    uint8_t      *status;
>> +
>>      /* XIVE internal tables */
>>      XiveIVE      *ivt;
>>  };
>>  
>> -bool spapr_xive_irq_set(sPAPRXive *xive, uint32_t lisn);
>> +static inline bool spapr_xive_irq_is_lsi(sPAPRXive *xive, int lisn)
>> +{
>> +    return xive->status[lisn] & XIVE_STATUS_LSI;
>> +}
>> +
>> +bool spapr_xive_irq_set(sPAPRXive *xive, uint32_t lisn, bool lsi);
>>  bool spapr_xive_irq_unset(sPAPRXive *xive, uint32_t lisn);
>>  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>>  
>