Re: [PATCH v7 01/15] nEPT: Support LOAD_IA32_EFER entry/exit controls for L1

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2960 bytes --]

On 2013-08-05 13:40, Gleb Natapov wrote:
> On Mon, Aug 05, 2013 at 07:27:33PM +0800, Arthur Chunqi Li wrote:
>> On Mon, Aug 5, 2013 at 4:07 PM, Gleb Natapov <gleb@redhat.com> wrote:
>>> From: Nadav Har'El <nyh@il.ibm.com>
>>>
>>> Recent KVM, since http://kerneltrap.org/mailarchive/linux-kvm/2010/5/2/6261577
>>> switch the EFER MSR when EPT is used and the host and guest have different
>>> NX bits. So if we add support for nested EPT (L1 guest using EPT to run L2)
>>> and want to be able to run recent KVM as L1, we need to allow L1 to use this
>>> EFER switching feature.
>>>
>>> To do this EFER switching, KVM uses VM_ENTRY/EXIT_LOAD_IA32_EFER if available,
>>> and if it isn't, it uses the generic VM_ENTRY/EXIT_MSR_LOAD. This patch adds
>>> support for the former (the latter is still unsupported).
>>>
>>> Nested entry and exit emulation (prepare_vmcs_02 and load_vmcs12_host_state,
>>> respectively) already handled VM_ENTRY/EXIT_LOAD_IA32_EFER correctly. So all
>>> that's left to do in this patch is to properly advertise this feature to L1.
>>>
>>> Note that vmcs12's VM_ENTRY/EXIT_LOAD_IA32_EFER are emulated by L0, by using
>>> vmx_set_efer (which itself sets one of several vmcs02 fields), so we always
>>> support this feature, regardless of whether the host supports it.
>>>
>>> Reviewed-by: Orit Wasserman <owasserm@redhat.com>
>>> Signed-off-by: Nadav Har'El <nyh@il.ibm.com>
>>> Signed-off-by: Jun Nakajima <jun.nakajima@intel.com>
>>> Signed-off-by: Xinhao Xu <xinhao.xu@intel.com>
>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>>> Signed-off-by: Gleb Natapov <gleb@redhat.com>
>>> ---
>>>  arch/x86/kvm/vmx.c |   23 ++++++++++++++++-------
>>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> index e999dc7..27efa6a 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -2198,7 +2198,8 @@ static __init void nested_vmx_setup_ctls_msrs(void)
>>>  #else
>>>         nested_vmx_exit_ctls_high = 0;
>>>  #endif
>>> -       nested_vmx_exit_ctls_high |= VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;
>>> +       nested_vmx_exit_ctls_high |= (VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
>>> +                                     VM_EXIT_LOAD_IA32_EFER);
>> Gleb, why we don't need to check whether host supports
>> VM_EXIT_LOAD_IA32_EFER here, as what you noted in my
>> VM_EXIT_LOAD_IA32_PAT patch?
> Nested VMX completely emulates the capability.

No, it doesn't. The values for host/guest are handled over via the
corresponding VMCS fields, physically, even though the actual loading is
emulated then. So we must not expose this feature unconditionally.
That's the same pattern as with PAT, Arthur is right.

I've a patch on top of v7 to fix this (and also the incorrect
VM_ENTRY_IA32E_MODE reporting on 32-bit). Will post it soon as part of
an updated version of my unrestricted guest mode series.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]