From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WiMsh-00033a-Sj for qemu-devel@nongnu.org; Thu, 08 May 2014 07:53:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WiMsb-0002zF-JJ for qemu-devel@nongnu.org; Thu, 08 May 2014 07:52:55 -0400 Message-ID: <536B700E.709@suse.de> Date: Thu, 08 May 2014 13:52:46 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1399442518-26303-1-git-send-email-aik@ozlabs.ru> <1399442518-26303-2-git-send-email-aik@ozlabs.ru> In-Reply-To: <1399442518-26303-2-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 05/07/2014 08:01 AM, 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 > --- > hw/intc/xics.c | 21 ++++++++++++++------- > hw/intc/xics_kvm.c | 5 ++--- > include/hw/ppc/xics.h | 5 ++++- > 3 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 64aabe7..1f89a00 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_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_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_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_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,19 @@ 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) > +{ > + ics->irqs[srcno].flags |= > + lsi ? XICS_FLAGS_LSI : XICS_FLAGS_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); What if this gets called with MSI first, then LSI on the same irq number? > } > > /* > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 09476ae..456fc2c 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_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_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..aad48cf 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,10 @@ struct ICSIRQState { > #define XICS_STATUS_REJECTED 0x4 > #define XICS_STATUS_MASKED_PENDING 0x8 > uint8_t status; > +/* @flags == 0 measn the interrupt is not allocated */ > +#define XICS_FLAGS_LSI 0x1 > +#define XICS_FLAGS_MSI 0x2 Please define a mask for the interrupt type. Alex > + uint8_t flags; > }; > > qemu_irq xics_get_qirq(XICSState *icp, int irq);