Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request

[Chao Gao]

>> +static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
>> +						   const void *sig, unsigned int sig_size)
>> +{
>> +	struct seamldr_params *params;
>> +	const u8 *ptr;
>> +	int i;
>> +
>> +	if (WARN_ON_ONCE(!is_vmalloc_addr(module) || !is_vmalloc_addr(sig)))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	/*
>> +	 * Check that input buffers satisfy P-SEAMLDR's size and alignment
>> +	 * constraints so they can be passed directly to P-SEAMLDR without
>> +	 * relocation or copy.
>> +	 */
>> +	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);
>> +
>> +	params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
>> +	if (!params)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	params->scenario = SEAMLDR_SCENARIO_UPDATE;
>
>Add a comment for why params->version isn't initialized explicitly?

Because the page is zero-allocated, the version is implicitly 0.

But I just found that 16KB sigstructs require version 1, so I'll make the
version explicit:

	/* Only version 1 supports >4KB sigstruct */
	if (sig_size > SZ_4K)
		params->version = 1;
	else
		params->version = 0;

Note that we can't always use version 1 since existing P-SEAMLDR versions don't
support it.

<snip>

>> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
>> +{
>> +	const struct tdx_blob *blob = (const void *)data;
>> +	int module_size, sig_size;
>> +	const void *sig, *module;
>> +
>> +	if (size < sizeof(struct tdx_blob) || blob->offset_of_module >= size)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	if (blob->version != 0x100) {
>Do we need a macro for this 0x100?

Maybe not, as this is a one-off check (i.e., the version/macro won't be used
anywhere else). If someone has a strong opinion on this, I can add one.