Re: [PATCH 5/6] x86/virt/tdx: Make TDX module initialize the extensions
From: Xu Yilun
Date: Tue Aug 25 2026 - 06:06:36 EST
On Mon, Aug 24, 2026 at 05:58:07PM +0000, Edgecombe, Rick P wrote:
> On Tue, 2026-08-25 at 01:14 +0800, Xu Yilun wrote:
> > > If that could technically work, I guess the benefit of the current approach
> > > is that it allows to use contiguous physical allocations. Not sure if you
> > > think that simpler snippet would actually be that simple in the real world.
> >
> > I think there are several simplifications here, let's break down:
> >
> > 1. Forget about the 512-page limitation for hpa_list_info, always add 1 page
> > at a time.
> > This can also be applied to current flow. So put it aside.
> >
> > 2. The loop strategy:
> > - read total memory size vs. - loop on error code
> > - prealloc all memory - alloc a page
> > - loop on size - memory add
> > - memory add
> >
> > The main saving is that we don't read memory_pool_required_pages any more.
> > Others are similar lines of code.
>
> If we don't read memory_pool_required_pages, then the only real option is to add
> 1 page at a time. Or I'd think you end up giving extra memory. Unless
> memory_pool_required_pages is rounded up to some higher page order? I think no.
>
> But otherwise, if you are going to add pages many at at time, you need to read
> memory_pool_required_pages to make sure you are not going to give extra memory.
> At that point doing the allocation upfront (before the error code) is simpler.
> Hence, the design in this patch.
>
> So I'm not suggesting to change the design in the patch. Just that if 1 and 2
> are really slightly simpler, it's worth justifying the design in this patch as
> for the purpose of reducing fragmentation. Otherwise it looks unnecessarily
Good to me.
> complicated.
>
> >
> > 3. But we'd better keep reading ext_required. I tested when ext_required ==
> > 0:
> > - tdh_sys_init() returns TDX_EXT_MEMORY_POOL_REQUIRED,
>
> If we don't config any extensions, tdh_sys_init() returns
> TDX_EXT_MEMORY_POOL_REQUIRED? Seems like a bug.
Maybe. But it also indicates calling tdh_sys_init() when ext_required == 0
is not architecturly defined now, we should not rely on that.