Messages in this thread | | | Subject | Re: [PATCH 1/2] KVM: MMU: fix ept=0/pte.u=0/pte.w=0/CR0.WP=0/CR4.SMEP=1/EFER.NX=0 combo | From | Paolo Bonzini <> | Date | Thu, 10 Mar 2016 11:09:27 +0100 |
| |
On 10/03/2016 09:27, Xiao Guangrong wrote: >> > >> + if (!enable_ept) { >> + guest_efer |= EFER_NX; >> + ignore_bits |= EFER_NX; > > Update ignore_bits is not necessary i think.
More precisely, ignore_bits is only needed if guest EFER.NX=0 and we're not in this CR0.WP=1/CR4.SMEP=0 situation. In theory you could have guest EFER.NX=1 and host EFER.NX=0.
This is what I came up with (plus some comments :)):
u64 guest_efer = vmx->vcpu.arch.efer; u64 ignore_bits = 0;
if (!enable_ept) { if (boot_cpu_has(X86_FEATURE_SMEP)) guest_efer |= EFER_NX; else if (!(guest_efer & EFER_NX)) ignore_bits |= EFER_NX; }
>> - guest_efer = vmx->vcpu.arch.efer; >> if (!(guest_efer & EFER_LMA)) >> guest_efer &= ~EFER_LME; >> if (guest_efer != host_efer) >> add_atomic_switch_msr(vmx, MSR_EFER, >> guest_efer, host_efer); > > So, why not set EFER_NX (if !ept) just in this branch to make the fix > more simpler?
I didn't like having
guest_efer = vmx->vcpu.arch.efer; ... if (!enable_ept) guest_efer |= EFER_NX; guest_efer &= ~ignore_bits; guest_efer |= host_efer & ignore_bits; ... if (...) { guest_efer = vmx->vcpu.arch.efer; if (!enable_ept) guest_efer |= EFER_NX; ... }
My patch is bigger but the resulting code is smaller and easier to follow:
guest_efer = vmx->vcpu.arch.efer; if (!enable_ept) guest_efer |= EFER_NX; ... if (...) { ... } else { guest_efer &= ~ignore_bits; guest_efer |= host_efer & ignore_bits; }
Paolo
| |