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

[Xing, Cedric]

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@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> > ---
> >   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@xxxxxxxxx>
Agreed. I'll change -EINVAL to -ENXIO as this would be due to the 
inability to convert VA to GPA.

> [..]