From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48295) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WYESS-0001Nj-DE for qemu-devel@nongnu.org; Thu, 10 Apr 2014 08:52:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WYESM-0000r7-Gc for qemu-devel@nongnu.org; Thu, 10 Apr 2014 08:51:56 -0400 Message-ID: <534693E4.1050204@suse.de> Date: Thu, 10 Apr 2014 14:51:48 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1394770689-29039-1-git-send-email-aik@ozlabs.ru> <1394770689-29039-7-git-send-email-aik@ozlabs.ru> In-Reply-To: <1394770689-29039-7-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 6/8] spapr: move interrupt allocator to xics List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, =?ISO-8859-1?Q?Andreas_F=E4rber?= On 14.03.14 05:18, Alexey Kardashevskiy wrote: > The current allocator returns IRQ numbers from a pool and does not > support IRQs reuse in any form as it did not keep track of what it > previously returned, it only had the last returned IRQ. > However migration may change interrupts for devices depending on > their order in the command line. Wtf? Nonono, this sounds very bogus and wrong. Migration shouldn't change anything. Alex > This moves an allocator from SPAPR to XICS. > > This switches IRQ users to use new API. > > This uses LSI/MSI flags to know if interrupt is in use. > > Signed-off-by: Alexey Kardashevskiy > --- > hw/intc/xics.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/ppc/spapr.c | 67 ------------------------------------------ > hw/ppc/spapr_events.c | 2 +- > hw/ppc/spapr_pci.c | 6 ++-- > hw/ppc/spapr_vio.c | 2 +- > include/hw/ppc/spapr.h | 10 ------- > include/hw/ppc/xics.h | 2 ++ > trace-events | 3 ++ > 8 files changed, 90 insertions(+), 82 deletions(-) > > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index e5195bf..8d101a3 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -690,6 +690,86 @@ void xics_set_irq_type(XICSState *icp, int irq, bool lsi) > ics_set_irq_type(&icp->ics[server], irq, lsi); > } > > +#define XICS_IRQ_FREE(ics, n) \ > + (!((ics)->irqs[(n) - (ics)->offset].flags & \ > + (XICS_FLAGS_LSI | XICS_FLAGS_MSI))) > + > +static int ics_find_free_block(ICSState *ics, int num, int alignnum) > +{ > + int first, i; > + > + for (first = 0; first < ics->nr_irqs; first += alignnum) { > + if (num > (ics->nr_irqs - first)) { > + return -1; > + } > + for (i = first; i < first + num; ++i) { > + if (!XICS_IRQ_FREE(ics, i + ics->offset)) { > + break; > + } > + } > + if (i == (first + num)) { > + return first + ics->offset; > + } > + } > + > + return -1; > +} > + > +int xics_alloc(XICSState *icp, int server, int irq, bool lsi) > +{ > + ICSState *ics = &icp->ics[server]; > + > + if (irq) { > + assert(server == xics_find_server(icp, irq)); > + if (!XICS_IRQ_FREE(ics, irq)) { > + trace_xics_alloc_failed(server, irq); > + return -1; > + } > + } else { > + irq = ics_find_free_block(ics, 1, 1); > + } > + > + ics_set_irq_type(ics, irq, lsi); > + trace_xics_alloc(server, irq); > + > + return irq; > +} > + > +/* > + * Allocate block of consequtive IRQs, returns a number of the first. > + * If align==true, aligns the first IRQ number to num. > + */ > +int xics_alloc_block(XICSState *icp, int server, int num, bool lsi, bool align) > +{ > + int i, first = -1; > + ICSState *ics = &icp->ics[server]; > + > + assert(server == 0); > + /* > + * MSIMesage::data is used for storing VIRQ so > + * it has to be aligned to num to support multiple > + * MSI vectors. MSI-X is not affected by this. > + * The hint is used for the first IRQ, the rest should > + * be allocated continuously. > + */ > + if (align) { > + assert((num == 1) || (num == 2) || (num == 4) || > + (num == 8) || (num == 16) || (num == 32)); > + first = ics_find_free_block(ics, num, num); > + } else { > + first = ics_find_free_block(ics, num, 1); > + } > + > + if (first > 0) { > + for (i = first; i < first + num; ++i) { > + ics_set_irq_type(ics, i, lsi); > + } > + } > + trace_xics_alloc_block(server, first, num, lsi, align); > + > + return first; > +} > + > /* > * Guest interfaces > */ > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 12adc21..29ca2e0 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -83,73 +83,6 @@ > > sPAPREnvironment *spapr; > > -int spapr_allocate_irq(int hint, bool lsi) > -{ > - int irq; > - > - if (hint) { > - irq = hint; > - if (hint >= spapr->next_irq) { > - spapr->next_irq = hint + 1; > - } > - /* FIXME: we should probably check for collisions somehow */ > - } else { > - irq = spapr->next_irq++; > - } > - > - /* Configure irq type */ > - if (!xics_get_qirq(spapr->icp, irq)) { > - return 0; > - } > - > - xics_set_irq_type(spapr->icp, irq, lsi); > - > - return irq; > -} > - > -/* > - * Allocate block of consequtive IRQs, returns a number of the first. > - * If msi==true, aligns the first IRQ number to num. > - */ > -int spapr_allocate_irq_block(int num, bool lsi, bool msi) > -{ > - int first = -1; > - int i, hint = 0; > - > - /* > - * MSIMesage::data is used for storing VIRQ so > - * it has to be aligned to num to support multiple > - * MSI vectors. MSI-X is not affected by this. > - * The hint is used for the first IRQ, the rest should > - * be allocated continuously. > - */ > - if (msi) { > - assert((num == 1) || (num == 2) || (num == 4) || > - (num == 8) || (num == 16) || (num == 32)); > - hint = (spapr->next_irq + num - 1) & ~(num - 1); > - } > - > - for (i = 0; i < num; ++i) { > - int irq; > - > - irq = spapr_allocate_irq(hint, lsi); > - if (!irq) { > - return -1; > - } > - > - if (0 == i) { > - first = irq; > - hint = 0; > - } > - > - /* If the above doesn't create a consecutive block then that's > - * an internal bug */ > - assert(irq == (first + i)); > - } > - > - return first; > -} > - > static XICSState *try_create_xics(const char *type, int nr_servers, > int nr_irqs) > { > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index 16fa49e..e605430 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -314,7 +314,7 @@ static void check_exception(PowerPCCPU *cpu, sPAPREnvironment *spapr, > > void spapr_events_init(sPAPREnvironment *spapr) > { > - spapr->epow_irq = spapr_allocate_msi(0); > + spapr->epow_irq = xics_alloc_block(spapr->icp, 0, 1, false, false); > spapr->epow_notifier.notify = spapr_powerdown_req; > qemu_register_powerdown_notifier(&spapr->epow_notifier); > spapr_rtas_register("check-exception", check_exception); > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index cbef095..4eaf364 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -343,8 +343,8 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, > > /* There is no cached config, allocate MSIs */ > if (!phb->msi_table[ndev].nvec) { > - irq = spapr_allocate_irq_block(req_num, false, > - ret_intr_type == RTAS_TYPE_MSI); > + irq = xics_alloc_block(spapr->icp, 0, req_num, false, > + ret_intr_type == RTAS_TYPE_MSI); > if (irq < 0) { > error_report("Cannot allocate MSIs for device#%d", ndev); > rtas_st(rets, 0, RTAS_OUT_HW_ERROR); > @@ -623,7 +623,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > for (i = 0; i < PCI_NUM_PINS; i++) { > uint32_t irq; > > - irq = spapr_allocate_lsi(0); > + irq = xics_alloc_block(spapr->icp, 0, 1, true, false); > if (!irq) { > error_setg(errp, "spapr_allocate_lsi failed"); > return; > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index 4e33f46..8aeb263 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -448,7 +448,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev) > dev->qdev.id = id; > } > > - dev->irq = spapr_allocate_msi(dev->irq); > + dev->irq = xics_alloc(spapr->icp, 0, dev->irq, false); > if (!dev->irq) { > return -1; > } > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 18332fd..5d577ff 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -323,16 +323,6 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode, > int spapr_allocate_irq(int hint, bool lsi); > int spapr_allocate_irq_block(int num, bool lsi, bool msi); > > -static inline int spapr_allocate_msi(int hint) > -{ > - return spapr_allocate_irq(hint, false); > -} > - > -static inline int spapr_allocate_lsi(int hint) > -{ > - return spapr_allocate_irq(hint, true); > -} > - > /* RTAS return codes */ > #define RTAS_OUT_SUCCESS 0 > #define RTAS_OUT_NO_ERRORS_FOUND 1 > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index 4b30e9a..337398d 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -157,6 +157,8 @@ struct ICSIRQState { > > qemu_irq xics_get_qirq(XICSState *icp, int irq); > void xics_set_irq_type(XICSState *icp, int irq, bool lsi); > +int xics_alloc(XICSState *icp, int server, int irq, bool lsi); > +int xics_alloc_block(XICSState *icp, int server, int num, bool lsi, bool align); > > void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu); > > diff --git a/trace-events b/trace-events > index 002c260..ad7400e 100644 > --- a/trace-events > +++ b/trace-events > @@ -1143,6 +1143,9 @@ xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]" > xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_xive: irq %#x [src %d] server %#x prio %#x" > xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]" > xics_ics_eoi(int nr) "ics_eoi: irq %#x" > +xics_alloc(int server, int irq) "server#%d, irq %d" > +xics_alloc_failed(int server, int irq) "server#%d, irq %d" > +xics_alloc_block(int server, int first, int num, bool lsi, int align) "server#%d, first irq %d, %d irqs, lsi=%d, alignnum %d" > > # hw/ppc/spapr_iommu.c > spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64