Re: [PATCH 1/2] xen/*/nospec: Provide common versions of evaluate_nospec/block_speculation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.03.2024 17:10, Andrew Cooper wrote:
> --- a/xen/arch/x86/include/asm/nospec.h
> +++ b/xen/arch/x86/include/asm/nospec.h
> @@ -23,20 +23,20 @@ static always_inline bool barrier_nospec_false(void)
>      return false;
>  }
>  
> -/* Allow to protect evaluation of conditionals with respect to speculation */
> -static always_inline bool evaluate_nospec(bool condition)
> +static always_inline bool arch_evaluate_nospec(bool condition)
>  {
>      if ( condition )
>          return barrier_nospec_true();
>      else
>          return barrier_nospec_false();
>  }
> +#define arch_evaluate_nospec arch_evaluate_nospec
>  
> -/* Allow to block speculative execution in generic code */
> -static always_inline void block_speculation(void)
> +static always_inline void arch_block_speculation(void)
>  {
>      barrier_nospec_true();
>  }
> +#define arch_block_speculation arch_block_speculation

I'm having some difficulty seeing the need for the renaming (adding
or the arch_ prefixes): Why can't the original names be kept, and
...

> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -9,6 +9,29 @@
>  
>  #include <asm/nospec.h>
>  
> +/*
> + * Protect a conditional branch from bad speculation.  Architectures *must*
> + * provide arch_evaluate_nospec() for this to be effective.
> + */
> +static always_inline bool evaluate_nospec(bool cond)
> +{
> +#ifndef arch_evaluate_nospec
> +#define arch_evaluate_nospec(cond) cond
> +#endif
> +    return arch_evaluate_nospec(cond);
> +}
> +
> +/*
> + * Halt speculation unconditonally.  Architectures *must* provide
> + * arch_block_speculation() for this to be effective.
> + */
> +static always_inline void block_speculation(void)
> +{
> +#ifdef arch_block_speculation
> +    arch_block_speculation();
> +#endif
> +}

... stubs be introduced here if the original names aren't #define-d?
IOW what does the extra layer gain us?

Jan