Re: [Qemu-devel] [PATCH v2 1/8] xics: Add flags for interrupts

[Alexander Graf <agraf@suse.de>]

On 21.05.14 08:30, Alexey Kardashevskiy wrote:
> On 05/15/2014 07:59 PM, Alexey Kardashevskiy wrote:
>> The existing interrupt allocation scheme in SPAPR assumes that
>> interrupts are allocated at the start time, continously and the config
>> will not change. However, there are cases when this is not going to work
>> such as:
>>
>> 1. migration - we will have to have an ability to choose interrupt
>> numbers for devices in the command line and this will create gaps in
>> interrupt space.
>>
>> 2. PCI hotplug - interrupts from unplugged device need to be returned
>> back to interrupt pool, otherwise we will quickly run out of interrupts.
>>
>> This replaces a separate lslsi[] array with a byte in the ICSIRQState
>> struct and defines "LSI" and "MSI" flags. Neither of these flags set
>> signals that the descriptor is not allocated and not in use.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/intc/xics.c        | 23 ++++++++++++++++-------
>>   hw/intc/xics_kvm.c    |  5 ++---
>>   include/hw/ppc/xics.h |  6 +++++-
>>   3 files changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 64aabe7..220ca0e 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -438,7 +438,7 @@ static void ics_set_irq(void *opaque, int srcno, int val)
>>   {
>>       ICSState *ics = (ICSState *)opaque;
>>   
>> -    if (ics->islsi[srcno]) {
>> +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>>           set_irq_lsi(ics, srcno, val);
>>       } else {
>>           set_irq_msi(ics, srcno, val);
>> @@ -475,7 +475,7 @@ static void ics_write_xive(ICSState *ics, int nr, int server,
>>   
>>       trace_xics_ics_write_xive(nr, srcno, server, priority);
>>   
>> -    if (ics->islsi[srcno]) {
>> +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>>           write_xive_lsi(ics, srcno);
>>       } else {
>>           write_xive_msi(ics, srcno);
>> @@ -497,7 +497,7 @@ static void ics_resend(ICSState *ics)
>>   
>>       for (i = 0; i < ics->nr_irqs; i++) {
>>           /* FIXME: filter by server#? */
>> -        if (ics->islsi[i]) {
>> +        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
>>               resend_lsi(ics, i);
>>           } else {
>>               resend_msi(ics, i);
>> @@ -512,7 +512,7 @@ static void ics_eoi(ICSState *ics, int nr)
>>   
>>       trace_xics_ics_eoi(nr);
>>   
>> -    if (ics->islsi[srcno]) {
>> +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) {
>>           irq->status &= ~XICS_STATUS_SENT;
>>       }
>>   }
>> @@ -609,7 +609,6 @@ static void ics_realize(DeviceState *dev, Error **errp)
>>           return;
>>       }
>>       ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>> -    ics->islsi = g_malloc0(ics->nr_irqs * sizeof(bool));
>>       ics->qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs);
>>   }
>>   
>> @@ -646,11 +645,21 @@ qemu_irq xics_get_qirq(XICSState *icp, int irq)
>>       return icp->ics->qirqs[irq - icp->ics->offset];
>>   }
>>   
>> +static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi)
>> +{
>> +    assert(!(ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MASK));
>> +
>> +    ics->irqs[srcno].flags |=
>> +        lsi ? XICS_FLAGS_IRQ_LSI : XICS_FLAGS_IRQ_MSI;
>> +}
>> +
>>   void xics_set_irq_type(XICSState *icp, int irq, bool lsi)
>>   {
>> -    assert(ics_valid_irq(icp->ics, irq));
>> +    ICSState *ics = icp->ics;
>>   
>> -    icp->ics->islsi[irq - icp->ics->offset] = lsi;
>> +    assert(ics_valid_irq(ics, irq));
>> +
>> +    ics_set_irq_type(ics, irq - ics->offset, lsi);
>>   }
>>   
>>   /*
>> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
>> index 09476ae..8719a88 100644
>> --- a/hw/intc/xics_kvm.c
>> +++ b/hw/intc/xics_kvm.c
>> @@ -224,7 +224,7 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
>>               state |= KVM_XICS_MASKED;
>>           }
>>   
>> -        if (ics->islsi[i]) {
>> +        if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) {
>>               state |= KVM_XICS_LEVEL_SENSITIVE;
>>               if (irq->status & XICS_STATUS_ASSERTED) {
>>                   state |= KVM_XICS_PENDING;
>> @@ -253,7 +253,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val)
>>       int rc;
>>   
>>       args.irq = srcno + ics->offset;
>> -    if (!ics->islsi[srcno]) {
>> +    if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MSI) {
>>           if (!val) {
>>               return;
>>           }
>> @@ -290,7 +290,6 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp)
>>           return;
>>       }
>>       ics->irqs = g_malloc0(ics->nr_irqs * sizeof(ICSIRQState));
>> -    ics->islsi = g_malloc0(ics->nr_irqs * sizeof(bool));
>>       ics->qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics->nr_irqs);
>>   }
>>   
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 0d7673d..fa8e9c2 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -136,7 +136,6 @@ struct ICSState {
>>       uint32_t nr_irqs;
>>       uint32_t offset;
>>       qemu_irq *qirqs;
>> -    bool *islsi;
>>       ICSIRQState *irqs;
>>       XICSState *icp;
>>   };
>> @@ -150,6 +149,11 @@ struct ICSIRQState {
>>   #define XICS_STATUS_REJECTED           0x4
>>   #define XICS_STATUS_MASKED_PENDING     0x8
>>       uint8_t status;
>> +/* (flags & XICS_FLAGS_IRQ_MASK) == 0 means the interrupt is not allocated */
>> +#define XICS_FLAGS_IRQ_LSI             0x1
>> +#define XICS_FLAGS_IRQ_MSI             0x2
>> +#define XICS_FLAGS_IRQ_MASK            0x3
>> +    uint8_t flags;
>>   };
>>   
>>   qemu_irq xics_get_qirq(XICSState *icp, int irq);
>>
> This is missing. But I suspect they still mmm not perfect so no point in
> reposting them?
>
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index f6a2066..c920410 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -579,6 +579,7 @@ static const VMStateDescription vmstate_ics_irq = {
>           VMSTATE_UINT8(priority, ICSIRQState),
>           VMSTATE_UINT8(saved_priority, ICSIRQState),
>           VMSTATE_UINT8(status, ICSIRQState),
> +        VMSTATE_UINT8(flags, ICSIRQState),

The version change is also missing.


Alex