Re: [PATCH v3 3/5] x86/tdx: Add tdx_mcall_extend_rtmr() interface

["Xing, Cedric" <cedric.xing@intel.com>]

On 4/9/2025 12:10 AM, Dan Williams wrote:
> Cedric Xing wrote:
>> The TDX guest exposes one MRTD (Build-time Measurement Register) and four
>> RTMR (Run-time Measurement Register) registers to record the build and boot
>> measurements of a virtual machine (VM). These registers are similar to PCR
>> (Platform Configuration Register) registers in the TPM (Trusted Platform
>> Module) space. This measurement data is used to implement security features
>> like attestation and trusted boot.
>>
>> To facilitate updating the RTMR registers, the TDX module provides support
>> for the `TDG.MR.RTMR.EXTEND` TDCALL which can be used to securely extend
>> the RTMR registers.
>>
>> Add helper function to update RTMR registers. It will be used by the TDX
>> guest driver in enabling RTMR extension support.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> Typically Signed-off-by without Co-developed-by means that the patch was
> submitted upstream be Sathya, so did you also intend to add a
> Co-developed-by or should this solo tag just be Reviewed-by?
> 
I did modify slightly this commit from Sathya. I could be wrong but was 
told that "Signed-off-by:" was necessary to certify I had the authority 
to submit this patch. scripts/checkpatch.py complained about the 
co-existence of "Co-developed-by" and "Signed-off-by". So I had to keep 
the "Signed-off-by" tag only.

>> Signed-off-by: Cedric Xing <cedric.xing@intel.com>
>> ---
>>   arch/x86/coco/tdx/tdx.c           | 36 ++++++++++++++++++++++++++++++++++++
>>   arch/x86/include/asm/shared/tdx.h |  1 +
>>   arch/x86/include/asm/tdx.h        |  2 ++
>>   3 files changed, 39 insertions(+)
>>
>> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
>> index edab6d6049be..b042ca74bcd3 100644
>> --- a/arch/x86/coco/tdx/tdx.c
>> +++ b/arch/x86/coco/tdx/tdx.c
>> @@ -36,6 +36,7 @@
>>   /* TDX Module call error codes */
>>   #define TDCALL_RETURN_CODE(a)	((a) >> 32)
>>   #define TDCALL_INVALID_OPERAND	0xc0000100
>> +#define TDCALL_OPERAND_BUSY	0x80000200
>>   
>>   #define TDREPORT_SUBTYPE_0	0
>>   
>> @@ -136,6 +137,41 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
>>   }
>>   EXPORT_SYMBOL_GPL(tdx_mcall_get_report0);
>>   
>> +/**
>> + * tdx_mcall_extend_rtmr() - Wrapper to extend RTMR registers using
>> + *			     TDG.MR.RTMR.EXTEND TDCALL.
>> + * @index: Index of RTMR register to be extended.
>> + * @data: Address of the input buffer with RTMR register extend data.
>> + *
>> + * Refer to section titled "TDG.MR.RTMR.EXTEND leaf" in the TDX Module
>> + * v1.0 specification for more information on TDG.MR.RTMR.EXTEND TDCALL.
>> + * It is used in the TDX guest driver module to allow user extend the
>> + * RTMR registers (index > 1).
>> + *
>> + * Return 0 on success, -EINVAL for invalid operands, -EBUSY for busy
>> + * operation or -EIO on other TDCALL failures.
>> + */
>> +int tdx_mcall_extend_rtmr(u8 index, u8 *data)
>> +{
>> +	struct tdx_module_args args = {
>> +		.rcx = virt_to_phys(data),
>> +		.rdx = index,
>> +	};
>> +	u64 ret;
>> +
>> +	ret = __tdcall(TDG_MR_RTMR_EXTEND, &args);
>> +	if (ret) {
>> +		if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
>> +			return -EINVAL;
> 
> Here is where the ABI documentation can help to make sure that userspace
> can tell the difference between userspace bugs, kernel bugs, or TDX
> internal errors. So perhaps translate this EINVAL to
> ENXIO in tsm-mr.c. Otherwise, this patch looks good to me:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> 
Agreed. I'll change -EINVAL to -ENXIO as this would be due to the 
inability to convert VA to GPA.

> [..]