Re: [PATCH 5/6] x86/virt/tdx: Make TDX module initialize the extensions

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.