Re: [PATCH v1 07/26] x86/virt/tdx: Read TDX global metadata for TDX Module Extensions

Xu Yilun
2025-11-18T13:14:52Z

On Mon, Nov 17, 2025 at 08:52:36AM -0800, Dave Hansen wrote:
> On 11/16/25 18:22, Xu Yilun wrote:
> > +static __init int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> > +{
> > + int ret = 0;
> > + u64 val;
> > +
> > + if (!ret && !(ret = read_sys_metadata_field(0x3100000100000000, &val)))
> > + sysinfo_ext->memory_pool_required_pages = val;
> > + if (!ret && !(ret = read_sys_metadata_field(0x3100000100000001, &val)))
> > + sysinfo_ext->ext_required = val;
> > +
> > + return ret;
> > +}
>
> These were OK-ish when they were being generated by a script.
>
> Now that they're being generated by and edited by humans, they
> need to actually be readable.

I agree. Further more, let me figure out if we could require minimum
boilerplate code when a new field is added.

>
> Can we please get this down to something that looks more like:
>
> MACRO(&sysinfo_ext->memory_pool_required_pages, 0x3100000100000000);
> MACRO(&sysinfo_ext->ext_required, 0x3100000100000001);
>
> You can generate code in that macro, or generate a struct like
> this:
>
> static __init int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> {
> int ret = 0;
> struct tdx_metadata_init[] = {
> MACRO(&sysinfo_ext->memory_pool_required_pages, 0x3100000100000000),
> MACRO(&sysinfo_ext->ext_required, 0x3100000100000001),
> {},
> };
>
> return tdx_...(sysinfo_ext, tdx_metadata_init);
> }
>
> and have the helper parse the structure.
>
> But, either way, the method that's being proposed here needs to go.

I'll try and may need a seperate refactoring patch for the existing
code.