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

dan.j.williams
2026-02-11T16:24:32Z

Dave Hansen wrote:
> On 11/16/25 18:22, Xu Yilun wrote:
> > + struct page *root __free(__free_page) = alloc_page(GFP_KERNEL |
> > + __GFP_ZERO);
> > + if (!root)
> > + return NULL;
>
> Why don't you just kcalloc() this like the rest of them?
>
> Then you won't need "mm: Add __free() support for __free_page()" either,
> right?

I saw a preview of what this comment does to the implementation and I am
not sure it is an improvement to avoid the __free() support for
alloc_page().

The seamcall prototype is:

u64 tdh_ide_stream_km(u64 spdm_id, u64 stream_id, u64 operation,
struct page *spdm_rsp, struct page *spdm_req,
u64 *spdm_req_len);

The interdiff of the effect of this review feedback is:

@@ drivers/virt/coco/tdx-host/tdx-host.c: static void tdx_spdm_session_teardown(str
+
+ do {
+ r = tdh_ide_stream_km(tlink->spdm_id, tlink->stream_id, op,
-+ tlink->in_msg, tlink->out_msg,
++ virt_to_page(tlink->in_msg),
++ virt_to_page(tlink->out_msg),
+ &out_msg_sz);
+ ret = tdx_link_event_handler(tlink, r, out_msg_sz);
+ } while (ret == -EAGAIN);

This is unfortunate because tdh_ide_stream_km() will just turn around
and call page_to_phys() on those arguments. It forfeits type safety and
inflicts the mental hurdle of "->in_msg and ->out_msg are direct-mapped
page-aligned virtual addresses, right, right!?".

So alloc_page() + __free_page() is more suitable than kzalloc(PAGE_SIZE,
...) as long as the seamcall requires 'struct page *' arguments. Another
path is the seamcall semantic is updated to handle virt_to_phys()
without alignment concerns, but it is a bit late to make that change. I
considered the concept of passing a "va page frame number" around, but
'struct page *' *is* the idiomatic expression for that concept.