Re: [PATCH v3 1/4] x86/spec: print the built-in SPECULATIVE_HARDEN_* options

2024-02-27 Thread Roger Pau Monné 
On Mon, Feb 26, 2024 at 01:39:49PM +0100, Jan Beulich wrote:
> On 26.02.2024 12:07, Roger Pau Monne wrote:
> > Just like it's done for INDIRECT_THUNK and SHADOW_PAGING.
> > 
> > Reported-by: Jan Beulich 
> > Signed-off-by: Roger Pau Monné 
> 
> In principle
> Reviewed-by: Jan Beulich 
> but ...
> 
> > --- a/xen/arch/x86/spec_ctrl.c
> > +++ b/xen/arch/x86/spec_ctrl.c
> > @@ -466,13 +466,25 @@ static void __init print_details(enum ind_thunk thunk)
> > (e21a & cpufeat_mask(X86_FEATURE_SBPB))   ? " SBPB" 
> >   : "");
> >  
> >  /* Compiled-in support which pertains to mitigations. */
> > -if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || 
> > IS_ENABLED(CONFIG_SHADOW_PAGING) )
> > +if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || 
> > IS_ENABLED(CONFIG_SHADOW_PAGING) ||
> > + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_ARRAY) ||
> > + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_BRANCH) ||
> > + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS) )
> >  printk("  Compiled-in support:"
> >  #ifdef CONFIG_INDIRECT_THUNK
> > " INDIRECT_THUNK"
> >  #endif
> >  #ifdef CONFIG_SHADOW_PAGING
> > " SHADOW_PAGING"
> > +#endif
> > +#ifdef CONFIG_SPECULATIVE_HARDEN_ARRAY
> > +   " SPECULATIVE_HARDEN_ARRAY"
> > +#endif
> > +#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
> > +   " SPECULATIVE_HARDEN_BRANCH"
> > +#endif
> > +#ifdef CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS
> > +   " SPECULATIVE_HARDEN_GUEST_ACCESS"
> >  #endif
> 
> ... I'd like to suggest to drop the SPECULATIVE_ from the string literals.
> They're relevant in the Kconfig identifiers, but they're imo redundant in
> the context of these log messages. (Happy to adjust while committing, if
> need be.)

Oh, yes, indeed, we already print HARDEN_BRANCH instead of
SPECULATIVE_HARDEN_BRANCH.  Please adjust at commit if you don't mind.

Thanks, Roger.

Re: [PATCH v3 1/4] x86/spec: print the built-in SPECULATIVE_HARDEN_* options

2024-02-26 Thread Jan Beulich 
On 26.02.2024 12:07, Roger Pau Monne wrote:
> Just like it's done for INDIRECT_THUNK and SHADOW_PAGING.
> 
> Reported-by: Jan Beulich 
> Signed-off-by: Roger Pau Monné 

In principle
Reviewed-by: Jan Beulich 
but ...

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -466,13 +466,25 @@ static void __init print_details(enum ind_thunk thunk)
> (e21a & cpufeat_mask(X86_FEATURE_SBPB))   ? " SBPB"   
> : "");
>  
>  /* Compiled-in support which pertains to mitigations. */
> -if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || 
> IS_ENABLED(CONFIG_SHADOW_PAGING) )
> +if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || 
> IS_ENABLED(CONFIG_SHADOW_PAGING) ||
> + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_ARRAY) ||
> + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_BRANCH) ||
> + IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS) )
>  printk("  Compiled-in support:"
>  #ifdef CONFIG_INDIRECT_THUNK
> " INDIRECT_THUNK"
>  #endif
>  #ifdef CONFIG_SHADOW_PAGING
> " SHADOW_PAGING"
> +#endif
> +#ifdef CONFIG_SPECULATIVE_HARDEN_ARRAY
> +   " SPECULATIVE_HARDEN_ARRAY"
> +#endif
> +#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
> +   " SPECULATIVE_HARDEN_BRANCH"
> +#endif
> +#ifdef CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS
> +   " SPECULATIVE_HARDEN_GUEST_ACCESS"
>  #endif

... I'd like to suggest to drop the SPECULATIVE_ from the string literals.
They're relevant in the Kconfig identifiers, but they're imo redundant in
the context of these log messages. (Happy to adjust while committing, if
need be.)

Jan

[PATCH v3 1/4] x86/spec: print the built-in SPECULATIVE_HARDEN_* options

2024-02-26 Thread Roger Pau Monne 
Just like it's done for INDIRECT_THUNK and SHADOW_PAGING.

Reported-by: Jan Beulich 
Signed-off-by: Roger Pau Monné 
---
Changes since v2:
 - New in this version.
---
 xen/arch/x86/spec_ctrl.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 421fe3f640df..9f5ed8772533 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -466,13 +466,25 @@ static void __init print_details(enum ind_thunk thunk)
(e21a & cpufeat_mask(X86_FEATURE_SBPB))   ? " SBPB" 
  : "");
 
 /* Compiled-in support which pertains to mitigations. */
-if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || IS_ENABLED(CONFIG_SHADOW_PAGING) 
)
+if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) || IS_ENABLED(CONFIG_SHADOW_PAGING) 
||
+ IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_ARRAY) ||
+ IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_BRANCH) ||
+ IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS) )
 printk("  Compiled-in support:"
 #ifdef CONFIG_INDIRECT_THUNK
" INDIRECT_THUNK"
 #endif
 #ifdef CONFIG_SHADOW_PAGING
" SHADOW_PAGING"
+#endif
+#ifdef CONFIG_SPECULATIVE_HARDEN_ARRAY
+   " SPECULATIVE_HARDEN_ARRAY"
+#endif
+#ifdef CONFIG_SPECULATIVE_HARDEN_BRANCH
+   " SPECULATIVE_HARDEN_BRANCH"
+#endif
+#ifdef CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS
+   " SPECULATIVE_HARDEN_GUEST_ACCESS"
 #endif
"\n");
 
-- 
2.43.0