[PATCH] x86/spec: fix reporting of BHB clearing usage from guest entry points

[PATCH] x86/spec: fix reporting of BHB clearing usage from guest entry points

[Roger Pau Monne]

Reporting whether the BHB clearing on entry is done for the different domains
types based on cpu_has_bhb_seq is incorrect, as that variable signals whether
there's a BHB clearing sequence selected, but that alone doesn't imply that
such sequence is used from the PV and/or HVM entry points.

Instead use opt_bhb_entry_{pv,hvm} which do signal whether BHB clearing is
performed on entry from PV/HVM.

Fixes: 689ad48ce9cf ('x86/spec-ctrl: Wire up the Native-BHI software sequences')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/spec_ctrl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index dd01e30844a1..29f5248d01d1 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -643,7 +643,7 @@ static void __init print_details(enum ind_thunk thunk)
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
            opt_verw_hvm                              ? " VERW"          : "",
            boot_cpu_has(X86_FEATURE_IBPB_ENTRY_HVM)  ? " IBPB-entry"    : "",
-           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
+           opt_bhb_entry_hvm                         ? " BHB-entry"     : "");
 
 #endif
 #ifdef CONFIG_PV
@@ -658,7 +658,7 @@ static void __init print_details(enum ind_thunk thunk)
            opt_eager_fpu                             ? " EAGER_FPU"     : "",
            opt_verw_pv                               ? " VERW"          : "",
            boot_cpu_has(X86_FEATURE_IBPB_ENTRY_PV)   ? " IBPB-entry"    : "",
-           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
+           opt_bhb_entry_pv                          ? " BHB-entry"     : "");
 
     printk("  XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
            opt_xpti_hwdom ? "enabled" : "disabled",
-- 
2.44.0

Re: [PATCH] x86/spec: fix reporting of BHB clearing usage from guest entry points

[Jan Beulich]

On 15.04.2024 16:17, Roger Pau Monne wrote:
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -643,7 +643,7 @@ static void __init print_details(enum ind_thunk thunk)
>             opt_eager_fpu                             ? " EAGER_FPU"     : "",
>             opt_verw_hvm                              ? " VERW"          : "",
>             boot_cpu_has(X86_FEATURE_IBPB_ENTRY_HVM)  ? " IBPB-entry"    : "",
> -           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
> +           opt_bhb_entry_hvm                         ? " BHB-entry"     : "");
>  
>  #endif
>  #ifdef CONFIG_PV
> @@ -658,7 +658,7 @@ static void __init print_details(enum ind_thunk thunk)
>             opt_eager_fpu                             ? " EAGER_FPU"     : "",
>             opt_verw_pv                               ? " VERW"          : "",
>             boot_cpu_has(X86_FEATURE_IBPB_ENTRY_PV)   ? " IBPB-entry"    : "",
> -           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
> +           opt_bhb_entry_pv                          ? " BHB-entry"     : "");
>  
>      printk("  XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
>             opt_xpti_hwdom ? "enabled" : "disabled",

For one the surrounding if() conditions need similar adjustment.

But then I wonder whether the further uses of cpu_has_bhb_seq don't
need switching, too (when determining whether to force
X86_SPEC_NO_LFENCE_ENTRY_*). At which point the question is why
cpu_has_bhb_seq exists in the first place: It in particular does not
affect the default calculation of opt_bhb_entry_*, afaics.

Jan

Re: [PATCH] x86/spec: fix reporting of BHB clearing usage from guest entry points

[Roger Pau Monné]

On Thu, Apr 18, 2024 at 12:44:26PM +0200, Jan Beulich wrote:
> On 15.04.2024 16:17, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/spec_ctrl.c
> > +++ b/xen/arch/x86/spec_ctrl.c
> > @@ -643,7 +643,7 @@ static void __init print_details(enum ind_thunk thunk)
> >             opt_eager_fpu                             ? " EAGER_FPU"     : "",
> >             opt_verw_hvm                              ? " VERW"          : "",
> >             boot_cpu_has(X86_FEATURE_IBPB_ENTRY_HVM)  ? " IBPB-entry"    : "",
> > -           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
> > +           opt_bhb_entry_hvm                         ? " BHB-entry"     : "");
> >  
> >  #endif
> >  #ifdef CONFIG_PV
> > @@ -658,7 +658,7 @@ static void __init print_details(enum ind_thunk thunk)
> >             opt_eager_fpu                             ? " EAGER_FPU"     : "",
> >             opt_verw_pv                               ? " VERW"          : "",
> >             boot_cpu_has(X86_FEATURE_IBPB_ENTRY_PV)   ? " IBPB-entry"    : "",
> > -           cpu_has_bhb_seq                           ? " BHB-entry"     : "");
> > +           opt_bhb_entry_pv                          ? " BHB-entry"     : "");
> >  
> >      printk("  XPTI (64-bit PV only): Dom0 %s, DomU %s (with%s PCID)\n",
> >             opt_xpti_hwdom ? "enabled" : "disabled",
> 
> For one the surrounding if() conditions need similar adjustment.

Oh, I see, you mean the printing of "None" if no options are active.

> But then I wonder whether the further uses of cpu_has_bhb_seq don't
> need switching, too (when determining whether to force
> X86_SPEC_NO_LFENCE_ENTRY_*). At which point the question is why
> cpu_has_bhb_seq exists in the first place: It in particular does not
> affect the default calculation of opt_bhb_entry_*, afaics.

Hm, yes, that's indeed not very helpful.  The current code is overly
restrictive, as the lfence won't be elided even when the BHB sequence
is not being used by that specific guest type, as long as some
sequence is configured.  Will add a further patch to fix that and then
remove cpu_has_bhb_seq.

Thanks, Roger.