| Message ID | 20191027152323.24326-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: vmx, svm: always run with EFER.NXE=1 when shadow paging is active | expand |
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?
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
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...
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
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.
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(-)