Re: [for-5.0 PATCH v2 2/4] spapr: Pass the maximum number of vCPUs to the KVM interrupt controller

[Cédric Le Goater]

On 26/11/2019 17:46, Greg Kurz wrote:
> The XIVE and XICS-on-XIVE KVM devices on POWER9 hosts can greatly reduce
> their consumption of some scarce HW resources, namely Virtual Presenter
> identifiers, if they know the maximum number of vCPUs that may run in the
> VM.
> 
> Prepare ground for this by passing the value down to xics_kvm_connect()
> and kvmppc_xive_connect(). This is purely mechanical, no functional
> change.
> 
> Signed-off-by: Greg Kurz <address@hidden>

Reviewed-by: Cédric Le Goater <address@hidden>

> ---
>  hw/intc/spapr_xive.c        |    6 ++++--
>  hw/intc/spapr_xive_kvm.c    |    3 ++-
>  hw/intc/xics_kvm.c          |    3 ++-
>  hw/intc/xics_spapr.c        |    5 +++--
>  hw/ppc/spapr_irq.c          |    8 +++++---
>  include/hw/ppc/spapr_irq.h  |   10 ++++++++--
>  include/hw/ppc/spapr_xive.h |    3 ++-
>  include/hw/ppc/xics_spapr.h |    3 ++-
>  8 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 729246e906c9..8364f61c786c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -651,12 +651,14 @@ 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, Error **errp)
> +static int spapr_xive_activate(SpaprInterruptController *intc,
> +                               uint32_t nr_servers, Error **errp)
>  {
>      SpaprXive *xive = SPAPR_XIVE(intc);
>  
>      if (kvm_enabled()) {
> -        int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, errp);
> +        int rc = spapr_irq_init_kvm(kvmppc_xive_connect, intc, nr_servers,
> +                                    errp);
>          if (rc < 0) {
>              return rc;
>          }
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 69e73552f1ef..46c7609bd8d0 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -728,7 +728,8 @@ static void *kvmppc_xive_mmap(SpaprXive *xive, int pgoff, 
> size_t len,
>   * All the XIVE memory regions are now backed by mappings from the KVM
>   * XIVE device.
>   */
> -int kvmppc_xive_connect(SpaprInterruptController *intc, Error **errp)
> +int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers,
> +                        Error **errp)
>  {
>      SpaprXive *xive = SPAPR_XIVE(intc);
>      XiveSource *xsrc = &xive->source;
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 954c424b3652..a1f1b7b0d37c 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -342,7 +342,8 @@ void ics_kvm_set_irq(ICSState *ics, int srcno, int val)
>      }
>  }
>  
> -int xics_kvm_connect(SpaprInterruptController *intc, Error **errp)
> +int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers,
> +                     Error **errp)
>  {
>      ICSState *ics = ICS_SPAPR(intc);
>      int rc;
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index b3705dab0e8a..8ae4f41459c3 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -422,10 +422,11 @@ static int 
> xics_spapr_post_load(SpaprInterruptController *intc, int version_id)
>      return 0;
>  }
>  
> -static int xics_spapr_activate(SpaprInterruptController *intc, Error **errp)
> +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, errp);
> +        return spapr_irq_init_kvm(xics_kvm_connect, intc, nr_servers, errp);
>      }
>      return 0;
>  }
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d4a54afc8662..07e08d6544a0 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -70,15 +70,16 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int 
> irq, uint32_t num)
>      bitmap_clear(spapr->irq_map, irq - SPAPR_IRQ_MSI, num);
>  }
>  
> -int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **),
> +int spapr_irq_init_kvm(SpaprInterruptControllerInitKvm fn,
>                         SpaprInterruptController *intc,
> +                       uint32_t nr_servers,
>                         Error **errp)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
>      Error *local_err = NULL;
>  
>      if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> -        if (fn(intc, &local_err) < 0) {
> +        if (fn(intc, nr_servers, &local_err) < 0) {
>              if (machine_kernel_irqchip_required(machine)) {
>                  error_prepend(&local_err,
>                                "kernel_irqchip requested but unavailable: ");
> @@ -481,6 +482,7 @@ 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 +500,7 @@ static void set_active_intc(SpaprMachineState *spapr,
>  
>      sicc = SPAPR_INTC_GET_CLASS(new_intc);
>      if (sicc->activate) {
> -        sicc->activate(new_intc, &error_fatal);
> +        sicc->activate(new_intc, nr_servers, &error_fatal);
>      }
>  
>      spapr->active_intc = new_intc;
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index ff814d13de37..ca8cb4421374 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -43,7 +43,8 @@ typedef struct SpaprInterruptController 
> SpaprInterruptController;
>  typedef struct SpaprInterruptControllerClass {
>      InterfaceClass parent;
>  
> -    int (*activate)(SpaprInterruptController *intc, Error **errp);
> +    int (*activate)(SpaprInterruptController *intc, uint32_t nr_servers,
> +                    Error **errp);
>      void (*deactivate)(SpaprInterruptController *intc);
>  
>      /*
> @@ -98,8 +99,13 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq);
>  int spapr_irq_post_load(SpaprMachineState *spapr, int version_id);
>  void spapr_irq_reset(SpaprMachineState *spapr, Error **errp);
>  int spapr_irq_get_phandle(SpaprMachineState *spapr, void *fdt, Error **errp);
> -int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **),
> +
> +typedef int (*SpaprInterruptControllerInitKvm)(SpaprInterruptController *,
> +                                               uint32_t, Error **);
> +
> +int spapr_irq_init_kvm(SpaprInterruptControllerInitKvm fn,
>                         SpaprInterruptController *intc,
> +                       uint32_t nr_servers,
>                         Error **errp);
>  
>  /*
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 742b7e834f2a..3a103c224d44 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -66,7 +66,8 @@ int spapr_xive_end_to_target(uint8_t end_blk, uint32_t 
> end_idx,
>  /*
>   * KVM XIVE device helpers
>   */
> -int kvmppc_xive_connect(SpaprInterruptController *intc, Error **errp);
> +int kvmppc_xive_connect(SpaprInterruptController *intc, uint32_t nr_servers,
> +                        Error **errp);
>  void kvmppc_xive_disconnect(SpaprInterruptController *intc);
>  void kvmppc_xive_reset(SpaprXive *xive, Error **errp);
>  void kvmppc_xive_set_source_config(SpaprXive *xive, uint32_t lisn, XiveEAS 
> *eas,
> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
> index 28b87038c89a..1c65c96e3c7a 100644
> --- a/include/hw/ppc/xics_spapr.h
> +++ b/include/hw/ppc/xics_spapr.h
> @@ -32,7 +32,8 @@
>  #define TYPE_ICS_SPAPR "ics-spapr"
>  #define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR)
>  
> -int xics_kvm_connect(SpaprInterruptController *intc, Error **errp);
> +int xics_kvm_connect(SpaprInterruptController *intc, uint32_t nr_servers,
> +                     Error **errp);
>  void xics_kvm_disconnect(SpaprInterruptController *intc);
>  bool xics_kvm_has_broken_disconnect(SpaprMachineState *spapr);
>  
>