linux-kernel - Re: [PATCH 1/2] KVM: VMX: replace 0x180 with EPT_VIOLATION

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: <Yj0IBGTu45TZkqGi@google.com>
Date:   Fri, 25 Mar 2022 00:08:36 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     SU Hang <darcy.sh@...group.com>
Cc:     kvm@...r.kernel.org,
        赖江山 <jiangshan.ljs@...group.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] KVM: VMX:  replace 0x180 with EPT_VIOLATION_*
 definition

On Mon, Mar 21, 2022, SU Hang wrote:
> Using self-expressing macro definition EPT_VIOLATION_GVA_VALIDATION
> and EPT_VIOLATION_GVA_TRANSLATED instead of 0x180
> in FNAME(walk_addr_generic)().
> 
> Signed-off-by: SU Hang <darcy.sh@...group.com>
> ---
>  arch/x86/include/asm/vmx.h     | 2 ++
>  arch/x86/kvm/mmu/paging_tmpl.h | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 0ffaa3156a4e..a6789fe9b56e 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -546,6 +546,7 @@ enum vm_entry_failure_code {
>  #define EPT_VIOLATION_READABLE_BIT	3
>  #define EPT_VIOLATION_WRITABLE_BIT	4
>  #define EPT_VIOLATION_EXECUTABLE_BIT	5
> +#define EPT_VIOLATION_GVA_VALIDATION_BIT 7

VALIDATION isn't quite right, EPT_VIOLATION_GVA_IS_VALID is more appropriate.
VALIDATION makes it sound like the CPU has does some form of validation on the GVA.

>  #define EPT_VIOLATION_GVA_TRANSLATED_BIT 8
>  #define EPT_VIOLATION_ACC_READ		(1 << EPT_VIOLATION_ACC_READ_BIT)
>  #define EPT_VIOLATION_ACC_WRITE		(1 << EPT_VIOLATION_ACC_WRITE_BIT)
> @@ -553,6 +554,7 @@ enum vm_entry_failure_code {
>  #define EPT_VIOLATION_READABLE		(1 << EPT_VIOLATION_READABLE_BIT)
>  #define EPT_VIOLATION_WRITABLE		(1 << EPT_VIOLATION_WRITABLE_BIT)
>  #define EPT_VIOLATION_EXECUTABLE	(1 << EPT_VIOLATION_EXECUTABLE_BIT)
> +#define EPT_VIOLATION_GVA_VALIDATION	(1 << EPT_VIOLATION_GVA_VALIDATION_BIT)
>  #define EPT_VIOLATION_GVA_TRANSLATED	(1 << EPT_VIOLATION_GVA_TRANSLATED_BIT)
>  
>  /*
> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 95367f5ca998..7853c7ef13a1 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -523,7 +523,8 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>  	 * The other bits are set to 0.
>  	 */
>  	if (!(errcode & PFERR_RSVD_MASK)) {
> -		vcpu->arch.exit_qualification &= 0x180;
> +		vcpu->arch.exit_qualification &= (EPT_VIOLATION_GVA_VALIDATION
> +			| EPT_VIOLATION_GVA_TRANSLATED);

Please put the | before the newline, and align the stuff inside the parantheses.
That makes it must easier to see what the code is doing at a glance.

		vcpu->arch.exit_qualification &= (EPT_VIOLATION_GVA_IS_VALID |
						  EPT_VIOLATION_GVA_TRANSLATED);

>  		if (write_fault)
>  			vcpu->arch.exit_qualification |= EPT_VIOLATION_ACC_WRITE;
>  		if (user_fault)
> -- 
> 2.32.0.3.g01195cf9f
>