* [PATCH v2 0/2] Add wrapper to read GPR of INVPCID, INVVPID, and INVEPT
@ 2021-11-03 18:32 Vipin Sharma
2021-11-03 18:32 ` [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for " Vipin Sharma
2021-11-03 18:32 ` [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma
0 siblings, 2 replies; 6+ messages in thread
From: Vipin Sharma @ 2021-11-03 18:32 UTC (permalink / raw)
To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma
Hello,
VMX code to handle INVPCID, INVVPID, and INVEPT read the same GPR index
in VM exit info. This patch series improves that handling by adding a
common wrapper function for them.
This series also moves INVPCID type check from both SVM and VMX to
common place in kvm_handle_invpcid().
Overall, this series is just reducing duplicate code.
Changes in v2:
- Keeping the register read visible in the functions.
- Removed INVPCID type check hardcoding and moved error condition to common
function.
[v1] https://lore.kernel.org/lkml/20211011194615.2955791-1-vipinsh@google.com/
Vipin Sharma (2):
KVM: VMX: Add a wrapper to read index of GPR for INVPCID, INVVPID, and
INVEPT
KVM: Move INVPCID type check from vmx and svm to the common
kvm_handle_invpcid()
arch/x86/kvm/svm/svm.c | 5 -----
arch/x86/kvm/vmx/nested.c | 10 ++++++----
arch/x86/kvm/vmx/vmx.c | 9 +++------
arch/x86/kvm/vmx/vmx.h | 5 +++++
arch/x86/kvm/x86.c | 3 ++-
5 files changed, 16 insertions(+), 16 deletions(-)
--
2.33.1.1089.g2158813163f-goog
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for INVPCID, INVVPID, and INVEPT 2021-11-03 18:32 [PATCH v2 0/2] Add wrapper to read GPR of INVPCID, INVVPID, and INVEPT Vipin Sharma @ 2021-11-03 18:32 ` Vipin Sharma 2021-11-03 23:00 ` Sean Christopherson 2021-11-03 18:32 ` [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma 1 sibling, 1 reply; 6+ messages in thread From: Vipin Sharma @ 2021-11-03 18:32 UTC (permalink / raw) To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma handle_invept(), handle_invvpid(), handle_invpcid() read the same reg2 on VM exit. Move them to a common wrapper function. Signed-off-by: Vipin Sharma <vipinsh@google.com> --- arch/x86/kvm/vmx/nested.c | 10 ++++++---- arch/x86/kvm/vmx/vmx.c | 4 +++- arch/x86/kvm/vmx/vmx.h | 5 +++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index b4ee5e9f9e20..f73d4e31dd99 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -5379,7 +5379,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) struct { u64 eptp, gpa; } operand; - int i, r; + int i, r, gpr_index; if (!(vmx->nested.msrs.secondary_ctls_high & SECONDARY_EXEC_ENABLE_EPT) || @@ -5392,7 +5392,8 @@ static int handle_invept(struct kvm_vcpu *vcpu) return 1; vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); - type = kvm_register_read(vcpu, (vmx_instruction_info >> 28) & 0xf); + gpr_index = vmx_get_instr_info_reg2(vmx_instruction_info); + type = kvm_register_read(vcpu, gpr_index); types = (vmx->nested.msrs.ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6; @@ -5459,7 +5460,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) u64 gla; } operand; u16 vpid02; - int r; + int r, gpr_index; if (!(vmx->nested.msrs.secondary_ctls_high & SECONDARY_EXEC_ENABLE_VPID) || @@ -5472,7 +5473,8 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) return 1; vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); - type = kvm_register_read(vcpu, (vmx_instruction_info >> 28) & 0xf); + gpr_index = vmx_get_instr_info_reg2(vmx_instruction_info); + type = kvm_register_read(vcpu, gpr_index); types = (vmx->nested.msrs.vpid_caps & VMX_VPID_EXTENT_SUPPORTED_MASK) >> 8; diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 71f54d85f104..e41d207e3298 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5494,6 +5494,7 @@ static int handle_invpcid(struct kvm_vcpu *vcpu) u64 pcid; u64 gla; } operand; + int gpr_index; if (!guest_cpuid_has(vcpu, X86_FEATURE_INVPCID)) { kvm_queue_exception(vcpu, UD_VECTOR); @@ -5501,7 +5502,8 @@ 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); + gpr_index = vmx_get_instr_info_reg2(vmx_instruction_info); + type = kvm_register_read(vcpu, gpr_index); if (type > 3) { kvm_inject_gp(vcpu, 0); diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h index e7db42e3b0ce..95c9bca45cdd 100644 --- a/arch/x86/kvm/vmx/vmx.h +++ b/arch/x86/kvm/vmx/vmx.h @@ -522,4 +522,9 @@ static inline bool vmx_guest_state_valid(struct kvm_vcpu *vcpu) void dump_vmcs(struct kvm_vcpu *vcpu); +static inline int vmx_get_instr_info_reg2(u32 vmx_instr_info) +{ + return (vmx_instr_info >> 28) & 0xf; +} + #endif /* __KVM_X86_VMX_H */ -- 2.33.1.1089.g2158813163f-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for INVPCID, INVVPID, and INVEPT 2021-11-03 18:32 ` [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for " Vipin Sharma @ 2021-11-03 23:00 ` Sean Christopherson 0 siblings, 0 replies; 6+ messages in thread From: Sean Christopherson @ 2021-11-03 23:00 UTC (permalink / raw) To: Vipin Sharma; +Cc: pbonzini, jmattson, dmatlack, kvm, linux-kernel On Wed, Nov 03, 2021, Vipin Sharma wrote: > handle_invept(), handle_invvpid(), handle_invpcid() read the same reg2 "same reg2" doesn't provide any context as to what "reg2" is, or what it's used for. > on VM exit. Move them to a common wrapper function. "Move them to a common helper function" reads as if the patch is moving the whole handle_*() code :-) handle_invept(), handle_invvpid(), handle_invpcid() read the same reg2 field in vmcs.VMX_INSTRUCTION_INFO to get the index of the GPR that holds the invalidation type. Add a helper to retrieve reg2 from VMX instr info to consolidate and document the shift+mask magic. > Signed-off-by: Vipin Sharma <vipinsh@google.com> Changelog nits aside, Reviewed-by: Sean Christopherson <seanjc@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() 2021-11-03 18:32 [PATCH v2 0/2] Add wrapper to read GPR of INVPCID, INVVPID, and INVEPT Vipin Sharma 2021-11-03 18:32 ` [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for " Vipin Sharma @ 2021-11-03 18:32 ` Vipin Sharma 2021-11-03 20:00 ` Sean Christopherson 1 sibling, 1 reply; 6+ messages in thread From: Vipin Sharma @ 2021-11-03 18:32 UTC (permalink / raw) To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma This check will be done in switch statement of kvm_handle_invpcid(), used by both VMX and SVM. It also removes (type > 3) check. Signed-off-by: Vipin Sharma <vipinsh@google.com> --- arch/x86/kvm/svm/svm.c | 5 ----- arch/x86/kvm/vmx/vmx.c | 5 ----- arch/x86/kvm/x86.c | 3 ++- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 21bb81710e0f..ccbf96876ec6 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 e41d207e3298..a3bb9854f4d2 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5505,11 +5505,6 @@ static int handle_invpcid(struct kvm_vcpu *vcpu) gpr_index = vmx_get_instr_info_reg2(vmx_instruction_info); type = kvm_register_read(vcpu, gpr_index); - 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 ac83d873d65b..134585027e92 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12443,7 +12443,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); -- 2.33.1.1089.g2158813163f-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() 2021-11-03 18:32 ` [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma @ 2021-11-03 20:00 ` Sean Christopherson 2021-11-03 20:23 ` Vipin Sharma 0 siblings, 1 reply; 6+ messages in thread From: Sean Christopherson @ 2021-11-03 20:00 UTC (permalink / raw) To: Vipin Sharma; +Cc: pbonzini, jmattson, dmatlack, kvm, linux-kernel On Wed, Nov 03, 2021, Vipin Sharma wrote: > This check will be done in switch statement of kvm_handle_invpcid(), Please make the changelog a stand on its own, i.e. don't rely on the shortlog for context. > used by both VMX and SVM. It also removes (type > 3) check. Use imperative mood, i.e. state what you're doing as a "command", don't refer to the patch from a third-person point of view. The changelog also needs to call out that, unlike INVVPID and INVEPT, INVPCID is not explicitly documented as checking the "type" before reading the operand from memory. I.e. there's a subtle, undocumented functional change in this patch. Something like: Handle #GP on INVCPID due to an invalid type in the common switch statement instead of relying on callers to manually verify the type is valid. Unlike INVVPID and INVPET, INVPCID is not explicitly documented as checking the type before reading the operand from memory, so deferring the type validity check until after that point is architecturally allowed. For the code: Reviewed-by: Sean Christopherson <seanjc@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() 2021-11-03 20:00 ` Sean Christopherson @ 2021-11-03 20:23 ` Vipin Sharma 0 siblings, 0 replies; 6+ messages in thread From: Vipin Sharma @ 2021-11-03 20:23 UTC (permalink / raw) To: Sean Christopherson; +Cc: pbonzini, jmattson, dmatlack, kvm, linux-kernel On Wed, Nov 3, 2021 at 1:00 PM Sean Christopherson <seanjc@google.com> wrote: > > On Wed, Nov 03, 2021, Vipin Sharma wrote: > > This check will be done in switch statement of kvm_handle_invpcid(), > > Please make the changelog a stand on its own, i.e. don't rely on the shortlog > for context. > > > used by both VMX and SVM. It also removes (type > 3) check. > > Use imperative mood, i.e. state what you're doing as a "command", don't refer to > the patch from a third-person point of view. > > The changelog also needs to call out that, unlike INVVPID and INVEPT, INVPCID is > not explicitly documented as checking the "type" before reading the operand from > memory. I.e. there's a subtle, undocumented functional change in this patch. > > Something like: > > Handle #GP on INVCPID due to an invalid type in the common switch statement > instead of relying on callers to manually verify the type is valid. Unlike > INVVPID and INVPET, INVPCID is not explicitly documented as checking the type > before reading the operand from memory, so deferring the type validity check > until after that point is architecturally allowed. > Thanks. I will update it and send out v3. > For the code: > > Reviewed-by: Sean Christopherson <seanjc@google.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-03 23:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-03 18:32 [PATCH v2 0/2] Add wrapper to read GPR of INVPCID, INVVPID, and INVEPT Vipin Sharma 2021-11-03 18:32 ` [PATCH v2 1/2] KVM: VMX: Add a wrapper to read index of GPR for " Vipin Sharma 2021-11-03 23:00 ` Sean Christopherson 2021-11-03 18:32 ` [PATCH v2 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma 2021-11-03 20:00 ` Sean Christopherson 2021-11-03 20:23 ` Vipin Sharma
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).