| Message ID | 20230515144259.1009245-6-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: Introduce MSR_ARCH_CAPS into featuresets | expand |
On 15.05.2023 16:42, Andrew Cooper wrote: > Extend x86_cpu_policy_fill_native() with a read of ARCH_CAPS based on the > CPUID information just read, which removes the need handling it specially in > calculate_raw_cpu_policy(). > > Extend generic_identify() to read ARCH_CAPS into x86_capability[], which is > fed into the Host Policy. This in turn means there's no need to special case > arch_caps in calculate_host_policy(). > > No practical change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> I have a question though, which I think would be nice if the description had covered: > --- a/xen/lib/x86/cpuid.c > +++ b/xen/lib/x86/cpuid.c > @@ -226,7 +226,12 @@ void x86_cpu_policy_fill_native(struct cpu_policy *p) > p->hv_limit = 0; > p->hv2_limit = 0; > > - /* TODO MSRs */ > +#ifdef __XEN__ > + /* TODO MSR_PLATFORM_INFO */ > + > + if ( p->feat.arch_caps ) > + rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw); > +#endif What about non-Xen environments re-using this code? In particular the test harnesses would be nice if they didn't run with the two fields all blank at all times. Jan
On 16/05/2023 1:53 pm, Jan Beulich wrote: > On 15.05.2023 16:42, Andrew Cooper wrote: >> Extend x86_cpu_policy_fill_native() with a read of ARCH_CAPS based on the >> CPUID information just read, which removes the need handling it specially in >> calculate_raw_cpu_policy(). >> >> Extend generic_identify() to read ARCH_CAPS into x86_capability[], which is >> fed into the Host Policy. This in turn means there's no need to special case >> arch_caps in calculate_host_policy(). >> >> No practical change. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > > I have a question though, which I think would be nice if the description > had covered: > >> --- a/xen/lib/x86/cpuid.c >> +++ b/xen/lib/x86/cpuid.c >> @@ -226,7 +226,12 @@ void x86_cpu_policy_fill_native(struct cpu_policy *p) >> p->hv_limit = 0; >> p->hv2_limit = 0; >> >> - /* TODO MSRs */ >> +#ifdef __XEN__ >> + /* TODO MSR_PLATFORM_INFO */ >> + >> + if ( p->feat.arch_caps ) >> + rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw); >> +#endif > What about non-Xen environments re-using this code? In particular the > test harnesses would be nice if they didn't run with the two fields > all blank at all times. Right now, I don't have an answer. In Linux in lockdown mode, there isn't even a way to access this info userspace, because /proc/cpu/$/msr goes away. It's only a unit test, and this doesn't break it. ~Andrew
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c index 49f5465ec445..dfd9abd8564c 100644 --- a/xen/arch/x86/cpu-policy.c +++ b/xen/arch/x86/cpu-policy.c @@ -354,9 +354,6 @@ void calculate_raw_cpu_policy(void) /* 0x000000ce MSR_INTEL_PLATFORM_INFO */ /* Was already added by probe_cpuid_faulting() */ - - if ( cpu_has_arch_caps ) - rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw); } static void __init calculate_host_policy(void) @@ -409,15 +406,6 @@ static void __init calculate_host_policy(void) /* 0x000000ce MSR_INTEL_PLATFORM_INFO */ /* probe_cpuid_faulting() sanity checks presence of MISC_FEATURES_ENABLES */ p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting; - - /* Temporary, until we have known_features[] for feature bits in MSRs. */ - p->arch_caps.raw = raw_cpu_policy.arch_caps.raw & - (ARCH_CAPS_RDCL_NO | ARCH_CAPS_IBRS_ALL | ARCH_CAPS_RSBA | - ARCH_CAPS_SKIP_L1DFL | ARCH_CAPS_SSB_NO | ARCH_CAPS_MDS_NO | - ARCH_CAPS_IF_PSCHANGE_MC_NO | ARCH_CAPS_TSX_CTRL | ARCH_CAPS_TAA_NO | - ARCH_CAPS_SBDR_SSDP_NO | ARCH_CAPS_FBSDP_NO | ARCH_CAPS_PSDP_NO | - ARCH_CAPS_FB_CLEAR | ARCH_CAPS_RRSBA | ARCH_CAPS_BHI_NO | - ARCH_CAPS_PBRSB_NO); } static void __init guest_common_default_feature_adjustments(uint32_t *fs) diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index edc4db1335eb..a3a341fd7db2 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -474,6 +474,11 @@ static void generic_identify(struct cpuinfo_x86 *c) cpuid_count(0xd, 1, &c->x86_capability[FEATURESET_Da1], &tmp, &tmp, &tmp); + + if (test_bit(X86_FEATURE_ARCH_CAPS, c->x86_capability)) + rdmsr(MSR_ARCH_CAPABILITIES, + c->x86_capability[FEATURESET_10Al], + c->x86_capability[FEATURESET_10Ah]); } /* diff --git a/xen/lib/x86/cpuid.c b/xen/lib/x86/cpuid.c index a9f31858aeff..dfd377cfb7ef 100644 --- a/xen/lib/x86/cpuid.c +++ b/xen/lib/x86/cpuid.c @@ -226,7 +226,12 @@ void x86_cpu_policy_fill_native(struct cpu_policy *p) p->hv_limit = 0; p->hv2_limit = 0; - /* TODO MSRs */ +#ifdef __XEN__ + /* TODO MSR_PLATFORM_INFO */ + + if ( p->feat.arch_caps ) + rdmsrl(MSR_ARCH_CAPABILITIES, p->arch_caps.raw); +#endif x86_cpu_policy_recalc_synth(p); }
Extend x86_cpu_policy_fill_native() with a read of ARCH_CAPS based on the CPUID information just read, which removes the need handling it specially in calculate_raw_cpu_policy(). Extend generic_identify() to read ARCH_CAPS into x86_capability[], which is fed into the Host Policy. This in turn means there's no need to special case arch_caps in calculate_host_policy(). No practical change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/arch/x86/cpu-policy.c | 12 ------------ xen/arch/x86/cpu/common.c | 5 +++++ xen/lib/x86/cpuid.c | 7 ++++++- 3 files changed, 11 insertions(+), 13 deletions(-)