linux-kernel - Re: [PATCH v3 19/25] KVM: VMX: Adjust CR3/INVPLG interception for EPT=y at runtime, not setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <085a6f661672da1e507422ea4404e144abbd5562.camel@redhat.com>
Date:   Tue, 12 Jul 2022 14:58:27 +0300
From:   Maxim Levitsky <mlevitsk@...hat.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>
Cc:     Anirudh Rayabharam <anrayabh@...ux.microsoft.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 19/25] KVM: VMX: Adjust CR3/INVPLG interception for
 EPT=y at runtime, not setup

On Fri, 2022-07-08 at 16:42 +0200, Vitaly Kuznetsov wrote:
> From: Sean Christopherson <seanjc@...gle.com>
> 
> Clear the CR3 and INVLPG interception controls at runtime based on
> whether or not EPT is being _used_, as opposed to clearing the bits at
> setup if EPT is _supported_ in hardware, and then restoring them when EPT
> is not used.  Not mucking with the base config will allow using the base
> config as the starting point for emulating the VMX capability MSRs.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> Reviewed-by: Jim Mattson <jmattson@...gle.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 9771c771c8f5..eca6875d6732 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2501,13 +2501,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>         rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP,
>                 &vmx_cap->ept, &vmx_cap->vpid);
>  
> -       if (_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_EPT) {
> -               /* CR3 accesses and invlpg don't need to cause VM Exits when EPT
> -                  enabled */
> -               _cpu_based_exec_control &= ~(CPU_BASED_CR3_LOAD_EXITING |
> -                                            CPU_BASED_CR3_STORE_EXITING |
> -                                            CPU_BASED_INVLPG_EXITING);
> -       } else if (vmx_cap->ept) {
> +       if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_EPT) &&
> +           vmx_cap->ept) {
>                 pr_warn_once("EPT CAP should not exist if not support "
>                                 "1-setting enable EPT VM-execution control\n");
>  
> @@ -4264,10 +4259,11 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
>                 exec_control |= CPU_BASED_CR8_STORE_EXITING |
>                                 CPU_BASED_CR8_LOAD_EXITING;
>  #endif
> -       if (!enable_ept)
> -               exec_control |= CPU_BASED_CR3_STORE_EXITING |
> -                               CPU_BASED_CR3_LOAD_EXITING  |
> -                               CPU_BASED_INVLPG_EXITING;
> +       /* No need to intercept CR3 access or INVPLG when using EPT. */
> +       if (enable_ept)
> +               exec_control &= ~(CPU_BASED_CR3_LOAD_EXITING |
> +                                 CPU_BASED_CR3_STORE_EXITING |
> +                                 CPU_BASED_INVLPG_EXITING);
>         if (kvm_mwait_in_guest(vmx->vcpu.kvm))
>                 exec_control &= ~(CPU_BASED_MWAIT_EXITING |
>                                 CPU_BASED_MONITOR_EXITING);


Makes sense, although the 'runtime' word a bit misleading, as we don't allow to change
'enable_ept' after kvm_intel is loaded I think.


Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>

Best regards,
	Maxim Levitsky