linux-kernel - Re: [PATCH 04/21] KVM: x86/mmu: Allow non-zero value for non-present SPTE and removed SPTE

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]

Date: Tue, 12 Mar 2024 12:26:35 +1300
From: "Huang, Kai" <kai.huang@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>, <linux-kernel@...r.kernel.org>,
	<kvm@...r.kernel.org>
CC: <seanjc@...gle.com>, <michael.roth@....com>, <isaku.yamahata@...el.com>,
	<thomas.lendacky@....com>, Binbin Wu <binbin.wu@...ux.intel.com>
Subject: Re: [PATCH 04/21] KVM: x86/mmu: Allow non-zero value for non-present
 SPTE and removed SPTE

>   
> +/*
> + * Non-present SPTE value for both VMX and SVM for TDP MMU.

In the previous patch, SHADOW_NONPRESENT_VALUE is also used in the 
shadow MMU code.  So here when you change SHADOW_NONPRESENT_VALUE to a 
non-zero value, the "for TDP MMU" part doesn't stand.

I am wondering whether we can just avoid using SHADOW_NONPRESENT_VALUE 
in shadow MMU code in the previous patch, and state explicitly that we 
are only going to support TDP MMU for non-zero value for non-present SPTE?

> + * For SVM NPT, for non-present spte (bit 0 = 0), other bits are ignored.
> + * For VMX EPT, bit 63 is ignored if #VE is disabled. (EPT_VIOLATION_VE=0)
> + *              bit 63 is #VE suppress if #VE is enabled. (EPT_VIOLATION_VE=1)
> + * For TDX:
> + *   TDX module sets EPT_VIOLATION_VE for Secure-EPT and conventional EPT
> + */
> +#ifdef CONFIG_X86_64
> +#define SHADOW_NONPRESENT_VALUE	BIT_ULL(63)
> +static_assert(!(SHADOW_NONPRESENT_VALUE & SPTE_MMU_PRESENT_MASK));
> +#else
>   #define SHADOW_NONPRESENT_VALUE	0ULL
> +#endif
>   
>   extern u64 __read_mostly shadow_host_writable_mask;
>   extern u64 __read_mostly shadow_mmu_writable_mask;
> @@ -196,7 +209,7 @@ extern u64 __read_mostly shadow_nonpresent_or_rsvd_mask;
>    *
>    * Only used by the TDP MMU.
>    */
> -#define REMOVED_SPTE	0x5a0ULL
> +#define REMOVED_SPTE	(SHADOW_NONPRESENT_VALUE | 0x5a0ULL)

I kinda prefer moving this chunk to the previous patch, because the 
reason to have SHADOW_NONPRESENT_VALUE is to have a non-zero value for 
non-present SPTEs, which include the REMOVED_SPTE.

But just my 2cents.