Re: [PATCH 0/3] KVM: x86: SVM INVPCID fix, and cleanups

[Paolo Bonzini <pbonzini@redhat.com>]

On 12/02/21 01:34, Sean Christopherson wrote:
> Fix an INVPCID bug on SVM where it fails to injected a #UD when INVPCID is
> supported but not exposed to the guest.  Do a bit of cleanup in patch 02
> now that both VMX and SVM support PCID/INVPCID.
> 
> Patch 03 address KVM behavior that has long confused the heck out of me.
> KVM currently allows enabling INVPCID if and only if PCID is also enabled
> for the guest, the justification being that the guest will see incorrect
> fault behavior (#UD instead of #GP) due to the way the VMCS control works.
> 
> But that makes no sense, because nothing is forcing KVM to disable INVCPID
> in the VMCS when PCID is disabled.  AFACIT, the myth was the result of a
> bug in the original _submission_, not even the original _commit_ was buggy.
> 
> Digging back, the very original submission had this code, where
> vmx_pcid_supported() was further conditioned on EPT being enabled.  This
> would lead to the buggy scenario of unexpected #UD, as a host with PCID
> and INVCPID would fail to enable INVPCID if EPT was disabled.
> 
>>> +	if (vmx_pcid_supported()) {
>>> +		exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>>> +		if (exec_control & SECONDARY_EXEC_ENABLE_INVPCID) {
>>> +			best = kvm_find_cpuid_entry(vcpu, 0x1, 0);
>>> +			if (best && (best->ecx & bit(X86_FEATURE_PCID)))
>>> +				vmx->invpcid_enabled = true;
>>> +			else {
>>> +				exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
>>> +				vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
>>> +						exec_control);
>>> +				best = kvm_find_cpuid_entry(vcpu, 0x7, 0);
>>> +				best->ecx &= ~bit(X86_FEATURE_INVPCID);
>>> +			}
>>> +		}
>>> +	}
> 
> The incorrect behavior is especially problematic now that SVM also
> supports INVCPID, as KVM allows !PCID && INVPCID on SVM but not on VMX.
> 
> Patches to fix kvm-unit-tests are also incoming...
> 
> Sean Christopherson (3):
>    KVM: SVM: Intercept INVPCID when it's disabled to inject #UD
>    KVM: x86: Advertise INVPCID by default
>    KVM: VMX: Allow INVPCID in guest without PCID
> 
>   arch/x86/kvm/cpuid.c   |  2 +-
>   arch/x86/kvm/svm/svm.c | 11 ++++-------
>   arch/x86/kvm/vmx/vmx.c | 14 ++------------
>   3 files changed, 7 insertions(+), 20 deletions(-)
> 

Queued, thanks.

Paolo