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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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),
         VMSTATE_END_OF_LIST()
     },
 };




-- 
Alexey