kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxim Levitsky <mlevitsk@redhat.com>
To: Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: 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,
	Xiaoyao Li <xiaoyao.li@intel.com>,
	Reiji Watanabe <reijiw@google.com>
Subject: Re: [PATCH 04/15] KVM: x86: Move RDPID emulation intercept to its own enum
Date: Mon, 10 May 2021 11:14:39 +0300	[thread overview]
Message-ID: <01c04a2335c913437b98e3ea874357689b097990.camel@redhat.com> (raw)
In-Reply-To: <20210504171734.1434054-5-seanjc@google.com>

On Tue, 2021-05-04 at 10:17 -0700, Sean Christopherson wrote:
> Add a dedicated intercept enum for RDPID instead of piggybacking RDTSCP.
> Unlike VMX's ENABLE_RDTSCP, RDPID is not bound to SVM's RDTSCP intercept.
> 
> Fixes: fb6d4d340e05 ("KVM: x86: emulate RDPID")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/emulate.c     | 2 +-
>  arch/x86/kvm/kvm_emulate.h | 1 +
>  arch/x86/kvm/vmx/vmx.c     | 3 ++-
>  3 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index abd9a4db11a8..8fc71e70857d 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4502,7 +4502,7 @@ static const struct opcode group8[] = {
>   * from the register case of group9.
>   */
>  static const struct gprefix pfx_0f_c7_7 = {
> -	N, N, N, II(DstMem | ModRM | Op3264 | EmulateOnUD, em_rdpid, rdtscp),
> +	N, N, N, II(DstMem | ModRM | Op3264 | EmulateOnUD, em_rdpid, rdpid),
>  };
>  
>  
> diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
> index 0d359115429a..f016838faedd 100644
> --- a/arch/x86/kvm/kvm_emulate.h
> +++ b/arch/x86/kvm/kvm_emulate.h
> @@ -468,6 +468,7 @@ enum x86_intercept {
>  	x86_intercept_clgi,
>  	x86_intercept_skinit,
>  	x86_intercept_rdtscp,
> +	x86_intercept_rdpid,
>  	x86_intercept_icebp,
>  	x86_intercept_wbinvd,
>  	x86_intercept_monitor,
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 82404ee2520e..99591e523b47 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7437,8 +7437,9 @@ static int vmx_check_intercept(struct kvm_vcpu *vcpu,
>  	/*
>  	 * RDPID causes #UD if disabled through secondary execution controls.
>  	 * Because it is marked as EmulateOnUD, we need to intercept it here.
> +	 * Note, RDPID is hidden behind ENABLE_RDTSCP.
>  	 */
> -	case x86_intercept_rdtscp:
> +	case x86_intercept_rdpid:
Shoudn't this path still handle the x86_intercept_rdtscp as I described below,
or should we remove it from the SVM side as well?

>  		if (!nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_RDTSCP)) {
>  			exception->vector = UD_VECTOR;
>  			exception->error_code_valid = false;

I have a maybe unrelated question that caught my eye:
I see this:

	DIP(SrcNone, rdtscp, check_rdtsc),

As far as I can see this means that if a nested guest executes
the rdtscp, and L1 intercepts it, then we will emulate the rdtscp by doing a nested
VM exit, but if we emulate a rdtscp for L1, we will fail since there is no .execute callback.

Is this intentional? As I understand it, at least in theory the emulator can be called
on any instruction due to things like lack of unrestricted guest, and/or emulating an
instruction on page fault (although the later is usually done by re-executing the instruction).

I know that the x86 emulator is far from being complete for such cases but I 
do wonder why rdtspc has different behavior in regard to nested and not nested case.

So this patch (since it removes the x86_intercept_rdtscp handling from the VMX),
should break the rdtscp emulation for the nested guest on VMX, although it is probably
not used anyway and should be removed.

Best regards,
	Maxim Levitsky



  parent reply	other threads:[~2021-05-10  8:14 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04 17:17 [PATCH 00/15] KVM: x86: RDPID/RDTSCP fixes and uret MSR cleanups Sean Christopherson
2021-05-04 17:17 ` [PATCH 01/15] KVM: VMX: Do not adverise RDPID if ENABLE_RDTSCP control is unsupported Sean Christopherson
2021-05-04 17:37   ` Jim Mattson
2021-05-04 17:53     ` Jim Mattson
2021-05-04 18:14       ` Sean Christopherson
2021-05-05  3:04   ` Reiji Watanabe
2021-05-10  8:03   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 02/15] KVM: x86: Emulate RDPID only if RDTSCP is supported Sean Christopherson
2021-05-04 17:50   ` Jim Mattson
2021-05-05  3:51   ` Reiji Watanabe
2021-05-05  8:01     ` Paolo Bonzini
2021-05-10  8:08   ` Maxim Levitsky
2021-05-10 17:20     ` Sean Christopherson
2021-05-11 12:32       ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 03/15] KVM: SVM: Inject #UD on RDTSCP when it should be disabled in the guest Sean Christopherson
2021-05-04 21:45   ` Jim Mattson
2021-05-04 21:53     ` Sean Christopherson
2021-05-04 21:56       ` Jim Mattson
2021-05-04 22:10         ` Sean Christopherson
2021-05-04 22:24           ` Jim Mattson
2021-05-04 21:57       ` Paolo Bonzini
2021-05-04 21:58         ` Jim Mattson
2021-05-10  8:08           ` Maxim Levitsky
2021-05-10 16:56             ` Sean Christopherson
2021-05-11 12:34               ` Maxim Levitsky
2021-05-18 10:59               ` Paolo Bonzini
2021-05-18 19:24                 ` Sean Christopherson
2021-05-05  4:26   ` Reiji Watanabe
2021-05-10  8:08   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 04/15] KVM: x86: Move RDPID emulation intercept to its own enum Sean Christopherson
2021-05-04 23:24   ` Jim Mattson
2021-05-10  8:14   ` Maxim Levitsky [this message]
2021-05-04 17:17 ` [PATCH 05/15] KVM: VMX: Disable preemption when probing user return MSRs Sean Christopherson
2021-05-04 23:36   ` Jim Mattson
2021-05-10  8:18   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 06/15] KVM: SVM: Probe and load MSR_TSC_AUX regardless of RDTSCP support in host Sean Christopherson
2021-05-10  8:20   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 07/15] KVM: x86: Add support for RDPID without RDTSCP Sean Christopherson
2021-05-10  8:20   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 08/15] KVM: VMX: Configure list of user return MSRs at module init Sean Christopherson
2021-05-10  8:23   ` Maxim Levitsky
2021-05-10 15:13     ` Sean Christopherson
2021-05-11 12:34       ` Maxim Levitsky
2021-05-11 20:10         ` Sean Christopherson
2021-05-04 17:17 ` [PATCH 09/15] KVM: VMX: Use flag to indicate "active" uret MSRs instead of sorting list Sean Christopherson
2021-05-08  3:31   ` Reiji Watanabe
2021-05-10 16:43     ` Sean Christopherson
2021-05-10 17:55       ` Reiji Watanabe
2021-05-10  8:25   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 10/15] KVM: VMX: Use common x86's uret MSR list as the one true list Sean Christopherson
2021-05-10  8:25   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 11/15] KVM: VMX: Disable loading of TSX_CTRL MSR the more conventional way Sean Christopherson
2021-05-05  8:49   ` Paolo Bonzini
2021-05-05 15:36     ` Sean Christopherson
2021-05-05 15:50       ` Paolo Bonzini
2021-05-10  8:26   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 12/15] KVM: x86: Export the number of uret MSRs to vendor modules Sean Christopherson
2021-05-10  8:27   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 13/15] KVM: x86: Move uret MSR slot management to common x86 Sean Christopherson
2021-05-10  8:28   ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 14/15] KVM: x86: Tie Intel and AMD behavior for MSR_TSC_AUX to guest CPU model Sean Christopherson
2021-05-10  8:29   ` Maxim Levitsky
2021-05-10 16:50     ` Sean Christopherson
2021-05-10 17:11       ` Jim Mattson
2021-05-11 12:34         ` Maxim Levitsky
2021-05-04 17:17 ` [PATCH 15/15] KVM: x86: Hide RDTSCP and RDPID if MSR_TSC_AUX probing failed Sean Christopherson
2021-05-10  8:29   ` Maxim Levitsky
2021-05-05  8:51 ` [PATCH 00/15] KVM: x86: RDPID/RDTSCP fixes and uret MSR cleanups 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=01c04a2335c913437b98e3ea874357689b097990.camel@redhat.com \
    --to=mlevitsk@redhat.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=reijiw@google.com \
    --cc=seanjc@google.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=xiaoyao.li@intel.com \
    --subject='Re: [PATCH 04/15] KVM: x86: Move RDPID emulation intercept to its own enum' \
    /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

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).