On Wed, Aug 21, 2019 at 01:59:20PM -0700, Jim Mattson wrote: > On Mon, Aug 19, 2019 at 3:11 PM Sean Christopherson > <sean.j.christopherson@xxxxxxxxx> wrote: > > > > On Tue, Aug 20, 2019 at 12:46:49AM +0300, Nikita Leshenko wrote: > > > Before this commit, userspace could disable the GUEST_ACTIVITY_HLT bit in > > > VMX_MISC yet KVM would happily accept GUEST_ACTIVITY_HLT activity state in > > > VMCS12. We can fix it by either failing VM entries with HLT activity state when > > > it's not supported or by disallowing clearing this bit. > > > > > > The latter is preferable. If we go with the former, to disable > > > GUEST_ACTIVITY_HLT userspace also has to make CPU_BASED_HLT_EXITING a "must be > > > 1" control, otherwise KVM will be presenting a bogus model to L1. > > > > > > Don't fail writes that disable GUEST_ACTIVITY_HLT to maintain backwards > > > compatibility. > > > > Paolo, do we actually need to maintain backwards compatibility in this > > case? This seems like a good candidate for "fix the bug and see who yells". > > Google's userspace clears bit 6. Please don't fail that write! Booooo. Requiring CPU_BASED_HLT_EXITING to be forced to 1 is probably off the table since the bits are in two separate MSRs, i.e. we run into a chicken-and-egg scenario. Silently forcing GUEST_ACTIVITY_HLT seems wrong, especially if userspace has also forced CPU_BASED_HLT_EXITING, e.g. KVM would be letting L1 do something userspace explicitly asked KVM to prevent. Generally speaking, KVM lets userspace do dumb things so long as it doesn't break the kernel, so, what if we change sync_vmcs02_to_vmcs12() to propagate GUEST_ACTIVITY_HLT to vmcs12 if and only if the activity state exists? That might be better than causing a nested VM-Enter to fail? I'm also a-ok doing nothing and fulling putting the onus on userspace to get the config correct.
