Re: [PATCH v3 31/34] spapr, xics, xive: Move SpaprIrq::post_load hook to backends

From: Greg Kurz <groug@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: "Jason Wang" <jasowang@redhat.com>,
	"Riku Voipio" <riku.voipio@iki.fi>,
	qemu-devel@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>,
	qemu-ppc@nongnu.org, clg@kaod.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	philmd@redhat.com
Subject: Re: [PATCH v3 31/34] spapr, xics, xive: Move SpaprIrq::post_load hook to backends
Date: Wed, 2 Oct 2019 11:50:50 +0200	[thread overview]
Message-ID: <20191002115050.052cb2fe@bahia.lan> (raw)
In-Reply-To: <20191002025208.3487-32-david@gibson.dropbear.id.au>

On Wed,  2 Oct 2019 12:52:05 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The remaining logic in the post_load hook really belongs to the interrupt
> controller backends, and just needs to be called on the active controller
> (after the active controller is set to the right thing based on the
> incoming migration in the generic spapr_irq_post_load() logic).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/intc/spapr_xive.c        |  5 +++--
>  hw/intc/xics_spapr.c        | 13 +++++++++++
>  hw/ppc/spapr_irq.c          | 45 ++++---------------------------------
>  include/hw/ppc/spapr_irq.h  |  3 +--
>  include/hw/ppc/spapr_xive.h |  1 -
>  5 files changed, 21 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index e8b946982c..ab68e6eaf6 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -462,10 +462,10 @@ static int vmstate_spapr_xive_pre_save(void *opaque)
>   * Called by the sPAPR IRQ backend 'post_load' method at the machine
>   * level.
>   */
> -int spapr_xive_post_load(SpaprXive *xive, int version_id)
> +static int spapr_xive_post_load(SpaprInterruptController *intc, int version_id)
>  {
>      if (kvm_irqchip_in_kernel()) {
> -        return kvmppc_xive_post_load(xive, version_id);
> +        return kvmppc_xive_post_load(SPAPR_XIVE(intc), version_id);
>      }
>  
>      return 0;
> @@ -699,6 +699,7 @@ static void spapr_xive_class_init(ObjectClass *klass, void *data)
>      sicc->set_irq = spapr_xive_set_irq;
>      sicc->print_info = spapr_xive_print_info;
>      sicc->dt = spapr_xive_dt;
> +    sicc->post_load = spapr_xive_post_load;
>  }
>  
>  static const TypeInfo spapr_xive_info = {
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 8abbc799ba..9590eedc3d 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -395,6 +395,18 @@ static void xics_spapr_print_info(SpaprInterruptController *intc, Monitor *mon)
>      ics_pic_print_info(ics, mon);
>  }
>  
> +static int xics_spapr_post_load(SpaprInterruptController *intc, int version_id)
> +{
> +    if (!kvm_irqchip_in_kernel()) {
> +        CPUState *cs;
> +        CPU_FOREACH(cs) {
> +            PowerPCCPU *cpu = POWERPC_CPU(cs);
> +            icp_resend(spapr_cpu_state(cpu)->icp);
> +        }
> +    }
> +    return 0;
> +}
> +
>  static void xics_spapr_activate(SpaprInterruptController *intc, Error **errp)
>  {
>      if (kvm_enabled()) {
> @@ -425,6 +437,7 @@ static void ics_spapr_class_init(ObjectClass *klass, void *data)
>      sicc->set_irq = xics_spapr_set_irq;
>      sicc->print_info = xics_spapr_print_info;
>      sicc->dt = xics_spapr_dt;
> +    sicc->post_load = xics_spapr_post_load;
>  }
>  
>  static const TypeInfo ics_spapr_info = {
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index f70b331f44..f3d18b1dad 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -100,43 +100,22 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **),
>   * XICS IRQ backend.
>   */
>  
> -static int spapr_irq_post_load_xics(SpaprMachineState *spapr, int version_id)
> -{
> -    if (!kvm_irqchip_in_kernel()) {
> -        CPUState *cs;
> -        CPU_FOREACH(cs) {
> -            PowerPCCPU *cpu = POWERPC_CPU(cs);
> -            icp_resend(spapr_cpu_state(cpu)->icp);
> -        }
> -    }
> -    return 0;
> -}
> -
>  SpaprIrq spapr_irq_xics = {
>      .nr_xirqs    = SPAPR_NR_XIRQS,
>      .nr_msis     = SPAPR_NR_MSIS,
>      .xics        = true,
>      .xive        = false,
> -
> -    .post_load   = spapr_irq_post_load_xics,
>  };
>  
>  /*
>   * XIVE IRQ backend.
>   */
>  
> -static int spapr_irq_post_load_xive(SpaprMachineState *spapr, int version_id)
> -{
> -    return spapr_xive_post_load(spapr->xive, version_id);
> -}
> -
>  SpaprIrq spapr_irq_xive = {
>      .nr_xirqs    = SPAPR_NR_XIRQS,
>      .nr_msis     = SPAPR_NR_MSIS,
>      .xics        = false,
>      .xive        = true,
> -
> -    .post_load   = spapr_irq_post_load_xive,
>  };
>  
>  /*
> @@ -148,21 +127,6 @@ SpaprIrq spapr_irq_xive = {
>   * activated after an extra machine reset.
>   */
>  
> -/*
> - * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
> - * default.
> - */
> -static SpaprIrq *spapr_irq_current(SpaprMachineState *spapr)
> -{
> -    return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
> -        &spapr_irq_xive : &spapr_irq_xics;
> -}
> -
> -static int spapr_irq_post_load_dual(SpaprMachineState *spapr, int version_id)
> -{
> -    return spapr_irq_current(spapr)->post_load(spapr, version_id);
> -}
> -
>  /*
>   * Define values in sync with the XIVE and XICS backend
>   */
> @@ -171,8 +135,6 @@ SpaprIrq spapr_irq_dual = {
>      .nr_msis     = SPAPR_NR_MSIS,
>      .xics        = true,
>      .xive        = true,
> -
> -    .post_load   = spapr_irq_post_load_dual,
>  };
>  
>  
> @@ -447,8 +409,11 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>  
>  int spapr_irq_post_load(SpaprMachineState *spapr, int version_id)
>  {
> +    SpaprInterruptControllerClass *sicc;
> +
>      spapr_irq_update_active_intc(spapr);
> -    return spapr->irq->post_load(spapr, version_id);
> +    sicc = SPAPR_INTC_GET_CLASS(spapr->active_intc);
> +    return sicc->post_load(spapr->active_intc, version_id);
>  }
>  
>  void spapr_irq_reset(SpaprMachineState *spapr, Error **errp)
> @@ -589,8 +554,6 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .nr_msis     = SPAPR_IRQ_XICS_LEGACY_NR_XIRQS,
>      .xics        = true,
>      .xive        = false,
> -
> -    .post_load   = spapr_irq_post_load_xics,
>  };
>  
>  static void spapr_irq_register_types(void)
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 1aff1c2eb7..4c2cd091da 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -62,6 +62,7 @@ typedef struct SpaprInterruptControllerClass {
>      void (*print_info)(SpaprInterruptController *intc, Monitor *mon);
>      void (*dt)(SpaprInterruptController *intc, uint32_t nr_servers,
>                 void *fdt, uint32_t phandle);
> +    int (*post_load)(SpaprInterruptController *intc, int version_id);
>  } SpaprInterruptControllerClass;
>  
>  void spapr_irq_update_active_intc(SpaprMachineState *spapr);
> @@ -82,8 +83,6 @@ typedef struct SpaprIrq {
>      uint32_t    nr_msis;
>      bool        xics;
>      bool        xive;
> -
> -    int (*post_load)(SpaprMachineState *spapr, int version_id);
>  } SpaprIrq;
>  
>  extern SpaprIrq spapr_irq_xics;
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 64972754f9..d84bd5c229 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -55,7 +55,6 @@ typedef struct SpaprXive {
>  #define SPAPR_XIVE_BLOCK_ID 0x0
>  
>  void spapr_xive_pic_print_info(SpaprXive *xive, Monitor *mon);
> -int spapr_xive_post_load(SpaprXive *xive, int version_id);
>  
>  void spapr_xive_hcall_init(SpaprMachineState *spapr);
>  void spapr_xive_set_tctx_os_cam(XiveTCTX *tctx);