Re: [PATCH v7 13/22] x86/virt/seamldr: Install a new TDX module

[Edgecombe, Rick P]

On Tue, 2026-03-31 at 05:41 -0700, Chao Gao wrote:
> Following the shutdown of the existing TDX module, the update process
> continues with installing the new module. P-SEAMLDR provides the
> SEAMLDR.INSTALL SEAMCALL to perform this installation, which must be
> executed on all CPUs.
> 
> Implement SEAMLDR.INSTALL and execute it on every CPU.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>
> Reviewed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>

It seems a pretty straight forward one. My only question would be if the log
needs a bit more info. Not sure what to put though...

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>

> ---
> v6:
>  - wrap seamldr_call(P_SEAMLDR_INSTALL..) in a helper [Kiryl]
> v5:
>  - drop "serially" from the changelog as it doesn't matter to
>    this patch
> ---
>  arch/x86/virt/vmx/tdx/seamldr.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index a8bfa30ee55f..3e46f3bfaa8b 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -19,6 +19,7 @@
>  
>  /* P-SEAMLDR SEAMCALL leaf function */
>  #define P_SEAMLDR_INFO			0x8000000000000000
> +#define P_SEAMLDR_INSTALL		0x8000000000000001
>  
>  #define SEAMLDR_MAX_NR_MODULE_4KB_PAGES	496
>  #define SEAMLDR_MAX_NR_SIG_4KB_PAGES	4
> @@ -73,6 +74,13 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>  
> +static int seamldr_install(const struct seamldr_params *params)
> +{
> +	struct tdx_module_args args = { .rcx = __pa(params) };

In an earlier patch you have a wrapper as:
struct tdx_module_args args = = {};
args.rxx = foo;

Why the style difference? It would be good to standardize, but the existing code
isn't standardized. What do you think about going with this style through the
series for the one arg ones?

> +
> +	return seamldr_call(P_SEAMLDR_INSTALL, &args);
> +}
> +
>  static void free_seamldr_params(struct seamldr_params *params)
>  {
>  	free_page((unsigned long)params);
> @@ -209,6 +217,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>  enum module_update_state {
>  	MODULE_UPDATE_START,
>  	MODULE_UPDATE_SHUTDOWN,
> +	MODULE_UPDATE_CPU_INSTALL,
>  	MODULE_UPDATE_DONE,
>  };
>  
> @@ -266,6 +275,9 @@ static int do_seamldr_install_module(void *seamldr_params)
>  				if (primary)
>  					ret = tdx_module_shutdown();
>  				break;
> +			case MODULE_UPDATE_CPU_INSTALL:
> +				ret = seamldr_install(seamldr_params);
> +				break;
>  			default:
>  				break;
>  			}