Re: [PATCH v2 05/31] x86/virt/tdx: Extend tdx_page_array to support IOMMU_MT

[Xu Yilun]

> A couple recommendations come to mind:
> 
> * s/tdx_page_array_free/tdx_page_array_destroy/
> 
>   ...since "destroy" mirrors create and matches other cases where only
>   metadata is managed.
> 
> * Create a new tdx_page_array_repopulate() helper to make it clear which
>   paths depend on being able to repopulate and move the WARN_ON_ONCE() out of
>   the common path that does not repopulate. "repopulate" can have
>   "realloc" semantics where it allocates on first use, but otherwise
>   "populate" gets to not care about the corner cases. Make the WARN case
>   fail repopulate.

Agree. I end up add a function like that:


/**
 * tdx_page_array_repopulate() - repopulate a tdx_page_array
 * @array: The array descriptor to reallocate for.
 * @pages: Pointer to struct page array for tdx_page_array populating
 * @nr_pages: Size of @pages array.
 * 
 * Re-populate the tdx_page_array. If @array is %NULL, it behaves exactly like
 * tdx_page_array_create().
 *
 * Return: Re-populated tdx_page_array or NULL on failure.
 */
static struct tdx_page_array *
tdx_page_array_repopulate(struct tdx_page_array *array, struct page **pages,
			  unsigned int nr_pages)
{
	struct tdx_page_array *tmp = array;
	int ret;

	if (tmp) {
		/* Don't pass in something partially initialized */
		if (!tmp->root || !tmp->pages || !tmp->nr_pages)
			return NULL;

		/*
		 * When re-populating, the old pages are no longer tracked.
		 * Theoretically they require cache flushing before reclaiming
		 * for other kernel usage, similar to tdx_page_array_destroy().
		 * Since there is no use case to repopulate and then reclaim
		 * old pages yet, just warn to prompt future improvement.
		 */
		if (WARN_ON_ONCE(tmp->need_phymem_page_wbinvd))
			return NULL;
	} else {
		tmp = tdx_page_array_alloc();
		if (!tmp)
			return NULL;
	}

	ret = tdx_page_array_populate(tmp, pages, nr_pages);
	if (ret) {
		/* Only destroy newly allocated object */
		if (!array)
			tdx_page_array_destroy(tmp);

		return NULL;
	}

	return tmp;
}