Re: [PATCH v1 06/26] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects

Xu Yilun
2025-12-23T10:23:49Z

> > +static bool tdx_page_array_validate_release(struct tdx_page_array *array,
> > + unsigned int offset,
> > + unsigned int nr_released,
> > + u64 released_hpa)
> > +{
> > + unsigned int nents;
> > + u64 *entries;
> > + int i;
> > +
> > + if (offset >= array->nr_pages)
> > + return false;
> > +
> > + nents = umin(array->nr_pages - offset, TDX_PAGE_ARRAY_MAX_NENTS);
> > +
> > + if (nents != nr_released) {
> > + pr_err("%s nr_released [%d] doesn't match page array nents [%d]\n",
> > + __func__, nr_released, nents);
> > + return false;
> > + }
> > +
> > + /*
> > + * Unfortunately TDX has multiple page allocation protocols, check the
> > + * "singleton" case required for HPA_ARRAY_T.
> > + */
> > + if (page_to_phys(array->pages[0]) == released_hpa &&
> > + array->nr_pages == 1)
> > + return true;
> > +
> > + /* Then check the "non-singleton" case */
> > + if (page_to_phys(array->root) == released_hpa) {
> > + entries = (u64 *)page_address(array->root);
>
> page_address() returns a void * so the cast here isn't needed and (to me
> at least) doesn't add value from readability point of view.

These casts disappear during the refactoring from alloc_page() to
kzalloc(PAGE_SIZE, ...) for my v2.

>
> I haven't checked later patches, but if this code doesn't change to use
> entries outside this scope then,
> u64 *entries = page_address(array->root);
> would be nice to restrict the scope and make the type here immediately
> visible.

Yes. Also for int i.

Thanks,
Yilun

>
> > + for (i = 0; i < nents; i++) {
> > + struct page *page = array->pages[offset + i];
> > + u64 val = page_to_phys(page);
> > +
> > + if (val != entries[i]) {
> > + pr_err("%s entry[%d] [0x%llx] doesn't match page hpa [0x%llx]\n",
> > + __func__, i, entries[i], val);
> > + return false;
> > + }
> > + }
> > +
> > + return true;
> > + }
> > +
> > + pr_err("%s failed to validate, released_hpa [0x%llx], root page hpa [0x%llx], page0 hpa [%#llx], number pages %u\n",
> > + __func__, released_hpa, page_to_phys(array->root),
> > + page_to_phys(array->pages[0]), array->nr_pages);
> > +
> > + return false;
> > +}
>
>