linux-kernel - Re: [PATCH v3 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT

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: <850f7ce0571cb54bc984c79861bdfd104e097eb9.camel@intel.com>
Date: Mon, 6 Oct 2025 19:34:03 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-coco@...ts.linux.dev"
	<linux-coco@...ts.linux.dev>, "Li, Xiaoyao" <xiaoyao.li@...el.com>, "Huang,
 Kai" <kai.huang@...el.com>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "kas@...nel.org"
	<kas@...nel.org>, "seanjc@...gle.com" <seanjc@...gle.com>, "mingo@...hat.com"
	<mingo@...hat.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"Yamahata, Isaku" <isaku.yamahata@...el.com>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "Annapurve, Vishal" <vannapurve@...gle.com>, "Gao,
 Chao" <chao.gao@...el.com>, "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org"
	<x86@...nel.org>, "Williams, Dan J" <dan.j.williams@...el.com>
CC: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v3 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic
 PAMT

On Fri, 2025-09-26 at 14:57 -0700, Edgecombe, Rick P wrote:
> Yes, it's not symmetrical. It was probably added manually. I don't see
> why we actually need the the part that makes it unsymmetrical
> (tdx_supports_dynamic_pamt() check). In fact I think it's bad because
> it depends on specific call order of the metadata reading to be valid.
> 
> Kirill, any reason we can't drop it?
> 

Ah, actually the check is needed. The reason is that if the TDX module doesn't
support DPAMT, then a new kernel with these changes could fail to read the
metadata and, in turn, fail the call. This means the TDX module initialization
would fail, when really we want to just continue without Dynamic PAMT. I guess
as we move off of the base support, the validity of the metadata struct members
will start to depend on if the relevant feature is actually supported.

For this single conditional field it's hard to justify more then the simple
tdx_supports_dynamic_pamt() check, but if this keeps coming up we might want to
look at re-arranging the metadata such that we don't end up with scattered
checks all over.

I'll add a comment for now, and we can keep an eye on it:

	/*
	 * Don't fail here if tdx_supports_dynamic_pamt() isn't supported. The
	 * TDX code can fallback to normal PAMT if it's not supported.
	 */
	if (!ret && tdx_supports_dynamic_pamt(&tdx_sysinfo) &&
	    !(ret = read_sys_metadata_field(0x9100000100000013, &val)))
		sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;

So the resulting logic is, if the module has the 'features0' bit set, then read
the field. If reading the field fails somehow, then fail the TDX module
initialization. We could also continue under normal PAMT, but then we would have
to make sure DPAMT wasn't tried if we allowed the initialization to continue
here. Let's keep it simpler.