x86/msr: fix handling of MSR_IA32_PERF_{STATUS/CTL}

Currently a PV hardware domain can also be given control over the CPU
frequency, and such guest is allowed to write to MSR_IA32_PERF_CTL.
However since commit 322ec7c89f the default behavior has been changed
to reject accesses to not explicitly handled MSRs, preventing PV
guests that manage CPU frequency from reading
MSR_IA32_PERF_{STATUS/CTL}.

Additionally some HVM guests (Windows at least) will attempt to read
MSR_IA32_PERF_CTL and will panic if given back a #GP fault:

  vmx.c:3035:d8v0 RDMSR 0x00000199 unimplemented
  d8v0 VIRIDIAN CRASH: 3b c0000096 fffff806871c1651 ffffda0253683720 0

Move the handling of MSR_IA32_PERF_{STATUS/CTL} to the common MSR
handling shared between HVM and PV guests, and add an explicit case
for reads to MSR_IA32_PERF_{STATUS/CTL}.

Restore previous behavior and allow PV guests with the required
permissions to read the contents of the mentioned MSRs. Non privileged
guests will get 0 when trying to read those registers, as writes to
MSR_IA32_PERF_CTL by such guest will already be silently dropped.

Fixes: 322ec7c89f ('x86/pv: disallow access to unknown MSRs')
Fixes: 84e848fd7a ('x86/hvm: disallow access to unknown MSRs')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 3059178798)
This commit is contained in:
Roger Pau Monné authored and Andrew Cooper committed 2020-11-10 17:43:56 +00:00
1 parent 6434a6348d
commit 33483f81ed
3 files changed
+51 -14

No files matched your search

+35
View File
@@ -208,6 +208,25 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
*val = msrs->misc_features_enables.raw;
break;
/*
* These MSRs are not enumerated in CPUID. They have been around
* since the Pentium 4, and implemented by other vendors.
*
* Some versions of Windows try reading these before setting up a #GP
* handler, and Linux has several unguarded reads as well. Provide
* RAZ semantics, in general, but permit a cpufreq controller dom0 to
* have full access.
*/
case MSR_IA32_PERF_STATUS:
case MSR_IA32_PERF_CTL:
if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
goto gp_fault;
*val = 0;
if ( likely(!is_cpufreq_controller(d)) || rdmsr_safe(msr, *val) == 0 )
break;
goto gp_fault;
case MSR_X2APIC_FIRST ... MSR_X2APIC_LAST:
if ( !is_hvm_domain(d) || v != curr )
goto gp_fault;
@@ -305,6 +324,7 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
case MSR_INTEL_CORE_THREAD_COUNT:
case MSR_INTEL_PLATFORM_INFO:
case MSR_ARCH_CAPABILITIES:
case MSR_IA32_PERF_STATUS:
/* Read-only */
case MSR_TSX_FORCE_ABORT:
case MSR_TSX_CTRL:
@@ -411,6 +431,21 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
break;
}
/*
* This MSR is not enumerated in CPUID. It has been around since the
* Pentium 4, and implemented by other vendors.
*
* To match the RAZ semantics, implement as write-discard, except for
* a cpufreq controller dom0 which has full access.
*/
case MSR_IA32_PERF_CTL:
if ( !(cp->x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR)) )
goto gp_fault;
if ( likely(!is_cpufreq_controller(d)) || wrmsr_safe(msr, val) == 0 )
break;
goto gp_fault;
case MSR_X2APIC_FIRST ... MSR_X2APIC_LAST:
if ( !is_hvm_domain(d) || v != curr )
goto gp_fault;
-14
View File
@@ -776,12 +776,6 @@ static inline uint64_t guest_misc_enable(uint64_t val)
return val;
}
static inline bool is_cpufreq_controller(const struct domain *d)
{
return ((cpufreq_controller == FREQCTL_dom0_kernel) &&
is_hardware_domain(d));
}
static int read_msr(unsigned int reg, uint64_t *val,
struct x86_emulate_ctxt *ctxt)
{
@@ -1026,14 +1020,6 @@ static int write_msr(unsigned int reg, uint64_t val,
return X86EMUL_OKAY;
break;
case MSR_IA32_PERF_CTL:
if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
break;
if ( likely(!is_cpufreq_controller(currd)) ||
wrmsr_safe(reg, val) == 0 )
return X86EMUL_OKAY;
break;
case MSR_IA32_THERM_CONTROL:
case MSR_IA32_ENERGY_PERF_BIAS:
if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL )
+16
View File
@@ -1057,6 +1057,22 @@ extern enum cpufreq_controller {
FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
} cpufreq_controller;
static always_inline bool is_cpufreq_controller(const struct domain *d)
{
/*
* A PV dom0 can be nominated as the cpufreq controller, instead of using
* Xen's cpufreq driver, at which point dom0 gets direct access to certain
* MSRs.
*
* This interface only works when dom0 is identity pinned and has the same
* number of vCPUs as pCPUs on the system.
*
* It would be far better to paravirtualise the interface.
*/
return (is_pv_domain(d) && is_hardware_domain(d) &&
cpufreq_controller == FREQCTL_dom0_kernel);
}
#define CPUPOOLID_NONE -1
struct cpupool *cpupool_get_by_id(int poolid);
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.