Re: [Qemu-devel] [PATCH v8 09/12] spapr: set the interrupt presenter at reset

From: "Cédric Le Goater" <clg@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [Qemu-devel] [PATCH v8 09/12] spapr: set the interrupt presenter at reset
Date: Thu, 13 Dec 2018 13:52:14 +0100	[thread overview]
Message-ID: <4a9ab4ce-c7d7-9744-e5ea-10df38806e9a@kaod.org> (raw)
In-Reply-To: <20181211223823.13770-10-clg@kaod.org>

On 12/11/18 11:38 PM, Cédric Le Goater wrote:
> Currently, the interrupt presenter of the vCPU is set at realize
> time. Setting it at reset will become necessary when the new machine
> supporting both interrupt modes is introduced. In this machine, the
> interrupt mode is chosen at CAS time and activated after a reset.

I think we need a second '->intc' pointer specific to XIVE instead. 
How about ->intc_xive ?  

We could just drop this patch and easly get rid of the XiveTCTX object 
leak in spapr_unrealize_vcpu().

C. 

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v7:
> 
>  - introduced spapr_irq_reset_xics(). 
> 
>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index 9e2821e4b31f..fc8ea9021656 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU *cpu)
>      return (sPAPRCPUState *)cpu->machine_data;
>  }
>  
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
> +
>  #endif
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 82666436e9b4..afc5d4f4e739 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindIntCArgs {
> +    const char *intc_type;
> +    Object *intc;
> +} ForeachFindIntCArgs;
> +
> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
> +{
> +    ForeachFindIntCArgs *args = opaque;
> +
> +    if (object_dynamic_cast(child, args->intc_type)) {
> +        args->intc = child;
> +    }
> +
> +    return args->intc != NULL;
> +}
> +
> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
> +{
> +    ForeachFindIntCArgs args = { intc_type, NULL };
> +
> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
> +    g_assert(args.intc);
> +
> +    cpu->intc = args.intc;
> +}
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0999a2b2d69c..814500f22d34 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -12,6 +12,7 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "hw/ppc/spapr.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/spapr_xive.h"
>  #include "hw/ppc/xics.h"
>  #include "sysemu/kvm.h"
> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState *spapr, int version_id)
>      return 0;
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +   CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> +    }
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -225,6 +235,7 @@ sPAPRIrq spapr_irq_xics = {
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
> +    .reset       = spapr_irq_reset_xics,
>  };
>  
>  /*
> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>      CPU_FOREACH(cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>  
> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
> +
>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>      }
> 