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....@citrix.com>
>>>
>>> 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....@citrix.com>
>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
>>
>>> ---
>>> CC: Jan Beulich <jbeul...@suse.com>
>>> CC: Roger Pau Monné <roger....@citrix.com>
>>> CC: Wei Liu <w...@xen.org>
>>>
>>> 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
