| Message ID | 20191009060818.29719-16-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: IRQ subsystem cleanup | expand |
On 09/10/2019 08:08, David Gibson wrote: > The nr_msis value we use here has to line up with whether we're using > legacy or modern irq allocation. Therefore it's safer to derive it based > on legacy_irq_allocation rather than having SpaprIrq contain a canned > value. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> one minor typo below, > --- > hw/ppc/spapr.c | 5 ++--- > hw/ppc/spapr_irq.c | 26 +++++++++++++++++--------- > hw/ppc/spapr_pci.c | 7 ++++--- > include/hw/pci-host/spapr.h | 4 ++-- > include/hw/ppc/spapr_irq.h | 4 +--- > 5 files changed, 26 insertions(+), 20 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index e880db5d38..153cc54354 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1275,7 +1275,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr) > } > > QLIST_FOREACH(phb, &spapr->phbs, list) { > - ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, NULL); > + ret = spapr_dt_phb(spapr, phb, PHANDLE_INTC, fdt, NULL); > if (ret < 0) { > error_report("couldn't setup PCI devices in fdt"); > exit(1); > @@ -3910,8 +3910,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, > return -1; > } > > - if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis, > - fdt_start_offset)) { > + if (spapr_dt_phb(spapr, sphb, intc_phandle, fdt, fdt_start_offset)) { > error_setg(errp, "unable to create FDT node for PHB %d", sphb->index); > return -1; > } > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index f3d18b1dad..076da31501 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -29,9 +29,14 @@ static const TypeInfo spapr_intc_info = { > .class_size = sizeof(SpaprInterruptControllerClass), > }; > > -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis) > +static void spapr_irq_msi_init(SpaprMachineState *spapr) > { > - spapr->irq_map_nr = nr_msis; > + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { > + /* Legacy mode doesn't use this allocater */ allocater -> allocator > + return; > + } > + > + spapr->irq_map_nr = spapr_irq_nr_msis(spapr); > spapr->irq_map = bitmap_new(spapr->irq_map_nr); > } > > @@ -102,7 +107,6 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), > > SpaprIrq spapr_irq_xics = { > .nr_xirqs = SPAPR_NR_XIRQS, > - .nr_msis = SPAPR_NR_MSIS, > .xics = true, > .xive = false, > }; > @@ -113,7 +117,6 @@ SpaprIrq spapr_irq_xics = { > > SpaprIrq spapr_irq_xive = { > .nr_xirqs = SPAPR_NR_XIRQS, > - .nr_msis = SPAPR_NR_MSIS, > .xics = false, > .xive = true, > }; > @@ -132,7 +135,6 @@ SpaprIrq spapr_irq_xive = { > */ > SpaprIrq spapr_irq_dual = { > .nr_xirqs = SPAPR_NR_XIRQS, > - .nr_msis = SPAPR_NR_MSIS, > .xics = true, > .xive = true, > }; > @@ -247,6 +249,15 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, > sicc->dt(spapr->active_intc, nr_servers, fdt, phandle); > } > > +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr) > +{ > + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { > + return spapr->irq->nr_xirqs; > + } else { > + return SPAPR_XIRQ_BASE + spapr->irq->nr_xirqs - SPAPR_IRQ_MSI; > + } > +} > + > void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > { > MachineState *machine = MACHINE(spapr); > @@ -267,9 +278,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > } > > /* Initialize the MSI IRQ allocator. */ > - if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { > - spapr_irq_msi_init(spapr, spapr->irq->nr_msis); > - } > + spapr_irq_msi_init(spapr); > > if (spapr->irq->xics) { > Error *local_err = NULL; > @@ -551,7 +560,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > > SpaprIrq spapr_irq_xics_legacy = { > .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, > - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, > .xics = true, > .xive = false, > }; > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 01ff41d4c4..cc0e7829b6 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -2277,8 +2277,8 @@ static void spapr_phb_pci_enumerate(SpaprPhbState *phb) > > } > > -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, > - uint32_t nr_msis, int *node_offset) > +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, > + uint32_t intc_phandle, void *fdt, int *node_offset) > { > int bus_off, i, j, ret; > uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) }; > @@ -2343,7 +2343,8 @@ int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, > _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges)); > _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg))); > _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1)); > - _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", nr_msis)); > + _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", > + spapr_irq_nr_msis(spapr))); > > /* Dynamic DMA window */ > if (phb->ddw_enabled) { > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h > index 23506f05d9..8877ff51fb 100644 > --- a/include/hw/pci-host/spapr.h > +++ b/include/hw/pci-host/spapr.h > @@ -128,8 +128,8 @@ struct SpaprPhbState { > #define SPAPR_PCI_NV2ATSD_WIN_SIZE (NVGPU_MAX_NUM * NVGPU_MAX_LINKS * \ > 64 * KiB) > > -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, > - uint32_t nr_msis, int *node_offset); > +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, > + uint32_t intc_phandle, void *fdt, int *node_offset); > > void spapr_pci_rtas_init(void); > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index 08173e714c..befe8e01dc 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -27,7 +27,6 @@ > #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; > > @@ -73,14 +72,13 @@ void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); > void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, > void *fdt, uint32_t phandle); > > -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); > +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr); > int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, > Error **errp); > void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); > > typedef struct SpaprIrq { > uint32_t nr_xirqs; > - uint32_t nr_msis; > bool xics; > bool xive; > } SpaprIrq; >
On Wed, Oct 09, 2019 at 05:59:13PM +0200, Cédric Le Goater wrote: > On 09/10/2019 08:08, David Gibson wrote: > > The nr_msis value we use here has to line up with whether we're using > > legacy or modern irq allocation. Therefore it's safer to derive it based > > on legacy_irq_allocation rather than having SpaprIrq contain a canned > > value. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > one minor typo below, > > > --- > > hw/ppc/spapr.c | 5 ++--- > > hw/ppc/spapr_irq.c | 26 +++++++++++++++++--------- > > hw/ppc/spapr_pci.c | 7 ++++--- > > include/hw/pci-host/spapr.h | 4 ++-- > > include/hw/ppc/spapr_irq.h | 4 +--- > > 5 files changed, 26 insertions(+), 20 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index e880db5d38..153cc54354 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1275,7 +1275,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr) > > } > > > > QLIST_FOREACH(phb, &spapr->phbs, list) { > > - ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, NULL); > > + ret = spapr_dt_phb(spapr, phb, PHANDLE_INTC, fdt, NULL); > > if (ret < 0) { > > error_report("couldn't setup PCI devices in fdt"); > > exit(1); > > @@ -3910,8 +3910,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, > > return -1; > > } > > > > - if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis, > > - fdt_start_offset)) { > > + if (spapr_dt_phb(spapr, sphb, intc_phandle, fdt, fdt_start_offset)) { > > error_setg(errp, "unable to create FDT node for PHB %d", sphb->index); > > return -1; > > } > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index f3d18b1dad..076da31501 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -29,9 +29,14 @@ static const TypeInfo spapr_intc_info = { > > .class_size = sizeof(SpaprInterruptControllerClass), > > }; > > > > -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis) > > +static void spapr_irq_msi_init(SpaprMachineState *spapr) > > { > > - spapr->irq_map_nr = nr_msis; > > + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { > > + /* Legacy mode doesn't use this allocater */ > > allocater -> allocator Huh. I had surprising difficulty trying to confirm what the correct spelling really is here. Actual dictionaries seem to thing that neither "allocater" nor "allocator" are words. Still "allocator" seems to be by far the more common variant, so I've changed it.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e880db5d38..153cc54354 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1275,7 +1275,7 @@ static void *spapr_build_fdt(SpaprMachineState *spapr) } QLIST_FOREACH(phb, &spapr->phbs, list) { - ret = spapr_dt_phb(phb, PHANDLE_INTC, fdt, spapr->irq->nr_msis, NULL); + ret = spapr_dt_phb(spapr, phb, PHANDLE_INTC, fdt, NULL); if (ret < 0) { error_report("couldn't setup PCI devices in fdt"); exit(1); @@ -3910,8 +3910,7 @@ int spapr_phb_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr, return -1; } - if (spapr_dt_phb(sphb, intc_phandle, fdt, spapr->irq->nr_msis, - fdt_start_offset)) { + if (spapr_dt_phb(spapr, sphb, intc_phandle, fdt, fdt_start_offset)) { error_setg(errp, "unable to create FDT node for PHB %d", sphb->index); return -1; } diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index f3d18b1dad..076da31501 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -29,9 +29,14 @@ static const TypeInfo spapr_intc_info = { .class_size = sizeof(SpaprInterruptControllerClass), }; -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis) +static void spapr_irq_msi_init(SpaprMachineState *spapr) { - spapr->irq_map_nr = nr_msis; + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { + /* Legacy mode doesn't use this allocater */ + return; + } + + spapr->irq_map_nr = spapr_irq_nr_msis(spapr); spapr->irq_map = bitmap_new(spapr->irq_map_nr); } @@ -102,7 +107,6 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), SpaprIrq spapr_irq_xics = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = false, }; @@ -113,7 +117,6 @@ SpaprIrq spapr_irq_xics = { SpaprIrq spapr_irq_xive = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = false, .xive = true, }; @@ -132,7 +135,6 @@ SpaprIrq spapr_irq_xive = { */ SpaprIrq spapr_irq_dual = { .nr_xirqs = SPAPR_NR_XIRQS, - .nr_msis = SPAPR_NR_MSIS, .xics = true, .xive = true, }; @@ -247,6 +249,15 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, sicc->dt(spapr->active_intc, nr_servers, fdt, phandle); } +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr) +{ + if (SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { + return spapr->irq->nr_xirqs; + } else { + return SPAPR_XIRQ_BASE + spapr->irq->nr_xirqs - SPAPR_IRQ_MSI; + } +} + void spapr_irq_init(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); @@ -267,9 +278,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) } /* Initialize the MSI IRQ allocator. */ - if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { - spapr_irq_msi_init(spapr, spapr->irq->nr_msis); - } + spapr_irq_msi_init(spapr); if (spapr->irq->xics) { Error *local_err = NULL; @@ -551,7 +560,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) SpaprIrq spapr_irq_xics_legacy = { .nr_xirqs = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, - .nr_msis = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS, .xics = true, .xive = false, }; diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 01ff41d4c4..cc0e7829b6 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -2277,8 +2277,8 @@ static void spapr_phb_pci_enumerate(SpaprPhbState *phb) } -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, - uint32_t nr_msis, int *node_offset) +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, + uint32_t intc_phandle, void *fdt, int *node_offset) { int bus_off, i, j, ret; uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) }; @@ -2343,7 +2343,8 @@ int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges)); _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg))); _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1)); - _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", nr_msis)); + _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi", + spapr_irq_nr_msis(spapr))); /* Dynamic DMA window */ if (phb->ddw_enabled) { diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h index 23506f05d9..8877ff51fb 100644 --- a/include/hw/pci-host/spapr.h +++ b/include/hw/pci-host/spapr.h @@ -128,8 +128,8 @@ struct SpaprPhbState { #define SPAPR_PCI_NV2ATSD_WIN_SIZE (NVGPU_MAX_NUM * NVGPU_MAX_LINKS * \ 64 * KiB) -int spapr_dt_phb(SpaprPhbState *phb, uint32_t intc_phandle, void *fdt, - uint32_t nr_msis, int *node_offset); +int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb, + uint32_t intc_phandle, void *fdt, int *node_offset); void spapr_pci_rtas_init(void); diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 08173e714c..befe8e01dc 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -27,7 +27,6 @@ #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; @@ -73,14 +72,13 @@ void spapr_irq_print_info(SpaprMachineState *spapr, Monitor *mon); void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers, void *fdt, uint32_t phandle); -void spapr_irq_msi_init(SpaprMachineState *spapr, uint32_t nr_msis); +uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr); int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, Error **errp); void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); typedef struct SpaprIrq { uint32_t nr_xirqs; - uint32_t nr_msis; bool xics; bool xive; } SpaprIrq;