Re: [PATCH v2 11/21] x86/virt/seamldr: Allocate and populate a module update request

[Xu Yilun]

> +/* Allocate and populate a seamldr_params */
> +static struct seamldr_params *alloc_seamldr_params(const void *module, int module_size,
> +						   const void *sig, int sig_size)
> +{
> +	struct seamldr_params *params;
> +	const u8 *ptr;
> +	int i;
> +
> +	BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!IS_ALIGNED(module_size, SZ_4K) || !IS_ALIGNED(sig_size, SZ_4K) ||
> +	    !IS_ALIGNED((unsigned long)module, SZ_4K) ||
> +	    !IS_ALIGNED((unsigned long)sig, SZ_4K))
> +		return ERR_PTR(-EINVAL);
> +
> +	/* seamldr_params accepts one 4KB-page for sigstruct */
> +	if (sig_size != SZ_4K)

Why we check both IS_ALIGNED(sig_size, SZ_4K) and sig_size != SZ_4K, I
assume the former is redundant.

> +		return ERR_PTR(-EINVAL);
> +
> +	params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
> +	if (!params)
> +		return ERR_PTR(-ENOMEM);
> +
> +	params->scenario = SEAMLDR_SCENARIO_UPDATE;
> +	params->sigstruct_pa = (vmalloc_to_pfn(sig) << PAGE_SHIFT) +

This void * buffer comes from FW_UPLOAD callback, which is a kvmalloc
buffer, so we do vmalloc_to_pfn() here. But that knowledge resides in
FW_UPLOAD driver context, the kAPI entry, seamldr_install_module()
doesn't say that. So could we add the kernel-doc to specify this
"const u8 *data" at ...

... here
>  int seamldr_install_module(const u8 *data, u32 size)
>  {
>  	const struct seamldr_info *info = seamldr_get_info();
> @@ -82,6 +232,11 @@ int seamldr_install_module(const u8 *data, u32 size)
>  	if (!info->num_remaining_updates)
>  		return -ENOSPC;
>  
> +	struct seamldr_params *params __free(free_seamldr_params) =
> +						init_seamldr_params(data, size);
> +	if (IS_ERR(params))
> +		return PTR_ERR(params);
> +
>  	guard(cpus_read_lock)();
>  	if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
>  		pr_err("Cannot update TDX module if any CPU is offline\n");
> -- 
> 2.47.3
>