All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active
@ 2019-10-27 15:23 Paolo Bonzini
  2019-10-28  6:59 ` Sasha Levin
  2019-10-29  9:32 ` Joerg Roedel
  0 siblings, 2 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-10-27 15:23 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: stable, Joerg Roedel

VMX already does so if the host has SMEP, in order to support the combination of
CR0.WP=1 and CR4.SMEP=1.  However, it is perfectly safe to always do so, and in
fact VMX already ends up running with EFER.NXE=1 on old processors that lack the
"load EFER" controls, because it may help avoiding a slow MSR write.  Removing
all the conditionals simplifies the code.

SVM does not have similar code, but it should since recent AMD processors do
support SMEP.  So this patch also makes the code for the two vendors more similar
while fixing NPT=0, CR0.WP=1 and CR4.SMEP=1 on AMD processors.

Cc: stable@vger.kernel.org
Cc: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/svm.c     | 10 ++++++++--
 arch/x86/kvm/vmx/vmx.c | 14 +++-----------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index b6feb6a11a8d..2c452293c7cc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -732,8 +732,14 @@ static int get_npt_level(struct kvm_vcpu *vcpu)
 static void svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
 {
 	vcpu->arch.efer = efer;
-	if (!npt_enabled && !(efer & EFER_LMA))
-		efer &= ~EFER_LME;
+
+	if (!npt_enabled) {
+		/* Shadow paging assumes NX to be available.  */
+		efer |= EFER_NX;
+
+		if (!(efer & EFER_LMA))
+			efer &= ~EFER_LME;
+	}
 
 	to_svm(vcpu)->vmcb->save.efer = efer | EFER_SVME;
 	mark_dirty(to_svm(vcpu)->vmcb, VMCB_CR);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2a2ba277c676..e191d41afb34 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -896,17 +896,9 @@ static bool update_transition_efer(struct vcpu_vmx *vmx, int efer_offset)
 	u64 guest_efer = vmx->vcpu.arch.efer;
 	u64 ignore_bits = 0;
 
-	if (!enable_ept) {
-		/*
-		 * NX is needed to handle CR0.WP=1, CR4.SMEP=1.  Testing
-		 * host CPUID is more efficient than testing guest CPUID
-		 * or CR4.  Host SMEP is anyway a requirement for guest SMEP.
-		 */
-		if (boot_cpu_has(X86_FEATURE_SMEP))
-			guest_efer |= EFER_NX;
-		else if (!(guest_efer & EFER_NX))
-			ignore_bits |= EFER_NX;
-	}
+	/* Shadow paging assumes NX to be available.  */
+	if (!enable_ept)
+		guest_efer |= EFER_NX;
 
 	/*
 	 * LMA and LME handled by hardware; SCE meaningless outside long mode.
-- 
2.21.0


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

* Re: [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active
  2019-10-27 15:23 [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active Paolo Bonzini
@ 2019-10-28  6:59 ` Sasha Levin
  2019-10-28  7:56   ` Paolo Bonzini
  2019-10-29  9:32 ` Joerg Roedel
  1 sibling, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2019-10-28  6:59 UTC (permalink / raw)
  To: Sasha Levin, Paolo Bonzini, linux-kernel, kvm
  Cc: stable, Joerg Roedel, stable, Joerg Roedel, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.

v5.3.7: Build OK!
v4.19.80: Failed to apply! Possible dependencies:
    Unable to calculate

v4.14.150: Failed to apply! Possible dependencies:
    Unable to calculate

v4.9.197: Failed to apply! Possible dependencies:
    Unable to calculate

v4.4.197: Failed to apply! Possible dependencies:
    Unable to calculate


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks,
Sasha

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

* Re: [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active
  2019-10-28  6:59 ` Sasha Levin
@ 2019-10-28  7:56   ` Paolo Bonzini
  2019-10-28  8:02     ` Sasha Levin
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2019-10-28  7:56 UTC (permalink / raw)
  To: Sasha Levin, linux-kernel, kvm; +Cc: stable, Joerg Roedel

On 28/10/19 07:59, Sasha Levin wrote:
> 
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.
> 
> v5.3.7: Build OK!
> v4.19.80: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> v4.14.150: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> v4.9.197: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> v4.4.197: Failed to apply! Possible dependencies:
>     Unable to calculate
> 
> 
> NOTE: The patch will not be queued to stable trees until it is upstream.
> 
> How should we proceed with this patch?

It should apply just fine to all branches, just to arch/x86/kvm/vmx.c
instead of arch/x86/kvm/vmx/vmx.c.

Paolo


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

* Re: [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active
  2019-10-28  7:56   ` Paolo Bonzini
@ 2019-10-28  8:02     ` Sasha Levin
  0 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2019-10-28  8:02 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, stable, Joerg Roedel

On Mon, Oct 28, 2019 at 08:56:26AM +0100, Paolo Bonzini wrote:
>On 28/10/19 07:59, Sasha Levin wrote:
>>
>> This commit has been processed because it contains a -stable tag.
>> The stable tag indicates that it's relevant for the following trees: all
>>
>> The bot has tested the following trees: v5.3.7, v4.19.80, v4.14.150, v4.9.197, v4.4.197.
>>
>> v5.3.7: Build OK!
>> v4.19.80: Failed to apply! Possible dependencies:
>>     Unable to calculate
>>
>> v4.14.150: Failed to apply! Possible dependencies:
>>     Unable to calculate
>>
>> v4.9.197: Failed to apply! Possible dependencies:
>>     Unable to calculate
>>
>> v4.4.197: Failed to apply! Possible dependencies:
>>     Unable to calculate
>>
>>
>> NOTE: The patch will not be queued to stable trees until it is upstream.
>>
>> How should we proceed with this patch?
>
>It should apply just fine to all branches, just to arch/x86/kvm/vmx.c
>instead of arch/x86/kvm/vmx/vmx.c.

I wonder if we should be carrying symlinks in the stable branches to
make this smoother...

-- 
Thanks,
Sasha

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

* Re: [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active
  2019-10-27 15:23 [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active Paolo Bonzini
  2019-10-28  6:59 ` Sasha Levin
@ 2019-10-29  9:32 ` Joerg Roedel
  1 sibling, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2019-10-29  9:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, stable

Hi Paolo,

On Sun, Oct 27, 2019 at 04:23:23PM +0100, Paolo Bonzini wrote:
> VMX already does so if the host has SMEP, in order to support the combination of
> CR0.WP=1 and CR4.SMEP=1.  However, it is perfectly safe to always do so, and in
> fact VMX already ends up running with EFER.NXE=1 on old processors that lack the
> "load EFER" controls, because it may help avoiding a slow MSR write.  Removing
> all the conditionals simplifies the code.
> 
> SVM does not have similar code, but it should since recent AMD processors do
> support SMEP.  So this patch also makes the code for the two vendors more similar
> while fixing NPT=0, CR0.WP=1 and CR4.SMEP=1 on AMD processors.
> 
> Cc: stable@vger.kernel.org
> Cc: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good to me.

Reviewed-by: Joerg Roedel <jroedel@suse.de>

> ---
>  arch/x86/kvm/svm.c     | 10 ++++++++--
>  arch/x86/kvm/vmx/vmx.c | 14 +++-----------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index b6feb6a11a8d..2c452293c7cc 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -732,8 +732,14 @@ static int get_npt_level(struct kvm_vcpu *vcpu)
>  static void svm_set_efer(struct kvm_vcpu *vcpu, u64 efer)
>  {
>  	vcpu->arch.efer = efer;
> -	if (!npt_enabled && !(efer & EFER_LMA))
> -		efer &= ~EFER_LME;
> +
> +	if (!npt_enabled) {
> +		/* Shadow paging assumes NX to be available.  */
> +		efer |= EFER_NX;
> +
> +		if (!(efer & EFER_LMA))
> +			efer &= ~EFER_LME;
> +	}
>  
>  	to_svm(vcpu)->vmcb->save.efer = efer | EFER_SVME;
>  	mark_dirty(to_svm(vcpu)->vmcb, VMCB_CR);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 2a2ba277c676..e191d41afb34 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -896,17 +896,9 @@ static bool update_transition_efer(struct vcpu_vmx *vmx, int efer_offset)
>  	u64 guest_efer = vmx->vcpu.arch.efer;
>  	u64 ignore_bits = 0;
>  
> -	if (!enable_ept) {
> -		/*
> -		 * NX is needed to handle CR0.WP=1, CR4.SMEP=1.  Testing
> -		 * host CPUID is more efficient than testing guest CPUID
> -		 * or CR4.  Host SMEP is anyway a requirement for guest SMEP.
> -		 */
> -		if (boot_cpu_has(X86_FEATURE_SMEP))
> -			guest_efer |= EFER_NX;
> -		else if (!(guest_efer & EFER_NX))
> -			ignore_bits |= EFER_NX;
> -	}
> +	/* Shadow paging assumes NX to be available.  */
> +	if (!enable_ept)
> +		guest_efer |= EFER_NX;
>  
>  	/*
>  	 * LMA and LME handled by hardware; SCE meaningless outside long mode.
> -- 
> 2.21.0

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

end of thread, other threads:[~2019-10-29  9:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-27 15:23 [PATCH] KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active Paolo Bonzini
2019-10-28  6:59 ` Sasha Levin
2019-10-28  7:56   ` Paolo Bonzini
2019-10-28  8:02     ` Sasha Levin
2019-10-29  9:32 ` Joerg Roedel

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.