kvm: svm: Intercept RDPRU
diff mbox series

Message ID 20190919225917.36641-1-jmattson@google.com
State New
Headers show
Series
  • kvm: svm: Intercept RDPRU
Related show

Commit Message

Jim Mattson Sept. 19, 2019, 10:59 p.m. UTC
The RDPRU instruction gives the guest read access to the IA32_APERF
MSR and the IA32_MPERF MSR. According to volume 3 of the APM, "When
virtualization is enabled, this instruction can be intercepted by the
Hypervisor. The intercept bit is at VMCB byte offset 10h, bit 14."
Since we don't enumerate the instruction in KVM_SUPPORTED_CPUID,
intercept it and synthesize #UD.

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Drew Schmitt <dasch@google.com>
Reviewed-by: Jacob Xu <jacobhxu@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 arch/x86/include/asm/svm.h      | 1 +
 arch/x86/include/uapi/asm/svm.h | 1 +
 arch/x86/kvm/svm.c              | 8 ++++++++
 3 files changed, 10 insertions(+)

Comments

Krish Sadhukhan Sept. 20, 2019, 8:44 p.m. UTC | #1
On 9/19/19 3:59 PM, Jim Mattson wrote:
> The RDPRU instruction gives the guest read access to the IA32_APERF
> MSR and the IA32_MPERF MSR. According to volume 3 of the APM, "When
> virtualization is enabled, this instruction can be intercepted by the
> Hypervisor. The intercept bit is at VMCB byte offset 10h, bit 14."
> Since we don't enumerate the instruction in KVM_SUPPORTED_CPUID,
> intercept it and synthesize #UD.
>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Drew Schmitt <dasch@google.com>
> Reviewed-by: Jacob Xu <jacobhxu@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
>   arch/x86/include/asm/svm.h      | 1 +
>   arch/x86/include/uapi/asm/svm.h | 1 +
>   arch/x86/kvm/svm.c              | 8 ++++++++
>   3 files changed, 10 insertions(+)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index dec9c1e84c78..6ece8561ba66 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -52,6 +52,7 @@ enum {
>   	INTERCEPT_MWAIT,
>   	INTERCEPT_MWAIT_COND,
>   	INTERCEPT_XSETBV,
> +	INTERCEPT_RDPRU,
>   };
>   
>   
> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
> index a9731f8a480f..2e8a30f06c74 100644
> --- a/arch/x86/include/uapi/asm/svm.h
> +++ b/arch/x86/include/uapi/asm/svm.h
> @@ -75,6 +75,7 @@
>   #define SVM_EXIT_MWAIT         0x08b
>   #define SVM_EXIT_MWAIT_COND    0x08c
>   #define SVM_EXIT_XSETBV        0x08d
> +#define SVM_EXIT_RDPRU         0x08e
>   #define SVM_EXIT_NPF           0x400
>   #define SVM_EXIT_AVIC_INCOMPLETE_IPI		0x401
>   #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS	0x402
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 04fe21849b6e..cef00e959679 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1539,6 +1539,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>   	set_intercept(svm, INTERCEPT_SKINIT);
>   	set_intercept(svm, INTERCEPT_WBINVD);
>   	set_intercept(svm, INTERCEPT_XSETBV);
> +	set_intercept(svm, INTERCEPT_RDPRU);
>   	set_intercept(svm, INTERCEPT_RSM);
>   
>   	if (!kvm_mwait_in_guest(svm->vcpu.kvm)) {
> @@ -3830,6 +3831,12 @@ static int xsetbv_interception(struct vcpu_svm *svm)
>   	return 1;
>   }
>   
> +static int rdpru_interception(struct vcpu_svm *svm)
> +{
> +	kvm_queue_exception(&svm->vcpu, UD_VECTOR);
> +	return 1;
> +}
> +
>   static int task_switch_interception(struct vcpu_svm *svm)
>   {
>   	u16 tss_selector;
> @@ -4791,6 +4798,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>   	[SVM_EXIT_MONITOR]			= monitor_interception,
>   	[SVM_EXIT_MWAIT]			= mwait_interception,
>   	[SVM_EXIT_XSETBV]			= xsetbv_interception,
> +	[SVM_EXIT_RDPRU]			= rdpru_interception,
>   	[SVM_EXIT_NPF]				= npf_interception,
>   	[SVM_EXIT_RSM]                          = rsm_interception,
>   	[SVM_EXIT_AVIC_INCOMPLETE_IPI]		= avic_incomplete_ipi_interception,


Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Paolo Bonzini Sept. 24, 2019, 2:16 p.m. UTC | #2
On 20/09/19 00:59, Jim Mattson wrote:
> The RDPRU instruction gives the guest read access to the IA32_APERF
> MSR and the IA32_MPERF MSR. According to volume 3 of the APM, "When
> virtualization is enabled, this instruction can be intercepted by the
> Hypervisor. The intercept bit is at VMCB byte offset 10h, bit 14."
> Since we don't enumerate the instruction in KVM_SUPPORTED_CPUID,
> intercept it and synthesize #UD.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Drew Schmitt <dasch@google.com>
> Reviewed-by: Jacob Xu <jacobhxu@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>

Ugly stuff---Intel did the right thing in making the execution controls
"enable xxx" (xxx = RDRAND, RDSEED, etc.).

Queued, thanks.

Paolo

> ---
>  arch/x86/include/asm/svm.h      | 1 +
>  arch/x86/include/uapi/asm/svm.h | 1 +
>  arch/x86/kvm/svm.c              | 8 ++++++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index dec9c1e84c78..6ece8561ba66 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -52,6 +52,7 @@ enum {
>  	INTERCEPT_MWAIT,
>  	INTERCEPT_MWAIT_COND,
>  	INTERCEPT_XSETBV,
> +	INTERCEPT_RDPRU,
>  };
>  
>  
> diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
> index a9731f8a480f..2e8a30f06c74 100644
> --- a/arch/x86/include/uapi/asm/svm.h
> +++ b/arch/x86/include/uapi/asm/svm.h
> @@ -75,6 +75,7 @@
>  #define SVM_EXIT_MWAIT         0x08b
>  #define SVM_EXIT_MWAIT_COND    0x08c
>  #define SVM_EXIT_XSETBV        0x08d
> +#define SVM_EXIT_RDPRU         0x08e
>  #define SVM_EXIT_NPF           0x400
>  #define SVM_EXIT_AVIC_INCOMPLETE_IPI		0x401
>  #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS	0x402
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 04fe21849b6e..cef00e959679 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1539,6 +1539,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>  	set_intercept(svm, INTERCEPT_SKINIT);
>  	set_intercept(svm, INTERCEPT_WBINVD);
>  	set_intercept(svm, INTERCEPT_XSETBV);
> +	set_intercept(svm, INTERCEPT_RDPRU);
>  	set_intercept(svm, INTERCEPT_RSM);
>  
>  	if (!kvm_mwait_in_guest(svm->vcpu.kvm)) {
> @@ -3830,6 +3831,12 @@ static int xsetbv_interception(struct vcpu_svm *svm)
>  	return 1;
>  }
>  
> +static int rdpru_interception(struct vcpu_svm *svm)
> +{
> +	kvm_queue_exception(&svm->vcpu, UD_VECTOR);
> +	return 1;
> +}
> +
>  static int task_switch_interception(struct vcpu_svm *svm)
>  {
>  	u16 tss_selector;
> @@ -4791,6 +4798,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>  	[SVM_EXIT_MONITOR]			= monitor_interception,
>  	[SVM_EXIT_MWAIT]			= mwait_interception,
>  	[SVM_EXIT_XSETBV]			= xsetbv_interception,
> +	[SVM_EXIT_RDPRU]			= rdpru_interception,
>  	[SVM_EXIT_NPF]				= npf_interception,
>  	[SVM_EXIT_RSM]                          = rsm_interception,
>  	[SVM_EXIT_AVIC_INCOMPLETE_IPI]		= avic_incomplete_ipi_interception,
>
Jim Mattson Sept. 24, 2019, 4:53 p.m. UTC | #3
On Tue, Sep 24, 2019 at 7:16 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Ugly stuff---Intel did the right thing in making the execution controls
> "enable xxx" (xxx = RDRAND, RDSEED, etc.).

I agree. RDPRU should have been disabled by default, or at least
guarded by a new EFER bit, like MCOMMIT is.

Patch
diff mbox series

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index dec9c1e84c78..6ece8561ba66 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -52,6 +52,7 @@  enum {
 	INTERCEPT_MWAIT,
 	INTERCEPT_MWAIT_COND,
 	INTERCEPT_XSETBV,
+	INTERCEPT_RDPRU,
 };
 
 
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index a9731f8a480f..2e8a30f06c74 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -75,6 +75,7 @@ 
 #define SVM_EXIT_MWAIT         0x08b
 #define SVM_EXIT_MWAIT_COND    0x08c
 #define SVM_EXIT_XSETBV        0x08d
+#define SVM_EXIT_RDPRU         0x08e
 #define SVM_EXIT_NPF           0x400
 #define SVM_EXIT_AVIC_INCOMPLETE_IPI		0x401
 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS	0x402
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 04fe21849b6e..cef00e959679 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1539,6 +1539,7 @@  static void init_vmcb(struct vcpu_svm *svm)
 	set_intercept(svm, INTERCEPT_SKINIT);
 	set_intercept(svm, INTERCEPT_WBINVD);
 	set_intercept(svm, INTERCEPT_XSETBV);
+	set_intercept(svm, INTERCEPT_RDPRU);
 	set_intercept(svm, INTERCEPT_RSM);
 
 	if (!kvm_mwait_in_guest(svm->vcpu.kvm)) {
@@ -3830,6 +3831,12 @@  static int xsetbv_interception(struct vcpu_svm *svm)
 	return 1;
 }
 
+static int rdpru_interception(struct vcpu_svm *svm)
+{
+	kvm_queue_exception(&svm->vcpu, UD_VECTOR);
+	return 1;
+}
+
 static int task_switch_interception(struct vcpu_svm *svm)
 {
 	u16 tss_selector;
@@ -4791,6 +4798,7 @@  static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
 	[SVM_EXIT_MONITOR]			= monitor_interception,
 	[SVM_EXIT_MWAIT]			= mwait_interception,
 	[SVM_EXIT_XSETBV]			= xsetbv_interception,
+	[SVM_EXIT_RDPRU]			= rdpru_interception,
 	[SVM_EXIT_NPF]				= npf_interception,
 	[SVM_EXIT_RSM]                          = rsm_interception,
 	[SVM_EXIT_AVIC_INCOMPLETE_IPI]		= avic_incomplete_ipi_interception,