Re: [PATCH v1 00/26] PCI/TSM: TDX Connect: SPDM Session and IDE Establishment

dan.j.williams
2025-11-19T15:51:10Z

Dave Hansen wrote:
> Any chance we could use english in subjects? Isn't this something more
> along the lines of:
>
> PCI: Secure Device Passthrough For TDX, initial support
>
> I mean, "TDX Connect" is a pure marketing term that doesn't tell us
> much. Right?

Note that I generated that subject for this series during the RFC.
Yilun, apologies for setting you up for this feedback.

In retrospect, a better subject would be:

"PCI/TSM: PCIe Link Encryption Establishment via TDX platform services"

Most of the plain English description of this topic has gone through
multiple rounds of review in the core series to introduce and
re-introduce these acronyms [1].

This TDX series is one of many low-level architecture specific drivers,
and no, this phase of the enablement does not touch passthrough. It only
implements initial link protection details which are this acronym soup
that the PCI community at least is slowly learning to speak. It is a
pre-requisite for passthrough work.

The ordering of the enabling staging is detailed here [2].

I will add more decoder ring to that. At some point the coarse English
breaks down as the need talk about fine details picks up, i.e. specific
acronyms used by the specification need to be invoked. Granted, maybe
not in the subject.

[1]: http://lore.kernel.org/20251031212902.2256310-1-dan.j.williams@intel.com
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm.git/tree/Documentation/driver-api/pci/tsm.rst?h=staging

> I'll also say that even the beginning of the cover letter doesn't help a
> lot. Plain, acronym-free language when possible would be much
> appreciated there and across this series.
>
> For instance, there's not even a problem statement to be seen. I asked
> ChatGPT: "write me a short, three sentence plain language paragraph
> about why TDX Connect is needed on top of regular TDX"
>
> Regular TDX keeps a guest’s memory safe but leaves the path to
> physical devices exposed to the host. That means the host could
> spoof a device, alter configuration, or watch DMA traffic. TDX
> Connect closes that gap by letting the guest verify real devices
> and encrypt the I/O link so the host can’t interfere.

None of that is implemented in this series.

> That would be a great way to open a cover letter.

I think the break down is not reminding folks that this is a low-level
incremental implementation supporting core infrastructure that has
described the "first phase" problem on behalf of a class of these "TSM"
drivers that will follow. English is needed, yes, but I do not think we
need each submission to recreate an intro essay from the core
submission. Yes, we lost the link in this case, and that is my fault,
not Yilun's.

> On 11/16/25 18:22, Xu Yilun wrote:
> > arch/x86/virt/vmx/tdx/tdx.c | 740 ++++++++++++-
> > arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 32 +
>
> Let me know if anyone feels differently, but I really think the "TDX
> Host Extensions" need to be reviewed as a different patch set. Sure,
> they are a dependency for "TDX Connect". But, in the end, the host
> extension enabling does not interact at *ALL* with the later stuff. It's
> purely:

I feel differently.

> * Ask the TDX module how much memory it wants
> * Feed it with that much memory
>
> ... and voila! The fancy new extension works. Right?

Right, minor implementation detail of new ABIs. What does a "TDX
Host Extensions" patch set do that does not introduce new ABI? Linux
should not merge a patch that gives resources to the TDX Module
independent of a intent to use the ABIs.