* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jiri Pirko @ 2026-02-10 14:49 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <20260210124357.GD943673@ziepe.ca>
Tue, Feb 10, 2026 at 01:43:57PM +0100, jgg@ziepe.ca wrote:
>On Tue, Feb 10, 2026 at 10:14:08AM +0100, Jiri Pirko wrote:
>
>> >I'd advocate that the right design is for userspace to positively
>> >signal via this flag that it wants/accepts shared memory and without
>> >the flag shared memory should never be returned.
>>
>> We can have the same behaviour with the separate heap, can't we?
>> Userpace positively signals it wants/accepts the shared memory by
>> choosing "system_cc_decrypted" heap name.
>
>So what do the other heap names do? Always private? Do you ever get
>heaps that are unknowably private or shared (eg MMIO backed?)
If I understand the code correctly, you may get something like this:
$ ls /dev/dma_heap/
default_cma_region
protected,secure-video
protected,secure-video-record
protected,trusted-ui
system
The "protected*" ones are created by tee. I believe they handle
memory that is inaccesible to CPU.
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Leon Romanovsky @ 2026-02-10 12:48 UTC (permalink / raw)
To: Jiri Pirko
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, jgg, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <hwdezwktndbm6hoko3rz5lffgfljodegcygzf6rbdf2ferokj6@ftk2uk3rqfdq>
On Tue, Feb 10, 2026 at 10:05:14AM +0100, Jiri Pirko wrote:
> Mon, Feb 09, 2026 at 09:08:03PM +0100, jstultz@google.com wrote:
> >On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> From: Jiri Pirko <jiri@nvidia.com>
> >>
> >> Currently the flags, which are unused, are validated for all heaps.
> >> Since the follow-up patch introduces a flag valid for only one of the
> >> heaps, allow to specify the valid flags per-heap.
> >
> >I'm not really in this space anymore, so take my feedback with a grain of salt.
> >
> >While the heap allocate flags argument is unused, it was intended to
> >be used for generic allocation flags that would apply to all or at
> >least a wide majority of heaps.
> >
> >It was definitely not added to allow for per-heap or heap specific
> >flags (as this patch tries to utilize it). That was the mess we had
> >with ION driver that we were trying to avoid.
> >
> >The intent of dma-buf heaps is to try to abstract all the different
> >device memory constraints so there only needs to be a [usage] ->
> >[heap] mapping, and otherwise userland can be generalized so that it
> >doesn't need to be re-written to work with different devices/memory
> >types. Adding heap-specific allocation flags prevents that
> >generalization.
> >
> >So instead of adding heap specific flags, the general advice has been
> >to add a separate heap name for the flag property.
>
> Right, my original idea was to add a separate heap. Then I spotted the
> flags and seemed like a great fit. Was not aware or the history or
> original intention. Would be probably good to document it for
> future generations.
>
> So instead of flag, I will add heap named something
> like "system_cc_decrypted" to implement this.
It is problematic to expose a user‑visible API that depends on a name.
Such a design limits our ability to extend the functionality in the
future, should new use cases arise.
Thanks
>
> Thanks!
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jason Gunthorpe @ 2026-02-10 12:43 UTC (permalink / raw)
To: Jiri Pirko
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <tgvdjszwxggr53digbmddcbxvupzl4xcoprofkgrs2kgf6rknx@44ebljjpghjm>
On Tue, Feb 10, 2026 at 10:14:08AM +0100, Jiri Pirko wrote:
> >I'd advocate that the right design is for userspace to positively
> >signal via this flag that it wants/accepts shared memory and without
> >the flag shared memory should never be returned.
>
> We can have the same behaviour with the separate heap, can't we?
> Userpace positively signals it wants/accepts the shared memory by
> choosing "system_cc_decrypted" heap name.
So what do the other heap names do? Always private? Do you ever get
heaps that are unknowably private or shared (eg MMIO backed?)
Jason
^ permalink raw reply
* Re: [PATCH 5/5] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: kernel test robot @ 2026-02-10 12:02 UTC (permalink / raw)
To: Jiri Pirko, dri-devel, linaro-mm-sig, iommu, linux-media
Cc: llvm, oe-kbuild-all, sumit.semwal, benjamin.gaignard,
Brian.Starkey, jstultz, tjmercier, christian.koenig, m.szyprowski,
robin.murphy, jgg, leon, sean.anderson, ptesarik, catalin.marinas,
aneesh.kumar, suzuki.poulose, steven.price, thomas.lendacky,
john.allen, ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-6-jiri@resnulli.us>
Hi Jiri,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm-tip/drm-tip trace/for-next linus/master v6.19]
[cannot apply to next-20260209]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jiri-Pirko/dma-mapping-avoid-random-addr-value-print-out-on-error-path/20260209-234013
base: https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link: https://lore.kernel.org/r/20260209153809.250835-6-jiri%40resnulli.us
patch subject: [PATCH 5/5] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20260210/202602101926.lsquJdb1-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260210/202602101926.lsquJdb1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602101926.lsquJdb1-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/dma-buf/heaps/system_heap.c:66:8: error: call to undeclared function 'set_memory_decrypted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
66 | ret = set_memory_decrypted(addr, nr_pages);
| ^
>> drivers/dma-buf/heaps/system_heap.c:80:8: error: call to undeclared function 'set_memory_encrypted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
80 | ret = set_memory_encrypted(addr, nr_pages);
| ^
2 errors generated.
vim +/set_memory_decrypted +66 drivers/dma-buf/heaps/system_heap.c
59
60 static int system_heap_set_page_decrypted(struct page *page)
61 {
62 unsigned long addr = (unsigned long)page_address(page);
63 unsigned int nr_pages = 1 << compound_order(page);
64 int ret;
65
> 66 ret = set_memory_decrypted(addr, nr_pages);
67 if (ret)
68 pr_warn_ratelimited("dma-buf system heap: failed to decrypt page at %p\n",
69 page_address(page));
70
71 return ret;
72 }
73
74 static int system_heap_set_page_encrypted(struct page *page)
75 {
76 unsigned long addr = (unsigned long)page_address(page);
77 unsigned int nr_pages = 1 << compound_order(page);
78 int ret;
79
> 80 ret = set_memory_encrypted(addr, nr_pages);
81 if (ret)
82 pr_warn_ratelimited("dma-buf system heap: failed to re-encrypt page at %p, leaking memory\n",
83 page_address(page));
84
85 return ret;
86 }
87
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [RFC PATCH v5 08/45] KVM: x86/mmu: Propagate mirror SPTE removal to S-EPT in handle_changed_spte()
From: Yan Zhao @ 2026-02-10 10:54 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYYn0nf2cayYu8e7@google.com>
On Fri, Feb 06, 2026 at 09:41:38AM -0800, Sean Christopherson wrote:
> On Fri, Feb 06, 2026, Yan Zhao wrote:
> > On Thu, Feb 05, 2026 at 02:33:16PM -0800, Sean Christopherson wrote:
> > > On Thu, Feb 05, 2026, Yan Zhao wrote:
> > > > > > > if (was_present && !was_leaf &&
> > > > > > > (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> > > > > > > handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> > > > > > > + else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf)
> > > > > > Should we check !is_present instead of !is_leaf?
> > > > > > e.g. a transition from a present leaf entry to a present non-leaf entry could
> > > > > > also trigger this if case.
> > > > >
> > > > > No, the !is_leaf check is very intentional. At this point in the series, S-EPT
> > > > > doesn't support hugepages. If KVM manages to install a leaf SPTE and replaces
> > > > > that SPTE with a non-leaf SPTE, then we absolutely want the KVM_BUG_ON() in
> > > > > tdx_sept_remove_private_spte() to fire:
> > > > >
> > > > > /* TODO: handle large pages. */
> > > > > if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> > > > > return -EIO;
> > > > But the op is named remove_external_spte().
> > > > And the check of "level != PG_LEVEL_4K" is for removing large leaf entries.
> > >
> > > I agree that the naming at this point in the series is unfortunate, but I don't
> > > see it as outright wrong. That the TDP MMU could theoretically replace the leaf
> > > SPTE with a non-leaf SPTE doesn't change the fact that the old leaf SPTE *is*
> > > being removed.
> > Hmm, I can't agree with that. But I won't insist if you think it's ok :)
>
> If the code is read through a TDX lens, then I agree, it's seems wrong. Because
> then you *know* that TDX doesn't support back-to-back remove()=>add() operations
> to handle a page split.
>
> But from a TDP MMU perspective, this is entirely logical (ignoring that
> link_external_spt() is gone at this point in the series).
>
> else if (was_leaf && is_mirror_sptep(sptep) && !is_leaf) {
> kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_spte);
>
> /*
> * Link the new page table if a hugepage is being split, i.e.
> * if a leaf SPTE is being replaced with a non-leaf SPTE.
> */
> if (is_present)
> kvm_x86_call(link_external_spt)(kvm, gfn, level, ...);
> }
>
> And that is *exactly* why I want to get rid of the hyper-specific kvm_x86_ops
> hooks. They bleed all kinds of implementation details all over the TDP MMU, which
> makes it difficult to read and understand the relevant TDP MMU code if you don't
> already know the TDX rules. And I absolutely do not want to effectively require
> others to understand TDX's rules to be able to make changes to the TDP MMU.
Ok. I can understand your reasoning of checking !is_leaf now.
Thanks for the explanation!
Though I still think checking !is_present before calling op remove_external_spte()
in this patch is better, I have no strong opinion :)
...
> Nope, as above, 100% the opposite. Over ~3 patches, e.g.
>
> 1. Drop the KVM_BUG_ON()s or move them to TDX
> 2. Morph the !is_frozen_spte() check into a KVM_MMU_WARN_ON()
> 3. Rework the code to rely on __handle_changed_spte() to propagate writes to S-EPT
>
> That way, the _only_ path that updates external SPTEs is this:
>
> if (was_present && !was_leaf &&
> (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> else if (is_mirror_sptep(sptep))
> return kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
> new_spte, level);
>
> which is fully aligned with handle_changed_spte()'s role for !mirror roots: it
> exists to react to changes (the sole exception to that being aging SPTEs, which
> is a special case).
>
> Compile-tested only.
LGTM overall.
> arch/x86/kvm/mmu/tdp_mmu.c | 118 ++++++++++++++++++-------------------
> 1 file changed, 59 insertions(+), 59 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 847f2fcb6740..33a321aedac0 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -464,7 +464,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> }
>
> /**
> - * handle_changed_spte - handle bookkeeping associated with an SPTE change
> + * __handle_changed_spte - handle bookkeeping associated with an SPTE change
> * @kvm: kvm instance
> * @as_id: the address space of the paging structure the SPTE was a part of
> * @sptep: pointer to the SPTE
> @@ -480,9 +480,9 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> * dirty logging updates are handled in common code, not here (see make_spte()
> * and fast_pf_fix_direct_spte()).
> */
> -static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> - gfn_t gfn, u64 old_spte, u64 new_spte,
> - int level, bool shared)
> +static int __handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> + gfn_t gfn, u64 old_spte, u64 new_spte,
> + int level, bool shared)
> {
> bool was_present = is_shadow_present_pte(old_spte);
> bool is_present = is_shadow_present_pte(new_spte);
> @@ -518,7 +518,7 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> }
>
> if (old_spte == new_spte)
> - return;
> + return 0;
>
> trace_kvm_tdp_mmu_spte_changed(as_id, gfn, level, old_spte, new_spte);
>
> @@ -547,7 +547,7 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> "a temporary frozen SPTE.\n"
> "as_id: %d gfn: %llx old_spte: %llx new_spte: %llx level: %d",
> as_id, gfn, old_spte, new_spte, level);
> - return;
> + return 0;
> }
>
> if (is_leaf != was_leaf)
> @@ -559,30 +559,31 @@ static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> * SPTE being converted to a hugepage (leaf) or being zapped. Shadow
> * pages are kernel allocations and should never be migrated.
> *
> - * When modifying leaf entries in mirrored page tables, propagate the
> - * changes to the external SPTE. Bug the VM on failure, as callers
> - * aren't prepared to handle errors, e.g. due to lock contention in the
> - * TDX-Module. Note, changes to non-leaf mirror SPTEs are handled by
> - * handle_removed_pt() (the TDX-Module requires that child entries are
> - * removed before the parent SPTE), and changes to non-present mirror
> - * SPTEs are handled by __tdp_mmu_set_spte_atomic() (KVM needs to set
> - * the external SPTE while the mirror SPTE is frozen so that installing
> - * a new SPTE is effectively an atomic operation).
> + * When modifying leaf entries in mirrored page tables, propagate all
> + * changes to the external SPTE.
> */
> if (was_present && !was_leaf &&
> (is_leaf || !is_present || WARN_ON_ONCE(pfn_changed)))
> handle_removed_pt(kvm, spte_to_child_pt(old_spte, level), shared);
> - else if (was_leaf && is_mirror_sptep(sptep))
> - KVM_BUG_ON(kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
> - new_spte, level), kvm);
> + else if (is_mirror_sptep(sptep))
> + return kvm_x86_call(set_external_spte)(kvm, gfn, old_spte,
> + new_spte, level);
For TDX's future implementation of set_external_spte() for split splitting,
could we add a new param "bool shared" to op set_external_spte() in the
future? i.e.,
- when tdx_sept_split_private_spte() is invoked under write mmu_lock, it calls
tdh_do_no_vcpus() to retry BUSY error, and TDX_BUG_ON_2() then.
- when tdx_sept_split_private_spte() is invoked under read mmu_lock
(in the future when calling tdh_mem_range_block() in unnecessary), it could
directly return BUSY to TDP MMU on contention.
> + return 0;
> +}
> +
> +static void handle_changed_spte(struct kvm *kvm, int as_id, tdp_ptep_t sptep,
> + gfn_t gfn, u64 old_spte, u64 new_spte,
> + int level, bool shared)
> +{
Do we need "WARN_ON_ONCE(is_mirror_sptep(sptep) && shared)" here ?
> + KVM_BUG_ON(__handle_changed_spte(kvm, as_id, sptep, gfn, old_spte,
> + new_spte, level, shared), kvm);
> }
>
> static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> struct tdp_iter *iter,
> u64 new_spte)
> {
> - u64 *raw_sptep = rcu_dereference(iter->sptep);
> -
> /*
> * The caller is responsible for ensuring the old SPTE is not a FROZEN
> * SPTE. KVM should never attempt to zap or manipulate a FROZEN SPTE,
> @@ -591,40 +592,6 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> */
> WARN_ON_ONCE(iter->yielded || is_frozen_spte(iter->old_spte));
>
> - if (is_mirror_sptep(iter->sptep) && !is_frozen_spte(new_spte)) {
> - int ret;
> -
> - /*
> - * KVM doesn't currently support zapping or splitting mirror
> - * SPTEs while holding mmu_lock for read.
> - */
> - if (KVM_BUG_ON(is_shadow_present_pte(iter->old_spte), kvm) ||
> - KVM_BUG_ON(!is_shadow_present_pte(new_spte), kvm))
> - return -EBUSY;
> -
> - /*
> - * Temporarily freeze the SPTE until the external PTE operation
> - * has completed, e.g. so that concurrent faults don't attempt
> - * to install a child PTE in the external page table before the
> - * parent PTE has been written.
> - */
> - if (!try_cmpxchg64(raw_sptep, &iter->old_spte, FROZEN_SPTE))
> - return -EBUSY;
> -
> - /*
> - * Update the external PTE. On success, set the mirror SPTE to
> - * the desired value. On failure, restore the old SPTE so that
> - * the SPTE isn't frozen in perpetuity.
> - */
> - ret = kvm_x86_call(set_external_spte)(kvm, iter->gfn, iter->old_spte,
> - new_spte, iter->level);
> - if (ret)
> - __kvm_tdp_mmu_write_spte(iter->sptep, iter->old_spte);
> - else
> - __kvm_tdp_mmu_write_spte(iter->sptep, new_spte);
> - return ret;
> - }
> -
> /*
> * Note, fast_pf_fix_direct_spte() can also modify TDP MMU SPTEs and
> * does not hold the mmu_lock. On failure, i.e. if a different logical
> @@ -632,7 +599,7 @@ static inline int __must_check __tdp_mmu_set_spte_atomic(struct kvm *kvm,
> * the current value, so the caller operates on fresh data, e.g. if it
> * retries tdp_mmu_set_spte_atomic()
> */
> - if (!try_cmpxchg64(raw_sptep, &iter->old_spte, new_spte))
> + if (!try_cmpxchg64(rcu_dereference(iter->sptep), &iter->old_spte, new_spte))
> return -EBUSY;
>
> return 0;
> @@ -663,14 +630,44 @@ static inline int __must_check tdp_mmu_set_spte_atomic(struct kvm *kvm,
>
> lockdep_assert_held_read(&kvm->mmu_lock);
>
> - ret = __tdp_mmu_set_spte_atomic(kvm, iter, new_spte);
>
> + /* KVM should never freeze SPTEs using higher level APIs. */
> + KVM_MMU_WARN_ON(is_frozen_spte(new_spte));
What about
KVM_MMU_WARN_ON(is_frozen_spte(new_spte) ||
is_frozen_spte(iter->old_spte) || iter->yielded);
> + /*
> + * Temporarily freeze the SPTE until the external PTE operation has
> + * completed (unless the new SPTE itself will be frozen), e.g. so that
> + * concurrent faults don't attempt to install a child PTE in the
> + * external page table before the parent PTE has been written, or try
> + * to re-install a page table before the old one was removed.
> + */
> + if (is_mirror_sptep(iter->sptep))
> + ret = __tdp_mmu_set_spte_atomic(kvm, iter, FROZEN_SPTE);
> + else
> + ret = __tdp_mmu_set_spte_atomic(kvm, iter, new_spte);
and invoking open code try_cmpxchg64() directly?
> if (ret)
> return ret;
>
> - handle_changed_spte(kvm, iter->as_id, iter->sptep, iter->gfn,
> - iter->old_spte, new_spte, iter->level, true);
> + ret = __handle_changed_spte(kvm, iter->as_id, iter->sptep, iter->gfn,
> + iter->old_spte, new_spte, iter->level, true);
>
> - return 0;
> + /*
> + * Unfreeze the mirror SPTE. If updating the external SPTE failed,
> + * restore the old SPTE so that the SPTE isn't frozen in perpetuity,
> + * otherwise set the mirror SPTE to the new desired value.
> + */
> + if (is_mirror_sptep(iter->sptep)) {
> + if (ret)
> + __kvm_tdp_mmu_write_spte(iter->sptep, iter->old_spte);
> + else
> + __kvm_tdp_mmu_write_spte(iter->sptep, new_spte);
> + } else {
> + /*
> + * Bug the VM if handling the change failed, as failure is only
> + * allowed if KVM couldn't update the external SPTE.
> + */
> + KVM_BUG_ON(ret, kvm);
> + }
> + return ret;
> }
One concern for tdp_mmu_set_spte_atomic() to handle mirror SPTEs:
- Previously
1. set *iter->sptep to FROZEN_SPTE.
2. kvm_x86_call(set_external_spte)(old_spte, new_spte)
3. set *iter->sptep to new_spte
- Now with this diff
1. set *iter->sptep to FROZEN_SPTE.
2. __handle_changed_spte()
--> kvm_x86_call(set_external_spte)(iter->sptep, old_spte, new_spte)
3. set *iter->sptep to new_spte
what if __handle_changed_spte() reads *iter->sptep in step 2?
Passing in "bool is_mirror_sp" to __handle_changed_spte() instead?
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Huang, Kai @ 2026-02-10 10:54 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, ackerleytng@google.com, tglx@kernel.org,
sagis@google.com, Edgecombe, Rick P, bp@alien8.de,
Annapurve, Vishal, x86@kernel.org
In-Reply-To: <aYpjNrtGmogNzqwT@google.com>
>
> Doesn't work, because sp->external_spt will be non-NULL when KVM is freeing
> unused pages in tdp_mmu_split_huge_pages_root() and kvm_tdp_mmu_map(). That's
> solvable, but it's part of the asymmetry I don't love. AFAICT, unless we do
> something truly awful, there's no way to avoid having common KVM free unused
> S-EPT pages.
>
> That said, while I don't love the asymmetry, it's not a deal breaker, especially
> if we make the asymmetry super obvious and cleanly delineated. Specifically, if
> we differentiate between freeing unused page tables and freeing used (linked at
> any point) page tables.
>
> This would also allow us to address the naming than Yan doesn't like around
> reclaim_external_sp(), because we could have both free_external_sp() and
> free_unused_external_spt(), where the lack of "unused" gives the reader a hint
> that there's interesting work to be done for in-use external page tables.
>
> This won't apply cleanly due to other fixups. It's also at:
That's a bit unfortunate.
I hate to say, but should we just use option 1? :-)
As Dave mentioned, we can always improve the locking when there's real
performance issue.
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jiri Pirko @ 2026-02-10 9:14 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <20260210002927.GC943673@ziepe.ca>
Tue, Feb 10, 2026 at 01:29:27AM +0100, jgg@ziepe.ca wrote:
>On Mon, Feb 09, 2026 at 12:08:03PM -0800, John Stultz wrote:
>> On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> > From: Jiri Pirko <jiri@nvidia.com>
>> >
>> > Currently the flags, which are unused, are validated for all heaps.
>> > Since the follow-up patch introduces a flag valid for only one of the
>> > heaps, allow to specify the valid flags per-heap.
>>
>> I'm not really in this space anymore, so take my feedback with a grain of salt.
>>
>> While the heap allocate flags argument is unused, it was intended to
>> be used for generic allocation flags that would apply to all or at
>> least a wide majority of heaps.
>>
>> It was definitely not added to allow for per-heap or heap specific
>> flags (as this patch tries to utilize it). That was the mess we had
>> with ION driver that we were trying to avoid.
>
>I don't know alot about DMA heaps..
>
>On a CC VM system the shared/private property is universal and applies
>to every physical address. Not every address can dynamically change
>between shared and private, but every address does have a
>shared/private state.
>
>By default userspace process generally run exclusively in private
>memory and there are very few ways for userspace to even access shared
>memory.
>
>From a heaps perspective the API would be very strange, and perhaps
>even security dangerous, if it is returning shared memory to userspace
>without userspace knowing this is happening.
>
>I'd advocate that the right design is for userspace to positively
>signal via this flag that it wants/accepts shared memory and without
>the flag shared memory should never be returned.
We can have the same behaviour with the separate heap, can't we?
Userpace positively signals it wants/accepts the shared memory by
choosing "system_cc_decrypted" heap name.
[...]
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jiri Pirko @ 2026-02-10 9:05 UTC (permalink / raw)
To: John Stultz
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, tjmercier, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <CANDhNCoHEZsNRmU+3z5AbeAy05H7PTtUdTq1apNd5k0f9hWW8A@mail.gmail.com>
Mon, Feb 09, 2026 at 09:08:03PM +0100, jstultz@google.com wrote:
>On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Currently the flags, which are unused, are validated for all heaps.
>> Since the follow-up patch introduces a flag valid for only one of the
>> heaps, allow to specify the valid flags per-heap.
>
>I'm not really in this space anymore, so take my feedback with a grain of salt.
>
>While the heap allocate flags argument is unused, it was intended to
>be used for generic allocation flags that would apply to all or at
>least a wide majority of heaps.
>
>It was definitely not added to allow for per-heap or heap specific
>flags (as this patch tries to utilize it). That was the mess we had
>with ION driver that we were trying to avoid.
>
>The intent of dma-buf heaps is to try to abstract all the different
>device memory constraints so there only needs to be a [usage] ->
>[heap] mapping, and otherwise userland can be generalized so that it
>doesn't need to be re-written to work with different devices/memory
>types. Adding heap-specific allocation flags prevents that
>generalization.
>
>So instead of adding heap specific flags, the general advice has been
>to add a separate heap name for the flag property.
Right, my original idea was to add a separate heap. Then I spotted the
flags and seemed like a great fit. Was not aware or the history or
original intention. Would be probably good to document it for
future generations.
So instead of flag, I will add heap named something
like "system_cc_decrypted" to implement this.
Thanks!
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Yan Zhao @ 2026-02-10 8:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYprxnSHKHUtk7pt@google.com>
On Mon, Feb 09, 2026 at 03:20:38PM -0800, Sean Christopherson wrote:
> On Mon, Feb 09, 2026, Yan Zhao wrote:
> > On Fri, Feb 06, 2026 at 07:01:14AM -0800, Sean Christopherson wrote:
> > > @@ -2348,7 +2348,7 @@ void __tdx_pamt_put(u64 pfn)
> > > if (!atomic_dec_and_test(pamt_refcount))
> > > return;
> > >
> > > - scoped_guard(spinlock, &pamt_lock) {
> > > + scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
> > > /* Lost race with tdx_pamt_get(). */
> > > if (atomic_read(pamt_refcount))
> > > return;
> >
> > This option can get rid of the warning.
> >
> > However, given the pamt_lock is a global lock, which may be acquired even in the
> > softirq context, not sure if this irq disabled version is good.
>
> FWIW, the SEAMCALL itself disables IRQs (and everything else), so it's not _that_
> big of a change. But yeah, waiting on the spinlock with IRQs disabled isn't
> exactly idea.
Right. Though the SEAMCALL itself disables IRQs (which is no more than 18us from
my measurement), the time spent waiting for acquiring the spinlock with IRQs
disabled may scale with the number of contending threads. e.g.
When there're 4 threads trying to acquire the spinlock, the most unlucky thread
needs to wait with IRQs disabled for 3x18us=54us in the worst case.
> > For your reference, I measured some test data by concurrently launching and
> > destroying 4 TDs for 3 rounds:
> >
> > t0 ---------------------
> > scoped_guard(spinlock, &pamt_lock) { |->T1=t1-t0 |
> > t1 ---------- |
> > ... |
> > t2 ---------- |->T3=t4-t0
> > tdh_phymem_pamt_add/remove() |->T2=t3-t2 |
> > t3 ---------- |
> > ... |
> > t4 ---------------------
> > }
> >
> > (1) for __tdx_pamt_get()
> >
> > avg us min us max us
> > ------|---------------------------
> > T1 | 4 0 69
> > T2 | 4 2 18
> > T3 | 10 3 83
> >
> >
> > (2) for__tdx_pamt_put()
> >
> > avg us min us max us
> > ------|---------------------------
> > T1 | 0 0 5
> > T2 | 2 1 11
> > T3 | 3 2 15
> >
> >
> > > Option #2 would be to immediately free the page in tdx_sept_reclaim_private_sp(),
> > > so that pages that freed via handle_removed_pt() don't defer freeing the S-EPT
> > > page table (which, IIUC, is safe since the TDX-Module forces TLB flushes and exits).
> > >
> > > I really, really don't like this option (if it even works).
> > I don't like its asymmetry with tdx_sept_link_private_spt().
> >
> > However, do you think it would be good to have the PAMT pages of the sept pages
> > allocated from (*topup_private_mapping_cache) [1]?
>
> Hrm, dunno about "good", but it's definitely not terrible. To get the cache
> management right, it means adding yet another use of kvm_get_running_vcpu(), which
> I really dislike.
>
> On the other hand, if we combine that with TDX freeing in-use S-EPT page tables,
> unless I'm overly simplifying things, it would avoid having to extend
> kvm_mmu_memory_cache with the page_{get,free}() hook, and would then eliminate
> two kvm_x86_ops hooks, because the alloc/free of _unused_ S-EPT page tables is
> no different than regular page tables.
>
> As a bonus, we could keep the topup_external_cache() name and just clarify that
> the parameter specifies the number of page table pages, i.e. account for the +1
> for the mapping page in TDX code.
>
> All in all, I'm kinda leaning in this direction, because as much as I dislike
> kvm_get_running_vcpu(), it does minimize the number of kvm_x86_ops hooks.
>
> Something like this? Also pushed to
>
> https://github.com/sean-jc/linux.git x86/tdx_huge_sept_alt
>
It lacks the following change in tdx_sept_split_private_spte().
@@ -1836,46 +1841,70 @@ static int tdx_sept_split_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
if (!pamt_cache)
return -EIO;
+ r = tdx_pamt_get(page_to_pfn(external_spt), PG_LEVEL_4K, pamt_cache);
+ if (r)
+ return r;
+
err = tdh_do_no_vcpus(tdh_mem_range_block, kvm, &kvm_tdx->td, gpa,
level, &entry, &level_state);
- if (TDX_BUG_ON_2(err, TDH_MEM_RANGE_BLOCK, entry, level_state, kvm))
- return -EIO;
+ if (TDX_BUG_ON_2(err, TDH_MEM_RANGE_BLOCK, entry, level_state, kvm)) {
+ r = -EIO;
+ goto err;
+ }
tdx_track(kvm);
err = tdh_do_no_vcpus(tdh_mem_page_demote, kvm, &kvm_tdx->td, gpa,
level, spte_to_pfn(old_spte), external_spt,
pamt_cache, &entry, &level_state);
- if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_DEMOTE, entry, level_state, kvm))
- return -EIO;
+ if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_DEMOTE, entry, level_state, kvm)) {
+ r = -EIO;
+ goto err;
+ }
return 0;
+err:
+ tdx_pamt_put(page_to_pfn(external_spt), PG_LEVEL_4K);
+ return r;
}
Otherwise, LGTM except for the nits below.
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 6 +--
> arch/x86/include/asm/kvm_host.h | 15 ++------
> arch/x86/kvm/mmu/mmu.c | 3 --
> arch/x86/kvm/mmu/tdp_mmu.c | 23 +++++++-----
> arch/x86/kvm/vmx/tdx.c | 60 ++++++++++++++++++++----------
> 5 files changed, 61 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index 6083fb07cd3b..4b865617a421 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -94,11 +94,9 @@ KVM_X86_OP_OPTIONAL_RET0(set_tss_addr)
> KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
> KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
> KVM_X86_OP(load_mmu_pgd)
> -KVM_X86_OP_OPTIONAL(alloc_external_sp)
> -KVM_X86_OP_OPTIONAL(free_external_sp)
> -KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> -KVM_X86_OP_OPTIONAL(reclaim_external_sp)
> +KVM_X86_OP_OPTIONAL(reclaim_external_spt)
> KVM_X86_OP_OPTIONAL_RET0(topup_external_cache)
> +KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
> KVM_X86_OP(has_wbinvd_exit)
> KVM_X86_OP(get_l2_tsc_offset)
> KVM_X86_OP(get_l2_tsc_multiplier)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index cd3e7dc6ab9b..d3c31eaf18b1 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1850,19 +1850,12 @@ struct kvm_x86_ops {
> void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
> int root_level);
>
> - /*
> - * Callbacks to allocate and free external page tables, a.k.a. S-EPT,
> - * and to propagate changes in mirror page tables to the external page
> - * tables.
> - */
> - unsigned long (*alloc_external_sp)(gfp_t gfp);
> - void (*free_external_sp)(unsigned long addr);
> + void (*reclaim_external_spt)(struct kvm *kvm, gfn_t gfn,
> + struct kvm_mmu_page *sp);
> + int (*topup_external_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu,
> + int min_nr_spts);
> int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> u64 new_spte, enum pg_level level);
> - void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
> - struct kvm_mmu_page *sp);
> - int (*topup_external_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu, int min);
> -
>
> bool (*has_wbinvd_exit)(void);
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 62bf6bec2df2..f7cf456d9404 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -6714,9 +6714,6 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> if (!vcpu->arch.mmu_shadow_page_cache.init_value)
> vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
>
> - vcpu->arch.mmu_external_spt_cache.page_get = kvm_x86_ops.alloc_external_sp;
> - vcpu->arch.mmu_external_spt_cache.page_free = kvm_x86_ops.free_external_sp;
> -
> vcpu->arch.mmu = &vcpu->arch.root_mmu;
> vcpu->arch.walk_mmu = &vcpu->arch.root_mmu;
>
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index fef856323821..732548a678d8 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -53,14 +53,18 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
> rcu_barrier();
> }
>
> -static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> +static void __tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> {
> - if (sp->external_spt)
> - kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
> free_page((unsigned long)sp->spt);
> kmem_cache_free(mmu_page_header_cache, sp);
> }
>
> +static void tdp_mmu_free_unused_sp(struct kvm_mmu_page *sp)
> +{
> + free_page((unsigned long)sp->external_spt);
> + __tdp_mmu_free_sp(sp);
> +}
> +
> /*
> * This is called through call_rcu in order to free TDP page table memory
> * safely with respect to other kernel threads that may be operating on
> @@ -74,7 +78,8 @@ static void tdp_mmu_free_sp_rcu_callback(struct rcu_head *head)
> struct kvm_mmu_page *sp = container_of(head, struct kvm_mmu_page,
> rcu_head);
>
> - tdp_mmu_free_sp(sp);
> + WARN_ON_ONCE(sp->external_spt);
> + __tdp_mmu_free_sp(sp);
> }
>
> void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root)
> @@ -458,7 +463,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
> }
>
> if (is_mirror_sp(sp))
> - kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
> + kvm_x86_call(reclaim_external_spt)(kvm, base_gfn, sp);
>
> call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
> }
> @@ -1266,7 +1271,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
> * failed, e.g. because a different task modified the SPTE.
> */
> if (r) {
> - tdp_mmu_free_sp(sp);
> + tdp_mmu_free_unused_sp(sp);
> goto retry;
> }
>
> @@ -1461,7 +1466,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
> goto err_spt;
>
> if (is_mirror_sp) {
> - sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
> + sp->external_spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
> if (!sp->external_spt)
> goto err_external_spt;
>
> @@ -1472,7 +1477,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
> return sp;
>
> err_external_split:
> - kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
> + free_page((unsigned long)sp->external_spt);
> err_external_spt:
> free_page((unsigned long)sp->spt);
> err_spt:
> @@ -1594,7 +1599,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
> * installs its own sp in place of the last sp we tried to split.
> */
> if (sp)
> - tdp_mmu_free_sp(sp);
> + tdp_mmu_free_unused_sp(sp);
>
> return 0;
> }
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ae7b9beb3249..b0fc17baa1fc 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1701,7 +1701,7 @@ static struct tdx_pamt_cache *tdx_get_pamt_cache(struct kvm *kvm,
> }
>
> static int tdx_topup_external_pamt_cache(struct kvm *kvm,
> - struct kvm_vcpu *vcpu, int min)
> + struct kvm_vcpu *vcpu, int min_nr_spts)
> {
> struct tdx_pamt_cache *pamt_cache;
>
> @@ -1712,7 +1712,11 @@ static int tdx_topup_external_pamt_cache(struct kvm *kvm,
> if (!pamt_cache)
> return -EIO;
>
> - return tdx_topup_pamt_cache(pamt_cache, min);
> + /*
> + * Each S-EPT page tables requires a DPAMT pair, plus one more for the
> + * memory being mapped into the guest.
> + */
> + return tdx_topup_pamt_cache(pamt_cache, min_nr_spts + 1);
Nit:
S-EPT root page is a control page and it has no corresponding sp->external_spt.
So, do you think it would be good to check the root level?
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ae8b8438ae99..fff05052de27 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1643,16 +1643,23 @@ static struct tdx_pamt_cache *tdx_get_pamt_cache(struct kvm *kvm,
static int tdx_topup_external_pamt_cache(struct kvm *kvm,
struct kvm_vcpu *vcpu, int min_nr_spts)
{
+ int root_level = (kvm_gfn_direct_bits(kvm) == TDX_SHARED_BIT_PWL_5) ? 5 :4;
struct tdx_pamt_cache *pamt_cache;
if (!tdx_supports_dynamic_pamt(tdx_sysinfo))
return 0;
pamt_cache = tdx_get_pamt_cache(kvm, vcpu);
if (!pamt_cache)
return -EIO;
+ /*
+ * S-EPT root page is one of tdcs_pages whose PAMT pages have been installed in
+ * __tdx_td_init().
+ */
+ if (min_nr_spts == root_level)
+ min_nr_spts--;
+
/*
* Each S-EPT page tables requires a DPAMT pair, plus one more for the
* memory being mapped into the guest.
> }
>
> static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
> @@ -1911,23 +1915,41 @@ static int tdx_sept_split_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn, u64 new_spte,
> enum pg_level level)
> {
> + struct tdx_pamt_cache *pamt_cache;
> gpa_t gpa = gfn_to_gpa(gfn);
> u64 err, entry, level_state;
> struct page *external_spt;
> + int r;
>
> external_spt = tdx_spte_to_external_spt(kvm, gfn, new_spte, level);
> if (!external_spt)
> return -EIO;
>
> + pamt_cache = tdx_get_pamt_cache(kvm, kvm_get_running_vcpu());
> + if (!pamt_cache)
> + return -EIO;
> +
> + r = tdx_pamt_get(page_to_pfn(external_spt), PG_LEVEL_4K, pamt_cache);
> + if (r)
> + return r;
> +
> err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, external_spt,
> &entry, &level_state);
> - if (unlikely(IS_TDX_OPERAND_BUSY(err)))
> - return -EBUSY;
> + if (unlikely(IS_TDX_OPERAND_BUSY(err))) {
> + r = -EBUSY;
> + goto err;
> + }
>
> - if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm))
> - return -EIO;
> + if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm)) {
> + r = -EIO;
> + goto err;
> + }
>
> return 0;
> +
> +err:
> + tdx_pamt_put(page_to_pfn(external_spt), PG_LEVEL_4K);
> + return r;
> }
>
> static int tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
> @@ -1995,8 +2017,8 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> return tdx_sept_map_leaf_spte(kvm, gfn, new_spte, level);
> }
>
> -static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> - struct kvm_mmu_page *sp)
> +static void tdx_sept_reclaim_private_spt(struct kvm *kvm, gfn_t gfn,
> + struct kvm_mmu_page *sp)
> {
> /*
> * KVM doesn't (yet) zap page table pages in mirror page table while
> @@ -2014,7 +2036,16 @@ static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> */
> if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
> tdx_reclaim_page(virt_to_page(sp->external_spt)))
> - sp->external_spt = NULL;
> + goto out;
> +
> + /*
> + * Immediately free the S-EPT page as the TDX subsystem doesn't support
> + * freeing pages from RCU callbacks, and more importantly because
> + * TDH.PHYMEM.PAGE.RECLAIM ensures there are no outstanding readers.
> + */
> + tdx_free_control_page((unsigned long)sp->external_spt);
This creates another asymmetry, where there's nowhere to invoke
tdx_alloc_control_page() for the sp->external_spt.
Calling tdx_free_control_page() here could be confusing because:
- tdx_sept_reclaim_private_spt() is called only for non-root sps, whose
sp->external_spt is not allocated via tdx_alloc_control_page().
- The S-EPT root page is allocated via __tdx_alloc_control_page() by
__tdx_td_init(), but has no corresponding sp->external_spt.
So, could we just invoke
"__tdx_pamt_put(page_to_pfn(virt_to_page(sp->external_spt)))" in
tdx_sept_reclaim_private_sp()?
After tdx_sept_reclaim_private_spt() returns, sp goes back to unused by the
external page table. So, TDP MMU can invoke tdp_mmu_free_sp() without needing to
differentiate whether it's unused or not.
Something like below?
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 732548a678d8..d621e94d73c2 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -53,18 +53,15 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
rcu_barrier();
}
-static void __tdp_mmu_free_sp(struct kvm_mmu_page *sp)
+static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
{
+ free_page((unsigned long)sp->external_spt);
free_page((unsigned long)sp->spt);
kmem_cache_free(mmu_page_header_cache, sp);
}
-static void tdp_mmu_free_unused_sp(struct kvm_mmu_page *sp)
-{
- free_page((unsigned long)sp->external_spt);
- __tdp_mmu_free_sp(sp);
-}
-
/*
* This is called through call_rcu in order to free TDP page table memory
* safely with respect to other kernel threads that may be operating on
@@ -78,8 +75,7 @@ static void tdp_mmu_free_sp_rcu_callback(struct rcu_head *head)
struct kvm_mmu_page *sp = container_of(head, struct kvm_mmu_page,
rcu_head);
- WARN_ON_ONCE(sp->external_spt);
- __tdp_mmu_free_sp(sp);
+ tdp_mmu_free_sp(sp);
}
void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root)
@@ -1271,7 +1267,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
* failed, e.g. because a different task modified the SPTE.
*/
if (r) {
- tdp_mmu_free_unused_sp(sp);
+ tdp_mmu_free_sp(sp);
goto retry;
}
@@ -1599,7 +1595,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
* installs its own sp in place of the last sp we tried to split.
*/
if (sp)
- tdp_mmu_free_unused_sp(sp);
+ tdp_mmu_free_sp(sp);
return 0;
}
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b0fc17baa1fc..fbaf43b8cd46 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2035,17 +2035,12 @@ static void tdx_sept_reclaim_private_spt(struct kvm *kvm, gfn_t gfn,
* removal of the still-used PAMT entry.
*/
if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
- tdx_reclaim_page(virt_to_page(sp->external_spt)))
- goto out;
+ tdx_reclaim_page(virt_to_page(sp->external_spt))) {
+ sp->external_spt = NULL;
+ return;
+ }
- /*
- * Immediately free the S-EPT page as the TDX subsystem doesn't support
- * freeing pages from RCU callbacks, and more importantly because
- * TDH.PHYMEM.PAGE.RECLAIM ensures there are no outstanding readers.
- */
- tdx_free_control_page((unsigned long)sp->external_spt);
-out:
- sp->external_spt = NULL;
+ __tdx_pamt_put(page_to_pfn(virt_to_page(sp->external_spt)));
}
void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
^ permalink raw reply related
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Yan Zhao @ 2026-02-10 1:40 UTC (permalink / raw)
To: Dave Hansen
Cc: Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau, Paolo Bonzini,
linux-kernel, linux-coco, kvm, Kai Huang, Rick Edgecombe,
Vishal Annapurve, Ackerley Tng, Sagi Shahar, Binbin Wu,
Xiaoyao Li, Isaku Yamahata
In-Reply-To: <6a5e3e9f-69b2-4416-9465-92a859034391@intel.com>
On Mon, Feb 09, 2026 at 04:07:08PM -0800, Dave Hansen wrote:
> On 2/9/26 01:25, Yan Zhao wrote:
> > However, given the pamt_lock is a global lock, which may be acquired
> > even in the softirq context, not sure if this irq disabled version
> > is good.
>
> Generally, we try to avoid crap that's not scalable because it's hard to
> retrofit. But in this case, I'm just not sure how much of a bottleneck
> this lock is going to be in the real world.
>
> Let's be honest: starting and shutting down VMs in a loop doesn't mint
> money for cloud providers like running VMs does, so it's not exactly a
> real-world thing.
>
> That said, if this global lock _actually_ ever starts to bite anyone for
> real, it's not going to be rocket science to turn the single lock into 5
> or 10 or NR_CPUs, or whatever. So I think we can just keep it as-is and
> avert our eyes for the time being.
Hmm. One clarification: I'm not concerned about the global spinlock. My
concern is the attempt in the #1 solution [1] to turn off irq before acquiring
spinlock (spin_lock_irqsave()) to address the deadlock issue reported in [2].
[1] https://lore.kernel.org/all/aYYCOiMvWfSJR1AL@google.com/
[2] https://lore.kernel.org/all/aYW5CbUvZrLogsWF@yzhao56-desk.sh.intel.com/
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jason Gunthorpe @ 2026-02-10 0:29 UTC (permalink / raw)
To: John Stultz
Cc: Jiri Pirko, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <CANDhNCoHEZsNRmU+3z5AbeAy05H7PTtUdTq1apNd5k0f9hWW8A@mail.gmail.com>
On Mon, Feb 09, 2026 at 12:08:03PM -0800, John Stultz wrote:
> On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >
> > From: Jiri Pirko <jiri@nvidia.com>
> >
> > Currently the flags, which are unused, are validated for all heaps.
> > Since the follow-up patch introduces a flag valid for only one of the
> > heaps, allow to specify the valid flags per-heap.
>
> I'm not really in this space anymore, so take my feedback with a grain of salt.
>
> While the heap allocate flags argument is unused, it was intended to
> be used for generic allocation flags that would apply to all or at
> least a wide majority of heaps.
>
> It was definitely not added to allow for per-heap or heap specific
> flags (as this patch tries to utilize it). That was the mess we had
> with ION driver that we were trying to avoid.
I don't know alot about DMA heaps..
On a CC VM system the shared/private property is universal and applies
to every physical address. Not every address can dynamically change
between shared and private, but every address does have a
shared/private state.
By default userspace process generally run exclusively in private
memory and there are very few ways for userspace to even access shared
memory.
From a heaps perspective the API would be very strange, and perhaps
even security dangerous, if it is returning shared memory to userspace
without userspace knowing this is happening.
I'd advocate that the right design is for userspace to positively
signal via this flag that it wants/accepts shared memory and without
the flag shared memory should never be returned.
Even if the underyling heap only has shared memory in it (eg it is
mmio or something).
Otherwise making it implicit, perhaps based on heap name, sounds very
tricky for userspace to actually use fully securely.
Again, I don't know alot about heaps, but perhaps the missing part
here is that on a CC system all existing heaps, other than the one
using normal system pages, should be disabled for now. They can come
back once they are audited as to their shared/private state and
respect the new flag.
Another view is to ignore this affirmative handshake and just make it
implicit on something like the heap name and hope userspace lucks into
something that works for it, and doesn't accidently place, or become
tricked into placing, sensitive information into shared heap memory.
Again I know nothing about heaps, but this is a fuller picture of the
security sensitivity and what to think about with heaps and CC VM
systems.
> Now, there has been many discussions around "protected buffers" (which
> doesn't seem to map exactly to this confidental computing primitive,
> but sounds like it might be related)
I'm not sure what protected buffers are, but this CC VM shared/private
(or encrypted/decrypted) is a core kernel property that applies to
every physical address in the CC VM.
I assume protected buffers are something more platform specific and
hidden?
> But, it seems like the use case here is still far too narrow for a top
> level allocation flag.
CC certainly is a narrow use case, but within CC I don't think it is
narrow at all..
Jason
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Dave Hansen @ 2026-02-10 0:07 UTC (permalink / raw)
To: Yan Zhao, Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYmoIaFwgR6+hnGp@yzhao56-desk.sh.intel.com>
On 2/9/26 01:25, Yan Zhao wrote:
> However, given the pamt_lock is a global lock, which may be acquired
> even in the softirq context, not sure if this irq disabled version
> is good.
Generally, we try to avoid crap that's not scalable because it's hard to
retrofit. But in this case, I'm just not sure how much of a bottleneck
this lock is going to be in the real world.
Let's be honest: starting and shutting down VMs in a loop doesn't mint
money for cloud providers like running VMs does, so it's not exactly a
real-world thing.
That said, if this global lock _actually_ ever starts to bite anyone for
real, it's not going to be rocket science to turn the single lock into 5
or 10 or NR_CPUs, or whatever. So I think we can just keep it as-is and
avert our eyes for the time being.
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Sean Christopherson @ 2026-02-10 0:03 UTC (permalink / raw)
To: Dave Hansen
Cc: Yan Zhao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Kiryl Shutsemau, Paolo Bonzini, linux-kernel,
linux-coco, kvm, Kai Huang, Rick Edgecombe, Vishal Annapurve,
Ackerley Tng, Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <e81151ce-6e67-48e8-a722-da9ff03d686b@intel.com>
On Mon, Feb 09, 2026, Dave Hansen wrote:
> On 2/6/26 07:01, Sean Christopherson wrote:
> > /* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
> > int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
> > @@ -2272,7 +2272,7 @@ int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
> > if (ret)
> > goto out_free;
> >
> > - scoped_guard(spinlock, &pamt_lock) {
> > + scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
> > /*
> > * Lost race to other tdx_pamt_add(). Other task has already allocated
> > * PAMT memory for the HPA.
> > @@ -2348,7 +2348,7 @@ void __tdx_pamt_put(u64 pfn)
>
> Why does this need to be a raw spinlock? irqsave, sure, but raw?
Huh, TIL. (And just when I thought I finally had my head wrapped around RT "spinlocks"):
The hard interrupt related suffixes for spin_lock / spin_unlock operations
(_irq, _irqsave / _irqrestore) do not affect the CPU’s interrupt disabled state.
Ah, and running RCU callbacks from soft IRQ context is straight up disallowed for
PREEMPT_RT.
/* By default, use RCU_SOFTIRQ instead of rcuc kthreads. */
static bool use_softirq = !IS_ENABLED(CONFIG_PREEMPT_RT);
#ifndef CONFIG_PREEMPT_RT
module_param(use_softirq, bool, 0444);
#endif
So yeah, just spinlock_irqsave should be fine. Though the way things are trending,
it'll be a moot point if KVM ends up freeing S-EPT page tables from task context.
> The page allocator locks are used in this context and aren't raw.
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Dave Hansen @ 2026-02-09 23:40 UTC (permalink / raw)
To: Sean Christopherson, Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYYCOiMvWfSJR1AL@google.com>
On 2/6/26 07:01, Sean Christopherson wrote:
> /* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
> int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
> @@ -2272,7 +2272,7 @@ int __tdx_pamt_get(u64 pfn, struct tdx_pamt_cache *cache)
> if (ret)
> goto out_free;
>
> - scoped_guard(spinlock, &pamt_lock) {
> + scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
> /*
> * Lost race to other tdx_pamt_add(). Other task has already allocated
> * PAMT memory for the HPA.
> @@ -2348,7 +2348,7 @@ void __tdx_pamt_put(u64 pfn)
Why does this need to be a raw spinlock? irqsave, sure, but raw?
The page allocator locks are used in this context and aren't raw.
^ permalink raw reply
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Sean Christopherson @ 2026-02-09 23:20 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Kai Huang, Rick Edgecombe, Vishal Annapurve, Ackerley Tng,
Sagi Shahar, Binbin Wu, Xiaoyao Li, Isaku Yamahata
In-Reply-To: <aYmoIaFwgR6+hnGp@yzhao56-desk.sh.intel.com>
On Mon, Feb 09, 2026, Yan Zhao wrote:
> On Fri, Feb 06, 2026 at 07:01:14AM -0800, Sean Christopherson wrote:
> > @@ -2348,7 +2348,7 @@ void __tdx_pamt_put(u64 pfn)
> > if (!atomic_dec_and_test(pamt_refcount))
> > return;
> >
> > - scoped_guard(spinlock, &pamt_lock) {
> > + scoped_guard(raw_spinlock_irqsave, &pamt_lock) {
> > /* Lost race with tdx_pamt_get(). */
> > if (atomic_read(pamt_refcount))
> > return;
>
> This option can get rid of the warning.
>
> However, given the pamt_lock is a global lock, which may be acquired even in the
> softirq context, not sure if this irq disabled version is good.
FWIW, the SEAMCALL itself disables IRQs (and everything else), so it's not _that_
big of a change. But yeah, waiting on the spinlock with IRQs disabled isn't
exactly idea.
> For your reference, I measured some test data by concurrently launching and
> destroying 4 TDs for 3 rounds:
>
> t0 ---------------------
> scoped_guard(spinlock, &pamt_lock) { |->T1=t1-t0 |
> t1 ---------- |
> ... |
> t2 ---------- |->T3=t4-t0
> tdh_phymem_pamt_add/remove() |->T2=t3-t2 |
> t3 ---------- |
> ... |
> t4 ---------------------
> }
>
> (1) for __tdx_pamt_get()
>
> avg us min us max us
> ------|---------------------------
> T1 | 4 0 69
> T2 | 4 2 18
> T3 | 10 3 83
>
>
> (2) for__tdx_pamt_put()
>
> avg us min us max us
> ------|---------------------------
> T1 | 0 0 5
> T2 | 2 1 11
> T3 | 3 2 15
>
>
> > Option #2 would be to immediately free the page in tdx_sept_reclaim_private_sp(),
> > so that pages that freed via handle_removed_pt() don't defer freeing the S-EPT
> > page table (which, IIUC, is safe since the TDX-Module forces TLB flushes and exits).
> >
> > I really, really don't like this option (if it even works).
> I don't like its asymmetry with tdx_sept_link_private_spt().
>
> However, do you think it would be good to have the PAMT pages of the sept pages
> allocated from (*topup_private_mapping_cache) [1]?
Hrm, dunno about "good", but it's definitely not terrible. To get the cache
management right, it means adding yet another use of kvm_get_running_vcpu(), which
I really dislike.
On the other hand, if we combine that with TDX freeing in-use S-EPT page tables,
unless I'm overly simplifying things, it would avoid having to extend
kvm_mmu_memory_cache with the page_{get,free}() hook, and would then eliminate
two kvm_x86_ops hooks, because the alloc/free of _unused_ S-EPT page tables is
no different than regular page tables.
As a bonus, we could keep the topup_external_cache() name and just clarify that
the parameter specifies the number of page table pages, i.e. account for the +1
for the mapping page in TDX code.
All in all, I'm kinda leaning in this direction, because as much as I dislike
kvm_get_running_vcpu(), it does minimize the number of kvm_x86_ops hooks.
Something like this? Also pushed to
https://github.com/sean-jc/linux.git x86/tdx_huge_sept_alt
if it doesn't apply.
---
arch/x86/include/asm/kvm-x86-ops.h | 6 +--
arch/x86/include/asm/kvm_host.h | 15 ++------
arch/x86/kvm/mmu/mmu.c | 3 --
arch/x86/kvm/mmu/tdp_mmu.c | 23 +++++++-----
arch/x86/kvm/vmx/tdx.c | 60 ++++++++++++++++++++----------
5 files changed, 61 insertions(+), 46 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 6083fb07cd3b..4b865617a421 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -94,11 +94,9 @@ KVM_X86_OP_OPTIONAL_RET0(set_tss_addr)
KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
KVM_X86_OP(load_mmu_pgd)
-KVM_X86_OP_OPTIONAL(alloc_external_sp)
-KVM_X86_OP_OPTIONAL(free_external_sp)
-KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
-KVM_X86_OP_OPTIONAL(reclaim_external_sp)
+KVM_X86_OP_OPTIONAL(reclaim_external_spt)
KVM_X86_OP_OPTIONAL_RET0(topup_external_cache)
+KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
KVM_X86_OP(has_wbinvd_exit)
KVM_X86_OP(get_l2_tsc_offset)
KVM_X86_OP(get_l2_tsc_multiplier)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cd3e7dc6ab9b..d3c31eaf18b1 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1850,19 +1850,12 @@ struct kvm_x86_ops {
void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
int root_level);
- /*
- * Callbacks to allocate and free external page tables, a.k.a. S-EPT,
- * and to propagate changes in mirror page tables to the external page
- * tables.
- */
- unsigned long (*alloc_external_sp)(gfp_t gfp);
- void (*free_external_sp)(unsigned long addr);
+ void (*reclaim_external_spt)(struct kvm *kvm, gfn_t gfn,
+ struct kvm_mmu_page *sp);
+ int (*topup_external_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu,
+ int min_nr_spts);
int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, u64 old_spte,
u64 new_spte, enum pg_level level);
- void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
- struct kvm_mmu_page *sp);
- int (*topup_external_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu, int min);
-
bool (*has_wbinvd_exit)(void);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 62bf6bec2df2..f7cf456d9404 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6714,9 +6714,6 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
if (!vcpu->arch.mmu_shadow_page_cache.init_value)
vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
- vcpu->arch.mmu_external_spt_cache.page_get = kvm_x86_ops.alloc_external_sp;
- vcpu->arch.mmu_external_spt_cache.page_free = kvm_x86_ops.free_external_sp;
-
vcpu->arch.mmu = &vcpu->arch.root_mmu;
vcpu->arch.walk_mmu = &vcpu->arch.root_mmu;
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index fef856323821..732548a678d8 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -53,14 +53,18 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
rcu_barrier();
}
-static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
+static void __tdp_mmu_free_sp(struct kvm_mmu_page *sp)
{
- if (sp->external_spt)
- kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
free_page((unsigned long)sp->spt);
kmem_cache_free(mmu_page_header_cache, sp);
}
+static void tdp_mmu_free_unused_sp(struct kvm_mmu_page *sp)
+{
+ free_page((unsigned long)sp->external_spt);
+ __tdp_mmu_free_sp(sp);
+}
+
/*
* This is called through call_rcu in order to free TDP page table memory
* safely with respect to other kernel threads that may be operating on
@@ -74,7 +78,8 @@ static void tdp_mmu_free_sp_rcu_callback(struct rcu_head *head)
struct kvm_mmu_page *sp = container_of(head, struct kvm_mmu_page,
rcu_head);
- tdp_mmu_free_sp(sp);
+ WARN_ON_ONCE(sp->external_spt);
+ __tdp_mmu_free_sp(sp);
}
void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root)
@@ -458,7 +463,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
}
if (is_mirror_sp(sp))
- kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
+ kvm_x86_call(reclaim_external_spt)(kvm, base_gfn, sp);
call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
}
@@ -1266,7 +1271,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
* failed, e.g. because a different task modified the SPTE.
*/
if (r) {
- tdp_mmu_free_sp(sp);
+ tdp_mmu_free_unused_sp(sp);
goto retry;
}
@@ -1461,7 +1466,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
goto err_spt;
if (is_mirror_sp) {
- sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
+ sp->external_spt = (void *)__get_free_page(GFP_KERNEL_ACCOUNT);
if (!sp->external_spt)
goto err_external_spt;
@@ -1472,7 +1477,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
return sp;
err_external_split:
- kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
+ free_page((unsigned long)sp->external_spt);
err_external_spt:
free_page((unsigned long)sp->spt);
err_spt:
@@ -1594,7 +1599,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
* installs its own sp in place of the last sp we tried to split.
*/
if (sp)
- tdp_mmu_free_sp(sp);
+ tdp_mmu_free_unused_sp(sp);
return 0;
}
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ae7b9beb3249..b0fc17baa1fc 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1701,7 +1701,7 @@ static struct tdx_pamt_cache *tdx_get_pamt_cache(struct kvm *kvm,
}
static int tdx_topup_external_pamt_cache(struct kvm *kvm,
- struct kvm_vcpu *vcpu, int min)
+ struct kvm_vcpu *vcpu, int min_nr_spts)
{
struct tdx_pamt_cache *pamt_cache;
@@ -1712,7 +1712,11 @@ static int tdx_topup_external_pamt_cache(struct kvm *kvm,
if (!pamt_cache)
return -EIO;
- return tdx_topup_pamt_cache(pamt_cache, min);
+ /*
+ * Each S-EPT page tables requires a DPAMT pair, plus one more for the
+ * memory being mapped into the guest.
+ */
+ return tdx_topup_pamt_cache(pamt_cache, min_nr_spts + 1);
}
static int tdx_mem_page_add(struct kvm *kvm, gfn_t gfn, enum pg_level level,
@@ -1911,23 +1915,41 @@ static int tdx_sept_split_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
static int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn, u64 new_spte,
enum pg_level level)
{
+ struct tdx_pamt_cache *pamt_cache;
gpa_t gpa = gfn_to_gpa(gfn);
u64 err, entry, level_state;
struct page *external_spt;
+ int r;
external_spt = tdx_spte_to_external_spt(kvm, gfn, new_spte, level);
if (!external_spt)
return -EIO;
+ pamt_cache = tdx_get_pamt_cache(kvm, kvm_get_running_vcpu());
+ if (!pamt_cache)
+ return -EIO;
+
+ r = tdx_pamt_get(page_to_pfn(external_spt), PG_LEVEL_4K, pamt_cache);
+ if (r)
+ return r;
+
err = tdh_mem_sept_add(&to_kvm_tdx(kvm)->td, gpa, level, external_spt,
&entry, &level_state);
- if (unlikely(IS_TDX_OPERAND_BUSY(err)))
- return -EBUSY;
+ if (unlikely(IS_TDX_OPERAND_BUSY(err))) {
+ r = -EBUSY;
+ goto err;
+ }
- if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm))
- return -EIO;
+ if (TDX_BUG_ON_2(err, TDH_MEM_SEPT_ADD, entry, level_state, kvm)) {
+ r = -EIO;
+ goto err;
+ }
return 0;
+
+err:
+ tdx_pamt_put(page_to_pfn(external_spt), PG_LEVEL_4K);
+ return r;
}
static int tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn,
@@ -1995,8 +2017,8 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
return tdx_sept_map_leaf_spte(kvm, gfn, new_spte, level);
}
-static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
- struct kvm_mmu_page *sp)
+static void tdx_sept_reclaim_private_spt(struct kvm *kvm, gfn_t gfn,
+ struct kvm_mmu_page *sp)
{
/*
* KVM doesn't (yet) zap page table pages in mirror page table while
@@ -2014,7 +2036,16 @@ static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
*/
if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
tdx_reclaim_page(virt_to_page(sp->external_spt)))
- sp->external_spt = NULL;
+ goto out;
+
+ /*
+ * Immediately free the S-EPT page as the TDX subsystem doesn't support
+ * freeing pages from RCU callbacks, and more importantly because
+ * TDH.PHYMEM.PAGE.RECLAIM ensures there are no outstanding readers.
+ */
+ tdx_free_control_page((unsigned long)sp->external_spt);
+out:
+ sp->external_spt = NULL;
}
void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
@@ -3869,17 +3900,8 @@ void __init tdx_hardware_setup(void)
*/
vt_x86_ops.vm_size = max_t(unsigned int, vt_x86_ops.vm_size, sizeof(struct kvm_tdx));
- /*
- * TDX uses the external_spt cache to allocate S-EPT page table pages,
- * which (a) don't need to be initialized by KVM as the TDX-Module will
- * initialize the page (using the guest's encryption key), and (b) need
- * to use a custom allocator to be compatible with Dynamic PAMT.
- */
- vt_x86_ops.alloc_external_sp = tdx_alloc_control_page;
- vt_x86_ops.free_external_sp = tdx_free_control_page;
-
+ vt_x86_ops.reclaim_external_spt = tdx_sept_reclaim_private_spt;
vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
- vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
vt_x86_ops.gmem_convert = tdx_gmem_convert;
base-commit: 7adb9e428488cf7873a122043385a50dc1eebc8f
--
^ permalink raw reply related
* Re: [RFC PATCH v5 20/45] KVM: x86/mmu: Allocate/free S-EPT pages using tdx_{alloc,free}_control_page()
From: Sean Christopherson @ 2026-02-09 22:44 UTC (permalink / raw)
To: Kai Huang
Cc: Yan Y Zhao, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
Xiaoyao Li, dave.hansen@linux.intel.com, kas@kernel.org,
binbin.wu@linux.intel.com, mingo@redhat.com, pbonzini@redhat.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
Isaku Yamahata, sagis@google.com, tglx@kernel.org,
Rick P Edgecombe, bp@alien8.de, Vishal Annapurve, x86@kernel.org
In-Reply-To: <c753636171f82c3b6d64e7734be3a70c60775546.camel@intel.com>
On Mon, Feb 09, 2026, Kai Huang wrote:
> > Option #2 would be to immediately free the page in tdx_sept_reclaim_private_sp(),
> > so that pages that freed via handle_removed_pt() don't defer freeing the S-EPT
> > page table (which, IIUC, is safe since the TDX-Module forces TLB flushes and exits).
> >
> > I really, really don't like this option (if it even works).
> >
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index ae7b9beb3249..4726011ad624 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -2014,7 +2014,15 @@ static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
> > */
> > if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
> > tdx_reclaim_page(virt_to_page(sp->external_spt)))
> > - sp->external_spt = NULL;
> > + goto out;
> > +
> > + /*
> > + * Immediately free the control page, as the TDX subsystem doesn't
> > + * support freeing pages from RCU callbacks.
> > + */
> > + tdx_free_control_page((unsigned long)sp->external_spt);
> > +out:
> > + sp->external_spt = NULL;
> > }
> >
> > void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
>
> I don't think this is so bad, given we already have a bunch of
>
> is_mirror_sp(sp)
> kvm_x86_call(xx_external_spt)(..);
>
> in TDP MMU code?
>
> I suppose this won't make a lot of difference, but does below make you
> slightly happier?
Heh, slightly, but I still don't love it :-D
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 3181406c5e0b..3588265098a8 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -55,8 +55,7 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
>
> static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> {
> - if (sp->external_spt)
> - kvm_x86_call(free_external_sp)((unsigned long)sp-
> >external_spt);
> + WARN_ON_ONCE(sp->external_spt);
Doesn't work, because sp->external_spt will be non-NULL when KVM is freeing
unused pages in tdp_mmu_split_huge_pages_root() and kvm_tdp_mmu_map(). That's
solvable, but it's part of the asymmetry I don't love. AFAICT, unless we do
something truly awful, there's no way to avoid having common KVM free unused
S-EPT pages.
That said, while I don't love the asymmetry, it's not a deal breaker, especially
if we make the asymmetry super obvious and cleanly delineated. Specifically, if
we differentiate between freeing unused page tables and freeing used (linked at
any point) page tables.
This would also allow us to address the naming than Yan doesn't like around
reclaim_external_sp(), because we could have both free_external_sp() and
free_unused_external_spt(), where the lack of "unused" gives the reader a hint
that there's interesting work to be done for in-use external page tables.
This won't apply cleanly due to other fixups. It's also at:
https://github.com/sean-jc/linux.git x86/tdx_huge_sept
---
arch/x86/include/asm/kvm-x86-ops.h | 8 ++++----
arch/x86/include/asm/kvm_host.h | 10 +++++-----
arch/x86/kvm/mmu/mmu.c | 4 ++--
arch/x86/kvm/mmu/tdp_mmu.c | 27 +++++++++++++++++----------
arch/x86/kvm/vmx/tdx.c | 22 +++++++++++++++-------
5 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 87826176fa8d..9593d8d97f6b 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -94,11 +94,11 @@ KVM_X86_OP_OPTIONAL_RET0(set_tss_addr)
KVM_X86_OP_OPTIONAL_RET0(set_identity_map_addr)
KVM_X86_OP_OPTIONAL_RET0(get_mt_mask)
KVM_X86_OP(load_mmu_pgd)
-KVM_X86_OP_OPTIONAL(alloc_external_sp)
-KVM_X86_OP_OPTIONAL(free_external_sp)
-KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
-KVM_X86_OP_OPTIONAL(reclaim_external_sp)
+KVM_X86_OP_OPTIONAL_RET0(alloc_external_spt)
+KVM_X86_OP_OPTIONAL(free_external_spt)
+KVM_X86_OP_OPTIONAL(free_unused_external_spt)
KVM_X86_OP_OPTIONAL_RET0(topup_private_mapping_cache)
+KVM_X86_OP_OPTIONAL_RET0(set_external_spte)
KVM_X86_OP(has_wbinvd_exit)
KVM_X86_OP(get_l2_tsc_offset)
KVM_X86_OP(get_l2_tsc_multiplier)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7ff72c04d575..5fc25508548b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1855,13 +1855,13 @@ struct kvm_x86_ops {
* and to propagate changes in mirror page tables to the external page
* tables.
*/
- unsigned long (*alloc_external_sp)(gfp_t gfp);
- void (*free_external_sp)(unsigned long addr);
+ unsigned long (*alloc_external_spt)(gfp_t gfp);
+ void (*free_external_spt)(struct kvm *kvm, gfn_t gfn,
+ struct kvm_mmu_page *sp);
+ void (*free_unused_external_spt)(unsigned long external_spt);
+ int (*topup_private_mapping_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu);
int (*set_external_spte)(struct kvm *kvm, gfn_t gfn, u64 old_spte,
u64 new_spte, enum pg_level level);
- void (*reclaim_external_sp)(struct kvm *kvm, gfn_t gfn,
- struct kvm_mmu_page *sp);
- int (*topup_private_mapping_cache)(struct kvm *kvm, struct kvm_vcpu *vcpu);
bool (*has_wbinvd_exit)(void);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6a911aec075b..2ad417ac6e1f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6714,8 +6714,8 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
if (!vcpu->arch.mmu_shadow_page_cache.init_value)
vcpu->arch.mmu_shadow_page_cache.gfp_zero = __GFP_ZERO;
- vcpu->arch.mmu_external_spt_cache.page_get = kvm_x86_ops.alloc_external_sp;
- vcpu->arch.mmu_external_spt_cache.page_free = kvm_x86_ops.free_external_sp;
+ vcpu->arch.mmu_external_spt_cache.page_get = kvm_x86_ops.alloc_external_spt;
+ vcpu->arch.mmu_external_spt_cache.page_free = kvm_x86_ops.free_unused_external_spt;
vcpu->arch.mmu = &vcpu->arch.root_mmu;
vcpu->arch.walk_mmu = &vcpu->arch.root_mmu;
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index b7c13316181b..d43db86b12a7 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -53,12 +53,18 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm)
rcu_barrier();
}
-static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
+static void __tdp_mmu_free_sp(struct kvm_mmu_page *sp)
+{
+ free_page((unsigned long)sp->spt);
+ kmem_cache_free(mmu_page_header_cache, sp);
+}
+
+static void tdp_mmu_free_unused_sp(struct kvm_mmu_page *sp)
{
if (sp->external_spt)
- kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
- free_page((unsigned long)sp->spt);
- kmem_cache_free(mmu_page_header_cache, sp);
+ kvm_x86_call(free_unused_external_spt)((unsigned long)sp->external_spt);
+
+ __tdp_mmu_free_sp(sp);
}
/*
@@ -74,7 +80,8 @@ static void tdp_mmu_free_sp_rcu_callback(struct rcu_head *head)
struct kvm_mmu_page *sp = container_of(head, struct kvm_mmu_page,
rcu_head);
- tdp_mmu_free_sp(sp);
+ WARN_ON_ONCE(sp->external_spt);
+ __tdp_mmu_free_sp(sp);
}
void kvm_tdp_mmu_put_root(struct kvm *kvm, struct kvm_mmu_page *root)
@@ -458,7 +465,7 @@ static void handle_removed_pt(struct kvm *kvm, tdp_ptep_t pt, bool shared)
}
if (is_mirror_sp(sp))
- kvm_x86_call(reclaim_external_sp)(kvm, base_gfn, sp);
+ kvm_x86_call(free_external_spt)(kvm, base_gfn, sp);
call_rcu(&sp->rcu_head, tdp_mmu_free_sp_rcu_callback);
}
@@ -1266,7 +1273,7 @@ int kvm_tdp_mmu_map(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
* failed, e.g. because a different task modified the SPTE.
*/
if (r) {
- tdp_mmu_free_sp(sp);
+ tdp_mmu_free_unused_sp(sp);
goto retry;
}
@@ -1461,7 +1468,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
goto err_spt;
if (is_mirror_sp) {
- sp->external_spt = (void *)kvm_x86_call(alloc_external_sp)(GFP_KERNEL_ACCOUNT);
+ sp->external_spt = (void *)kvm_x86_call(alloc_external_spt)(GFP_KERNEL_ACCOUNT);
if (!sp->external_spt)
goto err_external_spt;
@@ -1472,7 +1479,7 @@ static struct kvm_mmu_page *tdp_mmu_alloc_sp_for_split(struct kvm *kvm,
return sp;
err_external_split:
- kvm_x86_call(free_external_sp)((unsigned long)sp->external_spt);
+ kvm_x86_call(free_unused_external_spt)((unsigned long)sp->external_spt);
err_external_spt:
free_page((unsigned long)sp->spt);
err_spt:
@@ -1594,7 +1601,7 @@ static int tdp_mmu_split_huge_pages_root(struct kvm *kvm,
* installs its own sp in place of the last sp we tried to split.
*/
if (sp)
- tdp_mmu_free_sp(sp);
+ tdp_mmu_free_unused_sp(sp);
return 0;
}
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 957fa59e9a65..aae7af26fe02 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1994,8 +1994,8 @@ static int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
return tdx_sept_map_leaf_spte(kvm, gfn, new_spte, level);
}
-static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
- struct kvm_mmu_page *sp)
+static void tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn,
+ struct kvm_mmu_page *sp)
{
/*
* KVM doesn't (yet) zap page table pages in mirror page table while
@@ -2013,7 +2013,16 @@ static void tdx_sept_reclaim_private_sp(struct kvm *kvm, gfn_t gfn,
*/
if (KVM_BUG_ON(is_hkid_assigned(to_kvm_tdx(kvm)), kvm) ||
tdx_reclaim_page(virt_to_page(sp->external_spt)))
- sp->external_spt = NULL;
+ goto out;
+
+ /*
+ * Immediately free the S-EPT page as the TDX subsystem doesn't support
+ * freeing pages from RCU callbacks, and more importantly because
+ * TDH.PHYMEM.PAGE.RECLAIM ensures there are no outstanding readers.
+ */
+ tdx_free_control_page((unsigned long)sp->external_spt);
+out:
+ sp->external_spt = NULL;
}
void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
@@ -3874,11 +3883,10 @@ void __init tdx_hardware_setup(void)
* initialize the page (using the guest's encryption key), and (b) need
* to use a custom allocator to be compatible with Dynamic PAMT.
*/
- vt_x86_ops.alloc_external_sp = tdx_alloc_control_page;
- vt_x86_ops.free_external_sp = tdx_free_control_page;
-
+ vt_x86_ops.alloc_external_spt = tdx_alloc_control_page;
+ vt_x86_ops.free_unused_external_spt = tdx_free_control_page;
+ vt_x86_ops.free_external_spt = tdx_sept_free_private_spt;
vt_x86_ops.set_external_spte = tdx_sept_set_private_spte;
- vt_x86_ops.reclaim_external_sp = tdx_sept_reclaim_private_sp;
vt_x86_ops.gmem_convert = tdx_gmem_convert;
base-commit: 0191132f233a66b5cb1ae9a09c18c6d6669c284a
--
^ permalink raw reply related
* Re: [PATCH] crypto: ccp - allow callers to use HV-Fixed page API when SEV is disabled
From: Tom Lendacky @ 2026-02-09 22:42 UTC (permalink / raw)
To: Ashish Kalra, john.allen, herbert, davem, bp
Cc: linux-crypto, linux-kernel, linux-coco
In-Reply-To: <20260206212645.125485-1-Ashish.Kalra@amd.com>
On 2/6/26 15:26, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> When SEV is disabled, the HV-Fixed page allocation call fails, which in
> turn causes SFS initialization to fail.
>
> Fix the HV-Fixed API so callers (for example, SFS) can use it even when
> SEV is disabled by performing normal page allocation and freeing.
>
> Fixes: e09701dcdd9c ("crypto: ccp - Add new HV-Fixed page allocation/free API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> drivers/crypto/ccp/sev-dev.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 1cdadddb744e..0d90b5f6a454 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1105,15 +1105,12 @@ struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages)
> {
> struct psp_device *psp_master = psp_get_master_device();
> struct snp_hv_fixed_pages_entry *entry;
> - struct sev_device *sev;
> unsigned int order;
> struct page *page;
>
> - if (!psp_master || !psp_master->sev_data)
> + if (!psp_master)
> return NULL;
>
> - sev = psp_master->sev_data;
> -
> order = get_order(PMD_SIZE * num_2mb_pages);
>
> /*
> @@ -1126,7 +1123,8 @@ struct page *snp_alloc_hv_fixed_pages(unsigned int num_2mb_pages)
> * This API uses SNP_INIT_EX to transition allocated pages to HV_Fixed
> * page state, fail if SNP is already initialized.
> */
> - if (sev->snp_initialized)
> + if (psp_master->sev_data &&
> + ((struct sev_device *)psp_master->sev_data)->snp_initialized)
> return NULL;
>
> /* Re-use freed pages that match the request */
> @@ -1162,7 +1160,7 @@ void snp_free_hv_fixed_pages(struct page *page)
> struct psp_device *psp_master = psp_get_master_device();
> struct snp_hv_fixed_pages_entry *entry, *nentry;
>
> - if (!psp_master || !psp_master->sev_data)
> + if (!psp_master)
> return;
>
> /*
^ permalink raw reply
* Re: [RFC PATCH v5 22/45] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Huang, Kai @ 2026-02-09 21:05 UTC (permalink / raw)
To: seanjc@google.com, Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Zhao, Yan Y, dave.hansen@linux.intel.com,
linux-kernel@vger.kernel.org, kas@kernel.org,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, ackerleytng@google.com, tglx@kernel.org,
Annapurve, Vishal, sagis@google.com, bp@alien8.de, x86@kernel.org
In-Reply-To: <ddf6c17664044bf66e7a9a0a58f2b6c1104dfbc4.camel@intel.com>
On Mon, 2026-02-09 at 17:08 +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-02-09 at 10:33 +0000, Huang, Kai wrote:
> > In the fault path, we already know the PFN after
> > kvm_mmu_faultin_pfn(), which is outside of MMU lock.
> >
> > What we still don't know is the actual mapping level, which is
> > currently done in kvm_tdp_mmu_map() via kvm_mmu_hugepage_adjust().
> >
> > However I don't see why we cannot move kvm_mmu_hugepage_adjust() out
> > of it to, e.g., right after kvm_mmu_faultin_pfn()?
> >
> > If we can do this, then AFAICT we can just do:
> >
> > r = kvm_x86_call(prepare_pfn)(vcpu, fault, pfn);
>
> What about the adjustments in disallowed_hugepage_adjust()?
AFAICT that's for preventing replacing existing small leafs with a huge
mapping, which is not supported by TDX in this series (PROMOTE isn't
supported).
Even we want to support PROMOTE in the future, it doesn't impact the logic
that we don't need to call tdx_pamt_get(pfn) if the fault->goal_level is 2M.
When KVM tries to replace the existing small leafs with huge SPTE for TDX,
the PROMOTE returns the now-unneeded PAMT pages and KVM can just free that.
There's no impact to non-TDX case too, I believe, because moving
kvm_mmu_hugepage_adjust() out of kvm_tdp_mmu_map() only moves it out, but
doesn't change the whole logic.
^ permalink raw reply
* Re: [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: John Stultz @ 2026-02-09 20:08 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, tjmercier, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <20260209153809.250835-5-jiri@resnulli.us>
On Mon, Feb 9, 2026 at 7:38 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@nvidia.com>
>
> Currently the flags, which are unused, are validated for all heaps.
> Since the follow-up patch introduces a flag valid for only one of the
> heaps, allow to specify the valid flags per-heap.
I'm not really in this space anymore, so take my feedback with a grain of salt.
While the heap allocate flags argument is unused, it was intended to
be used for generic allocation flags that would apply to all or at
least a wide majority of heaps.
It was definitely not added to allow for per-heap or heap specific
flags (as this patch tries to utilize it). That was the mess we had
with ION driver that we were trying to avoid.
The intent of dma-buf heaps is to try to abstract all the different
device memory constraints so there only needs to be a [usage] ->
[heap] mapping, and otherwise userland can be generalized so that it
doesn't need to be re-written to work with different devices/memory
types. Adding heap-specific allocation flags prevents that
generalization.
So instead of adding heap specific flags, the general advice has been
to add a separate heap name for the flag property.
Now, there has been many discussions around "protected buffers" (which
doesn't seem to map exactly to this confidental computing primitive,
but sounds like it might be related) , which have bounced between
being a allocation flag or a device specific heap without much
resolution. I appreciate in this patch seires you've pushed your
concept down into a DMA_ATTR_, as I do feel the kernel should have a
deeper sense of protected buffers (or any general propery like this)
as a concept if it is going to be a generic allocation flag, instead
of it being a somewhat thin creation of the outer heap-driver layer.
But, it seems like the use case here is still far too narrow for a top
level allocation flag.
So I'd advocate against introducing heap-specific flags like this.
thanks
-john
^ permalink raw reply
* Re: [RFC PATCH v5 22/45] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Edgecombe, Rick P @ 2026-02-09 17:08 UTC (permalink / raw)
To: seanjc@google.com, Huang, Kai
Cc: kvm@vger.kernel.org, Li, Xiaoyao, linux-coco@lists.linux.dev,
Zhao, Yan Y, dave.hansen@linux.intel.com, kas@kernel.org,
mingo@redhat.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
Yamahata, Isaku, ackerleytng@google.com,
linux-kernel@vger.kernel.org, tglx@kernel.org, sagis@google.com,
bp@alien8.de, Annapurve, Vishal, x86@kernel.org
In-Reply-To: <94f041b3aa32169fa2e1125edab7bd8fed3a6e59.camel@intel.com>
On Mon, 2026-02-09 at 10:33 +0000, Huang, Kai wrote:
> In the fault path, we already know the PFN after
> kvm_mmu_faultin_pfn(), which is outside of MMU lock.
>
> What we still don't know is the actual mapping level, which is
> currently done in kvm_tdp_mmu_map() via kvm_mmu_hugepage_adjust().
>
> However I don't see why we cannot move kvm_mmu_hugepage_adjust() out
> of it to, e.g., right after kvm_mmu_faultin_pfn()?
>
> If we can do this, then AFAICT we can just do:
>
> r = kvm_x86_call(prepare_pfn)(vcpu, fault, pfn);
What about the adjustments in disallowed_hugepage_adjust()?
^ permalink raw reply
* [PATCH 5/5] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: Jiri Pirko @ 2026-02-09 15:38 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Add a new DMA_HEAP_FLAG_DECRYPTED heap flag to allow userspace to
allocate decrypted (shared) memory from the dma-buf system heap for
confidential computing (CoCo) VMs.
On CoCo VMs, guest memory is encrypted by default. The hardware uses an
encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
Intel TDX) to control whether a given memory access is encrypted or
decrypted. The kernel's direct map is set up with encryption enabled,
so pages returned by alloc_pages() are encrypted in the direct map
by default. To make this memory usable for devices that do not support
DMA to encrypted memory (no TDISP support), it has to be explicitly
decrypted. A couple of things are needed to properly handle
decrypted memory for the dma-buf use case:
- set_memory_decrypted() on the direct map after allocation:
Besides clearing the encryption bit in the direct map PTEs, this
also notifies the hypervisor about the page state change. On free,
the inverse set_memory_encrypted() must be called before returning
pages to the allocator. If re-encryption fails, pages
are intentionally leaked to prevent decrypted memory from being
reused as private.
- pgprot_decrypted() for userspace and kernel virtual mappings:
Any new mapping of the decrypted pages, be it to userspace via
mmap or to kernel vmalloc space via vmap, creates PTEs independent
of the direct map. These must also have the encryption bit cleared,
otherwise accesses through them would see encrypted (garbage) data.
- DMA_ATTR_CC_DECRYPTED for DMA mapping:
Since the pages are already decrypted, the DMA API needs to be
informed via DMA_ATTR_CC_DECRYPTED so it can map them correctly
as unencrypted for device access.
On non-CoCo VMs the flag is rejected with -EOPNOTSUPP to prevent
misuse by userspace that does not understand the security implications
of explicitly decrypted memory.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
drivers/dma-buf/heaps/system_heap.c | 87 +++++++++++++++++++++++++++--
include/linux/dma-heap.h | 1 +
include/uapi/linux/dma-heap.h | 12 +++-
3 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 124dca56e4d8..0f80ecb660ec 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -10,6 +10,7 @@
* Andrew F. Davis <afd@ti.com>
*/
+#include <linux/cc_platform.h>
#include <linux/dma-buf.h>
#include <linux/dma-mapping.h>
#include <linux/dma-heap.h>
@@ -17,7 +18,9 @@
#include <linux/highmem.h>
#include <linux/mm.h>
#include <linux/module.h>
+#include <linux/pgtable.h>
#include <linux/scatterlist.h>
+#include <linux/set_memory.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
@@ -29,6 +32,7 @@ struct system_heap_buffer {
struct sg_table sg_table;
int vmap_cnt;
void *vaddr;
+ bool decrypted;
};
struct dma_heap_attachment {
@@ -36,6 +40,7 @@ struct dma_heap_attachment {
struct sg_table table;
struct list_head list;
bool mapped;
+ bool decrypted;
};
#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO)
@@ -52,6 +57,34 @@ static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP};
static const unsigned int orders[] = {8, 4, 0};
#define NUM_ORDERS ARRAY_SIZE(orders)
+static int system_heap_set_page_decrypted(struct page *page)
+{
+ unsigned long addr = (unsigned long)page_address(page);
+ unsigned int nr_pages = 1 << compound_order(page);
+ int ret;
+
+ ret = set_memory_decrypted(addr, nr_pages);
+ if (ret)
+ pr_warn_ratelimited("dma-buf system heap: failed to decrypt page at %p\n",
+ page_address(page));
+
+ return ret;
+}
+
+static int system_heap_set_page_encrypted(struct page *page)
+{
+ unsigned long addr = (unsigned long)page_address(page);
+ unsigned int nr_pages = 1 << compound_order(page);
+ int ret;
+
+ ret = set_memory_encrypted(addr, nr_pages);
+ if (ret)
+ pr_warn_ratelimited("dma-buf system heap: failed to re-encrypt page at %p, leaking memory\n",
+ page_address(page));
+
+ return ret;
+}
+
static int dup_sg_table(struct sg_table *from, struct sg_table *to)
{
struct scatterlist *sg, *new_sg;
@@ -90,6 +123,7 @@ static int system_heap_attach(struct dma_buf *dmabuf,
a->dev = attachment->dev;
INIT_LIST_HEAD(&a->list);
a->mapped = false;
+ a->decrypted = buffer->decrypted;
attachment->priv = a;
@@ -119,9 +153,11 @@ static struct sg_table *system_heap_map_dma_buf(struct dma_buf_attachment *attac
{
struct dma_heap_attachment *a = attachment->priv;
struct sg_table *table = &a->table;
+ unsigned long attrs;
int ret;
- ret = dma_map_sgtable(attachment->dev, table, direction, 0);
+ attrs = a->decrypted ? DMA_ATTR_CC_DECRYPTED : 0;
+ ret = dma_map_sgtable(attachment->dev, table, direction, attrs);
if (ret)
return ERR_PTR(ret);
@@ -188,8 +224,13 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
unsigned long addr = vma->vm_start;
unsigned long pgoff = vma->vm_pgoff;
struct scatterlist *sg;
+ pgprot_t prot;
int i, ret;
+ prot = vma->vm_page_prot;
+ if (buffer->decrypted)
+ prot = pgprot_decrypted(prot);
+
for_each_sgtable_sg(table, sg, i) {
unsigned long n = sg->length >> PAGE_SHIFT;
@@ -206,8 +247,7 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
if (addr + size > vma->vm_end)
size = vma->vm_end - addr;
- ret = remap_pfn_range(vma, addr, page_to_pfn(page),
- size, vma->vm_page_prot);
+ ret = remap_pfn_range(vma, addr, page_to_pfn(page), size, prot);
if (ret)
return ret;
@@ -225,6 +265,7 @@ static void *system_heap_do_vmap(struct system_heap_buffer *buffer)
struct page **pages = vmalloc(sizeof(struct page *) * npages);
struct page **tmp = pages;
struct sg_page_iter piter;
+ pgprot_t prot;
void *vaddr;
if (!pages)
@@ -235,7 +276,10 @@ static void *system_heap_do_vmap(struct system_heap_buffer *buffer)
*tmp++ = sg_page_iter_page(&piter);
}
- vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
+ prot = PAGE_KERNEL;
+ if (buffer->decrypted)
+ prot = pgprot_decrypted(prot);
+ vaddr = vmap(pages, npages, VM_MAP, prot);
vfree(pages);
if (!vaddr)
@@ -296,6 +340,14 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
for_each_sgtable_sg(table, sg, i) {
struct page *page = sg_page(sg);
+ /*
+ * Intentionally leak pages that cannot be re-encrypted
+ * to prevent decrypted memory from being reused.
+ */
+ if (buffer->decrypted &&
+ system_heap_set_page_encrypted(page))
+ continue;
+
__free_pages(page, compound_order(page));
}
sg_free_table(table);
@@ -344,6 +396,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
unsigned long size_remaining = len;
unsigned int max_order = orders[0];
+ bool decrypted = heap_flags & DMA_HEAP_FLAG_DECRYPTED;
struct dma_buf *dmabuf;
struct sg_table *table;
struct scatterlist *sg;
@@ -351,6 +404,15 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
struct page *page, *tmp_page;
int i, ret = -ENOMEM;
+ if (decrypted) {
+ if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
+ return ERR_PTR(-EOPNOTSUPP);
+#ifdef CONFIG_HIGHMEM
+ /* Sanity check, should not happen. */
+ return ERR_PTR(-EINVAL);
+#endif
+ }
+
buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
if (!buffer)
return ERR_PTR(-ENOMEM);
@@ -359,6 +421,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
mutex_init(&buffer->lock);
buffer->heap = heap;
buffer->len = len;
+ buffer->decrypted = decrypted;
INIT_LIST_HEAD(&pages);
i = 0;
@@ -393,6 +456,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
list_del(&page->lru);
}
+ if (decrypted) {
+ for_each_sgtable_sg(table, sg, i) {
+ ret = system_heap_set_page_decrypted(sg_page(sg));
+ if (ret)
+ goto free_pages;
+ }
+ }
+
/* create the dmabuf */
exp_info.exp_name = dma_heap_get_name(heap);
exp_info.ops = &system_heap_buf_ops;
@@ -410,6 +481,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
for_each_sgtable_sg(table, sg, i) {
struct page *p = sg_page(sg);
+ /*
+ * Intentionally leak pages that cannot be re-encrypted
+ * to prevent decrypted memory from being reused.
+ */
+ if (buffer->decrypted &&
+ system_heap_set_page_encrypted(p))
+ continue;
__free_pages(p, compound_order(p));
}
sg_free_table(table);
@@ -430,6 +508,7 @@ static int __init system_heap_create(void)
struct dma_heap_export_info exp_info = {
.name = "system",
.ops = &system_heap_ops,
+ .valid_heap_flags = DMA_HEAP_FLAG_DECRYPTED,
};
struct dma_heap *sys_heap;
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 7cfb531a9281..295a7eaa19ca 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -10,6 +10,7 @@
#define _DMA_HEAPS_H
#include <linux/types.h>
+#include <uapi/linux/dma-heap.h>
struct dma_heap;
diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
index a4cf716a49fa..6552c88e52f6 100644
--- a/include/uapi/linux/dma-heap.h
+++ b/include/uapi/linux/dma-heap.h
@@ -18,8 +18,16 @@
/* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
#define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
-/* Currently no heap flags */
-#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
+/**
+ * DMA_HEAP_FLAG_DECRYPTED - Allocate decrypted (shared) memory
+ *
+ * For confidential computing guests (AMD SEV, Intel TDX), this flag
+ * requests that the allocated memory be marked as decrypted (shared
+ * with the host).
+ */
+#define DMA_HEAP_FLAG_DECRYPTED (1ULL << 0)
+
+#define DMA_HEAP_VALID_HEAP_FLAGS (DMA_HEAP_FLAG_DECRYPTED)
/**
* struct dma_heap_allocation_data - metadata passed from userspace for
--
2.51.1
^ permalink raw reply related
* [PATCH 4/5] dma-buf: heaps: allow heap to specify valid heap flags
From: Jiri Pirko @ 2026-02-09 15:38 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Currently the flags, which are unused, are validated for all heaps.
Since the follow-up patch introduces a flag valid for only one of the
heaps, allow to specify the valid flags per-heap.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
drivers/dma-buf/dma-heap.c | 5 ++++-
include/linux/dma-heap.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8ab49924f8b7..4751bcef4b19 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -28,6 +28,7 @@
* @name: used for debugging/device-node name
* @ops: ops struct for this heap
* @priv: private data for this heap
+ * @valid_heap_flags: valid heap flags for this heap
* @heap_devt: heap device node
* @list: list head connecting to list of heaps
* @heap_cdev: heap char device
@@ -38,6 +39,7 @@ struct dma_heap {
const char *name;
const struct dma_heap_ops *ops;
void *priv;
+ u64 valid_heap_flags;
dev_t heap_devt;
struct list_head list;
struct cdev heap_cdev;
@@ -105,7 +107,7 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
return -EINVAL;
- if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
+ if (heap_allocation->heap_flags & ~heap->valid_heap_flags)
return -EINVAL;
fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
@@ -246,6 +248,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
heap->name = exp_info->name;
heap->ops = exp_info->ops;
heap->priv = exp_info->priv;
+ heap->valid_heap_flags = exp_info->valid_heap_flags;
/* Find unused minor number */
ret = xa_alloc(&dma_heap_minors, &minor, heap,
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 27d15f60950a..7cfb531a9281 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -31,6 +31,7 @@ struct dma_heap_ops {
* @name: used for debugging/device-node name
* @ops: ops struct for this heap
* @priv: heap exporter private data
+ * @valid_heap_flags: valid heap flags for this heap
*
* Information needed to export a new dmabuf heap.
*/
@@ -38,6 +39,7 @@ struct dma_heap_export_info {
const char *name;
const struct dma_heap_ops *ops;
void *priv;
+ u64 valid_heap_flags;
};
void *dma_heap_get_drvdata(struct dma_heap *heap);
--
2.51.1
^ permalink raw reply related
* [PATCH 3/5] dma-buf: heaps: use designated initializer for exp_info
From: Jiri Pirko @ 2026-02-09 15:38 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Use designated initializer for dma_heap_export_info instead of
separate field assignments and avoid the need to explicitly
zero fields in preparation to follow-up patch.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
drivers/dma-buf/heaps/cma_heap.c | 7 ++++---
drivers/dma-buf/heaps/system_heap.c | 9 ++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 42f88193eab9..d12c98be7fa9 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -388,7 +388,10 @@ static const struct dma_heap_ops cma_heap_ops = {
static int __init __add_cma_heap(struct cma *cma, const char *name)
{
- struct dma_heap_export_info exp_info;
+ struct dma_heap_export_info exp_info = {
+ .name = name,
+ .ops = &cma_heap_ops,
+ };
struct cma_heap *cma_heap;
cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
@@ -396,8 +399,6 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
return -ENOMEM;
cma_heap->cma = cma;
- exp_info.name = name;
- exp_info.ops = &cma_heap_ops;
exp_info.priv = cma_heap;
cma_heap->heap = dma_heap_add(&exp_info);
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 4c782fe33fd4..124dca56e4d8 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -427,13 +427,12 @@ static const struct dma_heap_ops system_heap_ops = {
static int __init system_heap_create(void)
{
- struct dma_heap_export_info exp_info;
+ struct dma_heap_export_info exp_info = {
+ .name = "system",
+ .ops = &system_heap_ops,
+ };
struct dma_heap *sys_heap;
- exp_info.name = "system";
- exp_info.ops = &system_heap_ops;
- exp_info.priv = NULL;
-
sys_heap = dma_heap_add(&exp_info);
if (IS_ERR(sys_heap))
return PTR_ERR(sys_heap);
--
2.51.1
^ permalink raw reply related
* [PATCH 2/5] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Jiri Pirko @ 2026-02-09 15:38 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
This is only relevant inside confidential computing (CoCo) virtual
machines, not on the hypervisor side.
Current CoCo designs don't place a vIOMMU in front of untrusted devices.
Instead, the DMA API forces all untrusted device DMA through swiotlb
bounce buffers (is_swiotlb_force_bounce()) which copies data into
decrypted memory on behalf of the device.
When a caller has already arranged for the memory to be decrypted
via set_memory_decrypted(), the DMA API needs to know so it can map
directly using the unencrypted physical address rather than bounce
buffering. Following the pattern of DMA_ATTR_MMIO, add
DMA_ATTR_CC_DECRYPTED for this purpose. Like the MMIO case, only the
caller knows what kind of memory it has and must inform the DMA API
for it to work correctly.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
include/linux/dma-mapping.h | 7 +++++++
include/trace/events/dma.h | 3 ++-
kernel/dma/direct.h | 14 +++++++++++---
3 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index aa36a0d1d9df..052235feb853 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -78,6 +78,13 @@
*/
#define DMA_ATTR_MMIO (1UL << 10)
+/*
+ * DMA_ATTR_CC_DECRYPTED: Indicates memory that has been explicitly decrypted
+ * (shared) for confidential computing guests. The caller must have
+ * called set_memory_decrypted(). A struct page is required.
+ */
+#define DMA_ATTR_CC_DECRYPTED (1UL << 11)
+
/*
* A dma_addr_t can hold any valid DMA or bus address for the platform. It can
* be given to a device to use as a DMA source or target. It is specific to a
diff --git a/include/trace/events/dma.h b/include/trace/events/dma.h
index b3fef140ae15..b3c2cee8841a 100644
--- a/include/trace/events/dma.h
+++ b/include/trace/events/dma.h
@@ -32,7 +32,8 @@ TRACE_DEFINE_ENUM(DMA_NONE);
{ DMA_ATTR_ALLOC_SINGLE_PAGES, "ALLOC_SINGLE_PAGES" }, \
{ DMA_ATTR_NO_WARN, "NO_WARN" }, \
{ DMA_ATTR_PRIVILEGED, "PRIVILEGED" }, \
- { DMA_ATTR_MMIO, "MMIO" })
+ { DMA_ATTR_MMIO, "MMIO" }, \
+ { DMA_ATTR_CC_DECRYPTED, "CC_DECRYPTED" })
DECLARE_EVENT_CLASS(dma_map,
TP_PROTO(struct device *dev, phys_addr_t phys_addr, dma_addr_t dma_addr,
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index 62f0d9d0ba02..ae5bc1919e1c 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -87,16 +87,24 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
dma_addr_t dma_addr;
if (is_swiotlb_force_bounce(dev)) {
- if (attrs & DMA_ATTR_MMIO)
- return DMA_MAPPING_ERROR;
+ if (!(attrs & DMA_ATTR_CC_DECRYPTED)) {
+ if (attrs & DMA_ATTR_MMIO)
+ return DMA_MAPPING_ERROR;
- return swiotlb_map(dev, phys, size, dir, attrs);
+ return swiotlb_map(dev, phys, size, dir, attrs);
+ }
+ } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
+ return DMA_MAPPING_ERROR;
}
if (attrs & DMA_ATTR_MMIO) {
dma_addr = phys;
if (unlikely(!dma_capable(dev, dma_addr, size, false)))
goto err_overflow;
+ } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
+ dma_addr = phys_to_dma_unencrypted(dev, phys);
+ if (unlikely(!dma_capable(dev, dma_addr, size, false)))
+ goto err_overflow;
} else {
dma_addr = phys_to_dma(dev, phys);
if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
--
2.51.1
^ permalink raw reply related
* [PATCH 1/5] dma-mapping: avoid random addr value print out on error path
From: Jiri Pirko @ 2026-02-09 15:38 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
dma_addr is unitialized in dma_direct_map_phys() when swiotlb is forced
and DMA_ATTR_MMIO is set which leads to random value print out in
warning. Fix that by just returning DMA_MAPPING_ERROR.
Fixes: e53d29f957b3 ("dma-mapping: convert dma_direct_*map_page to be phys_addr_t based")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
kernel/dma/direct.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index da2fadf45bcd..62f0d9d0ba02 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -88,7 +88,7 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
if (is_swiotlb_force_bounce(dev)) {
if (attrs & DMA_ATTR_MMIO)
- goto err_overflow;
+ return DMA_MAPPING_ERROR;
return swiotlb_map(dev, phys, size, dir, attrs);
}
--
2.51.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox