Re: [PATCH v4 04/16] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT

[Xu Yilun <yilun.xu@linux.intel.com>]

On Thu, Jan 08, 2026 at 04:52:10PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-01-08 at 20:53 +0800, Xu Yilun wrote:
> > I actually don't understand why a RDALL seamcall could eliminate
> > the check "if (some_optional_feature_exists) read_it;". IIUC, The
> > check
> > exists because kernel doesn't trust TDX Module so kernel wants to
> > verify
> > the correctness/consistency of the data, otherwise we could accept
> > whatever TDX Module tells us, do the below for each field:
> > 
> >   static int read_sys_metadata_field(u64 field_id, u64 *data)
> >   {
> > 	...
> > 	ret = seamcall(TDH_SYS_RD, &args);
> > 	if (ret == TDX_SUCCESS) {
> > 		*data = args.r8;
> > 		return 0;
> > 	}
> > 
> > 	/* The field doesn't exist */
> > 	if (ret == TDX_METADATA_FIELD_ID_INCORRECT) {
> > 		*data = 0;
> > 		return 0;
> > 	}
> > 
> > 	...
> > 
> > 	/* Real reading error */
> > 	return -EFAULT;
> >   }
> > 
> > The trustness doesn't change no matter how kernel retrieves these
> > data,
> > by a series of RD or a RDALL.
> 
> Having it be field specific behavior (like the diff I posted) means we
> don't need to worry about TDX module bugs where some field read fails
> and we don't catch it.

We still need this specific behavior when we bulk read. Can you accept
a successfully returned blob with TDX_FEATURES0_DYNAMIC_PAMT set but
pamt_page_bitmap_entry_bits field missing? I assume no, so we still
need:

	if (blob->tdx_feature0 & TDX_FEATURES0_DYNAMIC_PAMT)
		check blob->pamt_page_bitmap_entry_bits validity;

BTW: ret == TDX_METADATA_FIELD_ID_INCORRECT is not something bad, TDX
Module is effectively telling us the field doesn't exist.

> 
> By RDALL, I mean a simpler way to bulk read the metadata. So for future

I understand. But as I mentioned that has nothing to do with the
optional feature checking.

> looking changes, let's think about what we need and not try to find yet
> more clever ways to code around the current interface. The amount of

I don't think so, cause I don't see much benifit bulk reading could
bring to us. Bulk reading basically retrieves an _unverified_ blob from
TDX Module. I believe it could also been achieved by a simple iteration
of existing single reads. The major work, data verification, is still
there.

On the other hand, the cost of a newly designed firmware interface for
an already online functionality is not low, especially when you want
backward compatibility to old TDX Module. The worst case is we keep both
sets of the code...

> code and discussion on TDX metadata reading is just too high. Please go
> back and look at the earlier threads if you haven't yet.

I've read most of the threads before my first posting on this topic.
Most of the discussion/effort focus on the effective data verification.
But I haven't found any decisive evidence that bulk reading could
contribute on it.