* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Jason Gunthorpe @ 2026-07-10 16:19 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Catalin Marinas, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Jiri Pirko,
Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <yq5ajyr3h106.fsf@kernel.org>
On Fri, Jul 10, 2026 at 10:52:49AM +0530, Aneesh Kumar K.V wrote:
> >> > /*
> >> > * For host memory encryption and device requiring unencrypted DMA,
> >> > * MMIO memory is treated as shared by default.
> >> > */
> >> > if (attrs & DMA_ATTR_MMIO) {
> >> > if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) || force_dma_unencrypted(dev))
> >> > attrs |= DMA_ATTR_CC_SHARED;
> >> > }
> >>
> >> Yes, I think it does the trick, preserves the current semantics for AMD.
> >> I guess you could use a single 'if' for all checks (up to you).
> >
> > Please don't change it, MMIO P2P is broken on CC systems today and it
> > should stay broken. Passing DMA_ATTR_MMIO with DMA_ATTR_CC_SHARED is
> > an error that we need to correct in the drivers not make work in the
> > core code.
> >
>
> But the above changes are intended to handle HOST_MEM_ENCRYPT. In v7, we
> had the following diff:
To follow how the rest of the decrypted/encrypted stuff works the MMIO
has to be flaged with CC_SHARED for HOST_MEM_ENCRYPT too, just like
the PTEs.
> @@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
> {
> dma_addr_t dma_addr;
>
> + /*
> + * For a device requiring unencrypted DMA, MMIO memory is treated
> + * as shared by default.
> + */
> + if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
> + attrs |= DMA_ATTR_CC_SHARED;
force_dma_unencrypted() says nothing about the properties of the
address passed in, this was nonsense :\
> As we discussed [1], that can come in a later patch. In the meantime, adding
> the HOST_MEM_ENCRYPT check preserves the previous behavior for SME.
It never worked. When we added ATTR_MMIO it started to have a chance
to work but prior to that it was always broken anyhow. I don't see
there is much merit in preserving the narrow window when we
inadvertantly had a half working ATTR_MMIO.
But if you really want to it should be
cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) *only* and get rid of the wrong
force_dma_unencrypted().
But IMHO, I'd rather this series treat ATTR_MMIO as private MMIO and
ATTR_MMIO|CC_SHARED as shared MMIO and that's the right and correct
thing for the DMA API.
Jason
^ permalink raw reply
* Re: [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Kiryl Shutsemau @ 2026-07-10 15:56 UTC (permalink / raw)
To: Xu Yilun
Cc: x86, linux-kernel, rick.p.edgecombe, dave.hansen, dave.hansen,
yilun.xu, chao.gao, djbw, linux-coco, peter.fang, xiaoyao.li
In-Reply-To: <20260708170330.83850-1-yilun.xu@linux.intel.com>
On Thu, Jul 09, 2026 at 01:03:30AM +0800, Xu Yilun wrote:
> SEAMCALL invokes TDX module functions using a function number and
> parameters. To extend the functionalities of existing SEAMCALLs while
> keeping backward compatibility, TDX adds more numbered SEAMCALLs of the
> same family. This is just like syscalls, except that TDX defines a
> specific function number encoding pattern: a base function number and a
> version together encode the full function number.
>
> An existing SEAMCALL helper (TDH.VP.INIT) is already using the version
> field. Having the caller pack the version into the function number
> open-codes the ABI layout.
>
> Add a version field in struct tdx_module_args [1], so that most existing
> SEAMCALL helpers get a default "version == 0" behavior without code
> churn, while callers requiring extended functionalities can specify the
> version descriptively. Encode the tdx_module_args.version in the
> function number before calling into assembly code.
The commit message covers the mechanics, but is silent on compatibility
with older TDX modules. That context matters for the design choice, so
regardless of where the implementation discussion lands, I think it
needs to be spelled out here.
Who is responsible for picking a version the module supports?
TDH.VP.INIT can hardcode version 1 only because KVM already refuses to
enable TDX on modules without TOPOLOGY_ENUM.
The TDH.SYS.UPDATE user from [1] is the opposite case: it has to pick
version 0 or 1 at runtime depending on whether add-on features are
configured, to keep working on modules that don't support them.
The second point is the actual argument for a version field in struct
tdx_module_args rather than encoding the version in the leaf defines:
the version is not always a compile-time property of the call site.
Without TDH.SYS.UPDATE context, the patch seems pointless.
> Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@intel.com/ # [1]
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Sean Christopherson @ 2026-07-10 14:48 UTC (permalink / raw)
To: Peter Fang
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen
In-Reply-To: <20260710125211.GB2816371@pedri>
On Fri, Jul 10, 2026, Peter Fang wrote:
> On Wed, Jul 08, 2026 at 02:37:55PM -0700, Sean Christopherson wrote:
> > On Thu, Jun 18, 2026, Xu Yilun wrote:
> > And is keying off tdx_quote_enabled() and only tdx_quote_enabled() backwards
> > compatible? How do we know that cutting userspace out of the loop wont' break
> > anything?
>
> Yeah this needs explaining:
>
> The guest-observable difference between the two paths is that it sees
> quotes in different formats (the TDX module uses newer formats of
> course). And an old guest must be able to continue using its
> TDVMCALL<GetQuote> interface for quotes. The guest sends the quote to an
> external verifier to authenticate itself. The verifier (part of the
> cloud infrastructure) must know how to parse this format. So in other
> words, it's the verifier that must maintain backward compatibility if it
> wants to continue accepting old quotes. And it needs to be updated to
> support newer formats as well.
Uh, no, that's not how KVM's guest/host ABI works. Updating the host kernel and
breaking existing guest is not acceptable.
> > > +{
> > > + gfn_t gfn_start, gfn_end;
> > > + u64 end;
> > > +
> > > + if (!size)
> > > + return TDVMCALL_STATUS_INVALID_OPERAND;
> > > +
> > > + if (!PAGE_ALIGNED(gpa) || !PAGE_ALIGNED(size))
> > > + return TDVMCALL_STATUS_ALIGN_ERROR;
> > > +
> > > + if (check_add_overflow(gpa, size, &end))
> > > + return TDVMCALL_STATUS_INVALID_OPERAND;
> > > +
> > > + gfn_start = gpa_to_gfn(gpa);
> > > + gfn_end = gpa_to_gfn(end);
> > > +
> > > + /*
> > > + * Reject if the guest didn't explicitly convert its quote pages to
> > > + * shared.
> > > + */
> > > + if (!kvm_range_has_memory_attributes(vcpu->kvm, gfn_start, gfn_end,
> > > + KVM_MEMORY_ATTRIBUTE_PRIVATE, 0))
> >
> > TOCTOU?
>
> I struggled a bit with this actually... I can't just grab
> kvm->slots_lock here, and dropping kvm->srcu seems bad because the quote
> will be written to guest memory later. I settled on the idea that the
> guest should make sure these pages are converted to shared first. If it
> tries to convert them back to private after this check, before this
> TDVMCALL returns, then it's kind of a self-inflicted race and I don't
> see KVM doing any self damage in this case. Not sure if this makes sense
> to you? Or maybe I should just drop this check and go straight to
> kvm_vcpu_read_guest()?
Yes, drop it and rely on uaccess to do the right thing.
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Peter Fang @ 2026-07-10 12:52 UTC (permalink / raw)
To: Sean Christopherson
Cc: Xu Yilun, x86, kvm, linux-coco, linux-kernel, djbw, kas,
rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen
In-Reply-To: <ak7DMwbF4viiVoO5@google.com>
On Wed, Jul 08, 2026 at 02:37:55PM -0700, Sean Christopherson wrote:
> On Thu, Jun 18, 2026, Xu Yilun wrote:
> > void __init tdx_early_init(void);
> > diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
> > index ac8323a68b16..5e4b3aee0577 100644
> > --- a/arch/x86/kvm/vmx/tdx.h
> > +++ b/arch/x86/kvm/vmx/tdx.h
> > @@ -47,6 +47,12 @@ struct kvm_tdx {
> > * Set/unset is protected with kvm->mmu_lock.
> > */
> > bool wait_for_sept_zap;
> > +
> > + /*
> > + * Whether to get the quote directly in kernel, without exiting to
> > + * userspace.
> > + */
> > + bool get_quote_in_kernel;
>
> Why is this cached per-VM? IIUC, tdx_quote_enabled() is constant after initial
> TDX bringup.
Ah you're right. It shouldn't change. I think I did this because there
was an older version where quoting was not enabled during core TDX
bringup. So this was an attempt to make sure a guest doesn't see
tdx_quote_enabled() flip. It shouldn't be here. Sorry about that.
>
> And is keying off tdx_quote_enabled() and only tdx_quote_enabled() backwards
> compatible? How do we know that cutting userspace out of the loop wont' break
> anything?
Yeah this needs explaining:
The guest-observable difference between the two paths is that it sees
quotes in different formats (the TDX module uses newer formats of
course). And an old guest must be able to continue using its
TDVMCALL<GetQuote> interface for quotes. The guest sends the quote to an
external verifier to authenticate itself. The verifier (part of the
cloud infrastructure) must know how to parse this format. So in other
words, it's the verifier that must maintain backward compatibility if it
wants to continue accepting old quotes. And it needs to be updated to
support newer formats as well.
>
>
> > };
> >
> > /* TDX module vCPU states */
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 9f7c39e0d4b5..20558b0185b6 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -1538,11 +1538,133 @@ static int tdx_get_quote_user(struct kvm_vcpu *vcpu, u64 gpa, u64 size)
> > return 0;
> > }
> >
> > +static bool write_quote_status_to_guest(struct kvm_vcpu *vcpu, u64 status,
> > + gpa_t gpa)
> > +{
> > + if (kvm_vcpu_write_guest(vcpu,
> > + gpa + offsetof(struct tdx_quote_req, status),
> > + &status, sizeof(status)))
> > + return false;
> > +
> > + return true;
> > +}
>
> This is a completely useless wrapper. Actually, it's worse than useless, as it
> inverts the return value from the widely used kvm_vcpu_write_guest() API.
I'll remove this. I got feedback that tdx_get_quote_kernel() was full of
kvm_vcpu_write_guest()s and was hard to read, so I tried to break it
apart. Looks like it didn't help.
>
> > +
> > +static bool write_quote_to_guest(struct kvm_vcpu *vcpu, void *quote_data,
> > + u32 quote_len, gpa_t gpa)
> > +{
> > + if (kvm_vcpu_write_guest(vcpu,
> > + gpa + TDX_QUOTE_REQ_HDR_SIZE,
>
> This is unnecessarily confusing. I had to look at the definition of
> TDX_QUOTE_REQ_HDR_SIZE to understand that this just writing to tdx_quote_req.data.
> I might not care if this was the only write, but then the very next statement
> writes to a field that isn't obviously in the header.
I'll fix this. I can just spell out offsetof() to make this consistent.
>
> > + quote_data, quote_len))
> > + return false;
> > +
> > + if (kvm_vcpu_write_guest(vcpu,
> > + gpa + offsetof(struct tdx_quote_req, out_len),
> > + "e_len, sizeof(quote_len)))
> > + return false;
> > +
> > + return true;
> > +}
>
> So is this. There's literally one caller.
Yeah breaking the kvm_vcpu_write_guest()s apart didn't help...
>
> > +
> > +static u64 get_quote_kernel(struct kvm_vcpu *vcpu, struct tdx_quote_req *req,
> > + gpa_t req_gpa, size_t total_len)
> > +{
> > + struct tdx_td *td = &to_kvm_tdx(vcpu->kvm)->td;
> > +
> > + /* Only support version 1 as defined in the GHCI spec */
> > + if (req->version != 1)
> > + return TDX_QUOTE_STATUS_ERROR;
> > +
> > + /* Header + input data must fit in the page read from guest memory */
> > + if ((size_t)req->in_len + TDX_QUOTE_REQ_HDR_SIZE > PAGE_SIZE)
> > + return TDX_QUOTE_STATUS_ERROR;
> > +
> > + /* Caller owns the requested quote */
> > + void *quote_data __free(kvfree) =
> > + tdx_quote_generate(td, req->data, req->in_len, &req->out_len);
>
> Am I the only person that finds the __free() stuff to be difficult to follow?
> I'm sure there are flows where it's a big net positive, but IMO this isn't one.
> If the *unnecesary* layers of single-use helpers are stripped away, I suspect
> we'll naturally end up with a goto sequence that is more obvious that using __free().
The main concern I had was that the guest could give random message
lengths and mess with the host's memory allocator. Core TDX has a better
idea because it knows the maximum possible quote size through a metadata
field, so I just let the API return the final quote from the TDX module.
It's a bit annoying that the caller now has to free it.
Several folks suggested letting KVM allocate and free the buffer, so I
think I'll do that. It should make this easier to read, and this
__free() can be replaced with a much simpler pattern.
>
> > +
> > + if (!quote_data)
> > + return TDX_QUOTE_STATUS_UNAVAILABLE;
> > +
> > + if ((size_t)req->out_len + TDX_QUOTE_REQ_HDR_SIZE > total_len)
>
> Can we really not pre-check the size?
Yeah like the response above, KVM doesn't know what a "sane" total_len
is here. out_len comes from the TDX module so I used it to check against
total_len. If we let KVM allocate+free the quote buffer, then I'll also
let KVM get the maximum quote size from core TDX. I was trying to save
some code in KVM but looks like it's not worth it.
>
> > + return TDX_QUOTE_STATUS_ERROR;
> > +
> > + if (!write_quote_to_guest(vcpu, quote_data, req->out_len, req_gpa))
> > + return TDX_QUOTE_STATUS_ERROR;
> > +
> > + return TDX_QUOTE_STATUS_SUCCESS;
> > +}
> > +
> > +static u64 tdx_get_quote_check_args(struct kvm_vcpu *vcpu, u64 gpa, u64 size)
>
> Adding single use helpers does not make the code easier to read.
Yep. Will fix.
>
> > +{
> > + gfn_t gfn_start, gfn_end;
> > + u64 end;
> > +
> > + if (!size)
> > + return TDVMCALL_STATUS_INVALID_OPERAND;
> > +
> > + if (!PAGE_ALIGNED(gpa) || !PAGE_ALIGNED(size))
> > + return TDVMCALL_STATUS_ALIGN_ERROR;
> > +
> > + if (check_add_overflow(gpa, size, &end))
> > + return TDVMCALL_STATUS_INVALID_OPERAND;
> > +
> > + gfn_start = gpa_to_gfn(gpa);
> > + gfn_end = gpa_to_gfn(end);
> > +
> > + /*
> > + * Reject if the guest didn't explicitly convert its quote pages to
> > + * shared.
> > + */
> > + if (!kvm_range_has_memory_attributes(vcpu->kvm, gfn_start, gfn_end,
> > + KVM_MEMORY_ATTRIBUTE_PRIVATE, 0))
>
> TOCTOU?
I struggled a bit with this actually... I can't just grab
kvm->slots_lock here, and dropping kvm->srcu seems bad because the quote
will be written to guest memory later. I settled on the idea that the
guest should make sure these pages are converted to shared first. If it
tries to convert them back to private after this check, before this
TDVMCALL returns, then it's kind of a self-inflicted race and I don't
see KVM doing any self damage in this case. Not sure if this makes sense
to you? Or maybe I should just drop this check and go straight to
kvm_vcpu_read_guest()?
Thanks,
Peter
>
> > + return TDVMCALL_STATUS_INVALID_OPERAND;
> > +
> > + return TDVMCALL_STATUS_SUCCESS;
> > +}
^ permalink raw reply
* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Will Deacon @ 2026-07-10 10:50 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Aneesh Kumar K.V, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marc Zyngier, Steven Price,
Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <fc804746-4fb8-4ea9-997f-a4cae9ba8c14@samsung.com>
On Tue, Jul 07, 2026 at 03:03:48PM +0200, Marek Szyprowski wrote:
> On 07.07.2026 10:06, Aneesh Kumar K.V wrote:
> > "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> writes:
> >
> >> This series tracks confidential-computing shared DMA state through the
> >> dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted
> >> DMA buffers are handled consistently.
> >>
> >> Today, the direct DMA path mostly relies on force_dma_unencrypted() for
> >> shared/decrypted buffer handling. This series consolidates the
> >> force_dma_unencrypted() checks in the top-level functions and ensures
> >> that the remaining DMA interfaces use DMA attributes to make the correct
> >> decisions.
> >>
> >> The series separates mapping and allocation state:
> >> - DMA_ATTR_CC_SHARED describes the DMA address attribute requested for a
> >> mapping. It tells the DMA mapping path that the DMA address must target
> >> shared/decrypted memory.
> >> - __DMA_ATTR_ALLOC_CC_SHARED is an internal DMA-mapping attribute used only
> >> by allocation paths after the DMA core decides that the backing pages
> >> must be allocated as shared/decrypted memory.
> >>
> >> The series:
> >> - moves swiotlb-backed allocations out of __dma_direct_alloc_pages(),
> >> - uses __DMA_ATTR_ALLOC_CC_SHARED through the dma-direct alloc/free paths
> >> - teaches the atomic DMA pools to track encrypted versus decrypted
> >> state
> >> - tracks swiotlb pool encryption state and enforces strict pool
> >> selection
> >> - centralizes encrypted/decrypted pgprot handling in dma_pgprot() using
> >> DMA attributes
> >> - passes DMA attributes down to dma_capable() so capability checks can
> >> validate whether the selected DMA address encoding matches
> >> DMA_ATTR_CC_SHARED
> >> - makes dma_direct_map_phys() choose the DMA address encoding from
> >> DMA_ATTR_CC_SHARED and fall back to swiotlb when a shared DMA request
> >> cannot use the direct mapping, which lets arm64 and x86 CCA guests stop
> >> relying on SWIOTLB_FORCE for DMA mappings
> >> - use the selected swiotlb pool state to derive the returned DMA
> >> address
> >> - reports CC_ATTR_GUEST_MEM_ENCRYPT for arm64 Realms, powerpc secure
> >> guests, and s390 protected virtualization guests.
> >>
> >> Dependency:
> >> This series depends on the pKVM changes posted at:
> >> https://lore.kernel.org/all/20260603110522.3331819-1-smostafa@google.com
FYI, I'll probably take the second patch there as a fix, then I can put
the other two on a topic branch in the arm64 tree.
Will
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Peter Fang @ 2026-07-10 9:38 UTC (permalink / raw)
To: Nikolay Borisov
Cc: Edgecombe, Rick P, seanjc@google.com, djbw@kernel.org,
kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, dave.hansen@linux.intel.com,
baolu.lu@linux.intel.com, Hunter, Adrian, kas@kernel.org,
tony.lindgren@linux.intel.com, Xu, Yilun,
linux-kernel@vger.kernel.org, Mehta, Sohil, Duan, Zhenzhong,
Maloor, Kishen, yilun.xu@linux.intel.com, x86@kernel.org
In-Reply-To: <e6c0ecd4-c128-47c3-99ce-1d45113f6766@suse.com>
On Fri, Jul 10, 2026 at 12:01:41PM +0300, Nikolay Borisov wrote:
>
>
> On 7/4/26 08:43, Peter Fang wrote:
> > On Wed, Jul 01, 2026 at 11:45:53AM -0700, Edgecombe, Rick P wrote:
> > > On Wed, 2026-07-01 at 10:25 -0700, Sean Christopherson wrote:
> > > > > > That is a good question. The answer is partly historical reasons, but I
> > > > > > think the pros/cons don’t really move the needle too much.
> > > > > >
> > > > > > The main benefit of doing it with the host in the loop is that the guest
> > > > > > side TDVMCALL quoting interface can stay the same. There is also a wrinkle
> > > > > > in that there is a limited HW resource involved in the quoting,
> > > >
> > > > What is this magical resource?
> > >
> > > It's a HW crypto thing. I'll let Peter explain more.
> >
> > It's called S3M (Secured Startup Services). There was once a public
> > document about it, but it was removed for some reason. It basically
>
> Are you referring to this:
> https://www.scribd.com/document/970439243/S3M-Intel-White-Paper ?
Yep. The internet never forgets :)
>
> <snip>
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Nikolay Borisov @ 2026-07-10 9:01 UTC (permalink / raw)
To: Peter Fang, Edgecombe, Rick P
Cc: seanjc@google.com, djbw@kernel.org, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, Hansen, Dave,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
Hunter, Adrian, kas@kernel.org, tony.lindgren@linux.intel.com,
Xu, Yilun, linux-kernel@vger.kernel.org, Mehta, Sohil,
Duan, Zhenzhong, Maloor, Kishen, yilun.xu@linux.intel.com,
x86@kernel.org
In-Reply-To: <20260704054342.GB2169894@pedri>
On 7/4/26 08:43, Peter Fang wrote:
> On Wed, Jul 01, 2026 at 11:45:53AM -0700, Edgecombe, Rick P wrote:
>> On Wed, 2026-07-01 at 10:25 -0700, Sean Christopherson wrote:
>>>>> That is a good question. The answer is partly historical reasons, but I
>>>>> think the pros/cons don’t really move the needle too much.
>>>>>
>>>>> The main benefit of doing it with the host in the loop is that the guest
>>>>> side TDVMCALL quoting interface can stay the same. There is also a wrinkle
>>>>> in that there is a limited HW resource involved in the quoting,
>>>
>>> What is this magical resource?
>>
>> It's a HW crypto thing. I'll let Peter explain more.
>
> It's called S3M (Secured Startup Services). There was once a public
> document about it, but it was removed for some reason. It basically
Are you referring to this:
https://www.scribd.com/document/970439243/S3M-Intel-White-Paper ?
<snip>
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-07-10 5:22 UTC (permalink / raw)
To: Jason Gunthorpe, Catalin Marinas
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Jiri Pirko, Mostafa Saleh, Petr Tesarik,
Alexey Kardashevskiy, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260709181336.GM118978@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Thu, Jul 09, 2026 at 12:13:19PM +0100, Catalin Marinas wrote:
>> > > For AMD/SME, on host with memory encryption we now end up setting the C
>> > > bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
>> > > some other MMIO bus understands this attribute. Maybe we should stick to
>> > > something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
>> > > since this is not universally defined, just use the old dma_addr = phys
>> > > if MMIO and ignore any unlikely DMA offsets.
>> > >
>> >
>> > Considering for AMD/SME system an unencrypted dma addr is one without C
>> > bit, will this be good?
>> >
>> > /*
>> > * For host memory encryption and device requiring unencrypted DMA,
>> > * MMIO memory is treated as shared by default.
>> > */
>> > if (attrs & DMA_ATTR_MMIO) {
>> > if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) || force_dma_unencrypted(dev))
>> > attrs |= DMA_ATTR_CC_SHARED;
>> > }
>>
>> Yes, I think it does the trick, preserves the current semantics for AMD.
>> I guess you could use a single 'if' for all checks (up to you).
>
> Please don't change it, MMIO P2P is broken on CC systems today and it
> should stay broken. Passing DMA_ATTR_MMIO with DMA_ATTR_CC_SHARED is
> an error that we need to correct in the drivers not make work in the
> core code.
>
But the above changes are intended to handle HOST_MEM_ENCRYPT. In v7, we
had the following diff:
@@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
{
dma_addr_t dma_addr;
+ /*
+ * For a device requiring unencrypted DMA, MMIO memory is treated
+ * as shared by default.
+ */
+ if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
+ attrs |= DMA_ATTR_CC_SHARED;
+
This is now getting updated to
@@ -624,37 +626,44 @@ dma_addr_t dma_direct_map_phys(struct device *dev, phys_addr_t phys,
{
dma_addr_t dma_addr;
+ if (attrs & DMA_ATTR_MMIO) {
+ /*
+ * For host memory encryption and device requiring
+ * unencrypted DMA, MMIO memory is treated as shared by
+ * default.
+ */
+ if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) ||
+ force_dma_unencrypted(dev))
+ attrs |= DMA_ATTR_CC_SHARED;
+ }
+
I agree that we need to move that DMA_ATTR_CC_SHARED setting to
modified drivers/pci/p2pdma.c
@@ -285,6 +285,11 @@ int pcim_p2pdma_init(struct pci_dev *pdev)
continue;
p2p->mem[i].owner = &pdev->dev;
+
+ p2p->mem[i].dma_mapping_flags = DMA_ATTR_MMIO;
+ if (force_dma_unencrypted(dev))
+ p2p->mem[i].dma_mapping_flags |= DMA_ATTR_CC_SHARED;
+
As we discussed [1], that can come in a later patch. In the meantime, adding
the HOST_MEM_ENCRYPT check preserves the previous behavior for SME.
[1] https://lore.kernel.org/all/20260522132240.GD7702@ziepe.ca/
-aneesh
^ permalink raw reply
* Re: [PATCH 2/2] virt: tdx-guest: Allocate Quote buffer dynamically
From: Binbin Wu @ 2026-07-10 3:32 UTC (permalink / raw)
To: Peter Fang
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin, linux-kernel, linux-coco,
kvm
In-Reply-To: <20260612110853.3188196-3-peter.fang@intel.com>
On 6/12/2026 7:08 PM, Peter Fang wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> The TDX attestation driver currently uses a fixed 128 KB Quote buffer
> shared with the host VMM. This may be too small for Quotes using schemes
> such as post-quantum cryptography (PQC), where certificate chains can
> increase the Quote size to several megabytes.
Nit:
I think the description here should be more accurate.
"Several megabytes" colloquially implies sizes that could easily exceed 4 MB.
And it's a slight contradiction VS the 4MB limitation below.
>
> Allocate the Quote buffer based on the size reported by the TDX module
> instead of always reserving a fixed-size buffer. This avoids wasting
> memory on platforms that do not require larger Quotes. Older platforms
> fall back to the default 128 KB buffer.
>
> Because the Quote buffer must be physically contiguous, its size is
> bound by the buddy allocator's maximum page order (4 MB), which should
> be sufficient for current attestation needs.
>
> struct tdx_quote_buf has a trailing flexible array, so use offsetof()
> instead of sizeof() to calculate the header size.
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Assisted-by: Claude:claude-opus-4-7
> Assisted-by: GitHub Copilot:gpt-5.4
Same issue as pointed by Kiryl in the first patch.
> Signed-off-by: Peter Fang <peter.fang@intel.com>
Also the use of struct_size_t() instead of offsetof().
Otherwise looks okay to me:
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path
From: Yan Zhao @ 2026-07-10 2:47 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
x86@kernel.org
In-Reply-To: <6f19e79b49e6f6e32c34a87302a9ab475ad0f51c.camel@intel.com>
On Fri, Jul 10, 2026 at 09:34:48AM +0800, Edgecombe, Rick P wrote:
> On Thu, 2026-07-09 at 15:17 +0800, Yan Zhao wrote:
> > On Mon, May 25, 2026 at 07:35:12PM -0700, Rick Edgecombe wrote:
> > > When handling an EPT violation, KVM holds a spinlock while manipulating
> > > the EPT. Before entering the spinlock it doesn't know how many EPT page
> > > tables will need to be installed or whether a huge page will be used. For
> > > this reason it allocates a worst case number of page tables that it might
> > > need as part of servicing the EPT violation.
> > >
> > > Under Dynamic PAMT these pre-allocated pages will potentially need to have
> > > Dynamic PAMT backing pages installed for them. KVM already has helpers to
> > > manage topping up page caches before taking the MMU lock, but they cannot be
> > > passed from KVM to arch/x86 code.
> > >
> > > The problem of how and when to install the DPAMT backing pages for the
> > > pages given to the TDX module during the fault path has had a lot of
> > > design attempts.
> > > - Extracting KVM's MMU caches requires too much inlined code added to
> > > headers.
> > > - A few varieties of installing Dynamic PAMT backing when allocating the
> > > S-EPT page tables. [0][1]
> > IIUC, [0][1] here refer to design attempts that had various problems, right?
> > However, [1] looks exactly like the one being adopted in v6?
> >
> > Did you paste a wrong link? Should [1] instead be
> > https://lore.kernel.org/kvm/20260129011517.3545883-21-seanjc@google.com or
> > https://lore.kernel.org/kvm/aYYCOiMvWfSJR1AL@google.com ?
>
> Oh yea... I think it should be:
> Kai's original idea: https://lore.kernel.org/kvm/aXENNKjAKTM9UJNH@google.com/
> Sean's fixup of it:
> https://lore.kernel.org/kvm/20260129011517.3545883-20-seanjc@google.com/
> The problem with Sean's:
> https://lore.kernel.org/kvm/aYW5CbUvZrLogsWF@yzhao56-desk.sh.intel.com/
>
> TBH I'm a little on the fence about how useful the links will be in general. If
> someone tries to rip it out, it would be good breadcrumbs I guess
No problem. I am fine with either keeping or removing those links :)
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Yan Zhao @ 2026-07-10 2:43 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kirill.shutemov@linux.intel.com, linux-coco@lists.linux.dev,
Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
Annapurve, Vishal, tglx@kernel.org, bp@alien8.de,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
In-Reply-To: <aa669cd125b9cbbddd9105cfcbade50f130405db.camel@intel.com>
On Fri, Jul 10, 2026 at 09:04:57AM +0800, Edgecombe, Rick P wrote:
> On Thu, 2026-07-09 at 11:18 +0800, Yan Zhao wrote:
> > > > > page before gifting it to the TDX module, and tear it down after the
> > > > > page is reclaimed.
> > > > How about
> > > > "So the kernel must check whether it is necessary to install/remove PAMT
> > > > backing for each 4KB page, and do so when necessary before gifting the
> > > > page to the TDX module or after it is reclaimed." ?
> > >
> > > The checking part is true, but is it needed to understand this patch? I
> > > think it just needs a background of what is happening. How about:
> > >
> > > So the kernel must install PAMT backing for any 4KB page being gifting to
> > > the TDX module, and tear down the backing when the associated gifted pages
> > > are reclaimed.
> > Hmm. I'm not sure if it would be confusing for people without sufficient
> > background, because the kernel only needs to install PAMT backing before the
> > gifting the first 4KB page.
> >
> > How about:
> > So the kernel must ensure PAMT backing installed for any 4KB page being
> > gifting to the TDX module, and tear down the backing when all associated
> > gifted pages are reclaimed.
> I see how it's better. I added a few missed word tweaks and ended up with:
>
> So the kernel must ensure PAMT backing is installed for any 4KB page being
> gifting to the TDX module, and must tear down the backing when all associated
> gifted pages are reclaimed.
It's better! Thanks!
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Add helper to query maximum TD Quote size
From: Binbin Wu @ 2026-07-10 2:38 UTC (permalink / raw)
To: Kiryl Shutsemau, Peter Fang
Cc: Dave Hansen, Rick Edgecombe, Kuppuswamy Sathyanarayanan,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, linux-kernel, linux-coco, kvm
In-Reply-To: <aiv8mrJDgs_e8eLq@thinkstation>
On 6/12/2026 8:36 PM, Kiryl Shutsemau wrote:
>>
>> +/**
>> + * tdx_get_max_quote_size() - Get the maximum TD Quote size
>> + *
>> + * Read the maximum size of a TD Quote from a 4-byte TD metadata field. The TDX
>> + * guest driver uses it to size the buffer for Quote retrieval. Older TDX
>> + * modules do not support this field and return an error.
>> + *
>> + * Return: Maximum Quote size in bytes on success, or 0 on failure.
>> + */
>> +u32 tdx_get_max_quote_size(void)
>> +{
>> + u64 val, ret;
>> +
>> + ret = tdg_vm_rd(TDCS_QUOTE_MAX_SIZE, &val);
>> +
>> + return ret ? 0 : (u32)val;
>
> Cast is redundant.
I thought in Linux kernel, the explicit cast is often preferred?
E.g. as documentation of the metadata field’s width here?
>
>> +}
>> +EXPORT_SYMBOL_GPL(tdx_get_max_quote_size);
>> +
>> static void __noreturn tdx_panic(const char *msg)
>> {
>> struct tdx_module_args args = {
>
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Yan Zhao @ 2026-07-10 2:24 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kirill.shutemov@linux.intel.com, linux-coco@lists.linux.dev,
Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
Annapurve, Vishal, tglx@kernel.org, bp@alien8.de,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
In-Reply-To: <ad840587305878d544f3c1dd44ba6947498c167b.camel@intel.com>
On Fri, Jul 10, 2026 at 07:09:04AM +0800, Edgecombe, Rick P wrote:
> On Thu, 2026-07-09 at 10:50 +0800, Yan Zhao wrote:
> > > > > tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> > > > > backing for the 2MB region covering the passed pfn. However, multiple
> > > > > callers can concurrently operate on 4KB pages that fall within the same
> > > > > 2MB region. When this happens only one Dynamic PAMT page pair needs to
> > > > > be
> > > > What "this" stands for is not clear and a comma is missing after
> > > > "happens".
> > >
> > > Really this is not clear? The previous sentence is all about a senario being
> > > possible "multiple callers can concurrently operate on 4KB pages that fall
> > > within the same 2MB region". So I thought this would be clear.
> > Hmm, sorry for nitpicking.
> > But the previous sentence says "multiple callers can concurrently operate on
> > 4KB pages", where "operate" doesn't necessarily imply allocating pages.
> > However, the latter sentence assumes PAMT pages need to be installed. That's
> > why I think "it" is not clear.
>
> Not sure what you mean by "it", but I guess you are saying that "this" could
Oh, yes, "this"! :D
> refer to the first sentence and not the one that immediately preceded the "this"
> reference? That part still reads ok to me, but if you still feel it's unclear
> I'll change it to.
>
> Actually, another problem with this paragraph is focuses too much on concurrent
> callers, when the main thing this patch does is address get-ing multiple pages.
> The concurrency is added to make the refcounts work. I'd like to address that
> too. How is this to you?
This version looks great!
Thanks!
> x86/virt/tdx: Handle multiple callers in tdx_pamt_get/put()
>
> tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> backing for the 2MB region covering the passed page. However, multiple
> callers can add or remove 4KB pages that fall within the same 2MB region
> and in that scenario only a single PAMT entry is required.
>
> Make the helpers handle only adding/removing Dynamic PAMT backing when
> required, by refcounting each 2MB range. Gate the actual Dynamic PAMT add
> and remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> check and SEAMCALL with a global spinlock so the read-decide-act sequence
> is atomic. This also avoids TDX module BUSY errors, as the Dynamic PAMT add
> and remove SEAMCALLs take internal TDX module locks for the 2MB ranges of
> the specified PFN and the PAMT page pair PFNs. So simultaneous attempts on
> the same 2MB ranges of the PFNs would otherwise encounter and error, which
> would not be handleable in the put case.
>
> The lock is global and heavyweight. Use simple conditional logic to keep
> correctness obvious. This will be optimized in a later change.
>
> The pamt_refcount[]s are atomic_t's. They do not strictly need to be
Nit: Not sure if it's correct.
Just in case it's a typo: pamt_refcounts[].
> because all access is protected by pamt_lock. The overhead of an atomic_t
> in this situation is minuscule compared to the global lock. Leave the
> atomic_t in place to enable future optimization with minimal churn.
^ permalink raw reply
* Re: [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path
From: Edgecombe, Rick P @ 2026-07-10 1:34 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, kas@kernel.org, seanjc@google.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
x86@kernel.org
In-Reply-To: <ak9LDmWBIp52wv4X@yzhao56-desk.sh.intel.com>
On Thu, 2026-07-09 at 15:17 +0800, Yan Zhao wrote:
> On Mon, May 25, 2026 at 07:35:12PM -0700, Rick Edgecombe wrote:
> > When handling an EPT violation, KVM holds a spinlock while manipulating
> > the EPT. Before entering the spinlock it doesn't know how many EPT page
> > tables will need to be installed or whether a huge page will be used. For
> > this reason it allocates a worst case number of page tables that it might
> > need as part of servicing the EPT violation.
> >
> > Under Dynamic PAMT these pre-allocated pages will potentially need to have
> > Dynamic PAMT backing pages installed for them. KVM already has helpers to
> > manage topping up page caches before taking the MMU lock, but they cannot be
> > passed from KVM to arch/x86 code.
> >
> > The problem of how and when to install the DPAMT backing pages for the
> > pages given to the TDX module during the fault path has had a lot of
> > design attempts.
> > - Extracting KVM's MMU caches requires too much inlined code added to
> > headers.
> > - A few varieties of installing Dynamic PAMT backing when allocating the
> > S-EPT page tables. [0][1]
> IIUC, [0][1] here refer to design attempts that had various problems, right?
> However, [1] looks exactly like the one being adopted in v6?
>
> Did you paste a wrong link? Should [1] instead be
> https://lore.kernel.org/kvm/20260129011517.3545883-21-seanjc@google.com or
> https://lore.kernel.org/kvm/aYYCOiMvWfSJR1AL@google.com ?
Oh yea... I think it should be:
Kai's original idea: https://lore.kernel.org/kvm/aXENNKjAKTM9UJNH@google.com/
Sean's fixup of it:
https://lore.kernel.org/kvm/20260129011517.3545883-20-seanjc@google.com/
The problem with Sean's:
https://lore.kernel.org/kvm/aYW5CbUvZrLogsWF@yzhao56-desk.sh.intel.com/
TBH I'm a little on the fence about how useful the links will be in general. If
someone tries to rip it out, it would be good breadcrumbs I guess
>
> > - Using mempool_t to transfer the pages between KVM and arch/x86 doesn't
> > work because it is the component is designed more around maintaining a
> > pool of pages, rather than topping up a continually drained cache.
> >
> > So don't do these as they all had various problems. Instead just create a
> > small simple data structure to use for handing a pre-allocated list of
> > pages between KVM and arch/x86 code. Model this on KVM's existing MMU
> > memory caches.
> >
> > Add a tdx_pamt_cache arg to tdx_pamt_get() so it can draw pages from a
> > cache when needed. Not all DPAMT page installations will happen under
> > spinlock, for example control pages. So have tdx_pamt_get() maintain the
> Nit: In patch 9, S-EPT pages are regarded as control pages as well.
> So maybe
> "..., for example some control pages." or
Yea that is better. The patch 9 changes are after this, so it's technically
correct, but we can make it clearer:
..., for example TD and vCPU scoped control pages for
> "..., for example control pages other than S-EPT pages." ?
>
> > existing behavior of allocating from the page allocator when NULL is
> > passed for the struct tdx_pamt_cache arg. This prevents excess allocations
> > for cases where it can be avoided.
> >
> > Export the new helpers for KVM.
> >
> > Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> > Co-developed-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Link: https://lore.kernel.org/kvm/de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com/ [0]
> > Link: https://lore.kernel.org/kvm/aYprxnSHKHUtk7pt@google.com/ [1]
> > ---
> > v6:
> > - Filled out log from Sean's series
> > ---
> > arch/x86/include/asm/tdx.h | 17 ++++++++++
> > arch/x86/virt/vmx/tdx/tdx.c | 65 +++++++++++++++++++++++++++++++++----
> > 2 files changed, 76 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> > index 74e75db5728c7..191da84bbf2a1 100644
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -155,6 +155,23 @@ static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> > return false; /* To be enabled when kernel is ready */
> > }
> >
> > +/* Simple structure for pre-allocating Dynamic PAMT pages outside of locks. */
> outside of spinlocks?
>
> Pre-allocating Dynamic PAMT pages are still inside mutex, e.g., inside of
> kvm->slots_lock, vcpu->mutex...
Ok.
>
> > +struct tdx_pamt_cache {
> > + struct list_head page_list;
> > + int cnt;
> > +};
>
> The rest LGTM.
> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
>
Thanks!
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Edgecombe, Rick P @ 2026-07-10 1:04 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kirill.shutemov@linux.intel.com, linux-coco@lists.linux.dev,
Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
Annapurve, Vishal, tglx@kernel.org, bp@alien8.de,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
In-Reply-To: <ak8S6MiMkCEgJcP/@yzhao56-desk.sh.intel.com>
On Thu, 2026-07-09 at 11:18 +0800, Yan Zhao wrote:
> > > > page before gifting it to the TDX module, and tear it down after the
> > > > page is reclaimed.
> > > How about
> > > "So the kernel must check whether it is necessary to install/remove PAMT
> > > backing for each 4KB page, and do so when necessary before gifting the
> > > page to the TDX module or after it is reclaimed." ?
> >
> > The checking part is true, but is it needed to understand this patch? I
> > think it just needs a background of what is happening. How about:
> >
> > So the kernel must install PAMT backing for any 4KB page being gifting to
> > the TDX module, and tear down the backing when the associated gifted pages
> > are reclaimed.
> Hmm. I'm not sure if it would be confusing for people without sufficient
> background, because the kernel only needs to install PAMT backing before the
> gifting the first 4KB page.
>
> How about:
> So the kernel must ensure PAMT backing installed for any 4KB page being
> gifting to the TDX module, and tear down the backing when all associated
> gifted pages are reclaimed.
I see how it's better. I added a few missed word tweaks and ended up with:
So the kernel must ensure PAMT backing is installed for any 4KB page being
gifting to the TDX module, and must tear down the backing when all associated
gifted pages are reclaimed.
Thanks!
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Edgecombe, Rick P @ 2026-07-09 23:09 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kirill.shutemov@linux.intel.com, linux-coco@lists.linux.dev,
Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
Annapurve, Vishal, tglx@kernel.org, bp@alien8.de,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
In-Reply-To: <ak8MghJ4OKmsXAWE@yzhao56-desk.sh.intel.com>
On Thu, 2026-07-09 at 10:50 +0800, Yan Zhao wrote:
> > > > tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> > > > backing for the 2MB region covering the passed pfn. However, multiple
> > > > callers can concurrently operate on 4KB pages that fall within the same
> > > > 2MB region. When this happens only one Dynamic PAMT page pair needs to
> > > > be
> > > What "this" stands for is not clear and a comma is missing after
> > > "happens".
> >
> > Really this is not clear? The previous sentence is all about a senario being
> > possible "multiple callers can concurrently operate on 4KB pages that fall
> > within the same 2MB region". So I thought this would be clear.
> Hmm, sorry for nitpicking.
> But the previous sentence says "multiple callers can concurrently operate on
> 4KB pages", where "operate" doesn't necessarily imply allocating pages.
> However, the latter sentence assumes PAMT pages need to be installed. That's
> why I think "it" is not clear.
Not sure what you mean by "it", but I guess you are saying that "this" could
refer to the first sentence and not the one that immediately preceded the "this"
reference? That part still reads ok to me, but if you still feel it's unclear
I'll change it to.
Actually, another problem with this paragraph is focuses too much on concurrent
callers, when the main thing this patch does is address get-ing multiple pages.
The concurrency is added to make the refcounts work. I'd like to address that
too. How is this to you?
x86/virt/tdx: Handle multiple callers in tdx_pamt_get/put()
tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
backing for the 2MB region covering the passed page. However, multiple
callers can add or remove 4KB pages that fall within the same 2MB region
and in that scenario only a single PAMT entry is required.
Make the helpers handle only adding/removing Dynamic PAMT backing when
required, by refcounting each 2MB range. Gate the actual Dynamic PAMT add
and remove on refcount transitions (0->1 and 1->0). Serialize the refcount
check and SEAMCALL with a global spinlock so the read-decide-act sequence
is atomic. This also avoids TDX module BUSY errors, as the Dynamic PAMT add
and remove SEAMCALLs take internal TDX module locks for the 2MB ranges of
the specified PFN and the PAMT page pair PFNs. So simultaneous attempts on
the same 2MB ranges of the PFNs would otherwise encounter and error, which
would not be handleable in the put case.
The lock is global and heavyweight. Use simple conditional logic to keep
correctness obvious. This will be optimized in a later change.
The pamt_refcount[]s are atomic_t's. They do not strictly need to be
because all access is protected by pamt_lock. The overhead of an atomic_t
in this situation is minuscule compared to the global lock. Leave the
atomic_t in place to enable future optimization with minimal churn.
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Jason Gunthorpe @ 2026-07-09 18:13 UTC (permalink / raw)
To: Catalin Marinas
Cc: Aneesh Kumar K.V, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Jiri Pirko,
Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <ak-CT3oanlDfgTy4@arm.com>
On Thu, Jul 09, 2026 at 12:13:19PM +0100, Catalin Marinas wrote:
> > > For AMD/SME, on host with memory encryption we now end up setting the C
> > > bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
> > > some other MMIO bus understands this attribute. Maybe we should stick to
> > > something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
> > > since this is not universally defined, just use the old dma_addr = phys
> > > if MMIO and ignore any unlikely DMA offsets.
> > >
> >
> > Considering for AMD/SME system an unencrypted dma addr is one without C
> > bit, will this be good?
> >
> > /*
> > * For host memory encryption and device requiring unencrypted DMA,
> > * MMIO memory is treated as shared by default.
> > */
> > if (attrs & DMA_ATTR_MMIO) {
> > if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) || force_dma_unencrypted(dev))
> > attrs |= DMA_ATTR_CC_SHARED;
> > }
>
> Yes, I think it does the trick, preserves the current semantics for AMD.
> I guess you could use a single 'if' for all checks (up to you).
Please don't change it, MMIO P2P is broken on CC systems today and it
should stay broken. Passing DMA_ATTR_MMIO with DMA_ATTR_CC_SHARED is
an error that we need to correct in the drivers not make work in the
core code.
Jason
^ permalink raw reply
* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Jason Gunthorpe @ 2026-07-09 13:36 UTC (permalink / raw)
To: Dan Williams (nvidia)
Cc: linux-coco, linux-pci, driver-core, ankita, Aaron Tomlin,
Alexey Kardashevskiy, Alistair Francis, Aneesh Kumar K.V,
Arnd Bergmann, Bjorn Helgaas, Daniel Gomez, Danilo Krummrich,
Dexuan Cui, Donald Hunter, Greg Kroah-Hartman, Jakub Kicinski,
Luis Chamberlain, Lukas Wunner, Petr Pavlu, Rafael J. Wysocki,
Robin Murphy, Sami Tolvanen, Samuel Ortiz, Saravana Kannan,
Will Deacon, Xu Yilun
In-Reply-To: <6a4f0b35683b4_353c8910011@djbw-dev.notmuch>
On Wed, Jul 08, 2026 at 07:45:09PM -0700, Dan Williams (nvidia) wrote:
> > force_dma_unencrypted() does not *prevent* device access to private
> > memory and provides no security properties on its own. It's only
> > purpose is to inform the DMA API what the HW restrictions are for
> > doing DMA.
>
> Right, to be clear, this mode's security properties come from never
> asking the TSM to enable private DMA while the device is in RUN.
Ok, that's a twist I hadn't thought about. I don't see a reason to
support a driver probed with RUN but T=1 DMA disabled by the TSM.
Still, if we do this, I think having the TSM deal with it is probably
better than making a cross product of the trust level, something like:
echo only-shared-dma > $pdev/tsm/accept
So maybe accept should be tweaked:
echo full > $pdev/tsm/accept
? It doesn't matter much
> > > echo 1 > $pdev/tsm/accept
> >
> > And now it is RUN. So I don't see the issue with enabling DMA at the
> > same time as gonig to RUN? (though defering it to driver probe would
> > be a very nice touch as well)
>
> It requires "accept" to consider the trust level. E.g. what does it mean
> to do something like change requirements after accept?
> echo full $pdev/trust
> echo 1 > $pdev/tsm/accept
> echo adversary > $pdev/trust
> In that scenario this now adversary device may have been allowed to
> operate without an enforcing vIOMMU, and needs to unlock the device to
> correct that.
If accept enables DMA then it is up to userspace to ensure all policy
objectives are met before accepting, not after.
I think the statement kernel makes to userspace is the moment it
writes trust or accept the kernel is free to take action on it. ie
don't write a trust or accept until you mean it.
> > > There are also buses and paravisors that may know that private-DMA is
> > > enabled for a device by construction. In that case it is also a "trusted
> > > to access" signal, and not a "required to access" signal.
> >
> > In this case they wire force_dma_unencrypted()=false.
>
> Yeah, I just need a scheme where modular bus providers do not end up
> compromising the private / unexported method of changing the flag that
> force_dma_unencrypted() consumes.
Yeah, the bus and TSM really have to control this property, it is like
the other DMA attributes (eg dma seg boundary, etc)
> > At least if we omit the double check it can be fairly easy to add in
> > later if it really was needed for some time of use reason.
>
> As long as IOMMU presence can be enumerated prior to acceptance, T=1
> always includes private DMA, and the T=1 status is tracked independent
> of the trust level then yes, the cross product can be avoided.
Ok. iommu has to be setup the moment the struct device is created, it
can't add on later. I *think* the iommu related sysfs ordering is done
properly before any uevents, but I haven't validated that.
> 'struct device' grows some "request" policy for trust and T=1 that can
> be changed by modules etc. 'struct device_private' grows the operational
> trust level stable under device_lock() and a new flag to reflect T=1.
I guess
- The active trust level should be RO visible to the driver, iommu, etc
It should be stable under a bound driver
- The "dma require unencrypted" property needs to RO visible to the
DMA API and stable under a bound driver. This would input where
force_dma_unecrpyted() is in the flow [the name should align with
all the other per-device DMA API specific properties like seg
limit, boundary, mask, etc]
- The requested trust policy should be internal to the driver core and
be converted to the active trust level right before probe
- We should have ways to enable/disable all DMA before/after probe,
including both TSM and IOMMU approaches
Broadly all busses get some way to convert the requested trust policy
to the active trust level (eg by mixing in ACPI, etc, etc)
All busses get some way to set the "dma require unencrypted" property
IOMMU and drivers should be sensitive to the trust level
TSM is sensitive to accept, not the trust level
Userspace must ensure any security policy is met before accepting or
binding.
??
Jason
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Catalin Marinas @ 2026-07-09 11:13 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: iommu, linux-arm-kernel, linux-kernel, linux-coco, Robin Murphy,
Marek Szyprowski, Will Deacon, Marc Zyngier, Steven Price,
Suzuki K Poulose, Jiri Pirko, Jason Gunthorpe, Mostafa Saleh,
Petr Tesarik, Alexey Kardashevskiy, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley
In-Reply-To: <yq5apl0xgy89.fsf@kernel.org>
On Wed, Jul 08, 2026 at 11:28:14PM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
>
> > On Wed, Jul 01, 2026 at 11:19:20AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> index 97987f850a33..acf67c7064db 100644
> >> --- a/arch/arm64/mm/init.c
> >> +++ b/arch/arm64/mm/init.c
> >> @@ -338,10 +338,8 @@ void __init arch_mm_preinit(void)
> >> unsigned int flags = SWIOTLB_VERBOSE;
> >> bool swiotlb = max_pfn > PFN_DOWN(arm64_dma_phys_limit);
> >>
> >> - if (is_realm_world()) {
> >> + if (is_realm_world())
> >> swiotlb = true;
> >> - flags |= SWIOTLB_FORCE;
> >> - }
> >
> > For this part:
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> >
> >> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> >> index e05dc7649366..f3fc28f352ba 100644
> >> --- a/kernel/dma/direct.h
> >> +++ b/kernel/dma/direct.h
> >> @@ -88,37 +88,40 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
> >> {
> >> dma_addr_t dma_addr;
> >>
> >> + /*
> >> + * For a device requiring unencrypted DMA, MMIO memory is treated
> >> + * as shared by default.
> >> + */
> >> + if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
> >> + attrs |= DMA_ATTR_CC_SHARED;
> >> +
> >> if (is_swiotlb_force_bounce(dev)) {
> >> - if (!(attrs & DMA_ATTR_CC_SHARED)) {
> >> - if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> >> - return DMA_MAPPING_ERROR;
> >> + if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
> >> + return DMA_MAPPING_ERROR;
> >>
> >> - return swiotlb_map(dev, phys, size, dir, attrs);
> >> - }
> >> - } else if (attrs & DMA_ATTR_CC_SHARED) {
> >> - return DMA_MAPPING_ERROR;
> >> + return swiotlb_map(dev, phys, size, dir, attrs);
> >> }
> >>
> >> - if (attrs & DMA_ATTR_MMIO) {
> >> - dma_addr = phys;
> >> - if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
> >> - goto err_overflow;
> >> - } else if (attrs & DMA_ATTR_CC_SHARED) {
> >> + if (attrs & DMA_ATTR_CC_SHARED)
> >> dma_addr = phys_to_dma_unencrypted(dev, phys);
> >> + else
> >> + dma_addr = phys_to_dma_encrypted(dev, phys);
> >
> > For AMD/SME, on host with memory encryption we now end up setting the C
> > bit for DMA_ATTR_MMIO. This is fine for RAM but not sure whether
> > some other MMIO bus understands this attribute. Maybe we should stick to
> > something like __phys_to_dma() for the !CC_SHARED && MMIO path. Or,
> > since this is not universally defined, just use the old dma_addr = phys
> > if MMIO and ignore any unlikely DMA offsets.
> >
>
> Considering for AMD/SME system an unencrypted dma addr is one without C
> bit, will this be good?
>
> /*
> * For host memory encryption and device requiring unencrypted DMA,
> * MMIO memory is treated as shared by default.
> */
> if (attrs & DMA_ATTR_MMIO) {
> if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) || force_dma_unencrypted(dev))
> attrs |= DMA_ATTR_CC_SHARED;
> }
Yes, I think it does the trick, preserves the current semantics for AMD.
I guess you could use a single 'if' for all checks (up to you).
--
Catalin
^ permalink raw reply
* Re: [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-09 10:16 UTC (permalink / raw)
To: Dave Hansen
Cc: x86, linux-kernel, kas, rick.p.edgecombe, dave.hansen, yilun.xu,
chao.gao, djbw, linux-coco, peter.fang, xiaoyao.li
In-Reply-To: <e8aaca8c-6d88-490b-a3ae-c63a9e9a8b90@intel.com>
On Wed, Jul 08, 2026 at 11:03:14AM -0700, Dave Hansen wrote:
> On 7/8/26 10:03, Xu Yilun wrote:
> > +/*
> > + * SEAMCALL leaf:
> > + *
> > + * Bit 15:0 Leaf number
> > + * Bit 23:16 Version number
> > + */
> > +#define SEAMCALL_VERSION_MASK GENMASK_U64(23, 16)
> > +
> > static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> > struct tdx_module_args *args)
> > {
> > @@ -39,6 +48,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> > */
> > this_cpu_write(cache_state_incoherent, true);
> >
> > + FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> > return func(fn, args);
> > }
>
> This is really looking fragmented and inconsistent.
>
> What if someone *does* set the version bits in 'fn'? Also, if the "leaf
Then these bits would be ignored. FIELD_MODIFY() would overwrite the
version bits with the value in args->version.
> number" is just 16 bits, why is it a u64 in the API?
Because the "leaf number" is actually "bit 15:0 + bit 63". Seamldr calls
also use this path. And their leaf definitions include bit 63, such as:
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
#define P_SEAMLDR_INSTALL 0x8000000000000001
Sorry I only moved existing "SEAMCALL leaf" comments here, but the full
definition in TDX module SPEC is:
Bit 15:0 Leaf number
Bit 23:16 Version number
Bit 24 Interrupt mode /* Setting 1 causes irq-resume loop forever when irq disabled, Linux always sets 0 */
Bit 62:25 Reserved, must be 0
Bit 63 Invoke *P-SEAMLDR* calls
>
> Additionally, look at this:
>
> > /*
> > * Used in __tdcall*() to gather the input/output registers' values of the
> > * TDCALL instruction when requesting services from the TDX module. This is a
> > * software only structure and not part of the TDX module/VMM ABI
> > */
> > struct tdx_module_args {
>
> "version" doesn't fit this comment, does it? It's not a register.
OK, I think I can add a sentence to the comment a bit:
...when requesting service from the TDX module. The 'version' is an exception,
it is encoded in rax along with the Leaf number. This is a software...
[...]
> If we add a new argument to 'tdx_module_args' it seems like the most
> consistent thing to do would be to extend the assembly to marshal it too.
>
> We already have:
>
> /* Move Leaf ID to RAX */
> mov %rdi, %rax
>
> and it wouldn't be rocket science to add two instructions to get
> ->version in to place:
>
> /* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
> movzbl TDX_MODULE_version(%rsi), %eax
> shl $16, %eax
> /* Leaf number arg -> RAX[15:0]; Preserve [23:16]. */
> mov %di, %ax
If we want to keep seamldr call work as is, we can't lose bit 63:
/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
movzbl TDX_MODULE_version(%rsi), %eax
shl $16, %eax
/* Leaf number arg -> RCX[63] and RCX[15:0]. Zero rest of RCX */
mov %rdi, %rcx
movabs $0x800000000000ffff, %rdx
and %rdx, %rcx
/* Combine Leaf number and version -> RAX */
or %rcx, %rax
Or, I prefer more to assert that the Leaf numbers only touch bit 63 & bit [15:0]
in C code, this catches buggy bits rather than ignore them, and simplifies
assembly code:
@@ -58,6 +58,8 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
int retry = RDRAND_RETRY_LOOPS;
u64 ret;
+ BUILD_BUG_ON(fn & ~0x800000000000ffffULL); /* TODO: proper Macros */
then:
/* Leaf ABI version -> RAX[23:16]. Zero rest of RAX. */
movzbl TDX_MODULE_version(%rsi), %eax
shl $16, %eax
/* Combine Leaf ID and ABI version to RAX, they don't overlap */
or %rdi, %rax
^ permalink raw reply
* Re: [PATCH v8 1/6] firmware: smccc: Add an Arm SMCCC bus
From: Sudeep Holla @ 2026-07-09 8:12 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: linux-coco, linux-arm-kernel, linux-kernel, Sudeep Holla,
Catalin Marinas, Greg KH, Jeremy Linton, Jonathan Cameron,
Lorenzo Pieralisi, Mark Rutland, Will Deacon, Steven Price,
Suzuki K Poulose, Andre Przywara
In-Reply-To: <20260707081351.1680209-2-aneesh.kumar@kernel.org>
On Tue, Jul 07, 2026 at 01:43:45PM +0530, Aneesh Kumar K.V (Arm) wrote:
> SMCCC-discovered firmware services are currently represented by separate
> platform devices, such as smccc_trng and arm-cca-dev. Those devices do not
> represent independent DT/ACPI-described platform resources; they are
> features of the SMCCC firmware interface.
>
> Add an Arm SMCCC bus for services discovered through the SMCCC firmware
> interface. The bus provides SMCCC device and driver registration helpers,
> name-based matching, modalias generation, and a sysfs modalias attribute so
> SMCCC service drivers can bind to discovered firmware services and autoload
> as modules.
>
> Follow-up changes can then register SMCCC firmware services as arm-smccc
> devices instead of creating independent per-feature platform devices.
>
> Based on arm_ffa code
>
Reviewed-by: Sudeep Holla <sudeep.holla@kernel.org>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 13/15] PCI, device core: Add private memory access for DEVICE_TRUST_TCB
From: Alexey Kardashevskiy @ 2026-07-09 7:38 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Bjorn Helgaas, Dexuan Cui
In-Reply-To: <f8b5b966-ea36-4b4a-a9f5-58a20855a6fb@amd.com>
On 9/7/26 16:32, Alexey Kardashevskiy wrote:
> On 6/7/26 08:08, Dan Williams wrote:
>> A device that wants to access private memory needs to have its trust
>> elevated to DEVICE_TRUST_TCB. That trust is established either at compile
>> time (unlikely), the bus knows the device is within the TCB to start (some
>> paravisor setups), or the device is dynamically added to the TCB in
>> coordination with a TSM driver (primary TDISP use case) and the trust is
>> elevated by driver match.
>>
>> When a PCI device is associated with a TSM for security services the low
>> level TSM driver in the CC VM has the opportunity validate DMA access.
>> That validation happens at ->dma_configure() time when the device attaches
>> to a driver. The TSM driver is responsible for proving to the platform TSM
>> that the VM is enabling DMA with respect to the most recently generated
>> evidence. If that fails, driver attach fails.
>>
>> When a PCI device is not associated with a TSM provider for security
>> services, but the device is trusted there are 3 options.
>>
>> 1/ Arch requires all DMA enable events to be acked by TSM driver
>>
>> 2/ Arch does not require, but admin policy is responsible for knowing which
>> devices need TSM coordination to become active within the TCB.
>>
>> 3/ Device is approved by a paravisor to operate within the TCB, no TSM
>> coordination required.
>>
>> In cases 2 and 3 if the device needed TSM driver coordination, but the TSM
>> driver or association to the device is missing, it triggers hardware
>> errors. Those errors are a configuration error that the kernel does not
>> actively prevent.
>>
>> Architectures still need to fixup force_dma_unencrypted() to call
>> device_tcb_trusted() to tell the DMA layer that TCB access is granted.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>> Cc: Danilo Krummrich <dakr@kernel.org>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Dexuan Cui <decui@microsoft.com>
>> Signed-off-by: Dan Williams <djbw@kernel.org>
>> ---
>> drivers/base/Kconfig | 19 ++++++++++++++++++-
>> include/linux/device/trust.h | 4 ++++
>> drivers/base/trust.c | 8 ++++++++
>> drivers/pci/pci-driver.c | 25 ++++++++++++++++---------
>> 4 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
>> index a4233bdf9804..3597b39ee0e3 100644
>> --- a/drivers/base/Kconfig
>> +++ b/drivers/base/Kconfig
>> @@ -304,7 +304,17 @@ config DEVICE_TRUST_AUTO
>> help
>> Typical historical driver model, devices eagerly attempt to attach to
>> a driver and deploy all available mechanisms to allow performant
>> - direct memory access.
>> + direct memory access. This trust level does not include TCB privileges.
>> +
>> +config DEVICE_TRUST_TCB
>> + bool "TCB"
>> + depends on EXPERT
>> + help
>> + Devices are assumed to be within the Trusted Compute Boundary (TCB)
>> + with access to private TCB resources by default. This requires buses
>> + to have trust awareness and opt devices out of private operation,
>> + otherwise TCB integrity may fail the device's access or escalate to
>> + terminating the execution context (kill the CC VM).
>> endchoice
>> @@ -330,6 +340,13 @@ config BUILTIN_DEVICE_TRUST_ADVERSARY
>> help
>> Deploy mitigations in the IOMMU layer and driver to limit access.
>> +config BUILTIN_DEVICE_TRUST_TCB
>> + bool "TCB"
>> + depends on EXPERT
>> + help
>> + Devices bound by built-in drivers are assumed to be within the
>> + Trusted Compute Boundary with access to private/encrypted memory.
>> +
>> endchoice
>> endmenu
>> diff --git a/include/linux/device/trust.h b/include/linux/device/trust.h
>> index 283d3196e5e6..24951f5c56ba 100644
>> --- a/include/linux/device/trust.h
>> +++ b/include/linux/device/trust.h
>> @@ -13,6 +13,7 @@
>> * @DEVICE_TRUST_NONE: Blocked when idle, cannot bind
>> * @DEVICE_TRUST_ADVERSARY: Blocked when idle, constrained when active.
>> * @DEVICE_TRUST_AUTO: All typical privileges granted
>> + * @DEVICE_TRUST_TCB: AUTO privileges + private/encrypted memory access
>> *
>> * Devices flagged as adversarial are the ones that can potentially
>> * execute DMA attacks and similar. They are typically connected through
>> @@ -25,11 +26,13 @@ enum device_trust {
>> DEVICE_TRUST_NONE,
>> DEVICE_TRUST_ADVERSARY,
>> DEVICE_TRUST_AUTO,
>> + DEVICE_TRUST_TCB,
>> };
>> #define DEVICE_DEFAULT_TRUST \
>> (IS_ENABLED(CONFIG_DEVICE_TRUST_NONE) ? DEVICE_TRUST_NONE : \
>> IS_ENABLED(CONFIG_DEVICE_TRUST_ADVERSARY) ? DEVICE_TRUST_ADVERSARY : \
>> + IS_ENABLED(CONFIG_DEVICE_TRUST_TCB) ? DEVICE_TRUST_TCB : \
>> DEVICE_TRUST_AUTO)
>> struct device;
>> @@ -37,6 +40,7 @@ struct device_driver;
>> #ifdef CONFIG_DEVICE_TRUST
>> bool device_untrusted(struct device *dev);
>> +bool device_tcb_trusted(struct device *dev);
>> void module_driver_trust(struct module *mod, const char *val);
>> void module_driver_trust_init(struct module *mod, bool require_trust);
>> #else
>> diff --git a/drivers/base/trust.c b/drivers/base/trust.c
>> index 8efbe5c51250..21d374affbba 100644
>> --- a/drivers/base/trust.c
>> +++ b/drivers/base/trust.c
>> @@ -23,11 +23,18 @@ bool device_untrusted(struct device *dev)
>> return dev->bus_trust && dev->bus_trust <= DEVICE_TRUST_ADVERSARY;
>> }
>> +bool device_tcb_trusted(struct device *dev)
>> +{
>> + return dev->p->trust >= DEVICE_TRUST_TCB;
>> +}
>> +
>> /* Driver trust policy requires modules, builtin drivers always attach */
>> static enum device_trust builtin_driver_trust(void)
>> {
>> if (IS_ENABLED(CONFIG_BUILTIN_DEVICE_TRUST_ADVERSARY))
>> return DEVICE_TRUST_ADVERSARY;
>> + else if (IS_ENABLED(CONFIG_BUILTIN_DEVICE_TRUST_TCB))
>> + return DEVICE_TRUST_TCB;
>
>
> No plan to override this via the kernel cmdline?
Just tried - my test guest kernel has everything built-in. Since there is no module parameter - I did CONFIG_BUILTIN_DEVICE_TRUST_TCB=y. But now every driver gets this level, including virtio, which cannot ever be "TCB" (as anything emulated by QEMU). Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path
From: Yan Zhao @ 2026-07-09 7:17 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, kai.huang
In-Reply-To: <20260526023515.288829-9-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:12PM -0700, Rick Edgecombe wrote:
> When handling an EPT violation, KVM holds a spinlock while manipulating
> the EPT. Before entering the spinlock it doesn't know how many EPT page
> tables will need to be installed or whether a huge page will be used. For
> this reason it allocates a worst case number of page tables that it might
> need as part of servicing the EPT violation.
>
> Under Dynamic PAMT these pre-allocated pages will potentially need to have
> Dynamic PAMT backing pages installed for them. KVM already has helpers to
> manage topping up page caches before taking the MMU lock, but they cannot be
> passed from KVM to arch/x86 code.
>
> The problem of how and when to install the DPAMT backing pages for the
> pages given to the TDX module during the fault path has had a lot of
> design attempts.
> - Extracting KVM's MMU caches requires too much inlined code added to
> headers.
> - A few varieties of installing Dynamic PAMT backing when allocating the
> S-EPT page tables. [0][1]
IIUC, [0][1] here refer to design attempts that had various problems, right?
However, [1] looks exactly like the one being adopted in v6?
Did you paste a wrong link? Should [1] instead be
https://lore.kernel.org/kvm/20260129011517.3545883-21-seanjc@google.com or
https://lore.kernel.org/kvm/aYYCOiMvWfSJR1AL@google.com ?
> - Using mempool_t to transfer the pages between KVM and arch/x86 doesn't
> work because it is the component is designed more around maintaining a
> pool of pages, rather than topping up a continually drained cache.
>
> So don't do these as they all had various problems. Instead just create a
> small simple data structure to use for handing a pre-allocated list of
> pages between KVM and arch/x86 code. Model this on KVM's existing MMU
> memory caches.
>
> Add a tdx_pamt_cache arg to tdx_pamt_get() so it can draw pages from a
> cache when needed. Not all DPAMT page installations will happen under
> spinlock, for example control pages. So have tdx_pamt_get() maintain the
Nit: In patch 9, S-EPT pages are regarded as control pages as well.
So maybe
"..., for example some control pages." or
"..., for example control pages other than S-EPT pages." ?
> existing behavior of allocating from the page allocator when NULL is
> passed for the struct tdx_pamt_cache arg. This prevents excess allocations
> for cases where it can be avoided.
>
> Export the new helpers for KVM.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Link: https://lore.kernel.org/kvm/de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com/ [0]
> Link: https://lore.kernel.org/kvm/aYprxnSHKHUtk7pt@google.com/ [1]
> ---
> v6:
> - Filled out log from Sean's series
> ---
> arch/x86/include/asm/tdx.h | 17 ++++++++++
> arch/x86/virt/vmx/tdx/tdx.c | 65 +++++++++++++++++++++++++++++++++----
> 2 files changed, 76 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 74e75db5728c7..191da84bbf2a1 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -155,6 +155,23 @@ static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> return false; /* To be enabled when kernel is ready */
> }
>
> +/* Simple structure for pre-allocating Dynamic PAMT pages outside of locks. */
outside of spinlocks?
Pre-allocating Dynamic PAMT pages are still inside mutex, e.g., inside of
kvm->slots_lock, vcpu->mutex...
> +struct tdx_pamt_cache {
> + struct list_head page_list;
> + int cnt;
> +};
The rest LGTM.
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
^ permalink raw reply
* Re: [PATCH 13/15] PCI, device core: Add private memory access for DEVICE_TRUST_TCB
From: Alexey Kardashevskiy @ 2026-07-09 6:32 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Bjorn Helgaas, Dexuan Cui
In-Reply-To: <20260705220819.2472765-14-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> A device that wants to access private memory needs to have its trust
> elevated to DEVICE_TRUST_TCB. That trust is established either at compile
> time (unlikely), the bus knows the device is within the TCB to start (some
> paravisor setups), or the device is dynamically added to the TCB in
> coordination with a TSM driver (primary TDISP use case) and the trust is
> elevated by driver match.
>
> When a PCI device is associated with a TSM for security services the low
> level TSM driver in the CC VM has the opportunity validate DMA access.
> That validation happens at ->dma_configure() time when the device attaches
> to a driver. The TSM driver is responsible for proving to the platform TSM
> that the VM is enabling DMA with respect to the most recently generated
> evidence. If that fails, driver attach fails.
>
> When a PCI device is not associated with a TSM provider for security
> services, but the device is trusted there are 3 options.
>
> 1/ Arch requires all DMA enable events to be acked by TSM driver
>
> 2/ Arch does not require, but admin policy is responsible for knowing which
> devices need TSM coordination to become active within the TCB.
>
> 3/ Device is approved by a paravisor to operate within the TCB, no TSM
> coordination required.
>
> In cases 2 and 3 if the device needed TSM driver coordination, but the TSM
> driver or association to the device is missing, it triggers hardware
> errors. Those errors are a configuration error that the kernel does not
> actively prevent.
>
> Architectures still need to fixup force_dma_unencrypted() to call
> device_tcb_trusted() to tell the DMA layer that TCB access is granted.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> drivers/base/Kconfig | 19 ++++++++++++++++++-
> include/linux/device/trust.h | 4 ++++
> drivers/base/trust.c | 8 ++++++++
> drivers/pci/pci-driver.c | 25 ++++++++++++++++---------
> 4 files changed, 46 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index a4233bdf9804..3597b39ee0e3 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -304,7 +304,17 @@ config DEVICE_TRUST_AUTO
> help
> Typical historical driver model, devices eagerly attempt to attach to
> a driver and deploy all available mechanisms to allow performant
> - direct memory access.
> + direct memory access. This trust level does not include TCB privileges.
> +
> +config DEVICE_TRUST_TCB
> + bool "TCB"
> + depends on EXPERT
> + help
> + Devices are assumed to be within the Trusted Compute Boundary (TCB)
> + with access to private TCB resources by default. This requires buses
> + to have trust awareness and opt devices out of private operation,
> + otherwise TCB integrity may fail the device's access or escalate to
> + terminating the execution context (kill the CC VM).
>
> endchoice
>
> @@ -330,6 +340,13 @@ config BUILTIN_DEVICE_TRUST_ADVERSARY
> help
> Deploy mitigations in the IOMMU layer and driver to limit access.
>
> +config BUILTIN_DEVICE_TRUST_TCB
> + bool "TCB"
> + depends on EXPERT
> + help
> + Devices bound by built-in drivers are assumed to be within the
> + Trusted Compute Boundary with access to private/encrypted memory.
> +
> endchoice
>
> endmenu
> diff --git a/include/linux/device/trust.h b/include/linux/device/trust.h
> index 283d3196e5e6..24951f5c56ba 100644
> --- a/include/linux/device/trust.h
> +++ b/include/linux/device/trust.h
> @@ -13,6 +13,7 @@
> * @DEVICE_TRUST_NONE: Blocked when idle, cannot bind
> * @DEVICE_TRUST_ADVERSARY: Blocked when idle, constrained when active.
> * @DEVICE_TRUST_AUTO: All typical privileges granted
> + * @DEVICE_TRUST_TCB: AUTO privileges + private/encrypted memory access
> *
> * Devices flagged as adversarial are the ones that can potentially
> * execute DMA attacks and similar. They are typically connected through
> @@ -25,11 +26,13 @@ enum device_trust {
> DEVICE_TRUST_NONE,
> DEVICE_TRUST_ADVERSARY,
> DEVICE_TRUST_AUTO,
> + DEVICE_TRUST_TCB,
> };
>
> #define DEVICE_DEFAULT_TRUST \
> (IS_ENABLED(CONFIG_DEVICE_TRUST_NONE) ? DEVICE_TRUST_NONE : \
> IS_ENABLED(CONFIG_DEVICE_TRUST_ADVERSARY) ? DEVICE_TRUST_ADVERSARY : \
> + IS_ENABLED(CONFIG_DEVICE_TRUST_TCB) ? DEVICE_TRUST_TCB : \
> DEVICE_TRUST_AUTO)
>
> struct device;
> @@ -37,6 +40,7 @@ struct device_driver;
>
> #ifdef CONFIG_DEVICE_TRUST
> bool device_untrusted(struct device *dev);
> +bool device_tcb_trusted(struct device *dev);
> void module_driver_trust(struct module *mod, const char *val);
> void module_driver_trust_init(struct module *mod, bool require_trust);
> #else
> diff --git a/drivers/base/trust.c b/drivers/base/trust.c
> index 8efbe5c51250..21d374affbba 100644
> --- a/drivers/base/trust.c
> +++ b/drivers/base/trust.c
> @@ -23,11 +23,18 @@ bool device_untrusted(struct device *dev)
> return dev->bus_trust && dev->bus_trust <= DEVICE_TRUST_ADVERSARY;
> }
>
> +bool device_tcb_trusted(struct device *dev)
> +{
> + return dev->p->trust >= DEVICE_TRUST_TCB;
> +}
> +
> /* Driver trust policy requires modules, builtin drivers always attach */
> static enum device_trust builtin_driver_trust(void)
> {
> if (IS_ENABLED(CONFIG_BUILTIN_DEVICE_TRUST_ADVERSARY))
> return DEVICE_TRUST_ADVERSARY;
> + else if (IS_ENABLED(CONFIG_BUILTIN_DEVICE_TRUST_TCB))
> + return DEVICE_TRUST_TCB;
No plan to override this via the kernel cmdline?
> return DEVICE_TRUST_AUTO;
> }
>
> @@ -61,6 +68,7 @@ static const char * const device_trust_names[] = {
> [DEVICE_TRUST_NONE] = "none",
> [DEVICE_TRUST_ADVERSARY] = "adversary",
> [DEVICE_TRUST_AUTO] = "auto",
> + [DEVICE_TRUST_TCB] = "tcb",
> };
>
> static enum device_trust device_trust_parse(const char *name)
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index f36778e62ac1..e6a4bc3ca9b0 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -19,6 +19,7 @@
> #include <linux/kexec.h>
> #include <linux/of_device.h>
> #include <linux/acpi.h>
> +#include <linux/pci-tsm.h>
> #include <linux/dma-map-ops.h>
> #include <linux/iommu.h>
> #include "pci.h"
> @@ -1668,10 +1669,11 @@ static int pci_bus_num_vf(struct device *dev)
> static int pci_dma_configure(struct device *dev)
> {
> const struct device_driver *drv = READ_ONCE(dev->driver);
> - struct device *bridge;
> + struct pci_dev *pdev = to_pci_dev(dev);
> int ret = 0;
>
> - bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> + struct device *bridge __free(put_device) =
> + pci_get_host_bridge_device(pdev);
>
> if (IS_ENABLED(CONFIG_OF) && bridge->parent &&
> bridge->parent->of_node) {
> @@ -1688,16 +1690,21 @@ static int pci_dma_configure(struct device *dev)
> * the standard ACS capability but still support ACS via those
> * quirks.
> */
> - pci_enable_acs(to_pci_dev(dev));
> -
> - pci_put_host_bridge_device(bridge);
> + pci_enable_acs(pdev);
>
> /* @drv may not be valid when we're called from the IOMMU layer */
> - if (!ret && drv && !to_pci_driver(drv)->driver_managed_dma) {
> + if (ret || !drv)
> + return ret;
> +
> + if (device_tcb_trusted(dev))
> + ret = pci_tsm_enable_dma(pdev);
pci_tsm_disable_dma() is missing in pci_dma_cleanup() then. Otherwise works fine. Thanks!
> +
> + if (!ret && !to_pci_driver(drv)->driver_managed_dma)
> ret = iommu_device_use_default_domain(dev);
> - if (ret)
> - arch_teardown_dma_ops(dev);
> - }
> +
> + /* undo {of,acpi}_dma_configure() */
> + if (ret)
> + arch_teardown_dma_ops(dev);
>
> return ret;
> }
--
Alexey
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Yan Zhao @ 2026-07-09 3:18 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Hansen, Dave, kirill.shutemov@linux.intel.com, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, nik.borisov@suse.com,
linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <f686c26073761302b699c7374b668c933b0452e8.camel@intel.com>
On Thu, Jul 09, 2026 at 09:12:31AM +0800, Edgecombe, Rick P wrote:
> On Wed, 2026-07-08 at 17:13 +0800, Yan Zhao wrote:
> > On Mon, May 25, 2026 at 07:35:11PM -0700, Rick Edgecombe wrote:
> > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > >
> > > Use control page helpers for allocating and freeing TD control structures,
> > > such these operations can work for Dynamic PAMT.
> > >
> > > The TDX module tracks some state for each page of physical memory that it
> > > might use. It calls this state the PAMT. It includes separate state for
> > Nit: It records this state in the PAMT ?
>
> The text is trying to describe what PAMT is, so I'm not sure about this one.
Ok.
To me, PAMT stands for physical address metadata table, and the metadata the
table tracks is the "state" of a page.
Feel free to ignore the nit though :)
> > > each page size a physical page could be utilized at within the TDX module
> > > (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> > > allocated dynamically. So the kernel must install PAMT backing for each 4KB
> > How about
> > "..., only the backend to hold the 4KB page size state is allocated
> > dynamically" ?
>
> How about:
> In Dynamic PAMT, only the 4KB page size backing is allocated dynamically.
LGTM.
> > > page before gifting it to the TDX module, and tear it down after the page
> > > is reclaimed.
> > How about
> > "So the kernel must check whether it is necessary to install/remove PAMT
> > backing for each 4KB page, and do so when necessary before gifting the page
> > to the TDX module or after it is reclaimed." ?
>
> The checking part is true, but is it needed to understand this patch? I think it
> just needs a background of what is happening. How about:
>
> So the kernel must install PAMT backing for any 4KB page being gifting to the
> TDX module, and tear down the backing when the associated gifted pages are
> reclaimed.
Hmm. I'm not sure if it would be confusing for people without sufficient
background, because the kernel only needs to install PAMT backing before the
gifting the first 4KB page.
How about:
So the kernel must ensure PAMT backing installed for any 4KB page being gifting
to the TDX module, and tear down the backing when all associated gifted pages
are reclaimed.
^ permalink raw reply
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