lkml.org 
[lkml]   [2021]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] KVM: VMX: Add a wrapper for reading INVPCID/INVEPT/INVVPID type
On Mon, Oct 11, 2021, Jim Mattson wrote:
> On Mon, Oct 11, 2021 at 1:23 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Oct 11, 2021, Vipin Sharma wrote:
> > > - if (type > 3) {
> > > + if (type > INVPCID_TYPE_MAX) {
> >
> > Hrm, I don't love this because it's not auto-updating in the unlikely chance that
> > a new type is added. I definitely don't like open coding '3' either. What about
> > going with a verbose option of
> >
> > if (type != INVPCID_TYPE_INDIV_ADDR &&
> > type != INVPCID_TYPE_SINGLE_CTXT &&
> > type != INVPCID_TYPE_ALL_INCL_GLOBAL &&
> > type != INVPCID_TYPE_ALL_NON_GLOBAL) {
> > kvm_inject_gp(vcpu, 0);
> > return 1;
> > }
>
> Better, perhaps, to introduce a new function, valid_invpcid_type(),
> and squirrel away the ugliness there?

Oh, yeah, definitely. I missed that SVM's invpcid_interception() has the same
open-coded check.

Alternatively, could we handle the invalid type in the main switch statement? I
don't see anything in the SDM or APM that architecturally _requires_ the type be
checked before reading the INVPCID descriptor. Hardware may operate that way,
but that's uArch specific behavior unless there's explicit documentation.


diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 89077160d463..c8aade2e2a20 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3119,11 +3119,6 @@ static int invpcid_interception(struct kvm_vcpu *vcpu)
type = svm->vmcb->control.exit_info_2;
gva = svm->vmcb->control.exit_info_1;

- if (type > 3) {
- kvm_inject_gp(vcpu, 0);
- return 1;
- }
-
return kvm_handle_invpcid(vcpu, type, gva);
}

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 1c8b2b6e7ed9..ad2e794d4cb2 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5504,11 +5504,6 @@ static int handle_invpcid(struct kvm_vcpu *vcpu)
vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
type = kvm_register_read(vcpu, (vmx_instruction_info >> 28) & 0xf);

- if (type > 3) {
- kvm_inject_gp(vcpu, 0);
- return 1;
- }
-
/* According to the Intel instruction reference, the memory operand
* is read even if it isn't needed (e.g., for type==all)
*/
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d9273f536f9d..a3657db6daf9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12382,7 +12382,8 @@ int kvm_handle_invpcid(struct kvm_vcpu *vcpu, unsigned long type, gva_t gva)
return kvm_skip_emulated_instruction(vcpu);

default:
- BUG(); /* We have already checked above that type <= 3 */
+ kvm_inject_gp(vcpu, 0);
+ return 1;
}
}
EXPORT_SYMBOL_GPL(kvm_handle_invpcid);
\
 
 \ /
  Last update: 2021-10-14 18:55    [W:0.977 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site