| Message ID | 20201120174646.619395-7-groug@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: Address the confusion between IPI numbers and vCPU ids | expand |
On Fri, Nov 20, 2020 at 06:46:44PM +0100, Greg Kurz wrote: > The sPAPR ICS device exposes the range of vCPU ids it can handle in > the "ibm,interrupt-server-ranges" FDT property. The highest vCPU > id, ie. spapr_max_server_number(), is obtained from the machine > through the "nr_servers" argument of the generic spapr_irq_dt() call. > > We want to drop the "nr_servers" argument from the API because it > doesn't make sense for the sPAPR XIVE device actually. > > On POWER9, we also pass the highest vCPU id to the KVM XICS-on-XIVE > device, in order to optimize resource allocation in the HW. > > This is enough motivation to introduce an "nr-servers" property > and to use it for both purposes. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > include/hw/ppc/spapr.h | 4 ++-- > include/hw/ppc/xics_spapr.h | 21 +++++++++++++++++--- > hw/intc/xics_kvm.c | 2 +- > hw/intc/xics_spapr.c | 38 ++++++++++++++++++++++++------------- > hw/ppc/spapr.c | 5 +++-- > hw/ppc/spapr_irq.c | 7 +++++-- > 6 files changed, 54 insertions(+), 23 deletions(-) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 2e89e36cfbdc..b76c84a2f884 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -10,7 +10,7 @@ > #include "hw/ppc/spapr_irq.h" > #include "qom/object.h" > #include "hw/ppc/spapr_xive.h" /* For SpaprXive */ > -#include "hw/ppc/xics.h" /* For ICSState */ > +#include "hw/ppc/xics_spapr.h" /* For IcsSpaprState */ > #include "hw/ppc/spapr_tpm_proxy.h" > > struct SpaprVioBus; > @@ -230,7 +230,7 @@ struct SpaprMachineState { > SpaprIrq *irq; > qemu_irq *qirqs; > SpaprInterruptController *active_intc; > - ICSState *ics; > + IcsSpaprState *ics; > SpaprXive *xive; > > bool cmd_line_caps[SPAPR_CAP_NUM]; > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > index de752c0d2c7e..1a483a873b62 100644 > --- a/include/hw/ppc/xics_spapr.h > +++ b/include/hw/ppc/xics_spapr.h > @@ -28,12 +28,27 @@ > #define XICS_SPAPR_H > > #include "hw/ppc/spapr.h" > +#include "hw/ppc/xics.h" > #include "qom/object.h" > > +typedef struct IcsSpaprState { > + /*< private >*/ > + ICPState parent_obj; It's called "*Ics*SpaprState" and it's replacing an ICSState, but it's parent object is an *ICP*State - that doesn't seem right. > + > + /* > + * The ICS needs to know the upper limit to vCPU ids it > + * might be exposed to in order to size the vCPU id range > + * in "ibm,interrupt-server-ranges" and to optimize HW > + * resource allocation when using the XICS-on-XIVE KVM > + * device. It is the purpose of the "nr-servers" property > + * which *must* be set to a non-null value before realizing > + * the ICS. > + */ > + uint32_t nr_servers; That said, I'm a but dubious about attaching the number of servers to the ICS side, rather than the ICP side, since server numbers is basically an ICP concept. In fact... things about the overall topology are usually done via the XicsFabric, so I'm wondering if we should add a callback there for nr_servers. > +} IcsSpaprState; > + > #define TYPE_ICS_SPAPR "ics-spapr" > -/* This is reusing the ICSState typedef from TYPE_ICS */ > -DECLARE_INSTANCE_CHECKER(ICSState, ICS_SPAPR, > - TYPE_ICS_SPAPR) > +DECLARE_INSTANCE_CHECKER(IcsSpaprState, ICS_SPAPR, TYPE_ICS_SPAPR) > > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > Error **errp); > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 570d635bcc08..ecbbda0e249b 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -350,7 +350,7 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > int rc; > CPUState *cs; > Error *local_err = NULL; > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 8ae4f41459c3..ce147e8980ed 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -34,6 +34,7 @@ > #include "hw/ppc/xics.h" > #include "hw/ppc/xics_spapr.h" > #include "hw/ppc/fdt.h" > +#include "hw/qdev-properties.h" > #include "qapi/visitor.h" > > /* > @@ -154,7 +155,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno, server, priority; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -189,7 +190,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -221,7 +222,7 @@ static void rtas_int_off(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -254,7 +255,7 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -285,10 +286,13 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > > static void ics_spapr_realize(DeviceState *dev, Error **errp) > { > - ICSState *ics = ICS_SPAPR(dev); > - ICSStateClass *icsc = ICS_GET_CLASS(ics); > + IcsSpaprState *sics = ICS_SPAPR(dev); > + ICSStateClass *icsc = ICS_GET_CLASS(dev); > Error *local_err = NULL; > > + /* Set by spapr_irq_init() */ > + g_assert(sics->nr_servers); > + > icsc->parent_realize(dev, &local_err); > if (local_err) { > error_propagate(errp, local_err); > @@ -312,7 +316,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > void *fdt, uint32_t phandle) > { > uint32_t interrupt_server_ranges_prop[] = { > - 0, cpu_to_be32(nr_servers), > + 0, cpu_to_be32(ICS_SPAPR(intc)->nr_servers), > }; > int node; > > @@ -333,7 +337,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, > PowerPCCPU *cpu, Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > Object *obj; > SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > > @@ -364,7 +368,7 @@ static void xics_spapr_cpu_intc_destroy(SpaprInterruptController *intc, > static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > bool lsi, Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > > assert(ics); > assert(ics_valid_irq(ics, irq)); > @@ -380,7 +384,7 @@ static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > > static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > uint32_t srcno = irq - ics->offset; > > assert(ics_valid_irq(ics, irq)); > @@ -390,7 +394,7 @@ static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > > static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > uint32_t srcno = irq - ics->offset; > > ics_set_irq(ics, srcno, val); > @@ -398,7 +402,7 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > > static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > CPUState *cs; > > CPU_FOREACH(cs) { > @@ -426,7 +430,8 @@ static int xics_spapr_activate(SpaprInterruptController *intc, > uint32_t nr_servers, Error **errp) > { > if (kvm_enabled()) { > - return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp); > + return spapr_irq_init_kvm(xics_kvm_connect, intc, > + ICS_SPAPR(intc)->nr_servers, errp); > } > return 0; > } > @@ -438,6 +443,11 @@ static void xics_spapr_deactivate(SpaprInterruptController *intc) > } > } > > +static Property ics_spapr_properties[] = { > + DEFINE_PROP_UINT32("nr-servers", IcsSpaprState, nr_servers, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void ics_spapr_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -446,6 +456,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > > device_class_set_parent_realize(dc, ics_spapr_realize, > &isc->parent_realize); > + device_class_set_props(dc, ics_spapr_properties); > sicc->activate = xics_spapr_activate; > sicc->deactivate = xics_spapr_deactivate; > sicc->cpu_intc_create = xics_spapr_cpu_intc_create; > @@ -462,6 +473,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > static const TypeInfo ics_spapr_info = { > .name = TYPE_ICS_SPAPR, > .parent = TYPE_ICS, > + .instance_size = sizeof(IcsSpaprState), > .class_init = ics_spapr_class_init, > .interfaces = (InterfaceInfo[]) { > { TYPE_SPAPR_INTC }, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 12a012d9dd09..21de0456446b 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4218,15 +4218,16 @@ static void spapr_phb_placement(SpaprMachineState *spapr, uint32_t index, > static ICSState *spapr_ics_get(XICSFabric *dev, int irq) > { > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > + ICSState *ics = ICS(spapr->ics); > > - return ics_valid_irq(spapr->ics, irq) ? spapr->ics : NULL; > + return ics_valid_irq(ics, irq) ? ics : NULL; > } > > static void spapr_ics_resend(XICSFabric *dev) > { > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > > - ics_resend(spapr->ics); > + ics_resend(ICS(spapr->ics)); > } > > static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id) > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 2dacbecfd5fd..be6f4041e433 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -316,6 +316,9 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr), > &error_abort); > object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort); > + object_property_set_uint(obj, "nr-servers", > + spapr_max_server_number(spapr), > + &error_abort); > if (!qdev_realize(DEVICE(obj), NULL, errp)) { > return; > } > @@ -426,7 +429,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) > assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); > > if (spapr->ics) { > - assert(ics_valid_irq(spapr->ics, irq)); > + assert(ics_valid_irq(ICS(spapr->ics), irq)); > } > if (spapr->xive) { > assert(irq < spapr->xive->nr_irqs); > @@ -556,7 +559,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) > > int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > int first = -1; > > assert(ics);
On Mon, 23 Nov 2020 15:18:09 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Fri, Nov 20, 2020 at 06:46:44PM +0100, Greg Kurz wrote: > > The sPAPR ICS device exposes the range of vCPU ids it can handle in > > the "ibm,interrupt-server-ranges" FDT property. The highest vCPU > > id, ie. spapr_max_server_number(), is obtained from the machine > > through the "nr_servers" argument of the generic spapr_irq_dt() call. > > > > We want to drop the "nr_servers" argument from the API because it > > doesn't make sense for the sPAPR XIVE device actually. > > > > On POWER9, we also pass the highest vCPU id to the KVM XICS-on-XIVE > > device, in order to optimize resource allocation in the HW. > > > > This is enough motivation to introduce an "nr-servers" property > > and to use it for both purposes. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > include/hw/ppc/spapr.h | 4 ++-- > > include/hw/ppc/xics_spapr.h | 21 +++++++++++++++++--- > > hw/intc/xics_kvm.c | 2 +- > > hw/intc/xics_spapr.c | 38 ++++++++++++++++++++++++------------- > > hw/ppc/spapr.c | 5 +++-- > > hw/ppc/spapr_irq.c | 7 +++++-- > > 6 files changed, 54 insertions(+), 23 deletions(-) > > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 2e89e36cfbdc..b76c84a2f884 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -10,7 +10,7 @@ > > #include "hw/ppc/spapr_irq.h" > > #include "qom/object.h" > > #include "hw/ppc/spapr_xive.h" /* For SpaprXive */ > > -#include "hw/ppc/xics.h" /* For ICSState */ > > +#include "hw/ppc/xics_spapr.h" /* For IcsSpaprState */ > > #include "hw/ppc/spapr_tpm_proxy.h" > > > > struct SpaprVioBus; > > @@ -230,7 +230,7 @@ struct SpaprMachineState { > > SpaprIrq *irq; > > qemu_irq *qirqs; > > SpaprInterruptController *active_intc; > > - ICSState *ics; > > + IcsSpaprState *ics; > > SpaprXive *xive; > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > > index de752c0d2c7e..1a483a873b62 100644 > > --- a/include/hw/ppc/xics_spapr.h > > +++ b/include/hw/ppc/xics_spapr.h > > @@ -28,12 +28,27 @@ > > #define XICS_SPAPR_H > > > > #include "hw/ppc/spapr.h" > > +#include "hw/ppc/xics.h" > > #include "qom/object.h" > > > > +typedef struct IcsSpaprState { > > + /*< private >*/ > > + ICPState parent_obj; > > It's called "*Ics*SpaprState" and it's replacing an ICSState, but it's > parent object is an *ICP*State - that doesn't seem right. > Oops, typo :-\ > > + > > + /* > > + * The ICS needs to know the upper limit to vCPU ids it > > + * might be exposed to in order to size the vCPU id range > > + * in "ibm,interrupt-server-ranges" and to optimize HW > > + * resource allocation when using the XICS-on-XIVE KVM > > + * device. It is the purpose of the "nr-servers" property > > + * which *must* be set to a non-null value before realizing > > + * the ICS. > > + */ > > + uint32_t nr_servers; > > That said, I'm a but dubious about attaching the number of servers to > the ICS side, rather than the ICP side, since server numbers is > basically an ICP concept. In fact... things about the overall > topology are usually done via the XicsFabric, so I'm wondering if we > should add a callback there for nr_servers. > I agree this would be the way to go. I sent a patch to do so a year ago but it didn't reach consensus at the time: http://patchwork.ozlabs.org/project/qemu-devel/patch/157010405465.246126.7760334967989385566.stgit@bahia.lan/ I'll revisit the approach. > > +} IcsSpaprState; > > + > > #define TYPE_ICS_SPAPR "ics-spapr" > > -/* This is reusing the ICSState typedef from TYPE_ICS */ > > -DECLARE_INSTANCE_CHECKER(ICSState, ICS_SPAPR, > > - TYPE_ICS_SPAPR) > > +DECLARE_INSTANCE_CHECKER(IcsSpaprState, ICS_SPAPR, TYPE_ICS_SPAPR) > > > > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > > Error **errp); > > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > > index 570d635bcc08..ecbbda0e249b 100644 > > --- a/hw/intc/xics_kvm.c > > +++ b/hw/intc/xics_kvm.c > > @@ -350,7 +350,7 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) > > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > > Error **errp) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > int rc; > > CPUState *cs; > > Error *local_err = NULL; > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > > index 8ae4f41459c3..ce147e8980ed 100644 > > --- a/hw/intc/xics_spapr.c > > +++ b/hw/intc/xics_spapr.c > > @@ -34,6 +34,7 @@ > > #include "hw/ppc/xics.h" > > #include "hw/ppc/xics_spapr.h" > > #include "hw/ppc/fdt.h" > > +#include "hw/qdev-properties.h" > > #include "qapi/visitor.h" > > > > /* > > @@ -154,7 +155,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > > uint32_t nargs, target_ulong args, > > uint32_t nret, target_ulong rets) > > { > > - ICSState *ics = spapr->ics; > > + ICSState *ics = ICS(spapr->ics); > > uint32_t nr, srcno, server, priority; > > > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > > @@ -189,7 +190,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > > uint32_t nargs, target_ulong args, > > uint32_t nret, target_ulong rets) > > { > > - ICSState *ics = spapr->ics; > > + ICSState *ics = ICS(spapr->ics); > > uint32_t nr, srcno; > > > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > > @@ -221,7 +222,7 @@ static void rtas_int_off(PowerPCCPU *cpu, SpaprMachineState *spapr, > > uint32_t nargs, target_ulong args, > > uint32_t nret, target_ulong rets) > > { > > - ICSState *ics = spapr->ics; > > + ICSState *ics = ICS(spapr->ics); > > uint32_t nr, srcno; > > > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > > @@ -254,7 +255,7 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > > uint32_t nargs, target_ulong args, > > uint32_t nret, target_ulong rets) > > { > > - ICSState *ics = spapr->ics; > > + ICSState *ics = ICS(spapr->ics); > > uint32_t nr, srcno; > > > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > > @@ -285,10 +286,13 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > > > > static void ics_spapr_realize(DeviceState *dev, Error **errp) > > { > > - ICSState *ics = ICS_SPAPR(dev); > > - ICSStateClass *icsc = ICS_GET_CLASS(ics); > > + IcsSpaprState *sics = ICS_SPAPR(dev); > > + ICSStateClass *icsc = ICS_GET_CLASS(dev); > > Error *local_err = NULL; > > > > + /* Set by spapr_irq_init() */ > > + g_assert(sics->nr_servers); > > + > > icsc->parent_realize(dev, &local_err); > > if (local_err) { > > error_propagate(errp, local_err); > > @@ -312,7 +316,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > > void *fdt, uint32_t phandle) > > { > > uint32_t interrupt_server_ranges_prop[] = { > > - 0, cpu_to_be32(nr_servers), > > + 0, cpu_to_be32(ICS_SPAPR(intc)->nr_servers), > > }; > > int node; > > > > @@ -333,7 +337,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > > static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, > > PowerPCCPU *cpu, Error **errp) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > Object *obj; > > SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > > > > @@ -364,7 +368,7 @@ static void xics_spapr_cpu_intc_destroy(SpaprInterruptController *intc, > > static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > > bool lsi, Error **errp) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > > > assert(ics); > > assert(ics_valid_irq(ics, irq)); > > @@ -380,7 +384,7 @@ static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > > > > static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > uint32_t srcno = irq - ics->offset; > > > > assert(ics_valid_irq(ics, irq)); > > @@ -390,7 +394,7 @@ static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > > > > static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > uint32_t srcno = irq - ics->offset; > > > > ics_set_irq(ics, srcno, val); > > @@ -398,7 +402,7 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > > > > static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) > > { > > - ICSState *ics = ICS_SPAPR(intc); > > + ICSState *ics = ICS(intc); > > CPUState *cs; > > > > CPU_FOREACH(cs) { > > @@ -426,7 +430,8 @@ static int xics_spapr_activate(SpaprInterruptController *intc, > > uint32_t nr_servers, Error **errp) > > { > > if (kvm_enabled()) { > > - return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp); > > + return spapr_irq_init_kvm(xics_kvm_connect, intc, > > + ICS_SPAPR(intc)->nr_servers, errp); > > } > > return 0; > > } > > @@ -438,6 +443,11 @@ static void xics_spapr_deactivate(SpaprInterruptController *intc) > > } > > } > > > > +static Property ics_spapr_properties[] = { > > + DEFINE_PROP_UINT32("nr-servers", IcsSpaprState, nr_servers, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > static void ics_spapr_class_init(ObjectClass *klass, void *data) > > { > > DeviceClass *dc = DEVICE_CLASS(klass); > > @@ -446,6 +456,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > > > > device_class_set_parent_realize(dc, ics_spapr_realize, > > &isc->parent_realize); > > + device_class_set_props(dc, ics_spapr_properties); > > sicc->activate = xics_spapr_activate; > > sicc->deactivate = xics_spapr_deactivate; > > sicc->cpu_intc_create = xics_spapr_cpu_intc_create; > > @@ -462,6 +473,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > > static const TypeInfo ics_spapr_info = { > > .name = TYPE_ICS_SPAPR, > > .parent = TYPE_ICS, > > + .instance_size = sizeof(IcsSpaprState), > > .class_init = ics_spapr_class_init, > > .interfaces = (InterfaceInfo[]) { > > { TYPE_SPAPR_INTC }, > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 12a012d9dd09..21de0456446b 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -4218,15 +4218,16 @@ static void spapr_phb_placement(SpaprMachineState *spapr, uint32_t index, > > static ICSState *spapr_ics_get(XICSFabric *dev, int irq) > > { > > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > > + ICSState *ics = ICS(spapr->ics); > > > > - return ics_valid_irq(spapr->ics, irq) ? spapr->ics : NULL; > > + return ics_valid_irq(ics, irq) ? ics : NULL; > > } > > > > static void spapr_ics_resend(XICSFabric *dev) > > { > > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > > > > - ics_resend(spapr->ics); > > + ics_resend(ICS(spapr->ics)); > > } > > > > static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id) > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index 2dacbecfd5fd..be6f4041e433 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -316,6 +316,9 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > > object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr), > > &error_abort); > > object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort); > > + object_property_set_uint(obj, "nr-servers", > > + spapr_max_server_number(spapr), > > + &error_abort); > > if (!qdev_realize(DEVICE(obj), NULL, errp)) { > > return; > > } > > @@ -426,7 +429,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) > > assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); > > > > if (spapr->ics) { > > - assert(ics_valid_irq(spapr->ics, irq)); > > + assert(ics_valid_irq(ICS(spapr->ics), irq)); > > } > > if (spapr->xive) { > > assert(irq < spapr->xive->nr_irqs); > > @@ -556,7 +559,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) > > > > int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > > { > > - ICSState *ics = spapr->ics; > > + ICSState *ics = ICS(spapr->ics); > > int first = -1; > > > > assert(ics); >
On 11/20/20 6:46 PM, Greg Kurz wrote: > The sPAPR ICS device exposes the range of vCPU ids it can handle in > the "ibm,interrupt-server-ranges" FDT property. The highest vCPU > id, ie. spapr_max_server_number(), is obtained from the machine > through the "nr_servers" argument of the generic spapr_irq_dt() call. > > We want to drop the "nr_servers" argument from the API because it > doesn't make sense for the sPAPR XIVE device actually. > > On POWER9, we also pass the highest vCPU id to the KVM XICS-on-XIVE > device, in order to optimize resource allocation in the HW. > > This is enough motivation to introduce an "nr-servers" property > and to use it for both purposes. I don't see a strong justification for storing this information at the interrupt controller level. If it can be kept at the machine level, like it is today, I think it's better. C. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > include/hw/ppc/spapr.h | 4 ++-- > include/hw/ppc/xics_spapr.h | 21 +++++++++++++++++--- > hw/intc/xics_kvm.c | 2 +- > hw/intc/xics_spapr.c | 38 ++++++++++++++++++++++++------------- > hw/ppc/spapr.c | 5 +++-- > hw/ppc/spapr_irq.c | 7 +++++-- > 6 files changed, 54 insertions(+), 23 deletions(-) > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 2e89e36cfbdc..b76c84a2f884 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -10,7 +10,7 @@ > #include "hw/ppc/spapr_irq.h" > #include "qom/object.h" > #include "hw/ppc/spapr_xive.h" /* For SpaprXive */ > -#include "hw/ppc/xics.h" /* For ICSState */ > +#include "hw/ppc/xics_spapr.h" /* For IcsSpaprState */ > #include "hw/ppc/spapr_tpm_proxy.h" > > struct SpaprVioBus; > @@ -230,7 +230,7 @@ struct SpaprMachineState { > SpaprIrq *irq; > qemu_irq *qirqs; > SpaprInterruptController *active_intc; > - ICSState *ics; > + IcsSpaprState *ics; > SpaprXive *xive; > > bool cmd_line_caps[SPAPR_CAP_NUM]; > diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h > index de752c0d2c7e..1a483a873b62 100644 > --- a/include/hw/ppc/xics_spapr.h > +++ b/include/hw/ppc/xics_spapr.h > @@ -28,12 +28,27 @@ > #define XICS_SPAPR_H > > #include "hw/ppc/spapr.h" > +#include "hw/ppc/xics.h" > #include "qom/object.h" > > +typedef struct IcsSpaprState { > + /*< private >*/ > + ICPState parent_obj; > + > + /* > + * The ICS needs to know the upper limit to vCPU ids it > + * might be exposed to in order to size the vCPU id range > + * in "ibm,interrupt-server-ranges" and to optimize HW > + * resource allocation when using the XICS-on-XIVE KVM > + * device. It is the purpose of the "nr-servers" property > + * which *must* be set to a non-null value before realizing > + * the ICS. > + */ > + uint32_t nr_servers; > +} IcsSpaprState; > + > #define TYPE_ICS_SPAPR "ics-spapr" > -/* This is reusing the ICSState typedef from TYPE_ICS */ > -DECLARE_INSTANCE_CHECKER(ICSState, ICS_SPAPR, > - TYPE_ICS_SPAPR) > +DECLARE_INSTANCE_CHECKER(IcsSpaprState, ICS_SPAPR, TYPE_ICS_SPAPR) > > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > Error **errp); > diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c > index 570d635bcc08..ecbbda0e249b 100644 > --- a/hw/intc/xics_kvm.c > +++ b/hw/intc/xics_kvm.c > @@ -350,7 +350,7 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) > int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, > Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > int rc; > CPUState *cs; > Error *local_err = NULL; > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 8ae4f41459c3..ce147e8980ed 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -34,6 +34,7 @@ > #include "hw/ppc/xics.h" > #include "hw/ppc/xics_spapr.h" > #include "hw/ppc/fdt.h" > +#include "hw/qdev-properties.h" > #include "qapi/visitor.h" > > /* > @@ -154,7 +155,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno, server, priority; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -189,7 +190,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -221,7 +222,7 @@ static void rtas_int_off(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -254,7 +255,7 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > uint32_t nr, srcno; > > CHECK_EMULATED_XICS_RTAS(spapr, rets); > @@ -285,10 +286,13 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, > > static void ics_spapr_realize(DeviceState *dev, Error **errp) > { > - ICSState *ics = ICS_SPAPR(dev); > - ICSStateClass *icsc = ICS_GET_CLASS(ics); > + IcsSpaprState *sics = ICS_SPAPR(dev); > + ICSStateClass *icsc = ICS_GET_CLASS(dev); > Error *local_err = NULL; > > + /* Set by spapr_irq_init() */ > + g_assert(sics->nr_servers); > + > icsc->parent_realize(dev, &local_err); > if (local_err) { > error_propagate(errp, local_err); > @@ -312,7 +316,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > void *fdt, uint32_t phandle) > { > uint32_t interrupt_server_ranges_prop[] = { > - 0, cpu_to_be32(nr_servers), > + 0, cpu_to_be32(ICS_SPAPR(intc)->nr_servers), > }; > int node; > > @@ -333,7 +337,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, > static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, > PowerPCCPU *cpu, Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > Object *obj; > SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); > > @@ -364,7 +368,7 @@ static void xics_spapr_cpu_intc_destroy(SpaprInterruptController *intc, > static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > bool lsi, Error **errp) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > > assert(ics); > assert(ics_valid_irq(ics, irq)); > @@ -380,7 +384,7 @@ static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, > > static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > uint32_t srcno = irq - ics->offset; > > assert(ics_valid_irq(ics, irq)); > @@ -390,7 +394,7 @@ static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) > > static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > uint32_t srcno = irq - ics->offset; > > ics_set_irq(ics, srcno, val); > @@ -398,7 +402,7 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) > > static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) > { > - ICSState *ics = ICS_SPAPR(intc); > + ICSState *ics = ICS(intc); > CPUState *cs; > > CPU_FOREACH(cs) { > @@ -426,7 +430,8 @@ static int xics_spapr_activate(SpaprInterruptController *intc, > uint32_t nr_servers, Error **errp) > { > if (kvm_enabled()) { > - return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp); > + return spapr_irq_init_kvm(xics_kvm_connect, intc, > + ICS_SPAPR(intc)->nr_servers, errp); > } > return 0; > } > @@ -438,6 +443,11 @@ static void xics_spapr_deactivate(SpaprInterruptController *intc) > } > } > > +static Property ics_spapr_properties[] = { > + DEFINE_PROP_UINT32("nr-servers", IcsSpaprState, nr_servers, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > static void ics_spapr_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -446,6 +456,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > > device_class_set_parent_realize(dc, ics_spapr_realize, > &isc->parent_realize); > + device_class_set_props(dc, ics_spapr_properties); > sicc->activate = xics_spapr_activate; > sicc->deactivate = xics_spapr_deactivate; > sicc->cpu_intc_create = xics_spapr_cpu_intc_create; > @@ -462,6 +473,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) > static const TypeInfo ics_spapr_info = { > .name = TYPE_ICS_SPAPR, > .parent = TYPE_ICS, > + .instance_size = sizeof(IcsSpaprState), > .class_init = ics_spapr_class_init, > .interfaces = (InterfaceInfo[]) { > { TYPE_SPAPR_INTC }, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 12a012d9dd09..21de0456446b 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4218,15 +4218,16 @@ static void spapr_phb_placement(SpaprMachineState *spapr, uint32_t index, > static ICSState *spapr_ics_get(XICSFabric *dev, int irq) > { > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > + ICSState *ics = ICS(spapr->ics); > > - return ics_valid_irq(spapr->ics, irq) ? spapr->ics : NULL; > + return ics_valid_irq(ics, irq) ? ics : NULL; > } > > static void spapr_ics_resend(XICSFabric *dev) > { > SpaprMachineState *spapr = SPAPR_MACHINE(dev); > > - ics_resend(spapr->ics); > + ics_resend(ICS(spapr->ics)); > } > > static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id) > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 2dacbecfd5fd..be6f4041e433 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -316,6 +316,9 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr), > &error_abort); > object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort); > + object_property_set_uint(obj, "nr-servers", > + spapr_max_server_number(spapr), > + &error_abort); > if (!qdev_realize(DEVICE(obj), NULL, errp)) { > return; > } > @@ -426,7 +429,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) > assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); > > if (spapr->ics) { > - assert(ics_valid_irq(spapr->ics, irq)); > + assert(ics_valid_irq(ICS(spapr->ics), irq)); > } > if (spapr->xive) { > assert(irq < spapr->xive->nr_irqs); > @@ -556,7 +559,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) > > int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > { > - ICSState *ics = spapr->ics; > + ICSState *ics = ICS(spapr->ics); > int first = -1; > > assert(ics); >
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 2e89e36cfbdc..b76c84a2f884 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -10,7 +10,7 @@ #include "hw/ppc/spapr_irq.h" #include "qom/object.h" #include "hw/ppc/spapr_xive.h" /* For SpaprXive */ -#include "hw/ppc/xics.h" /* For ICSState */ +#include "hw/ppc/xics_spapr.h" /* For IcsSpaprState */ #include "hw/ppc/spapr_tpm_proxy.h" struct SpaprVioBus; @@ -230,7 +230,7 @@ struct SpaprMachineState { SpaprIrq *irq; qemu_irq *qirqs; SpaprInterruptController *active_intc; - ICSState *ics; + IcsSpaprState *ics; SpaprXive *xive; bool cmd_line_caps[SPAPR_CAP_NUM]; diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h index de752c0d2c7e..1a483a873b62 100644 --- a/include/hw/ppc/xics_spapr.h +++ b/include/hw/ppc/xics_spapr.h @@ -28,12 +28,27 @@ #define XICS_SPAPR_H #include "hw/ppc/spapr.h" +#include "hw/ppc/xics.h" #include "qom/object.h" +typedef struct IcsSpaprState { + /*< private >*/ + ICPState parent_obj; + + /* + * The ICS needs to know the upper limit to vCPU ids it + * might be exposed to in order to size the vCPU id range + * in "ibm,interrupt-server-ranges" and to optimize HW + * resource allocation when using the XICS-on-XIVE KVM + * device. It is the purpose of the "nr-servers" property + * which *must* be set to a non-null value before realizing + * the ICS. + */ + uint32_t nr_servers; +} IcsSpaprState; + #define TYPE_ICS_SPAPR "ics-spapr" -/* This is reusing the ICSState typedef from TYPE_ICS */ -DECLARE_INSTANCE_CHECKER(ICSState, ICS_SPAPR, - TYPE_ICS_SPAPR) +DECLARE_INSTANCE_CHECKER(IcsSpaprState, ICS_SPAPR, TYPE_ICS_SPAPR) int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, Error **errp); diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 570d635bcc08..ecbbda0e249b 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -350,7 +350,7 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val) int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers, Error **errp) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); int rc; CPUState *cs; Error *local_err = NULL; diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 8ae4f41459c3..ce147e8980ed 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -34,6 +34,7 @@ #include "hw/ppc/xics.h" #include "hw/ppc/xics_spapr.h" #include "hw/ppc/fdt.h" +#include "hw/qdev-properties.h" #include "qapi/visitor.h" /* @@ -154,7 +155,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = spapr->ics; + ICSState *ics = ICS(spapr->ics); uint32_t nr, srcno, server, priority; CHECK_EMULATED_XICS_RTAS(spapr, rets); @@ -189,7 +190,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, SpaprMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = spapr->ics; + ICSState *ics = ICS(spapr->ics); uint32_t nr, srcno; CHECK_EMULATED_XICS_RTAS(spapr, rets); @@ -221,7 +222,7 @@ static void rtas_int_off(PowerPCCPU *cpu, SpaprMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = spapr->ics; + ICSState *ics = ICS(spapr->ics); uint32_t nr, srcno; CHECK_EMULATED_XICS_RTAS(spapr, rets); @@ -254,7 +255,7 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = spapr->ics; + ICSState *ics = ICS(spapr->ics); uint32_t nr, srcno; CHECK_EMULATED_XICS_RTAS(spapr, rets); @@ -285,10 +286,13 @@ static void rtas_int_on(PowerPCCPU *cpu, SpaprMachineState *spapr, static void ics_spapr_realize(DeviceState *dev, Error **errp) { - ICSState *ics = ICS_SPAPR(dev); - ICSStateClass *icsc = ICS_GET_CLASS(ics); + IcsSpaprState *sics = ICS_SPAPR(dev); + ICSStateClass *icsc = ICS_GET_CLASS(dev); Error *local_err = NULL; + /* Set by spapr_irq_init() */ + g_assert(sics->nr_servers); + icsc->parent_realize(dev, &local_err); if (local_err) { error_propagate(errp, local_err); @@ -312,7 +316,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, void *fdt, uint32_t phandle) { uint32_t interrupt_server_ranges_prop[] = { - 0, cpu_to_be32(nr_servers), + 0, cpu_to_be32(ICS_SPAPR(intc)->nr_servers), }; int node; @@ -333,7 +337,7 @@ static void xics_spapr_dt(SpaprInterruptController *intc, uint32_t nr_servers, static int xics_spapr_cpu_intc_create(SpaprInterruptController *intc, PowerPCCPU *cpu, Error **errp) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); Object *obj; SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu); @@ -364,7 +368,7 @@ static void xics_spapr_cpu_intc_destroy(SpaprInterruptController *intc, static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, bool lsi, Error **errp) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); assert(ics); assert(ics_valid_irq(ics, irq)); @@ -380,7 +384,7 @@ static int xics_spapr_claim_irq(SpaprInterruptController *intc, int irq, static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); uint32_t srcno = irq - ics->offset; assert(ics_valid_irq(ics, irq)); @@ -390,7 +394,7 @@ static void xics_spapr_free_irq(SpaprInterruptController *intc, int irq) static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); uint32_t srcno = irq - ics->offset; ics_set_irq(ics, srcno, val); @@ -398,7 +402,7 @@ static void xics_spapr_set_irq(SpaprInterruptController *intc, int irq, int val) static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon) { - ICSState *ics = ICS_SPAPR(intc); + ICSState *ics = ICS(intc); CPUState *cs; CPU_FOREACH(cs) { @@ -426,7 +430,8 @@ static int xics_spapr_activate(SpaprInterruptController *intc, uint32_t nr_servers, Error **errp) { if (kvm_enabled()) { - return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp); + return spapr_irq_init_kvm(xics_kvm_connect, intc, + ICS_SPAPR(intc)->nr_servers, errp); } return 0; } @@ -438,6 +443,11 @@ static void xics_spapr_deactivate(SpaprInterruptController *intc) } } +static Property ics_spapr_properties[] = { + DEFINE_PROP_UINT32("nr-servers", IcsSpaprState, nr_servers, 0), + DEFINE_PROP_END_OF_LIST(), +}; + static void ics_spapr_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -446,6 +456,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) device_class_set_parent_realize(dc, ics_spapr_realize, &isc->parent_realize); + device_class_set_props(dc, ics_spapr_properties); sicc->activate = xics_spapr_activate; sicc->deactivate = xics_spapr_deactivate; sicc->cpu_intc_create = xics_spapr_cpu_intc_create; @@ -462,6 +473,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data) static const TypeInfo ics_spapr_info = { .name = TYPE_ICS_SPAPR, .parent = TYPE_ICS, + .instance_size = sizeof(IcsSpaprState), .class_init = ics_spapr_class_init, .interfaces = (InterfaceInfo[]) { { TYPE_SPAPR_INTC }, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 12a012d9dd09..21de0456446b 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4218,15 +4218,16 @@ static void spapr_phb_placement(SpaprMachineState *spapr, uint32_t index, static ICSState *spapr_ics_get(XICSFabric *dev, int irq) { SpaprMachineState *spapr = SPAPR_MACHINE(dev); + ICSState *ics = ICS(spapr->ics); - return ics_valid_irq(spapr->ics, irq) ? spapr->ics : NULL; + return ics_valid_irq(ics, irq) ? ics : NULL; } static void spapr_ics_resend(XICSFabric *dev) { SpaprMachineState *spapr = SPAPR_MACHINE(dev); - ics_resend(spapr->ics); + ics_resend(ICS(spapr->ics)); } static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 2dacbecfd5fd..be6f4041e433 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -316,6 +316,9 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr), &error_abort); object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort); + object_property_set_uint(obj, "nr-servers", + spapr_max_server_number(spapr), + &error_abort); if (!qdev_realize(DEVICE(obj), NULL, errp)) { return; } @@ -426,7 +429,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq) assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE)); if (spapr->ics) { - assert(ics_valid_irq(spapr->ics, irq)); + assert(ics_valid_irq(ICS(spapr->ics), irq)); } if (spapr->xive) { assert(irq < spapr->xive->nr_irqs); @@ -556,7 +559,7 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) { - ICSState *ics = spapr->ics; + ICSState *ics = ICS(spapr->ics); int first = -1; assert(ics);
The sPAPR ICS device exposes the range of vCPU ids it can handle in the "ibm,interrupt-server-ranges" FDT property. The highest vCPU id, ie. spapr_max_server_number(), is obtained from the machine through the "nr_servers" argument of the generic spapr_irq_dt() call. We want to drop the "nr_servers" argument from the API because it doesn't make sense for the sPAPR XIVE device actually. On POWER9, we also pass the highest vCPU id to the KVM XICS-on-XIVE device, in order to optimize resource allocation in the HW. This is enough motivation to introduce an "nr-servers" property and to use it for both purposes. Signed-off-by: Greg Kurz <groug@kaod.org> --- include/hw/ppc/spapr.h | 4 ++-- include/hw/ppc/xics_spapr.h | 21 +++++++++++++++++--- hw/intc/xics_kvm.c | 2 +- hw/intc/xics_spapr.c | 38 ++++++++++++++++++++++++------------- hw/ppc/spapr.c | 5 +++-- hw/ppc/spapr_irq.c | 7 +++++-- 6 files changed, 54 insertions(+), 23 deletions(-)