| Message ID | 1466601669-25398-6-git-send-email-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@arm.com> It looks good to me. CC'ing Konrad which is more knowledgeable than me about ALTERNATIVE. > 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 >
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@arm.com> > > 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 >> >
On Wed, Jul 20, 2016 at 8:43 AM, Julien Grall <julien.grall@arm.com> wrote: > Hi Konrad, > >>> 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: 42, eh? >>> >>> CHECK_WORKAROUND_HELPER(424242, ARM64_WORKAROUND_42424242, CONFIG_ARM64) >>> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >> >> >> It looks good to me. CC'ing Konrad which is more knowledgeable than me >> about ALTERNATIVE. > > > Do you have any opinions on this patch? Yes I do: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
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:
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@arm.com> --- xen/include/asm-arm/cpuerrata.h | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)