Re: [PATCH v4] x86/tdx: Dump TDX version During the TD Bootup

[Sun, Yi]

On 06.10.2023 17:48, Huang, Kai wrote:
> On Fri, 2023-10-06 at 14:28 +0800, Yi Sun wrote:
>
>
> It's better if you can maintain the code change history among different versions
> so that people can see what changes have been done:
>
> 	<your SoB>
> 	---
> 	code change history
> 	---
> 	...
>
> The "code change history" will be thrown away when the patch is applied.
I'll refine commit log and the change history.

...

> > +	td_sys->vendor_id = (u32)args.r8;
> > +
> > +	args.rdx = TDX_SYS_MAJOR_FID;
> > +	__tdcall_ret(TDX_SYS_RD, &args);
> > +
> > +	td_sys->major_version = (u16)args.r8;
> > +
> > +	args.rdx = TDX_SYS_MINOR_FID;
> > +	__tdcall_ret(TDX_SYS_RD, &args);
> > +
> > +	td_sys->minor_version = (u16)args.r8;
> > +
> > +	return;
> > +
> > +	/* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
> > +version_1_0:
> > +	td_sys->vendor_id = TDX_VENDOR_INTEL;
> > +	td_sys->major_version = 1;
> > +	td_sys->minor_version = 0;
>
> So the main reason that I think you can just print module version in this
> function instead of tdx_early_init() is because IIUC you are setting up some
> random value for TDX module 1.0.  Probably it's better to just print it's module
> 1.0 w/o mentioning major/minor.

Actually, it is not a random value here; it is the fixed value for TDX 1.0
becasue there is not such version info in TDX 1.0.
I assume we can state major=1 and minor=0 when TDX 1.0 is detected.
This will prevent confusion for the users.

> As replied above, I think you can print module info in tdg_get_sysinfo().
>
> If you do so, perhaps to rename the function to something like
> detect_tdx_version().
>
> You can either still keep
>
> 	pr_info("Guest detected\n");
>
> here, or remove it but embed it to the message when you print module version.
Get it. I'll do that.
> > diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
> > index f74695dea217..d326509832e6 100644
> > --- a/arch/x86/include/asm/shared/tdx.h
> > +++ b/arch/x86/include/asm/shared/tdx.h
> > @@ -17,6 +17,7 @@
> >  #define TDG_MR_REPORT			4
> >  #define TDG_MEM_PAGE_ACCEPT		6
> >  #define TDG_VM_WR			8
> > +#define TDX_SYS_RD			11
>
> TDG_SYS_RD, please.

Appreciate Kai's detailed comments.

Thanks
   --Sun, Yi