Mailing List Archive

commit 2cf3b4b92ab1a34e8cb1e859196e1492c89299de
Author: Andrew Cooper <andrew.cooper3@citrix.com>
AuthorDate: Thu Jun 10 11:01:06 2021 +0100
Commit: Andrew Cooper <andrew.cooper3@citrix.com>
CommitDate: Tue Jun 15 20:50:32 2021 +0100

x86/platform: Improve MSR permission handling for XENPF_resource_op

The logic to disallow writes to the TSC is out-of-place, and should be in
check_resource_access() rather than in resource_access().

Split the existing allow_access_msr() into two - msr_{read,write}_allowed() -
and move all permissions checks here.

Furthermore, guard access to MSR_IA32_CMT_{EVTSEL,CTR} to prohibit their use
on hardware which is lacking the QoS Monitoring feature. Introduce
cpu_has_pqe to help with the logic.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/platform_hypercall.c | 41 ++++++++++++++++++++++++++++-----------
xen/arch/x86/psr.c | 2 +-
xen/include/asm-x86/cpufeature.h | 1 +
3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 23fadbc782..41d8e59563 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -64,17 +64,33 @@ long cpu_frequency_change_helper(void *data)
return cpu_frequency_change((uint64_t)data);
}

-static bool allow_access_msr(unsigned int msr)
+static bool msr_read_allowed(unsigned int msr)
{
switch ( msr )
{
- /* MSR for CMT, refer to chapter 17.14 of Intel SDM. */
case MSR_IA32_CMT_EVTSEL:
case MSR_IA32_CMT_CTR:
+ return cpu_has_pqe;
+
case MSR_IA32_TSC:
return true;
}

+ if ( ppin_msr && msr == ppin_msr )
+ return true;
+
+ return false;
+}
+
+static bool msr_write_allowed(unsigned int msr)
+{
+ switch ( msr )
+ {
+ case MSR_IA32_CMT_EVTSEL:
+ case MSR_IA32_CMT_CTR:
+ return cpu_has_pqe;
+ }
+
return false;
}

@@ -96,15 +112,19 @@ void check_resource_access(struct resource_access *ra)
switch ( entry->u.cmd )
{
case XEN_RESOURCE_OP_MSR_READ:
- if ( ppin_msr && entry->idx == ppin_msr )
- break;
- /* fall through */
+ if ( entry->idx >> 32 )
+ ret = -EINVAL;
+ else if ( !msr_read_allowed(entry->idx) )
+ ret = -EPERM;
+ break;
+
case XEN_RESOURCE_OP_MSR_WRITE:
if ( entry->idx >> 32 )
ret = -EINVAL;
- else if ( !allow_access_msr(entry->idx) )
- ret = -EACCES;
+ else if ( !msr_write_allowed(entry->idx) )
+ ret = -EPERM;
break;
+
default:
ret = -EOPNOTSUPP;
break;
@@ -163,12 +183,11 @@ void resource_access(void *info)
}
}
break;
+
case XEN_RESOURCE_OP_MSR_WRITE:
- if ( unlikely(entry->idx == MSR_IA32_TSC) )
- ret = -EPERM;
- else
- ret = wrmsr_safe(entry->idx, entry->val);
+ ret = wrmsr_safe(entry->idx, entry->val);
break;
+
default:
BUG();
break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index d7f8864651..d805b85dc6 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -1558,7 +1558,7 @@ static void psr_cpu_init(void)
struct cpuid_leaf regs;
uint32_t feat_mask;

- if ( !psr_alloc_feat_enabled() || !boot_cpu_has(X86_FEATURE_PQE) )
+ if ( !psr_alloc_feat_enabled() || !cpu_has_pqe )
goto assoc_init;

if ( boot_cpu_data.cpuid_level < PSR_CPUID_LEVEL_CAT )
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index a539a4bacd..5f6b83f71c 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -94,6 +94,7 @@
#define cpu_has_bmi2 boot_cpu_has(X86_FEATURE_BMI2)
#define cpu_has_invpcid boot_cpu_has(X86_FEATURE_INVPCID)
#define cpu_has_rtm boot_cpu_has(X86_FEATURE_RTM)
+#define cpu_has_pqe boot_cpu_has(X86_FEATURE_PQE)
#define cpu_has_fpu_sel (!boot_cpu_has(X86_FEATURE_NO_FPU_SEL))
#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX)
#define cpu_has_avx512f boot_cpu_has(X86_FEATURE_AVX512F)
--
generated by git-patchbot for /home/xen/git/xen.git#staging