Re: [Xen-devel] [PATCH 5/9] xen/arm: Provide macros to help creating workaround helpers

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 14/07/16 14:57, Stefano Stabellini wrote:
> On Wed, 22 Jun 2016, Julien Grall wrote:
> > Workarounds may require to execute a different path when the platform
> > is affected by the associated erratum. Furthermore, this may need to
> > be called in the common code.
> >
> > To avoid too much intrusion/overhead, the workaround helpers need to
> > be a nop on architecture which will never have the workaround and have
> > to be quick to check whether the platform requires it.
> >
> > The alternative framework is used to transform the check in a single
> > instruction. When the framework is not available, the helper will have
> > ~6 instructions including 1 instruction load.
> >
> > The macro will create a handler called check_workaround_xxxxx with
> > xxxx the erratum number.
> >
> > For instance, the line bellow will create a workaround helper for
> > erratum #424242 which is enabled when the capability
> > ARM64_WORKAROUND_424242 is set and only available for ARM64:
> >
> > CHECK_WORKAROUND_HELPER(424242, ARM64_WORKAROUND_42424242, CONFIG_ARM64)
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> It looks good to me. CC'ing Konrad which is more knowledgeable than me
> about ALTERNATIVE.
Do you have any opinions on this patch?

Cheers,

> >  xen/include/asm-arm/cpuerrata.h | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> > index fe93beb..b9d8dfc 100644
> > --- a/xen/include/asm-arm/cpuerrata.h
> > +++ b/xen/include/asm-arm/cpuerrata.h
> > @@ -1,8 +1,47 @@
> >  #ifndef __ARM_CPUERRATA_H
> >  #define __ARM_CPUERRATA_H
> >
> > +#include <xen/config.h>
> > +#include <asm/cpufeature.h>
> > +#include <asm/alternative.h>
> > +
> >  void check_local_cpu_errata(void);
> >
> > +#ifdef CONFIG_ALTERNATIVE
> > +
> > +#define CHECK_WORKAROUND_HELPER(erratum, feature, arch)         \
> > +static inline bool_t check_workaround_##erratum(void)           \
> > +{                                                               \
> > +    if ( !IS_ENABLED(arch) )                                    \
> > +        return 0;                                               \
> > +    else                                                        \
> > +    {                                                           \
> > +        bool_t ret;                                             \
> > +                                                                \
> > +        asm volatile (ALTERNATIVE("mov %0, #0",                 \
> > +                                  "mov %0, #1",                 \
> > +                                  feature)                      \
> > +                      : "=r" (ret));                            \
> > +                                                                \
> > +        return unlikely(ret);                                   \
> > +    }                                                           \
> > +}
> > +
> > +#else /* CONFIG_ALTERNATIVE */
> > +
> > +#define CHECK_WORKAROUND_HELPER(erratum, feature, arch)         \
> > +static inline bool_t check_workaround_##erratum(void)           \
> > +{                                                               \
> > +    if ( !IS_ENABLED(arch) )                                    \
> > +        return 0;                                               \
> > +    else                                                        \
> > +        return unlikely(cpus_have_cap(feature));                \
> > +}
> > +
> > +#endif
> > +
> > +#undef CHECK_WORKAROUND_HELPER
> > +
> >  #endif /* __ARM_CPUERRATA_H */
> >  /*
> >   * Local variables:
> > --
> > 1.9.1
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel