linux-kernel - Re: [PATCH] KVM: VMX: Reduce redundant call and simplify code

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: <ZCb8/5/anTD24FSp@google.com>
Date:   Fri, 31 Mar 2023 08:32:15 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Wenyao Hai <haiwenyao@...ontech.com>
Cc:     pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: VMX: Reduce redundant call and simplify code

Nit, the call into common code is unnecessary/superfluous, not redundant.

On Fri, Mar 31, 2023, Wenyao Hai wrote:
> Use vcpu->arch.pat = data instead of kvm_set_msr_common() to
> simplify code, avoid redundant judgements.
> 
> Signed-off-by: Wenyao Hai <haiwenyao@...ontech.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index d2d6e1b6c788..abeeea21c8ef 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2320,12 +2320,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		    get_vmcs12(vcpu)->vm_exit_controls & VM_EXIT_SAVE_IA32_PAT)
>  			get_vmcs12(vcpu)->guest_ia32_pat = data;
>  
> -		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
> +		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT)
>  			vmcs_write64(GUEST_IA32_PAT, data);
> -			vcpu->arch.pat = data;
> -			break;
> -		}
> -		ret = kvm_set_msr_common(vcpu, msr_info);
> +
> +		vcpu->arch.pat = data;

After this, I'm pretty sure the PAT path in kvm_set_msr_common() is no longer used.
And good riddance, because it's not an MTRR, and then only reason to lump it in
with MTRRs would be to zap SPTEs when the memtype changes, but KVM doesn't even
do that because update_mtrr() bails early for PAT.

I'll send a small series next week to clean up handling of PAT and the related
code in kvm_set_msr_common().  I'll include this patch and the patch from Ke Guo[*]
as the first two patches in the series.

[*] https://lore.kernel.org/all/20230329081859.2571698-1-guoke@uniontech.com