Re: [PATCH v1 2/8] x86/virt/tdx: Name the TDX module global metadata field IDs

[Chao Gao]

On Tue, Aug 04, 2026 at 04:52:26PM -0700, Dave Hansen wrote:
>On 8/4/26 04:29, Chao Gao wrote:
>> From: Dave Hansen <dave@xxxxxxxx>
>> 
>> tdx_global_metadata.c currently identifies each field by a bare 64-bit hex
>> literal, extracted from the JSON file by an out-of-tree script:
>> 
>> 	if (!ret && !(ret = read_sys_metadata_field(0x9100000100000008, &val)))
>> 		sysinfo_tdmr->max_tdmrs = val;
>> 
>> That is unreviewable on its own. Verifying any one line requires the
>> reviewer to cross-reference the JSON file.
>
>I'm looking at "global_metadata.pdf" from this[1]. I see a line with
>"MAX_TDMRS" and "0x9100000100000008". That matches the lines above. That
>seems *FAR* from unreviewable. There's also no JSON in sight.
>
>What am I missing?
>
>Listen, I don't like how this turned out. I'm asking for it to be
>changed. But let's not justify it with things that just aren't true.

Sure. The only reason is to follow kernel convention for ABI constants
(for example, SEAMCALL leaf functions in the same header) and make the
use sites a bit more readable.

>
>> The hex literals in tdx_global_metadata.c are left alone because that file
>> will be replaced by a table-driven reader in the following patches.
>
>This part of the changelog is good.
>
>> No functional change intended.
>
>This is a bit much. :)

Ok. Will remove it.

>
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
>> index bdfd0e1e337a..5f567cb6c07a 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.h
>> +++ b/arch/x86/virt/vmx/tdx/tdx.h
>> @@ -58,6 +58,57 @@
>>   */
>>  #define TDX_VERSION_SHIFT		16
>>  
>> +/*
>> + * Global Scope Metadata field IDs.
>> + *
>> + * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
>> + * spec.
>> + *
>> + * A field ID is a 64-bit value that encodes the metadata "Class"
>> + * (which Linux mirrors in 'struct tdx_sys_info' sub-structures),
>> + * the element size, and a per-class field index. Each ID below
>> + * is paired with the C member that holds its value.
>> + */
>
>This is a bit verbose for my taste.

I will reduce this to:

/*
 * Global Scope Metadata field IDs.
 *
 * See "Global-Scope (TDX Module) Metadata" in the Intel TDX Module ABI
 * spec.
 */


>> +/*
>> + * Base IDs for the configurable-CPUID arrays.  The field ID of leaf
>> + * index @i is BASE + i; for the values array, sub-entry @j of index
>> + * @i is BASE + i*2 + j.
>> + */
>> +#define MD_FIELD_ID_CPUID_CONFIG_LEAVES		0x9900000300000400ULL
>> +#define MD_FIELD_ID_CPUID_CONFIG_VALUES		0x9900000300000500ULL
>
>This is complete jibberish without more context.
>

I will drop this comment and instead explain the indexing around the loop
that reads the configurable CPUID leaves and values.