linux-kernel - Re: [PATCH V4 1/2] x86/tdx: Eliminate duplicate code in tdx_clear

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] [day] [month] [year] [list]

Message-ID: <0b2a23d7c30e91e47cddd3fc7ef911249c5d8531.camel@intel.com>
Date: Wed, 23 Jul 2025 23:56:45 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>, "Hunter, Adrian"
	<adrian.hunter@...el.com>, "Annapurve, Vishal" <vannapurve@...gle.com>,
	"Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"seanjc@...gle.com" <seanjc@...gle.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "Li, Xiaoyao"
	<xiaoyao.li@...el.com>, "Luck, Tony" <tony.luck@...el.com>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "binbin.wu@...ux.intel.com"
	<binbin.wu@...ux.intel.com>, "Chatre, Reinette" <reinette.chatre@...el.com>,
	"kas@...nel.org" <kas@...nel.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"Yamahata, Isaku" <isaku.yamahata@...el.com>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>, "mingo@...hat.com" <mingo@...hat.com>,
	"hpa@...or.com" <hpa@...or.com>, "bp@...en8.de" <bp@...en8.de>, "Gao, Chao"
	<chao.gao@...el.com>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH V4 1/2] x86/tdx: Eliminate duplicate code in
 tdx_clear_page()

On Wed, 2025-07-23 at 23:26 +0000, Edgecombe, Rick P wrote:
> On Wed, 2025-07-23 at 23:01 +0000, Huang, Kai wrote:
> > Such renaming goes a little bit far IMHO.
> > 
> 
> I agree it's not quite necessary churn.
> 
> >   I respect the value of having
> > "quirk" in the name, but it also seems quite reasonable to me to hide such
> > "quirk" at the last level but just having "reset TDX pages" concept in the
> > higher levels.
> 
> Assuming all the comments get corrected, this still leaves "reset" as an
> operation that sometimes eagerly resets the page, or sometimes leaves it to be
> lazily done later by a random access. 
> 

Thanks for the point.

Yeah I agree it's better to convey such information in the function name.


> Maybe instead of reset which is an action
> that sometimes is skipped, something that says what state we want the page to be
> at the end - ready to use.
> 
> tdx_make_page_ready()
> tdx_make_page_usable()
> ...or something in that direction.
> 
> But this is still churn. Kai, what do you think about the other option of just
> putting the X86_BUG_TDX_PW_MCE in tdx_reset_page() and letting the
> initialization error path (tdmrs_reset_pamt_all()) keep always zeroing the
> pages. So:
> 
> static void tdx_reset_paddr(unsigned long base, unsigned long size)
> {
> 	/* doing MOVDIR64B ... */
> }
> 
> static void tdmr_reset_pamt(struct tdmr_info *tdmr)
> {
> 	tdmr_do_pamt_func(tdmr, tdx_reset_paddr);
> }
> 
> void tdx_quirk_reset_page(struct page *page)
> {
> 	if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> 		return;
> 
> 	tdx_reset_paddr(page_to_phys(page), PAGE_SIZE);
> }
> EXPORT_SYMBOL_GPL(tdx_reset_page);

I don't think it's good idea to treat PAMT and other types of TDX memory
differently.  I would rather go with the renaming as shown in Adrian's
patch.

So no objection from me. :-)