From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn25D-0004fo-A8 for qemu-devel@nongnu.org; Wed, 21 May 2014 04:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn256-0005s9-Ry for qemu-devel@nongnu.org; Wed, 21 May 2014 04:41:07 -0400 Message-ID: <537C669B.1060208@suse.de> Date: Wed, 21 May 2014 10:40:59 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1400147999-4793-1-git-send-email-aik@ozlabs.ru> <1400147999-4793-2-git-send-email-aik@ozlabs.ru> <537C47EC.6090907@ozlabs.ru> In-Reply-To: <537C47EC.6090907@ozlabs.ru> Content-Type: text/plain; charset=KOI8-R; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/8] xics: Add flags for interrupts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org 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 >> --- >> 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