On Fri, Sep 27, 2019 at 09:53:59AM +0200, Greg Kurz wrote: 65;5603;1c> On Fri, 27 Sep 2019 15:50:04 +1000 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > Both the XICS and XIVE interrupt backends have a "nr-irqs" property, but > > it means slightly different things. For XICS (or, strictly, the ICS) it > > indicates the number of "real" external IRQs. Those start at XICS_IRQ_BASE > > (0x1000) and don't include the special IPI vector. For XIVE, however, it > > includes the whole IRQ space, including XIVE's many IPI vectors. > > > > The spapr code currently doesn't handle this sensibly, with the > > nr_irqs value in SpaprIrq having different meanings depending on the > > backend. We fix this by renaming nr_irqs to nr_xirqs and making it > > always indicate just the number of external irqs, adjusting the value > > we pass to XIVE accordingly. We also move to using common constants > > in most of the irq configurations, to make it clearer that the IRQ > > space looks the same to the guest (and emulated devices), even if the > > backend is different. > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > Cedric and I have already given an R-b for this one.
Oh, yeah, not sure how I dropped that one. > Reviewed-by: Greg Kurz <gr...@kaod.org> > > > hw/ppc/spapr_irq.c | 53 ++++++++++++++------------------------ > > include/hw/ppc/spapr_irq.h | 19 +++++++++----- > > 2 files changed, 31 insertions(+), 41 deletions(-) > > > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index 8c26fa2d1e..3207b6bd01 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -92,7 +92,7 @@ static void spapr_irq_init_kvm(SpaprMachineState *spapr, > > * XICS IRQ backend. > > */ > > > > -static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_irqs, > > +static void spapr_irq_init_xics(SpaprMachineState *spapr, int nr_xirqs, > > Error **errp) > > { > > Object *obj; > > @@ -102,7 +102,7 @@ static void spapr_irq_init_xics(SpaprMachineState > > *spapr, int nr_irqs, > > object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > > object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr), > > &error_fatal); > > - object_property_set_int(obj, nr_irqs, "nr-irqs", &error_fatal); > > + object_property_set_int(obj, nr_xirqs, "nr-irqs", &error_fatal); > > object_property_set_bool(obj, true, "realized", &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > @@ -234,13 +234,9 @@ static void spapr_irq_init_kvm_xics(SpaprMachineState > > *spapr, Error **errp) > > } > > } > > > > -#define SPAPR_IRQ_XICS_NR_IRQS 0x1000 > > -#define SPAPR_IRQ_XICS_NR_MSIS \ > > - (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI) > > - > > SpaprIrq spapr_irq_xics = { > > - .nr_irqs = SPAPR_IRQ_XICS_NR_IRQS, > > - .nr_msis = SPAPR_IRQ_XICS_NR_MSIS, > > + .nr_xirqs = SPAPR_NR_XIRQS, > > + .nr_msis = SPAPR_NR_MSIS, > > .ov5 = SPAPR_OV5_XIVE_LEGACY, > > > > .init = spapr_irq_init_xics, > > @@ -260,7 +256,7 @@ SpaprIrq spapr_irq_xics = { > > /* > > * XIVE IRQ backend. > > */ > > -static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_irqs, > > +static void spapr_irq_init_xive(SpaprMachineState *spapr, int nr_xirqs, > > Error **errp) > > { > > uint32_t nr_servers = spapr_max_server_number(spapr); > > @@ -268,7 +264,7 @@ static void spapr_irq_init_xive(SpaprMachineState > > *spapr, int nr_irqs, > > int i; > > > > dev = qdev_create(NULL, TYPE_SPAPR_XIVE); > > - qdev_prop_set_uint32(dev, "nr-irqs", nr_irqs); > > + qdev_prop_set_uint32(dev, "nr-irqs", nr_xirqs + SPAPR_XIRQ_BASE); > > /* > > * 8 XIVE END structures per CPU. One for each available priority > > */ > > @@ -308,7 +304,7 @@ static qemu_irq spapr_qirq_xive(SpaprMachineState > > *spapr, int irq) > > { > > SpaprXive *xive = spapr->xive; > > > > - if (irq >= xive->nr_irqs) { > > + if ((irq < SPAPR_XIRQ_BASE) || (irq >= xive->nr_irqs)) { > > return NULL; > > } > > > > @@ -404,17 +400,9 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState > > *spapr, Error **errp) > > } > > } > > > > -/* > > - * XIVE uses the full IRQ number space. Set it to 8K to be compatible > > - * with XICS. > > - */ > > - > > -#define SPAPR_IRQ_XIVE_NR_IRQS 0x2000 > > -#define SPAPR_IRQ_XIVE_NR_MSIS (SPAPR_IRQ_XIVE_NR_IRQS - SPAPR_IRQ_MSI) > > - > > SpaprIrq spapr_irq_xive = { > > - .nr_irqs = SPAPR_IRQ_XIVE_NR_IRQS, > > - .nr_msis = SPAPR_IRQ_XIVE_NR_MSIS, > > + .nr_xirqs = SPAPR_NR_XIRQS, > > + .nr_msis = SPAPR_NR_MSIS, > > .ov5 = SPAPR_OV5_XIVE_EXPLOIT, > > > > .init = spapr_irq_init_xive, > > @@ -450,18 +438,18 @@ static SpaprIrq *spapr_irq_current(SpaprMachineState > > *spapr) > > &spapr_irq_xive : &spapr_irq_xics; > > } > > > > -static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_irqs, > > +static void spapr_irq_init_dual(SpaprMachineState *spapr, int nr_xirqs, > > Error **errp) > > { > > Error *local_err = NULL; > > > > - spapr_irq_xics.init(spapr, spapr_irq_xics.nr_irqs, &local_err); > > + spapr_irq_xics.init(spapr, spapr_irq_xics.nr_xirqs, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > return; > > } > > > > - spapr_irq_xive.init(spapr, spapr_irq_xive.nr_irqs, &local_err); > > + spapr_irq_xive.init(spapr, spapr_irq_xive.nr_xirqs, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > return; > > @@ -586,12 +574,9 @@ static const char > > *spapr_irq_get_nodename_dual(SpaprMachineState *spapr) > > /* > > * Define values in sync with the XIVE and XICS backend > > */ > > -#define SPAPR_IRQ_DUAL_NR_IRQS 0x2000 > > -#define SPAPR_IRQ_DUAL_NR_MSIS (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI) > > - > > SpaprIrq spapr_irq_dual = { > > - .nr_irqs = SPAPR_IRQ_DUAL_NR_IRQS, > > - .nr_msis = SPAPR_IRQ_DUAL_NR_MSIS, > > + .nr_xirqs = SPAPR_NR_XIRQS, > > + .nr_msis = SPAPR_NR_MSIS, > > .ov5 = SPAPR_OV5_XIVE_BOTH, > > > > .init = spapr_irq_init_dual, > > @@ -693,10 +678,10 @@ void spapr_irq_init(SpaprMachineState *spapr, Error > > **errp) > > spapr_irq_msi_init(spapr, spapr->irq->nr_msis); > > } > > > > - spapr->irq->init(spapr, spapr->irq->nr_irqs, errp); > > + spapr->irq->init(spapr, spapr->irq->nr_xirqs, errp); > > > > spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr, > > - spapr->irq->nr_irqs); > > + spapr->irq->nr_xirqs + > > SPAPR_XIRQ_BASE); > > } > > > > int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error > > **errp) > > @@ -804,11 +789,11 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, > > bool align, Error **errp) > > return first + ics->offset; > > } > > > > -#define SPAPR_IRQ_XICS_LEGACY_NR_IRQS 0x400 > > +#define SPAPR_IRQ_XICS_LEGACY_NR_XIRQS 0x400 > > > > SpaprIrq spapr_irq_xics_legacy = { > > - .nr_irqs = SPAPR_IRQ_XICS_LEGACY_NR_IRQS, > > - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_IRQS, > > + .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, > > + .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, > > .ov5 = SPAPR_OV5_XIVE_LEGACY, > > > > .init = spapr_irq_init_xics, > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > > index 5db305165c..a8f9a2ab11 100644 > > --- a/include/hw/ppc/spapr_irq.h > > +++ b/include/hw/ppc/spapr_irq.h > > @@ -16,13 +16,18 @@ > > * IRQ range offsets per device type > > */ > > #define SPAPR_IRQ_IPI 0x0 > > -#define SPAPR_IRQ_EPOW 0x1000 /* XICS_IRQ_BASE offset */ > > -#define SPAPR_IRQ_HOTPLUG 0x1001 > > -#define SPAPR_IRQ_VIO 0x1100 /* 256 VIO devices */ > > -#define SPAPR_IRQ_PCI_LSI 0x1200 /* 32+ PHBs devices */ > > > > -#define SPAPR_IRQ_MSI 0x1300 /* Offset of the dynamic range covered > > - * by the bitmap allocator */ > > +#define SPAPR_XIRQ_BASE XICS_IRQ_BASE /* 0x1000 */ > > +#define SPAPR_IRQ_EPOW (SPAPR_XIRQ_BASE + 0x0000) > > +#define SPAPR_IRQ_HOTPLUG (SPAPR_XIRQ_BASE + 0x0001) > > +#define SPAPR_IRQ_VIO (SPAPR_XIRQ_BASE + 0x0100) /* 256 VIO > > devices */ > > +#define SPAPR_IRQ_PCI_LSI (SPAPR_XIRQ_BASE + 0x0200) /* 32+ PHBs > > devices */ > > + > > +/* Offset of the dynamic range covered by the bitmap allocator */ > > +#define SPAPR_IRQ_MSI (SPAPR_XIRQ_BASE + 0x0300) > > + > > +#define SPAPR_NR_XIRQS 0x1000 > > +#define SPAPR_NR_MSIS (SPAPR_XIRQ_BASE + SPAPR_NR_XIRQS - > > SPAPR_IRQ_MSI) > > > > typedef struct SpaprMachineState SpaprMachineState; > > > > @@ -32,7 +37,7 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, > > uint32_t num, bool align, > > void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); > > > > typedef struct SpaprIrq { > > - uint32_t nr_irqs; > > + uint32_t nr_xirqs; > > uint32_t nr_msis; > > uint8_t ov5; > > > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature