linux-kernel - Re: [PATCH v2 08/25] x86/virt/tdx: Add SEAMCALL wrappers for TDX page cache management

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: <309d1c35713dd901098ae1a3d9c3c7afa62b74d3.camel@intel.com>
Date: Wed, 13 Nov 2024 20:51:12 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "pbonzini@...hat.com" <pbonzini@...hat.com>, "Hansen, Dave"
	<dave.hansen@...el.com>, "seanjc@...gle.com" <seanjc@...gle.com>
CC: "sean.j.christopherson@...el.com" <sean.j.christopherson@...el.com>, "Yao,
 Yuan" <yuan.yao@...el.com>, "Huang, Kai" <kai.huang@...el.com>,
	"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>, "Li, Xiaoyao"
	<xiaoyao.li@...el.com>, "isaku.yamahata@...il.com"
	<isaku.yamahata@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "tony.lindgren@...ux.intel.com"
	<tony.lindgren@...ux.intel.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"Zhao, Yan Y" <yan.y.zhao@...el.com>, "Chatre, Reinette"
	<reinette.chatre@...el.com>, "Yamahata, Isaku" <isaku.yamahata@...el.com>
Subject: Re: [PATCH v2 08/25] x86/virt/tdx: Add SEAMCALL wrappers for TDX page
 cache management

On Tue, 2024-11-12 at 16:20 -0800, Dave Hansen wrote:
> If this were, instead:
> 
> u64 tdh_phymem_page_reclaim(u64 page, u64 *type, u64 *owner, u64 *size)
> {
> 	...
> 	*type = args.rcx;
> 	*owner = args.rdx;
> 	*size = args.r8;
> 
> Then you wouldn't need the comment in the first place.  Then you could
> also be thinking about adding _some_ kind of type safety to the
> arguments.  The 'size' or the 'type' could totally be enums.

Yes, *rcx and *rdx stand out.

> 
> There's really zero value in having wrappers like these.  They don't
> have any type safety or add any readability or make the seamcall easier
> to use.  There's almost no value in having these versus just exporting
> seamcall_ret() itself.

Hoping to solicit some more thoughts on the value question...

I thought the main thing was to not export *all* SEAMCALLs. Future TDX modules
could add new leafs that do who-knows-what.

For this SEAMCALL wrapper, the only use of the out args is printing them in an
error message (based on other logic). So turning them into enums would just add
a layer of translation to be decoded. A developer would have to translate them
back into the registers they came from to try to extract meaning from the TDX
docs.

However, some future user of TDH.PHYMEM.PAGE.RECLAIM might want to do something
else where the enums could add code clarity. But this goes down the road of
building things that are not needed today.

Is there value in maintaining a sensible looking API to be exported, even if it
is not needed today?