Re: [RFC PATCH v3 23/59] KVM: x86: Allow host-initiated WRMSR to set X2APIC regardless of CPUID

Paolo Bonzini
2021-11-26T08:24:11Z

On 11/25/21 20:41, Thomas Gleixner wrote:
> On Wed, Nov 24 2021 at 16:20, isaku yamahata wrote:
>> Let userspace, or in the case of TDX, KVM itself, enable X2APIC even if
>> X2APIC is not reported as supported in the guest's CPU model. KVM
>> generally does not force specific ordering between ioctls(), e.g. this
>> forces userspace to configure CPUID before MSRs. And for TDX, vCPUs
>> will always run with X2APIC enabled, e.g. KVM will want/need to enable
>> X2APIC from time zero.
>
> This is complete crap. Fix the broken user space and do not add
> horrible hacks to the kernel.

tl;dr: I agree that it's a userspace issue but "configure CPUID before
MSR" is not the issue (in fact QEMU calls KVM_SET_CPUID2 before any call
to KVM_SET_MSRS).

We have quite a few other cases in which KVM_GET/SET_MSR is allowed to
get/set MSRs in ways that the guests are not allowed to do.

In general, there are several reasons for this:

- simplifying userspace so that it can use the same list of MSRs for all
guests (likely, the list that KVM provides with KVM_GET_MSR_INDEX_LIST).
For example MSR_TSC_AUX is only exposed to the guest if RDTSCP or
RDPID are available, but the host can always access it. This is usually
the reason why host accesses to MSRs override CPUID.

- simplifying userspace so that it does not have to go through the
various steps of a state machine; for example, it's okay if userspace
goes DISABLED->X2APIC instead of having to do DISABLED->XAPIC->X2APIC.

- allowing userspace to set a reset value, for example overriding the
lock bit in MSR_IA32_FEAT_CTL.

- read-only MSRs that are really "CPUID-like", i.e. they give the guest
information about processor features (for example the VMX feature MSRs)

- MSRs had some weird limitations that were lifted later by introducing
additional MSRs; for example KVM always allows the host to write to the
full-width MSR_IA32_PMC0 counters, because they are a saner version of
MSR_IA32_PERFCTR0 and there's no reason for userspace to inflict
MSR_IA32_PERFCTR0 on userspace.

So the host_initiated check doesn't _necessarily_ count as a horrible
hack in the kernel. However, in this case we have a trusted domain
without X2APIC. I'm not sure this configuration is clearly bogus. One
could imagine special-purpose VMs that don't need interrupts at all.
For full guests such as the ones that QEMU runs, I agree with Thomas
that userspace must be fixed to enforce x2apic for TDX guests.

Paolo