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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Li, Xiaoyao <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.

>
>> 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.

>
>> 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>
>> ---
>>  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.

Thanks
Zhenzhong