Re: [RFC PATCH 03/15] x86/tdx: Add TDG.TDI.RD module call wrapper for TDX Connect

[Edgecombe, Rick P]

On Tue, 2026-10-27 at 06:22 +0000, Duan, Zhenzhong wrote:


> 
> 
> > -----Original Message-----
> > From: Li, Xiaoyao <[xiaoyao.li@xxxxxxxxx](mailto:xiaoyao.li@xxxxxxxxx)>
> > Subject: Re: [RFC PATCH 03/15] x86/tdx: Add TDG.TDI.RD module call wrapper
> > for TDX Connect
> > 
> > On 9/24/2026 12:10 PM, Zhenzhong Duan wrote:
> > 
> > > Introduce wrapper tdx_mcall_tdi_read() to provide tdx-guest driver with a
> > > standard interface for querying specific metadata fields from a target
> > > Trust Device Interface Control Structure (TDI_CS).
> > > 
> > > Explicitly mapping architectural TDX error codes into Linux POSIX errno
> > > with two helpers, introducing tdx_mcall_to_errno() to handle general
> > > module calls, while introducing tdx_mcall_tdi_to_errno() specifically for
> > > TDI related module calls.
> > 
> > 
> > Set aside why mapping architectural TDX error codes into Linux POSIX errno
> > is
> > needed and what it brings, I think the common enabling order is one prep
> > patch
> > to introduce tdx_mcall_to_errno() and turn existing users to use it. Then
> > this
> > patch introduces tdx_mcall_tdi_to_errno() to use it as well.
> 
> 
> Ideally yes, we follow this way so that reviewers can focus on the core
> implementation and defer cleanup to a later series. We can revisit this
> if that rule changes.


...you are going to have justify the whole tdx_mcall_tdi_to_errno() thing first
as discussed in the other patch.



> 
> > 
> > 
> > > Unlike TDG.VP.VMCALL based hypercalls which communicate with the host,
> > > TDG.TDI.RD is a module call (mcall) that communicates directly with the
> > > TDX module. This distinction is reflected in the tdx_mcall_ naming
> > > prefix, as opposed to tdx_hcall_ used for host-directed hypercalls.
> > 
> > 
> > I would say this paragraph is not needed. It's not the first patch to
> > introduce
> > hcall and mcall.
> 
> 
> I think we need this brief explanation for the hcall/mcall naming distinction.
> Not all are familiar with TDX and know what hcall/mcall means.
> I'm open on this, if more people think no need, I'll remove this paragraph.

I think it's ok. Maybe make it clearer that this is explaining the pre-existing
naming schemes (which I think are a bit confusing since they all go through the
module).

> 
> 
> > 
> > 
> > > Note that while host-bound hypercalls (such as tdx_hcall_tdcm()) pass a
> > > platform-specific Device Identifier (devid), the func_id parameter used
> > > here is defined by the TDISP standard (refer to Section 11.2 "TDISP
> > > rules"). Although these values result in the same underlying bit format
> > > when the PCI segment is zero, they represent separate structural
> > > abstractions in their respective specifications.
> > 
> > 
> > This paragraph seems to provide help but it doesn't look to help (at least
> > for
> > me). Because until now there are only the helper defined but no user, I
> > still
> > don't know what specific values will be passed to them.
> 
> 
> In next version, we will pass in a pci_dev pointer to address Rick's comments,
> so you will see what specific values passed to mcall/hcall, e.g,
> 
> +int tdx_mcall_tdi_read(struct pci_dev *dev, u64 field, u64 *value)
> +{
> +       struct tdx_module_args args = {
> +               .rcx = pci_dev_id(dev),
> +               .rdx = field,
> +       };
> 
> So we can drop this paragraph in next version.
> 
> 
> > 
> > 
> > > Signed-off-by: Zhenzhong Duan
<[zhenzhong.duan@xxxxxxxxx](mailto:zhenzhong.duan@xxxxxxxxx)>
> > > ---
> > >  arch/x86/coco/tdx/tdx.c           |  5 ----
> > >  arch/x86/coco/tdx/tdx_connect.c   | 41 +++++++++++++++++++++++++++++++
> > >  arch/x86/include/asm/shared/tdx.h |  1 +
> > >  arch/x86/include/asm/tdx.h        | 20 +++++++++++++++
> > >  4 files changed, 62 insertions(+), 5 deletions(-)
> > > 
> > 
> > 
> > ...
> > 
> > 
> > > diff --git a/arch/x86/include/asm/shared/tdx.h
> > 
> > b/arch/x86/include/asm/shared/tdx.h
> > 
> > > index f46a3171a512..665c12925ff6 100644
> > > --- a/arch/x86/include/asm/shared/tdx.h
> > > +++ b/arch/x86/include/asm/shared/tdx.h
> > > @@ -20,6 +20,7 @@
> > >  #define TDG_MEM_PAGE_ACCEPT 6
> > >  #define TDG_VM_RD 7
> > >  #define TDG_VM_WR 8
> > > +#define TDG_TDI_READ 67
> > 
> > 
> > Please name it TDG_TDI_RD to keep it consistent with TDG_VM_RD
> 
> 
> Sure.
> 
Let's match the specs for these for easier doc reference. So yea TDG.TDI.RD becomes TDG_TDI_RD.