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@xxxxxxxxxxxxxxx
> Cc: Joerg Roedel <jroedel@xxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Looks good to me.
Reviewed-by: Joerg Roedel <jroedel@xxxxxxx>
> ---
> 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
