[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] x86/msr: fix handling of MSR_IA32_PERF_{STATUS/CTL}
On 10/11/2020 08:04, Jan Beulich wrote: > On 09.11.2020 19:31, Roger Pau Monné wrote: >> On Mon, Nov 09, 2020 at 05:38:19PM +0000, Andrew Cooper wrote: >>> From: Roger Pau Monné <roger.pau@xxxxxxxxxx> >>> >>> 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 322ec7c89f6 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: 322ec7c89f6 ('x86/pv: disallow access to unknown MSRs') >>> Fixes: 84e848fd7a1 ('x86/hvm: disallow access to unknown MSRs') >>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> >> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> >> >>> --- >>> CC: Jan Beulich <JBeulich@xxxxxxxx> >>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> >>> CC: Wei Liu <wl@xxxxxxx> >>> >>> v2: >>> * fix is_cpufreq_controller() to exclude PVH dom0, and collapse to nothing >>> in >>> !CONFIG_PV builds >>> * Drop the cross-vendor checks. It isn't possible to configure dom0 as >>> cross-vendor, and anyone using is_cpufreq_controller() without an exact >>> model match has far bigger problems. > This already answers ... > >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -1069,6 +1069,23 @@ 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. >>> + */ >> Would it be useful to add an assert here that the domain cpuid vendor >> and the BSP cpuid vendor match? >> >> Is it even possible to fake a different cpuid vendor for dom0? > ... your question here. Technically at the moment it is possible to configure a non-dom0 hardware domain as cross vendor, but anyone doing so can keep all the resulting pieces. ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |