Re: [PATCH for-6.0 7/8] spapr: Drop "nr_servers" argument of the sPAPR IC activate() operation

[David Gibson]

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;

-- 
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