arch/x86/virt/vmx/tdx/tdx.c | 222 +++++++++++++++++++- arch/x86/virt/vmx/tdx/tdx.h | 54 +++++ arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 -------------- 3 files changed, 275 insertions(+), 155 deletions(-) delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c
This series cleans up the TDX global metadata code. It has two goals:
1. Replace the generated code with a table-driven metadata reader.
2. Make the existing code easier to read and maintain, and simplify
adding new metadata fields.
During the v1 review, Dave raised concerns about signing off on
AI-generated code. I have since rewritten the affected patches based on
my own understanding of the code.
The main goal of this RFC is to agree on whether the table-driven reader is
the right replacement for the generated code. Please raise any concerns or
alternative design ideas.
Feedback on the patch organization is also welcome. In v2, each patch
converts one metadata reader. This should make the individual changes
easier to review, but results in more patches and some repetition in their
changelogs. I am not sure whether this is the best organization for the
series.
Dave, please feel free to ignore this RFC. Kirill, Rick, and other TDX
developers, please take a look.
Changes since v1:
=================
- Reimplemented the code and rewrote the changelogs for patches
generated entirely by AI. (Dave)
- Expanded the problem statement with the history of earlier TDX metadata
proposals (Rick)
- Dropped the false claim that literal u64 field IDs are unreviewable
(Dave)
- Rebase onto tip/x86/tdx and convert the metadata added by DPAMT.
- Name field IDs as their readers are converted instead of naming them
in a separate patch.
- Convert one metadata reader per patch.
- v1: https://lore.kernel.org/all/20260804112941.19894-1-chao.gao@intel.com/
The TDX module reports its capabilities and limits through a set of global
metadata fields, each read using a 64-bit field ID via TDH.SYS.RD. The
fields are grouped into classes, and the kernel mirrors each class it
needs in a sub-structure of struct tdx_sys_info.
Both those structures and the code that fills them were generated by an
out-of-tree script from a JSON file.
The script was not the first approach. Kai's first attempt paired each
field ID with its destination C member in a table and walked the table in a
loop to read every field. Two pieces of feedback on it drove everything
that followed [1]:
1. Compile-time type checking. Metadata fields have different sizes (u16
and u64), so a common helper takes a void * and a size instead of
a typed destination.
2. The check that a field ID's encoded size matches its destination
member ran at runtime, although both sizes are known at build time.
The discussion did not converge after several rounds of review. Dave noted
that, despite the void *, the size check provides the safety that matters:
it catches mismatched field and member widths [2]. That left one problem:
moving the size check from runtime to build time.
Before that was settled, the direction shifted to generating the code with a
script. At the time, the TDX ABI definitions were published as JSON, so
checking a field ID required consulting a machine-readable file by hand.
The script parsed the JSON and generated both the structures and their
readers [3]. Generation also made the size/type check unnecessary. The
field IDs and destination members came from the same input, so a mismatch
could only result from a bug in the script, which is less likely than a
mistake in hand-written code.
Dave concluded that the JSON experiment had failed [4] for two reasons:
1. The JSON file is not stable. The CPUID config arrays here were once
sized for a maximum of 32 entries, which has since increased to 128 [5].
2. The JSON file is not authoritative enough to write code from by itself.
TDX ABI definitions are now available in human-readable PDF specifications
[6]. So, stop relying on the out-of-tree script and maintain the code by
hand.
Yilun later proposed a single table whose entries carry the offset and size
of each mapped member in struct tdx_sys_info [7]. That design does not
cover the new handoff metadata because it is not cached in
struct tdx_sys_info.
This series gives every class its own table: each entry pairs a field ID
with the member that holds it, and a loop walks the table. This also covers
handoff metadata, which is read into a local structure rather than into
struct tdx_sys_info.
The common reader still takes a void *, but each mapping verifies at build
time that the destination member size matches the size encoded in the field
ID. A field/member width mismatch therefore fails the build.
AI usage
========
I used LLM tools to review the patches and refine the wording of the cover
letter and changelogs from my drafts. I reviewed all suggestions and adopted
those I agreed with. For example, AI review suggested the
read_sys_metadata_table() macro, which avoids repeating the table name when
passing both the table and its size.
Testing
=======
Built each patch individually and successfully launched TDs.
[1]: https://lore.kernel.org/kvm/66b16121c48f4_4fc729424@dwillia2-xfh.jf.intel.com.notmuch/
[2]: https://lore.kernel.org/kvm/c3b1e743-6d34-49ce-8e60-a41038f27c61@intel.com/
[3]: https://lore.kernel.org/kvm/f25673ea-08c5-474b-a841-095656820b67@intel.com/
[4]: https://lore.kernel.org/kvm/1e7bcbad-eb26-44b7-97ca-88ab53467212@intel.com/
[5]: https://lore.kernel.org/lkml/55f97ca1-8f32-4e33-96fb-d82ed2109f9a@intel.com/
[6]: https://www.intel.com/content/www/us/en/content-details/865803/abi-definitions-for-intel-tdx.html
[7]: https://lore.kernel.org/kvm/20251202050844.2520762-4-yilun.xu@linux.intel.com/
Chao Gao (10):
x86/virt/tdx: Add a helper to read a table of metadata fields
x86/virt/tdx: Convert the version metadata reader
x86/virt/tdx: Convert the features metadata reader
x86/virt/tdx: Convert the tdmr metadata reader
x86/virt/tdx: Convert the td_ctrl metadata reader
x86/virt/tdx: Convert the handoff metadata reader
x86/virt/tdx: Convert the td_conf metadata reader
x86/virt/tdx: Remove tdx_global_metadata.c
x86/virt/tdx: Use early returns in get_tdx_sys_info()
x86/virt/tdx: Verify structure member sizes against metadata field IDs
arch/x86/virt/vmx/tdx/tdx.c | 222 +++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 54 +++++
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 --------------
3 files changed, 275 insertions(+), 155 deletions(-)
delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c
--
2.52.0
On Fri, Sep 18, 2026 at 06:29:19AM -0700, Chao Gao wrote: > This series cleans up the TDX global metadata code. It has two goals: > > 1. Replace the generated code with a table-driven metadata reader. > > 2. Make the existing code easier to read and maintain, and simplify > adding new metadata fields. > > During the v1 review, Dave raised concerns about signing off on > AI-generated code. I have since rewritten the affected patches based on > my own understanding of the code. > > The main goal of this RFC is to agree on whether the table-driven reader is > the right replacement for the generated code. Please raise any concerns or > alternative design ideas. For alternatives, I wonder if grouping the metadata would help in the long run. Some of TDX metadata needs to be cached. Some of the metadata is init time data for how much memory to allocate etc. Some of the metadata needs to be re-read after TDX module update. Should we have the table-driven metadata reader use the TDX module metadata directly by default instead of always copying the data? That could be used for the init time data. And data could still be cached for CPUIDs etc.
On Wed, Sep 23, 2026 at 08:44:04AM +0300, Tony Lindgren wrote: >On Fri, Sep 18, 2026 at 06:29:19AM -0700, Chao Gao wrote: >> This series cleans up the TDX global metadata code. It has two goals: >> >> 1. Replace the generated code with a table-driven metadata reader. >> >> 2. Make the existing code easier to read and maintain, and simplify >> adding new metadata fields. >> >> During the v1 review, Dave raised concerns about signing off on >> AI-generated code. I have since rewritten the affected patches based on >> my own understanding of the code. >> >> The main goal of this RFC is to agree on whether the table-driven reader is >> the right replacement for the generated code. Please raise any concerns or >> alternative design ideas. > >For alternatives, I wonder if grouping the metadata would help in the long >run. Some of TDX metadata needs to be cached. Some of the metadata is init >time data for how much memory to allocate etc. Some of the metadata needs >to be re-read after TDX module update. I agree that TDX metadata should be cached only when there is a reason to. And yes, some init-time data is cached even though nothing reads it after init: max_tdmrs, max_reserved_per_tdmr and the pamt_*_entry_size fields are only needed while building the TDMRs. Only the version fields are re-read after a module update. Splitting them out of struct tdx_sys_info would be a good improvement, so the rest of the structure could then be __ro_after_init. These are cleanups that can be done in a separate series. > >Should we have the table-driven metadata reader use the TDX module >metadata directly by default instead of always copying the data? That >could be used for the init time data. And data could still be cached for >CPUIDs etc. The table-driven reader doesn't impose caching. It maps field IDs to members of whatever structure the caller passes, so the destination can be a cached global or a stack local. For init-time data, we can read it into a stack local the same way the handoff class is read in tdx_module_shutdown().
On Wed, Sep 23, 2026 at 04:19:12PM +0800, Chao Gao wrote: > On Wed, Sep 23, 2026 at 08:44:04AM +0300, Tony Lindgren wrote: > >On Fri, Sep 18, 2026 at 06:29:19AM -0700, Chao Gao wrote: > >> This series cleans up the TDX global metadata code. It has two goals: > >> > >> 1. Replace the generated code with a table-driven metadata reader. > >> > >> 2. Make the existing code easier to read and maintain, and simplify > >> adding new metadata fields. > >> > >> During the v1 review, Dave raised concerns about signing off on > >> AI-generated code. I have since rewritten the affected patches based on > >> my own understanding of the code. > >> > >> The main goal of this RFC is to agree on whether the table-driven reader is > >> the right replacement for the generated code. Please raise any concerns or > >> alternative design ideas. > > > >For alternatives, I wonder if grouping the metadata would help in the long > >run. Some of TDX metadata needs to be cached. Some of the metadata is init > >time data for how much memory to allocate etc. Some of the metadata needs > >to be re-read after TDX module update. > > I agree that TDX metadata should be cached only when there is a reason to. > > And yes, some init-time data is cached even though nothing reads it after > init: max_tdmrs, max_reserved_per_tdmr and the pamt_*_entry_size fields are > only needed while building the TDMRs. > > Only the version fields are re-read after a module update. Splitting them > out of struct tdx_sys_info would be a good improvement, so the rest of the > structure could then be __ro_after_init. > > These are cleanups that can be done in a separate series. Yes OK agreed. > >Should we have the table-driven metadata reader use the TDX module > >metadata directly by default instead of always copying the data? That > >could be used for the init time data. And data could still be cached for > >CPUIDs etc. > > The table-driven reader doesn't impose caching. It maps field IDs to > members of whatever structure the caller passes, so the destination can be > a cached global or a stack local. For init-time data, we can read it into a > stack local the same way the handoff class is read in tdx_module_shutdown(). OK thanks, nice to hear you have already considered the next steps. Might be worth updating the cover letter for the follow up options too.
On Fri, 2026-09-18 at 06:29 -0700, Chao Gao wrote: > Chao Gao (10): > x86/virt/tdx: Add a helper to read a table of metadata fields > x86/virt/tdx: Convert the version metadata reader > x86/virt/tdx: Convert the features metadata reader > x86/virt/tdx: Convert the tdmr metadata reader > x86/virt/tdx: Convert the td_ctrl metadata reader > x86/virt/tdx: Convert the handoff metadata reader > x86/virt/tdx: Convert the td_conf metadata reader > x86/virt/tdx: Remove tdx_global_metadata.c > x86/virt/tdx: Use early returns in get_tdx_sys_info() > x86/virt/tdx: Verify structure member sizes against metadata field IDs > > arch/x86/virt/vmx/tdx/tdx.c | 222 +++++++++++++++++++- > arch/x86/virt/vmx/tdx/tdx.h | 54 +++++ > arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 -------------- > 3 files changed, 275 insertions(+), 155 deletions(-) > delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c > > -- > 2.52.0 What is the base commit?
On Wed, Sep 23, 2026 at 08:47:13AM +0800, Edgecombe, Rick P wrote:
>On Fri, 2026-09-18 at 06:29 -0700, Chao Gao wrote:
>> Chao Gao (10):
>> x86/virt/tdx: Add a helper to read a table of metadata fields
>> x86/virt/tdx: Convert the version metadata reader
>> x86/virt/tdx: Convert the features metadata reader
>> x86/virt/tdx: Convert the tdmr metadata reader
>> x86/virt/tdx: Convert the td_ctrl metadata reader
>> x86/virt/tdx: Convert the handoff metadata reader
>> x86/virt/tdx: Convert the td_conf metadata reader
>> x86/virt/tdx: Remove tdx_global_metadata.c
>> x86/virt/tdx: Use early returns in get_tdx_sys_info()
>> x86/virt/tdx: Verify structure member sizes against metadata field IDs
>>
>> arch/x86/virt/vmx/tdx/tdx.c | 222 +++++++++++++++++++-
>> arch/x86/virt/vmx/tdx/tdx.h | 54 +++++
>> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 154 --------------
>> 3 files changed, 275 insertions(+), 155 deletions(-)
>> delete mode 100644 arch/x86/virt/vmx/tdx/tdx_global_metadata.c
>>
>> --
>> 2.52.0
>
>What is the base commit?
Commit e61c837817ac ("Documentation/x86: Add documentation for TDX's Dynamic PAMT")
from the tip/x86/tdx branch.
© 2016 - 2026 Red Hat, Inc.