Re: [PATCH v5 06/15] KVM: nVMX: Keep KVM updates to BNDCFGS ctrl bits across MSR write

[Paolo Bonzini]

On 6/7/22 23:35, Sean Christopherson wrote:
> From: Oliver Upton <oupton@xxxxxxxxxx>
>
> Since commit 5f76f6f5ff96 ("KVM: nVMX: Do not expose MPX VMX controls
> when guest MPX disabled"), KVM has taken ownership of the "load
> IA32_BNDCFGS" and "clear IA32_BNDCFGS" VMX entry/exit controls. The ABI
> is that these bits must be set in the IA32_VMX_TRUE_{ENTRY,EXIT}_CTLS
> MSRs if the guest's CPUID supports MPX, and clear otherwise.
>
> However, commit aedbaf4f6afd ("KVM: x86: Extract
> kvm_update_cpuid_runtime() from kvm_update_cpuid()") partially broke KVM
> ownership of the aforementioned bits. Before, kvm_update_cpuid() was
> exercised frequently when running a guest and constantly applied its own
> changes to the BNDCFGS bits. Now, the BNDCFGS bits are only ever
> updated after a KVM_SET_CPUID/KVM_SET_CPUID2 ioctl, meaning that a
> subsequent MSR write from userspace will clobber these values.
>
> Uphold the old ABI by reapplying KVM's tweaks to the BNDCFGS bits after
> an MSR write from userspace.
>
> Note, the old ABI that is being preserved is a KVM hack to workaround a
> userspace bug; see commit 5f76f6f5ff96 ("KVM: nVMX: Do not expose MPX VMX
> controls when guest MPX disabled").

Actually this is not a userspace bug.  It's an L1 workaround for running 
*correct* L1 userspace without a *kernel* bugfix on L0, namely commit 
691bd4340bef ("kvm: vmx: allow host to access guest MSR_IA32_BNDCFGS").

But thanks for writing the incorrect commit message, because now that 
I've actually looked at the history, I'm going to say screw 6 year old 
kernels used as L0.  Let's just revert commit 5f76f6f5ff96.

I've applied patches 1-5, let's see how bad the conflicts are in the 
rest of the series.

(This shows another reason why sometimes series seem to be cursed and 
don't get reviews: maintainers having a fuzzy feeling that *something* 
is wrong with them and putting off the review until it finally clicks).

Paolo