Re: [PATCH v4 01/17] x86/virt/tdx: Enhance tdx_pamt_get/put() to support huge pages

[Yan Zhao <yan.y.zhao@intel.com>]

On Thu, Oct 22, 2026 at 05:28:32AM +0800, Edgecombe, Rick P wrote:
> On Mon, 2026-10-19 at 14:35 +0800, Yan Zhao wrote:
> > > Then the contract is simply:
> > > 
> > >     There are only two possible mapping levels: 2MB and 4KB. There is a
> > > refcount
> > >     for each 2MB aligned range. The mapping level will be 4KB when there is
> > > any
> > >     refcount for 2MB aligned range.
> > >     
> > >     tdx_pamt_get() increments the refcount
> > >     tdx_pamt_put() decrements the refcount
> > I think refcounts should not appear in the contract. What is meaningful to KVM
> > is not the refcount adjustment, but preparing and unpreparing PAMT pages for
> > the physical page KVM wants to install/uninstall.
> 
> Well get and put are already refcount associated terms...
> 
> > 
> > Whenever KVM invokes tdx_pamt_get(), it wants to ensure DPAMT pages are
> > installed before installing a physical page; when KVM invokes tdx_pamt_put(),
> > it wants to ensure DPAMT pages can be torn down correctly after uninstalling a
> > physical page. So, perhaps a more accurate naming for
> > tdx_pamt_get()/tdx_pamt_put() would be
> > tdx_pamt_prepare()/tdx_pamt_unprepare()?
> 
> I don't see a reason to change the just settled pattern. It went through many
> rounds of reviews and no one ever objected to the get/put pattern. AFAICT the
> only scenario where it doesn't fit for huge pages if it is changed to accept
> args for other levels. But as before there is nothing to do for those other
> levels. And the callers already know this because they have to top up the cache
> appropriately. So there is no benefit that I can see. And following from that...
> no need to rename the functions already.
> 
> > 
> > The refcounts are internal implementation details of tdx_pamt_{get/put}() and
> > are hidden from KVM. It is tdx_pamt_{get/put}() that decides to use per-2MB
> > refcounts to track the invocation count, and ensures that PAMT pages are
> > (un)installed only once for each 2MB range.
> > 
> > The contract is therefore simple for KVM:
> > whenever KVM installs/uninstalls a physical page, invoke tdx_pamt_{get/put}()
> > and specify the physical range by providing the level.
> > 
> > >     tdh_mem_page_demote() increments the refcount for each 4KB page
> > Since tdh_mem_page_demote() only splits the mapping level of an already
> > installed physical page (without removing any physical page), KVM does not
> > need
> > to invoke tdx_pamt_{get/put}() for the guest private page. As refcounts are
> > hidden from KVM, KVM is unaware of the underlying refcount adjustment for the
> > split huge page.
> > 
> > On the other hand, since tdh_mem_page_demote() will install a new page table
> > page, KVM needs to invoke tdx_pamt_get() at level 4KB to prepare for the new
> > page table page (which is 4KB) before invoking tdh_mem_page_demote().
> > 
> > >     The mapping level will be determined by the refcount after each
> > > operation is
> > >     complete.
> > > 
> > > If you want to pass the level, somehow it has to be clear how the refcount
> > > and
> > > mapping level will be adjusted for each combination of operations.
> > When a level is passed to tdx_pamt_{get/put}(), it indicates that KVM wants to
> > prepare/unprepare PAMT pages for a physical page at the specified level.
> 
> As already discussed, there is only one level where we would need to do
> anything.
Hmm. Ok. Let's drop this patch then and have KVM check the level instead, as in
the v3 implementation:
https://lore.kernel.org/all/20260106102304.25211-1-yan.y.zhao@intel.com