Re: [PATCH v3 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot

[Julien Grall <julien.grall@arm.com>]

Hi Mirela,

On 27/04/18 18:12, Mirela Simonovic wrote:
> On boot, enabling errata workarounds will be triggered by the boot CPU
> from start_xen(). On CPU hotplug (non-boot scenario) this would not be
> done. This patch adds the code required to enable errata workarounds
> for a CPU being hotplugged after the system boots. This is triggered
> using a notifier. If the CPU fails to enable the errata Xen will panic.
> This is done because it is assumed that the CPU which is hotplugged
> after the system/Xen boots, was initially hotplugged during the
> system/Xen boot. Therefore, enabling errata workarounds should never
> fail.
> 
> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
> 
> ---
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> ---
>   xen/arch/arm/cpuerrata.c         | 35 +++++++++++++++++++++++++++++++++++
>   xen/arch/arm/cpufeature.c        | 23 +++++++++++++++++++++++
>   xen/include/asm-arm/cpufeature.h |  1 +
>   3 files changed, 59 insertions(+)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 1baa20654b..4040f781ec 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -5,6 +5,8 @@
>   #include <xen/spinlock.h>
>   #include <xen/vmap.h>
>   #include <xen/warning.h>
> +#include <xen/notifier.h>
> +#include <xen/cpu.h>
>   #include <asm/cpufeature.h>
>   #include <asm/cpuerrata.h>
>   #include <asm/psci.h>
> @@ -349,6 +351,39 @@ void __init enable_errata_workarounds(void)
>       enable_cpu_capabilities(arm_errata);
>   }
>   
> +static int cpu_errata_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    switch ( action )
> +    {
> +    case CPU_STARTING:
> +        enable_nonboot_cpu_caps(arm_errata);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_errata_nfb = {
> +    .notifier_call = cpu_errata_callback,
> +};
> +
> +static int __init cpu_errata_notifier_init(void)
> +{
> +    register_cpu_notifier(&cpu_errata_nfb);
> +    return 0;
> +}
> +/*
> + * Initialization has to be done at init rather than presmp_init phase because
> + * the callback should execute only after the secondary CPUs are initially
> + * booted (in hotplug scenarios when the system state is not boot). On boot,
> + * the enabling of errata workarounds will be triggered by the boot CPU from
> + * start_xen().
> + */
> +__initcall(cpu_errata_notifier_init);
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 525b45e22f..dd30f0d29c 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -68,6 +68,29 @@ void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
>       }
>   }
>   
> +/* Run through the enabled capabilities and enable() them on the calling CPU */
> +void enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
> +{
> +    ASSERT(system_state != SYS_STATE_boot);
> +
> +    for ( ; caps->matches; caps++ )
> +    {
> +        if ( !cpus_have_cap(caps->capability) )
> +            continue;
> +
> +        if ( caps->enable )
> +        {
> +            /*
> +             * Since the CPU has enabled errata workarounds on boot, it should

This function is not really about errata, it is about capabilities. 
Errata is just a sub-category of them.

> +             * never fail to enable them here.
> +             */
> +            if ( caps->enable((void *)caps) )
> +                panic("CPU%u failed to enable capability %u\n",
> +                      smp_processor_id(), caps->capability);

We should really avoid to use panic(...) if this is something the system 
can survive. In that specific case, it would only affect the current 
CPU. So it would be better to return an error and let the caller decide 
what to do.

Cheers,

> +        }
> +    }
> +}
> +
>   /*
>    * Local variables:
>    * mode: C
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index e557a095af..b14e226401 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -88,6 +88,7 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>                                const char *info);
>   
>   void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
> +void enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps);
>   
>   #endif /* __ASSEMBLY__ */
>   
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel