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: <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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ