Re: [PATCH v7] x86/tdx: Dump TDX Version During TD Bootup

[Huang, Kai]

> +/*
> + * Detect TDX Module version info from TDG.SYS.RD TDCALL
> + */
> +static void detect_tdx_version(void)
> +{
> +	struct tdx_module_args args = {};
> +	u16 major_version, minor_version;
> +	u32 vendor_id;
> +	u64 ret;
> +
> +	args.rdx = TDX_SYS_VENDOR_ID_FID;
> +	ret = __tdcall_ret(TDG_SYS_RD, &args);
> +	if (ret)
> +		goto err_out;

I am not sure we need to detect vendor ID?  I believe TDX must be from Intel?

Also, I think either "err" or "out" is sufficient?

> +
> +	vendor_id = args.r8;
> +
> +	memset(&args, 0, sizeof(args));
> +	args.rdx = TDX_SYS_MAJOR_FID;
> +	ret = __tdcall_ret(TDG_SYS_RD, &args);
> +	if (ret)
> +		goto err_out;
> +
> +	major_version = args.r8;
> +
> +	memset(&args, 0, sizeof(args));
> +	args.rdx = TDX_SYS_MINOR_FID;
> +	ret = __tdcall_ret(TDG_SYS_RD, &args);
> +	if (ret)
> +		goto err_out;
> +
> +	minor_version = args.r8;
> +
> +	pr_info("Guest detected. version:%u.%u VendorID:%x\n",
> +		major_version, minor_version, vendor_id);
> +
> +	return;
> +
> +err_out:
> +	if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
> +		pr_info("TDG.SYS.RD not available\n");
> +	else
> +		pr_info("TDG.SYS.RD unknown error (%llu), reading field %llu\n",
> +			ret, args.rdx);
> +
> +	pr_info("Assuming TDX version:1.x (x<5) VendorID:%x\n",
> +		TDX_VENDOR_INTEL);

You removed "Guest detected" in tdx_early_init(), but didn't add it here, so
unfortunately the "Guest detected" will get lost here.

Also, I suppose for the case TDG.SYS.RD isn't supported there's still some value
to print "Assume module version: 1.x (x<5)", but when TDG.SYS.RD failed
unexpectedly I don't think we can make that assumption, and printing "assuming
..." could give user misinformation which is worse than not printing IMHO.

So, I think you can keep the printing of "Guest detected" in tdx_early_init(),
and somehow refine the code to print something like below:

For module that does TDG.SYS.RD successfully:

  tdx: Guest detected
  tdx: Module version: %u.%u

For module that doesn't support TDG.SYS.RD:

  tdx: Guest detected
  tdx: Failed to get module version: TDG.SYS.RD unsupported. Assume module
version: 1.x (x < 5).

For module that TDG.SYS.RD failed unexpectedly:

  tdx: Guest detected
  tdx: Failed to get module version: TDG.SYS.RD failed to read 0x%llu: 0x%llu