| Message ID | 20201120174646.619395-8-groug@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: Address the confusion between IPI numbers and vCPU ids | expand |
On Fri, Nov 20, 2020 at 06:46:45PM +0100, Greg Kurz wrote: > This argument isn't used by the backends anymore. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > include/hw/ppc/spapr_irq.h | 3 +-- > hw/intc/spapr_xive.c | 3 +-- > hw/intc/xics_spapr.c | 3 +-- > hw/ppc/spapr_irq.c | 3 +-- > 4 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index c22a72c9e270..3e1c619d4c06 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -43,8 +43,7 @@ DECLARE_CLASS_CHECKERS(SpaprInterruptControllerClass, SPAPR_INTC, > struct SpaprInterruptControllerClass { > InterfaceClass parent; > > - int (*activate)(SpaprInterruptController *intc, uint32_t nr_servers, > - Error **errp); Hm. Thinking back on this, is the problem just that it's not clear here if the 'nr_servers' parameter here indicates the number of CPU targets, or the maximum index of CPU targets? If so, I wonder if it might be better to pass both numbers as parameters here, rather than shuffling the properties of the devices themselves. > + int (*activate)(SpaprInterruptController *intc, Error **errp); > void (*deactivate)(SpaprInterruptController *intc); > > /* > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > index d13a2955ce9b..8331026fdb12 100644 > --- a/hw/intc/spapr_xive.c > +++ b/hw/intc/spapr_xive.c > @@ -754,8 +754,7 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, > plat_res_int_priorities, sizeof(plat_res_int_priorities))); > } > > -static int spapr_xive_activate(SpaprInterruptController *intc, > - uint32_t nr_servers, Error **errp) > +static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp) > { > SpaprXive *xive = SPAPR_XIVE(intc); > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index ce147e8980ed..8810bd93c856 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -426,8 +426,7 @@ static int xics_spapr_post_load(SpaprInterruptController *intc, int version_id) > return 0; > } > > -static int xics_spapr_activate(SpaprInterruptController *intc, > - uint32_t nr_servers, Error **errp) > +static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp) > { > if (kvm_enabled()) { > return spapr_irq_init_kvm(xics_kvm_connect, intc, > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index be6f4041e433..f2897fbc942a 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -480,7 +480,6 @@ static void set_active_intc(SpaprMachineState *spapr, > SpaprInterruptController *new_intc) > { > SpaprInterruptControllerClass *sicc; > - uint32_t nr_servers = spapr_max_server_number(spapr); > > assert(new_intc); > > @@ -498,7 +497,7 @@ static void set_active_intc(SpaprMachineState *spapr, > > sicc = SPAPR_INTC_GET_CLASS(new_intc); > if (sicc->activate) { > - sicc->activate(new_intc, nr_servers, &error_fatal); > + sicc->activate(new_intc, &error_fatal); > } > > spapr->active_intc = new_intc;
On Mon, 23 Nov 2020 15:38:32 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Fri, Nov 20, 2020 at 06:46:45PM +0100, Greg Kurz wrote: > > This argument isn't used by the backends anymore. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > include/hw/ppc/spapr_irq.h | 3 +-- > > hw/intc/spapr_xive.c | 3 +-- > > hw/intc/xics_spapr.c | 3 +-- > > hw/ppc/spapr_irq.c | 3 +-- > > 4 files changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > > index c22a72c9e270..3e1c619d4c06 100644 > > --- a/include/hw/ppc/spapr_irq.h > > +++ b/include/hw/ppc/spapr_irq.h > > @@ -43,8 +43,7 @@ DECLARE_CLASS_CHECKERS(SpaprInterruptControllerClass, SPAPR_INTC, > > struct SpaprInterruptControllerClass { > > InterfaceClass parent; > > > > - int (*activate)(SpaprInterruptController *intc, uint32_t nr_servers, > > - Error **errp); > > Hm. Thinking back on this, is the problem just that it's not clear > here if the 'nr_servers' parameter here indicates the number of CPU > targets, or the maximum index of CPU targets? > AIUI 'nr_servers' has always been referring to the number of consecutive server ids that we put in the "ibm,interrupt-server-ranges" property for XICS. > If so, I wonder if it might be better to pass both numbers as > parameters here, rather than shuffling the properties of the devices > themselves. > Maybe. I'll give a try. > > + int (*activate)(SpaprInterruptController *intc, Error **errp); > > void (*deactivate)(SpaprInterruptController *intc); > > > > /* > > diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > > index d13a2955ce9b..8331026fdb12 100644 > > --- a/hw/intc/spapr_xive.c > > +++ b/hw/intc/spapr_xive.c > > @@ -754,8 +754,7 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, > > plat_res_int_priorities, sizeof(plat_res_int_priorities))); > > } > > > > -static int spapr_xive_activate(SpaprInterruptController *intc, > > - uint32_t nr_servers, Error **errp) > > +static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp) > > { > > SpaprXive *xive = SPAPR_XIVE(intc); > > > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > > index ce147e8980ed..8810bd93c856 100644 > > --- a/hw/intc/xics_spapr.c > > +++ b/hw/intc/xics_spapr.c > > @@ -426,8 +426,7 @@ static int xics_spapr_post_load(SpaprInterruptController *intc, int version_id) > > return 0; > > } > > > > -static int xics_spapr_activate(SpaprInterruptController *intc, > > - uint32_t nr_servers, Error **errp) > > +static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp) > > { > > if (kvm_enabled()) { > > return spapr_irq_init_kvm(xics_kvm_connect, intc, > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index be6f4041e433..f2897fbc942a 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -480,7 +480,6 @@ static void set_active_intc(SpaprMachineState *spapr, > > SpaprInterruptController *new_intc) > > { > > SpaprInterruptControllerClass *sicc; > > - uint32_t nr_servers = spapr_max_server_number(spapr); > > > > assert(new_intc); > > > > @@ -498,7 +497,7 @@ static void set_active_intc(SpaprMachineState *spapr, > > > > sicc = SPAPR_INTC_GET_CLASS(new_intc); > > if (sicc->activate) { > > - sicc->activate(new_intc, nr_servers, &error_fatal); > > + sicc->activate(new_intc, &error_fatal); > > } > > > > spapr->active_intc = new_intc; >
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index c22a72c9e270..3e1c619d4c06 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -43,8 +43,7 @@ DECLARE_CLASS_CHECKERS(SpaprInterruptControllerClass, SPAPR_INTC, struct SpaprInterruptControllerClass { InterfaceClass parent; - int (*activate)(SpaprInterruptController *intc, uint32_t nr_servers, - Error **errp); + int (*activate)(SpaprInterruptController *intc, Error **errp); void (*deactivate)(SpaprInterruptController *intc); /* diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c index d13a2955ce9b..8331026fdb12 100644 --- a/hw/intc/spapr_xive.c +++ b/hw/intc/spapr_xive.c @@ -754,8 +754,7 @@ static void spapr_xive_dt(SpaprInterruptController *intc, uint32_t nr_servers, plat_res_int_priorities, sizeof(plat_res_int_priorities))); } -static int spapr_xive_activate(SpaprInterruptController *intc, - uint32_t nr_servers, Error **errp) +static int spapr_xive_activate(SpaprInterruptController *intc, Error **errp) { SpaprXive *xive = SPAPR_XIVE(intc); diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index ce147e8980ed..8810bd93c856 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -426,8 +426,7 @@ static int xics_spapr_post_load(SpaprInterruptController *intc, int version_id) return 0; } -static int xics_spapr_activate(SpaprInterruptController *intc, - uint32_t nr_servers, Error **errp) +static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp) { if (kvm_enabled()) { return spapr_irq_init_kvm(xics_kvm_connect, intc, diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index be6f4041e433..f2897fbc942a 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -480,7 +480,6 @@ static void set_active_intc(SpaprMachineState *spapr, SpaprInterruptController *new_intc) { SpaprInterruptControllerClass *sicc; - uint32_t nr_servers = spapr_max_server_number(spapr); assert(new_intc); @@ -498,7 +497,7 @@ static void set_active_intc(SpaprMachineState *spapr, sicc = SPAPR_INTC_GET_CLASS(new_intc); if (sicc->activate) { - sicc->activate(new_intc, nr_servers, &error_fatal); + sicc->activate(new_intc, &error_fatal); } spapr->active_intc = new_intc;
This argument isn't used by the backends anymore. Signed-off-by: Greg Kurz <groug@kaod.org> --- include/hw/ppc/spapr_irq.h | 3 +-- hw/intc/spapr_xive.c | 3 +-- hw/intc/xics_spapr.c | 3 +-- hw/ppc/spapr_irq.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-)