Re: [Xen-devel] [PATCH 9/9] x86/vmx: Don't leak EFER.NXE into guest context

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/05/18 17:01, Roger Pau Monné wrote:
> On Tue, May 22, 2018 at 12:20:46PM +0100, Andrew Cooper wrote:
>> Intel hardware only uses 4 bits in MSR_EFER.  Changes to LME and LMA are
>> handled automatically via the VMENTRY_CTLS.IA32E_MODE bit.
>>
>> SCE is handled by ad-hoc logic in context_switch(), vmx_restore_guest_msrs()
>> and vmx_update_guest_efer(), and works by altering the host SCE value to 
>> match
>> the setting the guest wants.  This works because, in HVM vcpu context, Xen
>> never needs to execute a SYSCALL or SYSRET instruction.
>>
>> However, NXE has never been context switched.  Unlike SCE, NXE cannot be
>> context switched at vcpu boundaries because disabling NXE makes PTE.NX bits
>> reserved and cause a pagefault when encountered.  This means that the guest
>> always has Xen's setting in effect, irrespective of the bit it can see and
>> modify in its virtualised view of MSR_EFER.
>>
>> This isn't a major problem for production operating systems because they, 
>> like
>> Xen, always turn the NXE on when it is available.  However, it does have an
>> observable effect on which guest PTE bits are valid, and whether
>> PFEC_insn_fetch is visible in a #PF error code.
>>
>> Second generation VT-x hardware has host and guest EFER fields in the VMCS,
>> and support for loading and saving them automatically.  First generation VT-x
>> hardware needs to use MSR load/save lists to cause an atomic switch of
>> MSR_EFER on vmentry/exit.
>>
>> Therefore we update vmx_init_vmcs_config() to find and use guest/host EFER
>> support when available (and MSR load/save lists on older hardware) and drop
>> all ad-hoc alteration of SCE.
>>
>> There are two complications for shadow guests.  NXE, being a paging setting
>> needs to remain under host control, but that is fine as it is also Xen which
>> handles the pagefaults.  Also, it turns out that without EPT enabled, 
>> hardware
>> won't tolerate LME and LMA being different via either the GUEST_EFER VMCS
>> setting, or via the guest load list.  This doesn't matter in practice as we
>> intercept all writes to CR0 and reads from MSR_EFER, so can provide
>> architecturally consistent behaviour from the guests point of view.
>>
>> As a result of fixing EFER context switching, we can remove the Intel-special
>> case from hvm_nx_enabled() and let guest_walk_tables() work with the real
>> guest paging settings.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> LGTM:
>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> One question below though.
>
>> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h 
>> b/xen/include/asm-x86/hvm/vmx/vmcs.h
>> index cfd174c..6c6897c 100644
>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h
>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
>> @@ -306,6 +306,8 @@ extern u64 vmx_ept_vpid_cap;
>>      (vmx_cpu_based_exec_control & CPU_BASED_MONITOR_TRAP_FLAG)
>>  #define cpu_has_vmx_pat \
>>      (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_PAT)
>> +#define cpu_has_vmx_efer \
>> +    (vmx_vmentry_control & VM_ENTRY_LOAD_GUEST_EFER)
> Don't you also need a vmx_vmexit_control & VM_EXIT_SAVE_GUEST_EFER and
> vmx_vmexit_control & VM_EXIT_LOAD_HOST_EFER?
>
> Or can the presence of those two be inferred from
> VM_ENTRY_LOAD_GUEST_EFER?

They were introduced at the same time into hardware, so these settings
will be the same in practice.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel