Re: [PATCH v4 21/24] x86/virt/tdx: Avoid updates during update-sensitive operations

[Huang, Kai]

> 
> The TDX Module offers two solutions:
> 
> 1. Avoid updates during update-sensitive times
> 
>    The host VMM can instruct TDH.SYS.SHUTDOWN to fail if any of the TDs
>    are currently in any update-sensitive cases.
> 
> 2. Detect incompatibility after updates
> 
>    On TDH.SYS.UPDATE, the host VMM can configure the TDX Module to detect
>    actual incompatibility cases. The TDX Module will then return a special
>    error to signal the incompatibility, allowing the host VMM to restart
>    the update-sensitive operations.
> 
> Implement option #1 to fail updates if the feature is available. Also,
> distinguish this update failure from other failures by returning -EBUSY,
> which will be converted to a firmware update error code indicating that the
> firmware is busy.
> 
> Options like "do nothing" or option #2 are not viable [1] because the
> former allows damage to propagate to multiple, potentially unknown
> components (adding significant complexity to the whole ecosystem), while
> the latter may make existing KVM ioctls unstable.
> 

[...]

>  
> +#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE BIT(16)
> +
>  int tdx_module_shutdown(void)
>  {
>  	struct tdx_module_args args = {};
> -	int ret, cpu;
> +	u64 ret;
> +	int cpu;
>  
>  	/*
>  	 * Shut down the TDX Module and prepare handoff data for the next
> @@ -1189,9 +1192,21 @@ int tdx_module_shutdown(void)
>  	 * modules as new modules likely have higher handoff version.
>  	 */
>  	args.rcx = tdx_sysinfo.handoff.module_hv;
> -	ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> -	if (ret)
> -		return ret;
> +
> +	if (tdx_supports_update_compatibility(&tdx_sysinfo))
> +		args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
> +
> +	ret = seamcall(TDH_SYS_SHUTDOWN, &args);
> +
> +	/*
> +	 * Return -EBUSY to signal that there is one or more ongoing flows
> +	 * which may not be compatible with an updated TDX module, so that
> +	 * userspace can retry on this error.
> +	 */
> +	if ((ret & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
> +		return -EBUSY;
> +	else if (ret)
> +		return -EIO;
> 

The changelog says "doing nothing" isn't an option, and we need to depend on
TDH.SYS.SHUTDOWN to catch such incompatibilities.

To me this means we cannot support module update if TDH.SYS.SHUTDOWN doesn't
support this "AVOID_COMPAT_SENSITIVE" feature, because w/o it we cannot tell
whether the update is happening during any sensitive operation.

But the code above proceeds to TDH.SYS.SHUTDOWN anyway when this feature
isn't supported.  I don't think we should do that?