[PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup
@ 2019-12-10 23:24 Sean Christopherson
  2019-12-10 23:24 ` [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs Sean Christopherson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-10 23:24 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Chao Peng, Luwei Kang

Add a missing non-canonical check on writes to the RTIT address MSRs
and tack on a cleanup patch.

** ALL PATCHES ARE COMPLETELY UNTESTED **

Untested due to lack of hardware.

Sean Christopherson (2):
  KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
  KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic

 arch/x86/kvm/vmx/vmx.c | 57 ++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

-- 
2.24.0


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

* [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
  2019-12-10 23:24 [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Sean Christopherson
@ 2019-12-10 23:24 ` Sean Christopherson
  2019-12-11  2:16   ` Kang, Luwei
  2019-12-10 23:24 ` [PATCH 2/2] KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic Sean Christopherson
  2020-01-15 17:59 ` [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Paolo Bonzini
  2 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2019-12-10 23:24 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Chao Peng, Luwei Kang

Reject writes to RTIT address MSRs if the data being written is a
non-canonical address as the MSRs are subject to canonical checks, e.g.
KVM will trigger an unchecked #GP when loading the values to hardware
during pt_guest_enter().

Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 51e3b27f90ed..9aa2006dbe04 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2152,6 +2152,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			(index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
 					PT_CAP_num_address_ranges)))
 			return 1;
+		if (is_noncanonical_address(data, vcpu))
+			return 1;
 		if (index % 2)
 			vmx->pt_desc.guest.addr_b[index / 2] = data;
 		else
-- 
2.24.0


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

* RE: [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
  2019-12-10 23:24 ` [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs Sean Christopherson
@ 2019-12-11  2:16   ` Kang, Luwei
  2019-12-11 16:14     ` Sean Christopherson
  0 siblings, 1 reply; 6+ messages in thread
From: Kang, Luwei @ 2019-12-11  2:16 UTC (permalink / raw)
  To: Christopherson, Sean J, Paolo Bonzini
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Chao Peng



> -----Original Message-----
> From: Christopherson, Sean J <sean.j.christopherson@intel.com>
> Sent: Wednesday, December 11, 2019 7:25 AM
> To: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Christopherson, Sean J <sean.j.christopherson@intel.com>; Vitaly Kuznetsov <vkuznets@redhat.com>; Wanpeng Li
> <wanpengli@tencent.com>; Jim Mattson <jmattson@google.com>; Joerg Roedel <joro@8bytes.org>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; Chao Peng <chao.p.peng@linux.intel.com>; Kang, Luwei <luwei.kang@intel.com>
> Subject: [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
> 
> Reject writes to RTIT address MSRs if the data being written is a non-canonical address as the MSRs are subject to canonical checks,
> e.g.
> KVM will trigger an unchecked #GP when loading the values to hardware during pt_guest_enter().
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 51e3b27f90ed..9aa2006dbe04 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2152,6 +2152,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  			(index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
>  					PT_CAP_num_address_ranges)))
>  			return 1;
> +		if (is_noncanonical_address(data, vcpu))
> +			return 1;

Is this for live migrate a VM with 5 level page table to the VM with 4 level page table?

Thanks,
Luwei Kang

>  		if (index % 2)
>  			vmx->pt_desc.guest.addr_b[index / 2] = data;
>  		else
> --
> 2.24.0


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

* Re: [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
  2019-12-11  2:16   ` Kang, Luwei
@ 2019-12-11 16:14     ` Sean Christopherson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-11 16:14 UTC (permalink / raw)
  To: Kang, Luwei
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Chao Peng

On Tue, Dec 10, 2019 at 06:16:35PM -0800, Kang, Luwei wrote:
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index
> > 51e3b27f90ed..9aa2006dbe04 100644 --- a/arch/x86/kvm/vmx/vmx.c +++
> > b/arch/x86/kvm/vmx/vmx.c @@ -2152,6 +2152,8 @@ static int
> > vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) (index >= 2 *
> > intel_pt_validate_cap(vmx->pt_desc.caps, PT_CAP_num_address_ranges)))
> > return 1; +		if (is_noncanonical_address(data, vcpu)) +
> > return 1;
> 
> Is this for live migrate a VM with 5 level page table to the VM with 4 level
> page table?

This is orthogonal to live migration or 5-level paging.  Unless I'm missing
something, KVM simply fails to validate the incoming address.

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

* [PATCH 2/2] KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic
  2019-12-10 23:24 [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Sean Christopherson
  2019-12-10 23:24 ` [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs Sean Christopherson
@ 2019-12-10 23:24 ` Sean Christopherson
  2020-01-15 17:59 ` [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Paolo Bonzini
  2 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2019-12-10 23:24 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Chao Peng, Luwei Kang

Add a helper to consolidate the common checks for writing PT MSRs,
and opportunistically clean up the formatting of the affected code.

No functional change intended.

Cc: Chao Peng <chao.p.peng@linux.intel.com>
Cc: Luwei Kang <luwei.kang@intel.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 55 ++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 9aa2006dbe04..22e05e3fb0a3 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1057,6 +1057,12 @@ static unsigned long segment_base(u16 selector)
 }
 #endif
 
+static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
+{
+	return (pt_mode == PT_MODE_HOST_GUEST) &&
+	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
+}
+
 static inline void pt_load_msr(struct pt_ctx *ctx, u32 addr_range)
 {
 	u32 i;
@@ -2110,47 +2116,48 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		pt_update_intercept_for_msr(vmx);
 		break;
 	case MSR_IA32_RTIT_STATUS:
-		if ((pt_mode != PT_MODE_HOST_GUEST) ||
-			(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) ||
-			(data & MSR_IA32_RTIT_STATUS_MASK))
+		if (!pt_can_write_msr(vmx))
+			return 1;
+		if (data & MSR_IA32_RTIT_STATUS_MASK)
 			return 1;
 		vmx->pt_desc.guest.status = data;
 		break;
 	case MSR_IA32_RTIT_CR3_MATCH:
-		if ((pt_mode != PT_MODE_HOST_GUEST) ||
-			(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) ||
-			!intel_pt_validate_cap(vmx->pt_desc.caps,
-						PT_CAP_cr3_filtering))
+		if (!pt_can_write_msr(vmx))
+			return 1;
+		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
+					   PT_CAP_cr3_filtering))
 			return 1;
 		vmx->pt_desc.guest.cr3_match = data;
 		break;
 	case MSR_IA32_RTIT_OUTPUT_BASE:
-		if ((pt_mode != PT_MODE_HOST_GUEST) ||
-			(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) ||
-			(!intel_pt_validate_cap(vmx->pt_desc.caps,
-					PT_CAP_topa_output) &&
-			 !intel_pt_validate_cap(vmx->pt_desc.caps,
-					PT_CAP_single_range_output)) ||
-			(data & MSR_IA32_RTIT_OUTPUT_BASE_MASK))
+		if (!pt_can_write_msr(vmx))
+			return 1;
+		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
+					   PT_CAP_topa_output) &&
+		    !intel_pt_validate_cap(vmx->pt_desc.caps,
+					   PT_CAP_single_range_output))
+			return 1;
+		if (data & MSR_IA32_RTIT_OUTPUT_BASE_MASK)
 			return 1;
 		vmx->pt_desc.guest.output_base = data;
 		break;
 	case MSR_IA32_RTIT_OUTPUT_MASK:
-		if ((pt_mode != PT_MODE_HOST_GUEST) ||
-			(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) ||
-			(!intel_pt_validate_cap(vmx->pt_desc.caps,
-					PT_CAP_topa_output) &&
-			 !intel_pt_validate_cap(vmx->pt_desc.caps,
-					PT_CAP_single_range_output)))
+		if (!pt_can_write_msr(vmx))
+			return 1;
+		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
+					   PT_CAP_topa_output) &&
+		    !intel_pt_validate_cap(vmx->pt_desc.caps,
+					   PT_CAP_single_range_output))
 			return 1;
 		vmx->pt_desc.guest.output_mask = data;
 		break;
 	case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
+		if (!pt_can_write_msr(vmx))
+			return 1;
 		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
-		if ((pt_mode != PT_MODE_HOST_GUEST) ||
-			(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) ||
-			(index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
-					PT_CAP_num_address_ranges)))
+		if (index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
+						       PT_CAP_num_address_ranges))
 			return 1;
 		if (is_noncanonical_address(data, vcpu))
 			return 1;
-- 
2.24.0


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

* Re: [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup
  2019-12-10 23:24 [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Sean Christopherson
  2019-12-10 23:24 ` [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs Sean Christopherson
  2019-12-10 23:24 ` [PATCH 2/2] KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic Sean Christopherson
@ 2020-01-15 17:59 ` Paolo Bonzini
  2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2020-01-15 17:59 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel, Chao Peng, Luwei Kang

On 11/12/19 00:24, Sean Christopherson wrote:
> Add a missing non-canonical check on writes to the RTIT address MSRs
> and tack on a cleanup patch.
> 
> ** ALL PATCHES ARE COMPLETELY UNTESTED **
> 
> Untested due to lack of hardware.
> 
> Sean Christopherson (2):
>   KVM: VMX: Add non-canonical check on writes to RTIT address MSRs
>   KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic
> 
>  arch/x86/kvm/vmx/vmx.c | 57 ++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 24 deletions(-)
> 

Queued, thanks.

Paolo


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

end of thread, other threads:[~2020-01-15 18:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-10 23:24 [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Sean Christopherson
2019-12-10 23:24 ` [PATCH 1/2] KVM: VMX: Add non-canonical check on writes to RTIT address MSRs Sean Christopherson
2019-12-11  2:16   ` Kang, Luwei
2019-12-11 16:14     ` Sean Christopherson
2019-12-10 23:24 ` [PATCH 2/2] KVM: VMX: Add helper to consolidate up PT/RTIT WRMSR fault logic Sean Christopherson
2020-01-15 17:59 ` [PATCH 0/2] KVM: VMX: PT (RTIT) bug fix and cleanup Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.