Re: [Xen-devel] [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Stefano,

On 16/01/18 23:55, Stefano Stabellini wrote:
> On Tue, 16 Jan 2018, Julien Grall wrote:
> > Once Xen knows what features/workarounds present on the platform, it
> > might be necessary to configure each online CPU.
> >
> > Introduce a new callback "enable" that will be called on each online CPU to
> > configure the "capability".
> >
> > The code is based on Linux v4.14 (where cpufeature.c comes from), the
> > explanation of why using stop_machine_run is kept as we have similar
> > problem in the future.
> >
> > Lastly introduce enable_errata_workaround that will be called once CPUs
> > have booted and before the hardware domain is created.
> >
> > This is part of XSA-254.
> >
> > Signed-of-by: Julien Grall <julien.grall@xxxxxxxxxx>
> If you took the code from Linux, you need to add the original
> Signed-off-by from the Linux commit. Aside from that:
There are multiple commits touching this function. So I followed what we 
did in similar situation. By that I mean, mentioning the code was taken 
from Linux and not gathered the signed-off-by.
If you really want, I can gather all the signed-off-by of the commit 
touching this function.
Cheers,

> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>
>
> > ---
> >   xen/arch/arm/cpuerrata.c         |  6 ++++++
> >   xen/arch/arm/cpufeature.c        | 29 +++++++++++++++++++++++++++++
> >   xen/arch/arm/setup.c             |  1 +
> >   xen/include/asm-arm/cpuerrata.h  |  1 +
> >   xen/include/asm-arm/cpufeature.h |  3 +++
> >   5 files changed, 40 insertions(+)
> >
> > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> > index fe9e9facbe..772587c05a 100644
> > --- a/xen/arch/arm/cpuerrata.c
> > +++ b/xen/arch/arm/cpuerrata.c
> > @@ -64,6 +64,12 @@ void check_local_cpu_errata(void)
> >   {
> >       update_cpu_capabilities(arm_errata, "enabled workaround for");
> >   }
> > +
> > +void __init enable_errata_workarounds(void)
> > +{
> > +    enable_cpu_capabilities(arm_errata);
> > +}
> > +
> >   /*
> >    * Local variables:
> >    * mode: C
> > diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> > index 479c9fb011..525b45e22f 100644
> > --- a/xen/arch/arm/cpufeature.c
> > +++ b/xen/arch/arm/cpufeature.c
> > @@ -19,6 +19,7 @@
> >   #include <xen/types.h>
> >   #include <xen/init.h>
> >   #include <xen/smp.h>
> > +#include <xen/stop_machine.h>
> >   #include <asm/cpufeature.h>
> >   
> >   DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> > @@ -40,6 +41,34 @@ void update_cpu_capabilities(const struct 
> > arm_cpu_capabilities *caps,
> >   }
> >   
> >   /*
> > + * Run through the enabled capabilities and enable() it on all active
> > + * CPUs.
> > + */
> > +void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
> > +{
> > +    for ( ; caps->matches; caps++ )
> > +    {
> > +        if ( !cpus_have_cap(caps->capability) )
> > +            continue;
> > +
> > +        if ( caps->enable )
> > +        {
> > +            int ret;
> > +
> > +            /*
> > +             * Use stop_machine_run() as it schedules the work allowing
> > +             * us to modify PSTATE, instead of on_each_cpu() which uses
> > +             * an IPI, giving us a PSTATE that disappears when we
> > +             * return.
> > +             */
> > +            ret = stop_machine_run(caps->enable, (void *)caps, NR_CPUS);
> > +            /* stop_machine_run should never fail at this stage of the boot. */
> > +            BUG_ON(ret);
> > +        }
> > +    }
> > +}
> > +
> > +/*
> >    * Local variables:
> >    * mode: C
> >    * c-file-style: "BSD"
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 16a3b1be8e..032a6a882d 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -849,6 +849,7 @@ void __init start_xen(unsigned long boot_phys_offset,
> >        * stop_machine (tasklets initialized via an initcall).
> >        */
> >       apply_alternatives_all();
> > +    enable_errata_workarounds();
> >   
> >       /* Create initial domain 0. */
> >       /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> > diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> > index 8b158429c7..7de68361ff 100644
> > --- a/xen/include/asm-arm/cpuerrata.h
> > +++ b/xen/include/asm-arm/cpuerrata.h
> > @@ -5,6 +5,7 @@
> >   #include <asm/alternative.h>
> >   
> >   void check_local_cpu_errata(void);
> > +void enable_errata_workarounds(void);
> >   
> >   #ifdef CONFIG_HAS_ALTERNATIVE
> >   
> > diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> > index f00b6dbd39..21c65e198c 100644
> > --- a/xen/include/asm-arm/cpufeature.h
> > +++ b/xen/include/asm-arm/cpufeature.h
> > @@ -74,6 +74,7 @@ struct arm_cpu_capabilities {
> >       const char *desc;
> >       u16 capability;
> >       bool (*matches)(const struct arm_cpu_capabilities *);
> > +    int (*enable)(void *); /* Called on every active CPUs */
> >       union {
> >           struct {    /* To be used for eratum handling only */
> >               u32 midr_model;
> > @@ -85,6 +86,8 @@ struct arm_cpu_capabilities {
> >   void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> >                                const char *info);
> >   
> > +void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
> > +
> >   #endif /* __ASSEMBLY__ */
> >   
> >   #endif
> > --
> > 2.11.0
--
Julien Grall

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