[PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place
@ 2021-11-09 17:44 Vipin Sharma
  2021-11-09 17:44 ` [PATCH v4 1/2] KVM: VMX: Add a helper function to retrieve the GPR index for INVPCID, INVVPID, and INVEPT Vipin Sharma
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Vipin Sharma @ 2021-11-09 17:44 UTC (permalink / raw)
  To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma

INVPCID, INVVPID, and INVEPT instructions retrieve the GPR index
similarly to find the invalidation type. Patch 1 moves the shift and
mask magic to a single place.

INVPCID invalidation type check is same for both VMX and SVM. This
instruction is not documented to verify the type before reading the
operand from memory. So, moving the check to a common place in patch 2.

v4:
- Changed commit message of the patch 1

v3:
- https://lore.kernel.org/lkml/20211103205911.1253463-1-vipinsh@google.com/
- Patch 2's commit message is more detailed now.

v2:
- https://lore.kernel.org/lkml/20211103183232.1213761-1-vipinsh@google.com/
- 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 helper function to retrieve the GPR index 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.34.0.rc0.344.g81b53c2807-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 1/2] KVM: VMX: Add a helper function to retrieve the GPR index for INVPCID, INVVPID, and INVEPT
  2021-11-09 17:44 [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Vipin Sharma
@ 2021-11-09 17:44 ` Vipin Sharma
  2021-11-09 17:44 ` [PATCH v4 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma
  2021-11-11 14:06 ` [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Vipin Sharma @ 2021-11-09 17:44 UTC (permalink / raw)
  To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma

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
instruction info to consolidate and document the shift+mask magic.

Signed-off-by: Vipin Sharma <vipinsh@google.com>
Reviewed-by: Sean Christopherson <seanjc@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.34.0.rc0.344.g81b53c2807-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid()
  2021-11-09 17:44 [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Vipin Sharma
  2021-11-09 17:44 ` [PATCH v4 1/2] KVM: VMX: Add a helper function to retrieve the GPR index for INVPCID, INVVPID, and INVEPT Vipin Sharma
@ 2021-11-09 17:44 ` Vipin Sharma
  2021-11-11 14:06 ` [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Vipin Sharma @ 2021-11-09 17:44 UTC (permalink / raw)
  To: pbonzini, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel, Vipin Sharma

Handle #GP on INVPCID due to an invalid type in the common switch
statement instead of relying on the callers (VMX and SVM) to manually
validate the type.

Unlike INVVPID and INVEPT, INVPCID is not explicitly documented to check
the type before reading the operand from memory, so deferring the
type validity check until after that point is architecturally allowed.

Signed-off-by: Vipin Sharma <vipinsh@google.com>
Reviewed-by: Sean Christopherson <seanjc@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.34.0.rc0.344.g81b53c2807-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place
  2021-11-09 17:44 [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Vipin Sharma
  2021-11-09 17:44 ` [PATCH v4 1/2] KVM: VMX: Add a helper function to retrieve the GPR index for INVPCID, INVVPID, and INVEPT Vipin Sharma
  2021-11-09 17:44 ` [PATCH v4 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma
@ 2021-11-11 14:06 ` Paolo Bonzini
  2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-11-11 14:06 UTC (permalink / raw)
  To: Vipin Sharma, seanjc, jmattson; +Cc: dmatlack, kvm, linux-kernel

On 11/9/21 18:44, Vipin Sharma wrote:
> INVPCID, INVVPID, and INVEPT instructions retrieve the GPR index
> similarly to find the invalidation type. Patch 1 moves the shift and
> mask magic to a single place.
> 
> INVPCID invalidation type check is same for both VMX and SVM. This
> instruction is not documented to verify the type before reading the
> operand from memory. So, moving the check to a common place in patch 2.
> 
> v4:
> - Changed commit message of the patch 1
> 
> v3:
> - https://lore.kernel.org/lkml/20211103205911.1253463-1-vipinsh@google.com/
> - Patch 2's commit message is more detailed now.
> 
> v2:
> - https://lore.kernel.org/lkml/20211103183232.1213761-1-vipinsh@google.com/
> - 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 helper function to retrieve the GPR index 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(-)
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-11 14:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-09 17:44 [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Vipin Sharma
2021-11-09 17:44 ` [PATCH v4 1/2] KVM: VMX: Add a helper function to retrieve the GPR index for INVPCID, INVVPID, and INVEPT Vipin Sharma
2021-11-09 17:44 ` [PATCH v4 2/2] KVM: Move INVPCID type check from vmx and svm to the common kvm_handle_invpcid() Vipin Sharma
2021-11-11 14:06 ` [PATCH v4 0/2] Add a helper to read GPR index and move INVPCID validation to a common place Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®