All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Sean Christopherson <seanjc@google.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
Subject: Re: [PATCH v2 5/7] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
Date: Mon, 18 Oct 2021 14:46:06 +0200	[thread overview]
Message-ID: <265883f8-83a4-6386-fa56-b53b4897f5e1@redhat.com> (raw)
In-Reply-To: <20210827070249.924633-6-xiaoyao.li@intel.com>

On 27/08/21 09:02, Xiaoyao Li wrote:
> Per SDM, it triggers #GP for all the accessing of PT MSRs, if
> X86_FEATURE_INTEL_PT is not available.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> Changes in v2:
>   - allow userspace/host access regradless of PT bit, (Sean)
> ---
>   arch/x86/kvm/vmx/vmx.c | 38 +++++++++++++++++++++++++-------------
>   1 file changed, 25 insertions(+), 13 deletions(-)

Let's cache this in vmx->pt_desc.  More precisely:

- always call update_intel_pt_cfg from vmx_vcpu_after_set_cpuid

- add a field vmx->pt_desc.available matching guest_cpuid_has(vcpu, 
X86_FEATURE_INTEL_PT)

- if it is false, clear _all_ of vmx->pt_desc (with memcpy) and return 
early from update_intel_pt_cfg

Thanks,

Paolo

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index b9d640029c40..394ef4732838 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1007,10 +1007,21 @@ static unsigned long segment_base(u16 selector)
>   }
>   #endif
>   
> -static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
> +static inline bool pt_can_write_msr(struct vcpu_vmx *vmx,
> +				    struct msr_data *msr_info)
>   {
>   	return vmx_pt_mode_is_host_guest() &&
> -	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
> +	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN) &&
> +	       (msr_info->host_initiated ||
> +		guest_cpuid_has(&vmx->vcpu, X86_FEATURE_INTEL_PT));
> +}
> +
> +static inline bool pt_can_read_msr(struct kvm_vcpu *vcpu,
> +				   struct msr_data *msr_info)
> +{
> +	return vmx_pt_mode_is_host_guest() &&
> +	       (msr_info->host_initiated ||
> +		guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT));
>   }
>   
>   static inline bool pt_output_base_valid(struct kvm_vcpu *vcpu, u64 base)
> @@ -1852,24 +1863,24 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   							&msr_info->data);
>   		break;
>   	case MSR_IA32_RTIT_CTL:
> -		if (!vmx_pt_mode_is_host_guest())
> +		if (!pt_can_read_msr(vcpu, msr_info))
>   			return 1;
>   		msr_info->data = vmx->pt_desc.guest.ctl;
>   		break;
>   	case MSR_IA32_RTIT_STATUS:
> -		if (!vmx_pt_mode_is_host_guest())
> +		if (!pt_can_read_msr(vcpu, msr_info))
>   			return 1;
>   		msr_info->data = vmx->pt_desc.guest.status;
>   		break;
>   	case MSR_IA32_RTIT_CR3_MATCH:
> -		if (!vmx_pt_mode_is_host_guest() ||
> +		if (!pt_can_read_msr(vcpu, msr_info) ||
>   			!intel_pt_validate_cap(vmx->pt_desc.caps,
>   						PT_CAP_cr3_filtering))
>   			return 1;
>   		msr_info->data = vmx->pt_desc.guest.cr3_match;
>   		break;
>   	case MSR_IA32_RTIT_OUTPUT_BASE:
> -		if (!vmx_pt_mode_is_host_guest() ||
> +		if (!pt_can_read_msr(vcpu, msr_info) ||
>   			(!intel_pt_validate_cap(vmx->pt_desc.caps,
>   					PT_CAP_topa_output) &&
>   			 !intel_pt_validate_cap(vmx->pt_desc.caps,
> @@ -1878,7 +1889,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		msr_info->data = vmx->pt_desc.guest.output_base;
>   		break;
>   	case MSR_IA32_RTIT_OUTPUT_MASK:
> -		if (!vmx_pt_mode_is_host_guest() ||
> +		if (!pt_can_read_msr(vcpu, msr_info) ||
>   			(!intel_pt_validate_cap(vmx->pt_desc.caps,
>   					PT_CAP_topa_output) &&
>   			 !intel_pt_validate_cap(vmx->pt_desc.caps,
> @@ -1888,7 +1899,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		break;
>   	case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
>   		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
> -		if (!vmx_pt_mode_is_host_guest() ||
> +		if (!pt_can_read_msr(vcpu, msr_info) ||
>   		    (index >= 2 * vmx->pt_desc.nr_addr_ranges))
>   			return 1;
>   		if (index % 2)
> @@ -2156,6 +2167,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		return vmx_set_vmx_msr(vcpu, msr_index, data);
>   	case MSR_IA32_RTIT_CTL:
>   		if (!vmx_pt_mode_is_host_guest() ||
> +		    !guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT) ||
>   			vmx_rtit_ctl_check(vcpu, data) ||
>   			vmx->nested.vmxon)
>   			return 1;
> @@ -2164,14 +2176,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		pt_update_intercept_for_msr(vcpu);
>   		break;
>   	case MSR_IA32_RTIT_STATUS:
> -		if (!pt_can_write_msr(vmx))
> +		if (!pt_can_write_msr(vmx, msr_info))
>   			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_can_write_msr(vmx))
> +		if (!pt_can_write_msr(vmx, msr_info))
>   			return 1;
>   		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
>   					   PT_CAP_cr3_filtering))
> @@ -2179,7 +2191,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		vmx->pt_desc.guest.cr3_match = data;
>   		break;
>   	case MSR_IA32_RTIT_OUTPUT_BASE:
> -		if (!pt_can_write_msr(vmx))
> +		if (!pt_can_write_msr(vmx, msr_info))
>   			return 1;
>   		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
>   					   PT_CAP_topa_output) &&
> @@ -2191,7 +2203,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		vmx->pt_desc.guest.output_base = data;
>   		break;
>   	case MSR_IA32_RTIT_OUTPUT_MASK:
> -		if (!pt_can_write_msr(vmx))
> +		if (!pt_can_write_msr(vmx, msr_info))
>   			return 1;
>   		if (!intel_pt_validate_cap(vmx->pt_desc.caps,
>   					   PT_CAP_topa_output) &&
> @@ -2201,7 +2213,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		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))
> +		if (!pt_can_write_msr(vmx, msr_info))
>   			return 1;
>   		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
>   		if (index >= 2 * vmx->pt_desc.nr_addr_ranges)
> 


  reply	other threads:[~2021-10-18 12:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-27  7:02 [PATCH v2 0/7] KVM: VMX: PT (processor trace) optimization cleanup and fixes Xiaoyao Li
2021-08-27  7:02 ` [PATCH v2 1/7] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
2021-08-27  7:02 ` [PATCH v2 2/7] KVM: VMX: Use precomputed vmx->pt_desc.addr_range Xiaoyao Li
2021-08-27  7:02 ` [PATCH v2 3/7] KVM: VMX: Rename pt_desc.addr_range to pt_desc.nr_addr_range Xiaoyao Li
2021-10-18 12:41   ` Paolo Bonzini
2021-08-27  7:02 ` [PATCH v2 4/7] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit Xiaoyao Li
2021-08-27  7:02 ` [PATCH v2 5/7] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest Xiaoyao Li
2021-10-18 12:46   ` Paolo Bonzini [this message]
2021-08-27  7:02 ` [PATCH v2 6/7] KVM: VMX: Check Intel PT related CPUID leaves Xiaoyao Li
2021-09-09 21:41   ` Sean Christopherson
2021-09-10  1:59     ` Xiaoyao Li
2021-10-18  7:01       ` Xiaoyao Li
2021-10-18 12:47       ` Paolo Bonzini
2021-10-18 13:56         ` Xiaoyao Li
2021-10-18 17:26           ` Sean Christopherson
2021-10-19  1:46             ` Xiaoyao Li
2021-08-27  7:02 ` [PATCH v2 7/7] KVM: VMX: Only context switch some PT MSRs when they exist Xiaoyao Li
2021-10-18 13:08   ` Paolo Bonzini
2021-10-18 14:04     ` Xiaoyao Li
2021-10-18 15:20       ` Paolo Bonzini
2021-10-19 16:52 ` [PATCH v2 0/7] KVM: VMX: PT (processor trace) optimization cleanup and fixes Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=265883f8-83a4-6386-fa56-b53b4897f5e1@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=xiaoyao.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.