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

Message-ID: <8da9c9c9c53707ec805ccd1b7f8091081e3455e6.camel@intel.com>
Date: Wed, 23 Jul 2025 23:26:35 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>, "Hunter, Adrian"
	<adrian.hunter@...el.com>, "Annapurve, Vishal" <vannapurve@...gle.com>,
	"Huang, Kai" <kai.huang@...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>, "Zhao, Yan Y" <yan.y.zhao@...el.com>, "Luck, Tony"
	<tony.luck@...el.com>, "kas@...nel.org" <kas@...nel.org>,
	"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>, "Chatre, Reinette"
	<reinette.chatre@...el.com>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>, "mingo@...hat.com" <mingo@...hat.com>,
	"Yamahata, Isaku" <isaku.yamahata@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"hpa@...or.com" <hpa@...or.com>, "Gao, Chao" <chao.gao@...el.com>,
	"bp@...en8.de" <bp@...en8.de>, "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: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. 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);


> 
> E.g.,:
> 
> static void tdx_quirk_reset_paddr(unsigned long base, unsigned long size)
> {
> 	/* doing MOVDIR64B ... */
> }
> 
> static void tdx_reset_paddr(unsigned long base, unsigned long size)
> {
> 	if (!boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> 		return;
> 
> 	tdx_quirk_reset_paddr(base, size);
> }
> 
> void tdx_reset_page(struct page *page)
> {
> 	tdx_reset_paddr(page_to_phys(page), PAGE_SIZE);
> }
> EXPORT_SYMBOL_GPL(tdx_reset_page);