Re: [Xen-devel] [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, May 16, 2018 at 12:27:29PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:48, Wei Liu wrote:
> > On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
> >> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as 
> >> its
> >> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to 
> >> the
> >> MSR.
> >>
> >> Requested-by: Jan Beulich <JBeulich@xxxxxxxx>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> >> ---
> >> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> CC: Juergen Gross <jgross@xxxxxxxx>
> >> ---
> >>  xen/arch/x86/spec_ctrl.c          | 4 ++++
> >>  xen/include/asm-x86/cpufeatures.h | 1 +
> >>  xen/include/asm-x86/spec_ctrl.h   | 8 ++------
> >>  3 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> >> index a2328bd..f4a3165 100644
> >> --- a/xen/arch/x86/spec_ctrl.c
> >> +++ b/xen/arch/x86/spec_ctrl.c
> >> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
> >>      /* (Re)init BSP state now that default_spec_ctrl_flags has been 
> >> calculated. */
> >>      init_shadow_spec_ctrl_state();
> >>  
> >> +    /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. 
> >> */
> >> +    if ( default_xen_spec_ctrl )
> >> +        setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> >> +
> >>      xpti_init_default(false);
> >>      if ( opt_xpti == 0 )
> >>          setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
> >> diff --git a/xen/include/asm-x86/cpufeatures.h 
> >> b/xen/include/asm-x86/cpufeatures.h
> >> index 9d5d81e..b90aa2d 100644
> >> --- a/xen/include/asm-x86/cpufeatures.h
> >> +++ b/xen/include/asm-x86/cpufeatures.h
> >> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM,      (FSCAPINTS+0)*32+17) /* 
> >> MSR_SPEC_CTRL used by Xe
> >>  XEN_CPUFEATURE(SC_RSB_PV,       (FSCAPINTS+0)*32+18) /* RSB overwrite 
> >> needed for PV */
> >>  XEN_CPUFEATURE(SC_RSB_HVM,      (FSCAPINTS+0)*32+19) /* RSB overwrite 
> >> needed for HVM */
> >>  XEN_CPUFEATURE(NO_XPTI,         (FSCAPINTS+0)*32+20) /* XPTI mitigation 
> >> not in use */
> >> +XEN_CPUFEATURE(SC_MSR_IDLE,     (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || 
> >> SC_MSR_HVM) && default_xen_spec_ctrl */
> > I don't follow: the code above only depends on default_xen_spec_ctrl but
> > the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
> > consideration.
> 
> default_xen_spec_ctrl is only ever nonzero when SC_MSR_PV or SC_MSR_HVM
> is set.  In principle, I could assert that one of the two is set.
> 
> I have phrased it like this because it is going to become rather more
> complicated (than != 0) when adding IBRS_ATT mode.

OK, thanks for explaining.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel