Re: [Qemu-devel] [PATCH 23/25] spapr: toggle the ICP depending on the selected interrupt mode

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4703 bytes --]

On Thu, Nov 23, 2017 at 02:29:53PM +0100, Cédric Le Goater wrote:
> Each interrupt mode has its own specific interrupt presenter object,
> that we store under the CPU object, one for XICS and one for XIVE. The
> active presenter, corresponding to the current interrupt mode, is
> simply selected with a lookup on the children of the CPU.
> 
> Migration and CPU hotplug also need to reflect the current interrupt
> mode in use.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/spapr.c                  | 21 ++++++++++++++++++++-
>  hw/ppc/spapr_cpu_core.c         | 31 +++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr_cpu_core.h |  1 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a91ec1c0751a..b7389dbdf5ca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1128,8 +1128,10 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>  
>      /* /interrupt controller */
>      if (!spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +        spapr_cpu_core_set_icp(spapr->icp_type);
>          spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
>      } else {
> +        spapr_cpu_core_set_icp(TYPE_SPAPR_XIVE_ICP);

Again you shouldn't have non-DT side-effects from spapr_build_fdt().

>          /* Populate device tree for XIVE */
>          spapr_xive_populate(spapr, xics_max_server_number(), fdt, PHANDLE_XICP);
>          spapr_xive_mmio_map(spapr);
> @@ -1615,6 +1617,7 @@ static int spapr_post_load(void *opaque, int version_id)
>      }
>  
>      if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +        spapr_cpu_core_set_icp(TYPE_SPAPR_XIVE_ICP);
>          spapr_xive_mmio_map(spapr);
>      }
>  
> @@ -3610,7 +3613,7 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int vcpu_id)
>  Object *spapr_icp_create(sPAPRMachineState *spapr, CPUState *cs, Error **errp)
>  {
>      Error *local_err = NULL;
> -    Object *obj;
> +    Object *obj, *obj_xive;
>  
>      obj = icp_create(cs, spapr->icp_type, XICS_FABRIC(spapr), &local_err);
>      if (local_err) {
> @@ -3618,6 +3621,22 @@ Object *spapr_icp_create(sPAPRMachineState *spapr, CPUState *cs, Error **errp)
>          return NULL;
>      }
>  
> +    /* Add a XIVE interrupt presenter. The machine will switch the CPU
> +     * ICP depending on the interrupt model negotiated at CAS time.
> +     */
> +    obj_xive = icp_create(cs, TYPE_SPAPR_XIVE_ICP, XICS_FABRIC(spapr),
> +                          &local_err);

You shouldn't be using icp_create() a xics function, for xive.

> +    if (local_err) {
> +        object_unparent(obj);
> +        error_propagate(errp, local_err);
> +        return NULL;
> +    }
> +
> +    /* when hotplugged, the CPU should have the correct ICP */
> +    if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +        return obj_xive;
> +    }
> +
>      return obj;
>  }
>  
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 61a9850e688b..b0e39270f262 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -257,3 +257,34 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>  };
>  
>  DEFINE_TYPES(spapr_cpu_core_type_infos)
> +
> +typedef struct ForeachFindICPArgs {
> +    const char *icp_type;
> +    Object *icp;
> +} ForeachFindICPArgs;
> +
> +static int spapr_cpu_core_find_icp(Object *child, void *opaque)
> +{
> +    ForeachFindICPArgs *args = opaque;
> +
> +    if (object_dynamic_cast(child, args->icp_type)) {
> +        args->icp = child;
> +    }
> +
> +    return args->icp != NULL;
> +}
> +
> +void spapr_cpu_core_set_icp(const char *icp_type)
> +{
> +    CPUState *cs;
> +
> +    CPU_FOREACH(cs) {
> +        ForeachFindICPArgs args = { icp_type, NULL };
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +        object_child_foreach(OBJECT(cs), spapr_cpu_core_find_icp, &args);
> +        g_assert(args.icp);
> +
> +        cpu->intc = args.icp;
> +    }
> +}
> diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
> index f2d48d6a6786..a657dfb8863c 100644
> --- a/include/hw/ppc/spapr_cpu_core.h
> +++ b/include/hw/ppc/spapr_cpu_core.h
> @@ -38,4 +38,5 @@ typedef struct sPAPRCPUCoreClass {
>  } sPAPRCPUCoreClass;
>  
>  const char *spapr_get_cpu_core_type(const char *cpu_type);
> +void spapr_cpu_core_set_icp(const char *icp_type);
>  #endif

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]