Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects

[Xu Yilun]

> > A main difference is HPA_ARRAY_T requires singleton mode when
> > containing just 1 functional page (page0). In this mode the root page is
> > not needed and the HPA field of the raw value directly points to the
> > page0. But in this patch, root page is always allocated for user
> > friendly kAPIs.
> 
> I think this undersells the fact that "singleton mode" is a premature
> optimization that requires complication to take advantage of the benefit
> (sometimes save a single page allocation). The Linux implementation
> forfeits that small benefit for the larger gain of cleaner kAPIs.
> 
> > Another small difference is HPA_LIST_INFO contains a 'first entry' field
> > which could be filled by TDX Module. This simplifies host by providing
> > the same structure when re-invoke the interrupted SEAMCALL. No need for
> > host to touch this field.
> > 
> > Typical usages of the tdx_page_array:
> > 
> > 1. Add control pages:
> >  - struct tdx_page_array *array = tdx_page_array_create(nr_pages);
> >  - seamcall(TDH_XXX_CREATE, array, ...);
> > 
> > 2. Release control pages:
> >  - seamcall(TDX_XXX_DELETE, array, &nr_released, &released_hpa);
> 
> It is simply a bug if TDH_XXX_DELETE does not return every resource
> passed to TDH_XXX_CREATE. The only "leak" case to worry about is that
> TDH_XXX_DELETE fails. In that case it should be "fatal" (TDX_BUG_ON,
> system can keep hobbling along, but panic_on_warn() would not be
> unreasonable). If TDH_XXX_DELETE fails it indicates some catastrophic
> misunderstanding between Linux and the TDX Module.
> 
> So the seamcall in this case has no need for @nr_released or
> @released_hpa, those should already be known to the kernel.
> 
> What is missing is an architectural guarantee that TDH_XXX_DELETE
> success == "all resources you arranged at TDH_XXX_CREATE time are free".
> I would hope that is already the case and AUX_PAGE_PA is only an

Yes, it is already the case.

> unfortunate distraction. If it can ever be the case that CREATE and
> DELETE are asymmetric on success then that needs to be corrected and
> Linux will wait for a future module that can make that guarantee.

Agree. Thanks for this summary as well as the singleton-mode one.

> 
> I think that cleans up a bulk of the logic here to abandon caring that
> the module tries to remind us what we are releasing.

Yes, I'll delete all these "released page matching" logic.