| Message ID | 20240415141737.88236-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/spec: fix reporting of BHB clearing usage from guest entry points | expand |
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
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.
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",
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(-)