* Re: [PATCH 06/15] PCI/TSM: Add device evidence support
From: Dan Williams (nvidia) @ 2026-07-08 18:25 UTC (permalink / raw)
To: Alexey Kardashevskiy, Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Bjorn Helgaas, Alistair Francis,
Lukas Wunner, Xu Yilun, Aneesh Kumar K.V (Arm)
In-Reply-To: <e42b9ef3-a177-4e38-8aad-c3f4061f1b64@amd.com>
Alexey Kardashevskiy wrote:
> On 6/7/26 08:08, Dan Williams wrote:
> > Register the PCI Trusted Execution Environment Security Manager (TSM)
> > framework with the device-evidence netlink ABI. The security operations
> > that PCI core coordinates with a device security manager (DSM) through a
> > platform TEE security manager (TSM), builds upon the SPDM protocol.
> >
> > A TSM owns an SPDM session and publishes the corresponding evidence through
> > TSM firwmware ABIs. A low level TSM driver is responsible for creating a
> > 'struct device_evidence' context and carrying out refresh_evidence()
>
>
> a nit: so it is really refresh_measurements() as it won't refresh the report? may be call it so?
Sure.
> > requests for regenerating measurement transcripts with a nonce.
[..]
> > diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> > index a6435aba03f9..8bc16029d31e 100644
> > --- a/include/linux/pci-tsm.h
> > +++ b/include/linux/pci-tsm.h
> > @@ -3,7 +3,10 @@
> > #define __PCI_TSM_H
> > #include <linux/mutex.h>
> > #include <linux/pci.h>
> > +#include <linux/rwsem.h>
>
> then this file is missing extern struct rw_semaphore pci_tsm_rwsem;
This include was a mistake. pci_tsm_rwsem is private to
drivers/pci/tsm/, so I will remove this include. This is the only shared
data structure between drivers/pci/tsm/core.c and
drivers/pci/tsm/evidence.c. It could go go in a
drivers/pci/tsm/private.h, but given just the one declaration have
evidence.c do it directly.
> > #include <linux/sockptr.h>
> > +#include <linux/tsm.h>
> > +#include <linux/device/evidence.h>
> >
> > struct pci_tsm;
> > struct tsm_dev;
> > @@ -18,6 +21,7 @@ enum pci_tsm_req_scope;
> > * @devsec_ops: Lock, unlock, and interrogate the security state of the
> > * function via the platform TSM (typically virtual function
> > * operations).
> > + * @refresh_evidence: Common operation to regenerate attestation objects
> > *
> > * This operations are mutually exclusive either a tsm_dev instance
> > * manages physical link properties or it manages function security
> > @@ -75,6 +79,9 @@ struct pci_tsm_ops {
> > struct pci_dev *pdev);
> > void (*unlock)(struct pci_tsm *tsm);
> > );
> > +
> > + int (*refresh_evidence)(struct pci_tsm *tsm, const void *nonce,
> > + size_t nonce_len);
> > };
> >
> > /**
[..]
> > @@ -216,6 +226,10 @@ void pci_tsm_tdi_constructor(struct pci_dev *pdev, struct pci_tdi *tdi,
> > ssize_t pci_tsm_guest_req(struct pci_dev *pdev, enum pci_tsm_req_scope scope,
> > sockptr_t req_in, size_t in_len, sockptr_t req_out,
> > size_t out_len, u64 *tsm_code);
> > +static inline const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> > +{
> > + return tsm->tsm_dev->pci_ops;
> > +}
> > #else
> > static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> > {
> > diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm/core.c
> > similarity index 99%
> > rename from drivers/pci/tsm.c
> > rename to drivers/pci/tsm/core.c
>
> This move + related makefile changes + exposing pci_tsm_rwsem are better be in a separate patch. Thanks,
I'll do a prep patch.
^ permalink raw reply
* Re: [PATCH 13/15] PCI, device core: Add private memory access for DEVICE_TRUST_TCB
From: Aneesh Kumar K.V @ 2026-07-08 18:10 UTC (permalink / raw)
To: Dan Williams (nvidia), 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: <6a4e91aabfeb_2f05d5100da@djbw-dev.notmuch>
"Dan Williams (nvidia)" <djbw@kernel.org> writes:
> Aneesh Kumar K.V wrote:
>> Dan Williams <djbw@kernel.org> writes:
>>
>> > 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.
>> >
>>
>>
>> Do we have the last case, where the device is dynamically added to the
>> TCB in coordination with a TSM, implemented in this series? Do we expect
>> the CCA driver to set that up?
>
> In this case "dynamically added" means pre-existing device converted
> from shared to private operation. I.e. the model the work-in-progress
> CCA patches already cover if that was your question?
So we accept the device and then load the driver with trust=tcb ?. If not,
where do we set the device->bus_trust = DEVICE_TRUST_TCB?
-aneesh
^ permalink raw reply
* Re: [PATCH 13/15] PCI, device core: Add private memory access for DEVICE_TRUST_TCB
From: Dan Williams (nvidia) @ 2026-07-08 18:06 UTC (permalink / raw)
To: Aneesh Kumar K.V, 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: <yq5aa4s4i91v.fsf@kernel.org>
Aneesh Kumar K.V wrote:
> Dan Williams <djbw@kernel.org> writes:
>
> > 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.
> >
>
>
> Do we have the last case, where the device is dynamically added to the
> TCB in coordination with a TSM, implemented in this series? Do we expect
> the CCA driver to set that up?
In this case "dynamically added" means pre-existing device converted
from shared to private operation. I.e. the model the work-in-progress
CCA patches already cover if that was your question?
^ permalink raw reply
* Re: [PATCH 15/15] PCI/TSM: Add relative MMIO offset support?
From: Dan Williams (nvidia) @ 2026-07-08 18:05 UTC (permalink / raw)
To: Alexey Kardashevskiy, Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Xu Yilun, Aneesh Kumar K.V
In-Reply-To: <ae911d3b-682f-4c92-8f8d-d0d667388740@amd.com>
Alexey Kardashevskiy wrote:
> On 6/7/26 08:08, Dan Williams wrote:
> > The RMM specification, DEN0137-2.0-bet2 section A9.6.2 "Realm validation of
> > device memory mappings" documents the expectation that the
> > MMIO_REPORTING_OFFSET chosen for TDISP Interface Reports is always BAR
> > aligned.
> >
> > Ideally this change is not needed and all implementations share the same
> > expectation.
> >
> > If this semantic is already shipping in production and/or the PCI-SIG
> > clarifies that an implementation can hold this assumption then Linux will
> > need to ask the TSM drivers for this hint.
>
> We are changing it on SEV-TIO such that the host os calculates the
> offset to allow TDISP_OFFSET_BAR_ALIGN and passes it to the PSP during
> TDI_BIND (==interface start), others did not need it in the first
> place so I guess we can drop this one. Thanks,
Ok, yes, then this one can be dropped.
^ permalink raw reply
* Re: [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Dave Hansen @ 2026-07-08 18:03 UTC (permalink / raw)
To: Xu Yilun, x86, linux-kernel
Cc: kas, rick.p.edgecombe, 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 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
number" is just 16 bits, why is it a u64 in the API?
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.
If "fn" is just the 16-bit leaf number it should be a u16 everywhere.
Then there's no worry about the version number leaking in there
somewhere. The type can't even _carry_ the version number.
As a general rule, I dislike doing things in assembly that can be done
in C. But, in this case, we have some pretty darn simple assembly doing
a pretty simple job: marshaling data out of 'tdx_module_args' and in to
registers.
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
I don't *love* this. But, to be honest, I'd never even seen
FIELD_MODIFY() before.
Also, please double and triple check the assembly above. This does
depend pretty heavily on the rules around register naming and width, and
x86 is not exactly the most straightforward ISA in the world there.
I also wouldn't be shocked if there's a nicer way to do this that
someone else dreams up.
^ 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-08 17:58 UTC (permalink / raw)
To: 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, 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: <ak42F240d-53QeFN@arm.com>
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;
}
>
> In the other case, for an arm CCA guest, if the MMIO is shared we end up
> setting the shared attribute but that's fine, it's only an IPA address.
>
> --
> Catalin
-aneesh
^ permalink raw reply
* [PATCH v2] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-08 17:03 UTC (permalink / raw)
To: x86, linux-kernel
Cc: kas, rick.p.edgecombe, dave.hansen, dave.hansen, yilun.xu,
yilun.xu, chao.gao, djbw, linux-coco, peter.fang, xiaoyao.li
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.
Link: https://lore.kernel.org/kvm/4f4b0f29-424b-45ed-8cfd-c77da2ea390f@intel.com/ # [1]
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
Two alternative schemes were considered:
1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
This breaks naming consistency unless all existing stable function
macros are changed to TDH_XXX_V0.
2. Add an explicit 'version' parameter to the base seamcall() API. This
forces all stable SEAMCALL helpers to pass a meaningless '0'
argument. The magic '0' or '1' values at caller sites are not
descriptive.
Change in v2:
- Drop the C wrapper __seamcall_encode_fn()
- Rewrite the first paragraph of the changelog
- Make change log concise
- Add a link to the thread where this was suggested
- Move alternative schemes description below the separator
v1: https://lore.kernel.org/all/20260702144614.59464-1-yilun.xu@linux.intel.com/
---
arch/x86/include/asm/shared/tdx.h | 2 ++
arch/x86/virt/vmx/tdx/seamcall_internal.h | 10 ++++++++++
arch/x86/virt/vmx/tdx/tdx.h | 8 --------
arch/x86/virt/vmx/tdx/tdx.c | 5 +++--
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index f20e91d7ac35..b9aac2de233a 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -143,6 +143,8 @@ struct tdx_module_args {
u64 rbx;
u64 rdi;
u64 rsi;
+ /* ABI version, encoded in rax */
+ u8 version;
};
/* Used to communicate with the TDX module */
diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
index be5f446467df..53d7ab037750 100644
--- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
+++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
@@ -11,6 +11,7 @@
#ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
#define _X86_VIRT_SEAMCALL_INTERNAL_H
+#include <linux/bitfield.h>
#include <linux/printk.h>
#include <linux/types.h>
#include <asm/archrandom.h>
@@ -23,6 +24,14 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
+/*
+ * 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);
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index bdfd0e1e337a..63e3acfb5d0c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -50,14 +50,6 @@
#define TDH_SYS_UPDATE 53
#define TDH_SYS_DISABLE 69
-/*
- * SEAMCALL leaf:
- *
- * Bit 15:0 Leaf number
- * Bit 23:16 Version number
- */
-#define TDX_VERSION_SHIFT 16
-
/* TDX page types */
#define PT_NDA 0x0
#define PT_RSVD 0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..7a89e29b118c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
.rcx = vp->tdvpr_pa,
.rdx = initial_rcx,
.r8 = x2apicid,
+ /* apicid requires version == 1. */
+ .version = 1,
};
- /* apicid requires version == 1. */
- return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
+ return seamcall(TDH_VP_INIT, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Aneesh Kumar K.V @ 2026-07-08 15:09 UTC (permalink / raw)
To: 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, 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: <ak42F240d-53QeFN@arm.com>
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.
>
> In the other case, for an arm CCA guest, if the MMIO is shared we end up
> setting the shared attribute but that's fine, it's only an IPA address.
>
P2PDMA with CoCo is currently not supported, as discussed here:
https://lore.kernel.org/all/20260521175420.GA7702@ziepe.ca
https://lore.kernel.org/all/20260522132240.GD7702@ziepe.ca
Since I do not have much experience with P2PDMA, I have prepared a patch
that Leon Romanovsky <leonro@nvidia.com> has kindly agreed to review to
ensure it is correct. I have attached it below.
The conclusion is that P2P is not currently supported, so we get this
series merged first and handle P2PDMA support as a follow-up outside
this series.
commit f39bcb603c5cc4cfa738597715828ac45f01f142
Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
Date: Fri May 22 10:10:35 2026 +0530
PCI/P2PDMA: Carry host bridge DMA attrs through map and unmap
P2PDMA host bridge mappings currently pass DMA_ATTR_MMIO and rely on
dma-direct to add DMA_ATTR_CC_SHARED for devices that require unencrypted
DMA. That puts P2PDMA-specific confidential-computing policy in the generic
dma-direct MMIO mapping path and does not preserve the selected attributes
through all unmap paths.
Add DMA mapping flags to struct p2pdma_provider for host bridge mappings
and use those flags in the block, dma-buf and HMM mapping paths. Store the
attributes selected at map time where the unmap side no longer has provider
context, so that the unmap path uses the same attributes that were used for
mapping.
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index bfdb9ed70741..fafc6d6b8522 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -87,10 +87,11 @@ static bool blk_dma_map_bus(struct blk_dma_iter *iter, struct phys_vec *vec)
static bool blk_dma_map_direct(struct request *req, struct device *dma_dev,
struct blk_dma_iter *iter, struct phys_vec *vec)
{
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
+ attrs |= iter->p2pdma.mem->dma_mapping_flags;
+ iter->attrs = attrs;
iter->addr = dma_map_phys(dma_dev, vec->paddr, vec->len,
rq_dma_dir(req), attrs);
@@ -107,7 +108,7 @@ static bool blk_rq_dma_map_iova(struct request *req, struct device *dma_dev,
struct phys_vec *vec)
{
enum dma_data_direction dir = rq_dma_dir(req);
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
size_t mapped = 0;
int error;
@@ -115,7 +116,8 @@ static bool blk_rq_dma_map_iova(struct request *req, struct device *dma_dev,
iter->len = dma_iova_size(state);
if (iter->p2pdma.map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
+ attrs |= iter->p2pdma.mem->dma_mapping_flags;
+ iter->attrs = attrs;
do {
error = dma_iova_link(dma_dev, state, vec->paddr, mapped,
@@ -168,6 +170,7 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
struct phys_vec vec;
memset(&iter->p2pdma, 0, sizeof(iter->p2pdma));
+ iter->attrs = 0;
iter->status = BLK_STS_OK;
iter->p2pdma.map = PCI_P2PDMA_MAP_NONE;
diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
index 794acff2546a..e6d4ee8acd19 100644
--- a/drivers/dma-buf/dma-buf-mapping.c
+++ b/drivers/dma-buf/dma-buf-mapping.c
@@ -59,11 +59,13 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state,
* @sgt: Scatter-gather table
* @state: DMA IOVA state relevant in IOMMU-based DMA
* @size: Total size of DMA transfer
+ * @attrs: DMA attributes used for host bridge mappings
*/
struct dma_buf_dma {
struct sg_table sgt;
struct dma_iova_state *state;
size_t size;
+ unsigned long attrs;
};
/**
@@ -119,6 +121,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
*/
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
+ dma->attrs = provider->dma_mapping_flags;
dma->state = kzalloc_obj(*dma->state);
if (!dma->state) {
ret = -ENOMEM;
@@ -147,7 +150,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
ret = dma_iova_link(attach->dev, dma->state,
phys_vec[i].paddr, 0,
phys_vec[i].len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
if (ret)
goto err_unmap_dma;
@@ -155,7 +158,7 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
} else {
addr = dma_map_phys(attach->dev, phys_vec[i].paddr,
phys_vec[i].len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
ret = dma_mapping_error(attach->dev, addr);
if (ret)
goto err_unmap_dma;
@@ -195,11 +198,11 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
; /* Do nothing */
} else if (dma_use_iova(dma->state)) {
dma_iova_destroy(attach->dev, dma->state, mapped_len, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
} else {
for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
dma_unmap_phys(attach->dev, sg_dma_address(sgl),
- sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
+ sg_dma_len(sgl), dir, dma->attrs);
}
sg_free_table(&dma->sgt);
err_free_state:
@@ -231,13 +234,13 @@ void dma_buf_free_sgt(struct dma_buf_attachment *attach, struct sg_table *sgt,
; /* Do nothing */
} else if (dma_use_iova(dma->state)) {
dma_iova_destroy(attach->dev, dma->state, dma->size, dir,
- DMA_ATTR_MMIO);
+ dma->attrs);
} else {
struct scatterlist *sgl;
for_each_sgtable_dma_sg(sgt, sgl, i)
dma_unmap_phys(attach->dev, sg_dma_address(sgl),
- sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
+ sg_dma_len(sgl), dir, dma->attrs);
}
sg_free_table(sgt);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 69932d640b53..8f05f53b78fd 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -436,6 +436,7 @@ struct nvme_iod {
size_t total_len;
struct dma_iova_state dma_state;
+ unsigned long dma_attrs;
void *descriptors[NVME_MAX_NR_DESCRIPTORS];
struct nvme_dma_vec *dma_vecs;
unsigned int nr_dma_vecs;
@@ -443,6 +444,7 @@ struct nvme_iod {
dma_addr_t meta_dma;
size_t meta_total_len;
struct dma_iova_state meta_dma_state;
+ unsigned long meta_dma_attrs;
struct nvme_sgl_desc *meta_descriptor;
};
@@ -859,7 +861,7 @@ static void nvme_free_descriptors(struct request *req)
}
}
-static void nvme_free_prps(struct request *req, unsigned int attrs)
+static void nvme_free_prps(struct request *req, unsigned long attrs)
{
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
@@ -872,7 +874,7 @@ static void nvme_free_prps(struct request *req, unsigned int attrs)
}
static void nvme_free_sgls(struct request *req, struct nvme_sgl_desc *sge,
- struct nvme_sgl_desc *sg_list, unsigned int attrs)
+ struct nvme_sgl_desc *sg_list, unsigned long attrs)
{
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
enum dma_data_direction dir = rq_dma_dir(req);
@@ -899,7 +901,7 @@ static void nvme_unmap_metadata(struct request *req)
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct device *dma_dev = nvmeq->dev->dev;
struct nvme_sgl_desc *sge = iod->meta_descriptor;
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iod->flags & IOD_SINGLE_META_SEGMENT) {
dma_unmap_page(dma_dev, iod->meta_dma,
@@ -912,11 +914,11 @@ static void nvme_unmap_metadata(struct request *req)
map = PCI_P2PDMA_MAP_BUS_ADDR;
else if (iod->flags & IOD_META_MMIO) {
map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
- attrs |= DMA_ATTR_MMIO;
+ attrs = iod->meta_dma_attrs;
}
if (!blk_rq_dma_unmap(req, dma_dev, &iod->meta_dma_state,
- iod->meta_total_len, map)) {
+ iod->meta_total_len, map, attrs)) {
if (nvme_pci_cmd_use_meta_sgl(&iod->cmd))
nvme_free_sgls(req, sge, &sge[1], attrs);
else
@@ -935,7 +937,7 @@ static void nvme_unmap_data(struct request *req)
struct nvme_iod *iod = blk_mq_rq_to_pdu(req);
struct nvme_queue *nvmeq = req->mq_hctx->driver_data;
struct device *dma_dev = nvmeq->dev->dev;
- unsigned int attrs = 0;
+ unsigned long attrs = 0;
if (iod->flags & IOD_SINGLE_SEGMENT) {
static_assert(offsetof(union nvme_data_ptr, prp1) ==
@@ -949,11 +951,11 @@ static void nvme_unmap_data(struct request *req)
map = PCI_P2PDMA_MAP_BUS_ADDR;
else if (iod->flags & IOD_DATA_MMIO) {
map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
- attrs |= DMA_ATTR_MMIO;
+ attrs = iod->dma_attrs;
}
if (!blk_rq_dma_unmap(req, dma_dev, &iod->dma_state, iod->total_len,
- map)) {
+ map, attrs)) {
if (nvme_pci_cmd_use_sgl(&iod->cmd))
nvme_free_sgls(req, &iod->cmd.common.dptr.sgl,
iod->descriptors[0], attrs);
@@ -1270,6 +1272,7 @@ static blk_status_t nvme_map_data(struct request *req)
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
iod->flags |= IOD_DATA_MMIO;
+ iod->dma_attrs = iter.attrs;
break;
case PCI_P2PDMA_MAP_NONE:
break;
@@ -1305,6 +1308,7 @@ static blk_status_t nvme_pci_setup_meta_iter(struct request *req)
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
iod->flags |= IOD_META_MMIO;
+ iod->meta_dma_attrs = iter.attrs;
break;
case PCI_P2PDMA_MAP_NONE:
break;
@@ -1405,6 +1409,8 @@ static blk_status_t nvme_prep_rq(struct request *req)
iod->nr_descriptors = 0;
iod->total_len = 0;
iod->meta_total_len = 0;
+ iod->dma_attrs = 0;
+ iod->meta_dma_attrs = 0;
iod->nr_dma_vecs = 0;
ret = nvme_setup_cmd(req->q->queuedata, req);
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index b2d5266f8653..35ab3f8b53a8 100644
--- a/drivers/pci/p2pdma.c
+++ b/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;
+
p2p->mem[i].bus_offset =
pci_bus_address(pdev, i) - pci_resource_start(pdev, i);
}
diff --git a/include/linux/blk-mq-dma.h b/include/linux/blk-mq-dma.h
index 214c181ff2c9..fab72c0f8053 100644
--- a/include/linux/blk-mq-dma.h
+++ b/include/linux/blk-mq-dma.h
@@ -16,6 +16,7 @@ struct blk_dma_iter {
/* Output address range for this iteration */
dma_addr_t addr;
u32 len;
+ unsigned long attrs;
struct pci_p2pdma_map_state p2pdma;
/* Status code. Only valid when blk_rq_dma_map_iter_* returned false */
@@ -49,23 +50,19 @@ static inline bool blk_rq_dma_map_coalesce(struct dma_iova_state *state)
* @state: DMA IOVA state
* @mapped_len: number of bytes to unmap
* @map: peer-to-peer mapping type
+ * @attrs: DMA attributes used for the mapping
*
* Returns %false if the callers need to manually unmap every DMA segment
* mapped using @iter or %true if no work is left to be done.
*/
static inline bool blk_rq_dma_unmap(struct request *req, struct device *dma_dev,
struct dma_iova_state *state, size_t mapped_len,
- enum pci_p2pdma_map_type map)
+ enum pci_p2pdma_map_type map, unsigned long attrs)
{
if (map == PCI_P2PDMA_MAP_BUS_ADDR)
return true;
if (dma_use_iova(state)) {
- unsigned int attrs = 0;
-
- if (map == PCI_P2PDMA_MAP_THRU_HOST_BRIDGE)
- attrs |= DMA_ATTR_MMIO;
-
dma_iova_destroy(dma_dev, state, mapped_len, rq_dma_dir(req),
attrs);
return true;
diff --git a/include/linux/hmm-dma.h b/include/linux/hmm-dma.h
index f58b9fc71999..d9f967ed7ce1 100644
--- a/include/linux/hmm-dma.h
+++ b/include/linux/hmm-dma.h
@@ -14,12 +14,14 @@ struct pci_p2pdma_map_state;
* @state: DMA IOVA state
* @pfns: array of PFNs
* @dma_list: array of DMA addresses
+ * @dma_attrs: array of DMA attributes
* @dma_entry_size: size of each DMA entry in the array
*/
struct hmm_dma_map {
struct dma_iova_state state;
unsigned long *pfn_list;
dma_addr_t *dma_list;
+ unsigned long *dma_attrs;
size_t dma_entry_size;
};
diff --git a/include/linux/pci-p2pdma.h b/include/linux/pci-p2pdma.h
index 873de20a2247..402dc5e5d62b 100644
--- a/include/linux/pci-p2pdma.h
+++ b/include/linux/pci-p2pdma.h
@@ -21,10 +21,12 @@ struct scatterlist;
*
* A p2pdma provider is a range of MMIO address space available to the CPU.
* @owner: Device to which this provider belongs.
+ * @dma_mapping_flags: DMA attributes to use for host bridge mappings.
* @bus_offset: Bus offset for p2p communication.
*/
struct p2pdma_provider {
struct device *owner;
+ unsigned long dma_mapping_flags;
u64 bus_offset;
};
diff --git a/mm/hmm.c b/mm/hmm.c
index c72c9ddfdb2f..a871d8cb7dd9 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -720,6 +720,11 @@ int hmm_dma_map_alloc(struct device *dev, struct hmm_dma_map *map,
if (!map->pfn_list)
return -ENOMEM;
+ map->dma_attrs = kvcalloc(nr_entries, sizeof(*map->dma_attrs),
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!map->dma_attrs)
+ goto err_attrs;
+
use_iova = dma_iova_try_alloc(dev, &map->state, 0,
nr_entries * PAGE_SIZE);
if (!use_iova && dma_need_unmap(dev)) {
@@ -731,6 +736,8 @@ int hmm_dma_map_alloc(struct device *dev, struct hmm_dma_map *map,
return 0;
err_dma:
+ kvfree(map->dma_attrs);
+err_attrs:
kvfree(map->pfn_list);
return -ENOMEM;
}
@@ -749,6 +756,7 @@ void hmm_dma_map_free(struct device *dev, struct hmm_dma_map *map)
dma_iova_free(dev, &map->state);
kvfree(map->pfn_list);
kvfree(map->dma_list);
+ kvfree(map->dma_attrs);
}
EXPORT_SYMBOL_GPL(hmm_dma_map_free);
@@ -811,11 +819,12 @@ dma_addr_t hmm_dma_map_pfn(struct device *dev, struct hmm_dma_map *map,
case PCI_P2PDMA_MAP_NONE:
break;
case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
- attrs |= DMA_ATTR_MMIO;
+ attrs |= p2pdma_state->mem->dma_mapping_flags;
pfns[idx] |= HMM_PFN_P2PDMA;
break;
case PCI_P2PDMA_MAP_BUS_ADDR:
pfns[idx] |= HMM_PFN_P2PDMA_BUS | HMM_PFN_DMA_MAPPED;
+ map->dma_attrs[idx] = 0;
return pci_p2pdma_bus_addr_map(p2pdma_state->mem, paddr);
default:
return DMA_MAPPING_ERROR;
@@ -848,10 +857,12 @@ dma_addr_t hmm_dma_map_pfn(struct device *dev, struct hmm_dma_map *map,
if (dma_need_unmap(dev))
dma_addrs[idx] = dma_addr;
}
+ map->dma_attrs[idx] = attrs;
pfns[idx] |= HMM_PFN_DMA_MAPPED;
return dma_addr;
error:
pfns[idx] &= ~HMM_PFN_P2PDMA;
+ map->dma_attrs[idx] = 0;
return DMA_MAPPING_ERROR;
}
@@ -871,14 +882,11 @@ bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
struct dma_iova_state *state = &map->state;
dma_addr_t *dma_addrs = map->dma_list;
unsigned long *pfns = map->pfn_list;
- unsigned long attrs = DMA_ATTR_REQUIRE_COHERENT;
+ unsigned long attrs = DMA_ATTR_REQUIRE_COHERENT | map->dma_attrs[idx];
if ((pfns[idx] & valid_dma) != valid_dma)
return false;
- if (pfns[idx] & HMM_PFN_P2PDMA)
- attrs |= DMA_ATTR_MMIO;
-
if (pfns[idx] & HMM_PFN_P2PDMA_BUS)
; /* no need to unmap bus address P2P mappings */
else if (dma_use_iova(state))
@@ -890,6 +898,7 @@ bool hmm_dma_unmap_pfn(struct device *dev, struct hmm_dma_map *map, size_t idx)
pfns[idx] &=
~(HMM_PFN_DMA_MAPPED | HMM_PFN_P2PDMA | HMM_PFN_P2PDMA_BUS);
+ map->dma_attrs[idx] = 0;
return true;
}
EXPORT_SYMBOL_GPL(hmm_dma_unmap_pfn);
^ permalink raw reply related
* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Dave Hansen @ 2026-07-08 14:46 UTC (permalink / raw)
To: Edgecombe, Rick P, linux-kernel@vger.kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com, kas@kernel.org,
djbw@kernel.org, Fang, Peter, linux-coco@lists.linux.dev
In-Reply-To: <7d3eba7a5442cf3e84fe3658ae53337a183290b1.camel@intel.com>
On 7/6/26 14:44, Edgecombe, Rick P wrote:
>> Two alternative schemes were considered:
>>
>> 1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
>> However, this breaks naming consistency unless all existing stable
>> function macros are changed to TDH_XXX_V0.
>>
>> 2. Add an explicit 'version' parameter to the base seamcall() API. This
>> unnecessarily forces all stable SEAMCALL helpers to pass a
>> meaningless '0' argument. Additionally, the magic '0' or '1' values
>> at caller sites are not descriptive.
>>
> Dave was recently saying something to the effect of "make every word count". I
> think we could lose some filler words.
This can also just go below the --- so it doesn't make it to the final
changelog.
I like the discussion happening so far. I, too, dislike adding another
level of C wrappers. I'm looking forward to v2.
^ permalink raw reply
* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Jason Gunthorpe @ 2026-07-08 14:31 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: <6a4d95fcc92c2_2f05d5100f7@djbw-dev.notmuch>
On Tue, Jul 07, 2026 at 05:12:44PM -0700, Dan Williams (nvidia) wrote:
> 1/ We previously discussed a use case to operate a device's private MMIO
> while not allowing access to private memory (software encrypted NVME
> with private MMIO [1]). Many of the following comments are based on
> preserving this assumption so you can save some reading if we agree that
> use case can be abandoned.
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.
The use case I had in mind relied on the vIOMMU to police access to
the private memory. I think if you do T=1 and ADVERSARY you must have
a vIOMMU or it is an illogical configuration.
So, IMHO there is no use case for a T=1 device to only use shared
memory, I would abandon that combination.
> 2/ The confirmation of the trust level and the enabling of DMA are
> separated in time from setting the trust level and entering the RUN
> state.
>
> All the archs separate the RUN step from the ENABLE DMA step, and the
> implementation separates those steps in time.
Yes, until the device is fully accepted the DMA has to be kept off. If
you have to measure in the RUN state then we must have a second step
after measurement completes, however we are measuring in LOCKED right?
> echo tsm0 > $pdev/tsm/lock
> cat $nonce | device-evidence dump $pdev
> device-evidence validate $pdev $generation
So the device is in LOCKED here, DMA remains off both because of
LOCKED and becuase ENABLE DMA has not been done.
> 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)
With this API you can't set the devices to RUN and *then* collect the
evidence, is that restriction OK?
> echo full > $pdev/trust
> echo $pdev > $driver/bind
Then we can set the trust and bind the driver, knowing that the T=1
security property is in place.
> The configuration window where $pdev/trust and $pdev/accept can be
> dynamically changing should not be changing the force_dma_unencrypyted()
> result if only because that value will mismatch the hardware state until
> the next ->dma_configure() event.
T=x cannot be changed while a driver is bound, we need to block
that. Changing it will mess up all DMA and the DMA API won't work
anymore.
Thus force_dma_unencrypted() can only change when a driver is
unbound.
If a driver is unbound nothing ever reads force_dma_unencrypted() so
it is safe to change it, thus we can just synchronize T=1 and
force_dma_unencrypted() whenever T changes.
The error case has to wait for the driver to unbind if userspace wants
to go from ERROR->UNLOCKED. IMHO for RAS we are going to need a flow
to logically go from ERROR->RUN, probably with the 'same device
acceptance' I suggested earlier. In this case the DMAs will be halted,
the devices gets back to T=1 and RUN, then the DMAs are turned on
again. The driver remains in T=1 and force_dma_unencrypted() is always
false.
> 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.
> My answer, given the concerns of drivers dangerously flipping the
> force_dma_unencrypted() result at runtime was to place it in 'struct
> device_private'
Yes, this seems good idea in general
> Otherwise it is confusing when 2 devices are at FULL trust, but
> one has private memory access and the other does not. One is FULL the
> other is FULL+.
It is confusing when phrased like this, but this is also why I was
suggesting below that a pure trust level may not be sufficient as we
have quite a cross product of scenarios.
If the user wants to operate a device without T=1 and without
ADVERSARY then that's goofy, but technically the kernel doesn't really
need to care? It can do it.
IMHO a big question is how much do we want the kernel to nanny
userspace by having options to double check its work?
Either we trust the user to have checked all the policy conditions
before writing /sys/trust, or we encode some policy conditions and
tell the kernel so it can double-check prior to probe.
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.
> The question is whether these requirements belong on a central device
> trust mechanism bitmap or should be pushed out to other ABI.
Yes, for sure, I don't have a good sense what is better.
> For
> example, if the presence of the secure IOMMU is enumerated
> after the device is in the LOCKED state then userspace policy can know
> what it is getting into without needing to tell the kernel.
Yes, I think for almost all of this we could push it out to userspace
and have knobs all over. Like IOMMU already has a knob to switch to
DMA mode, we could add an IOMMU specific trust level to set ADVERSARY
also if we go in this direction.
But, I also do really like the idea of a central knob that was fairly
general and simple DISABLE/ADVERSARY/FULL which all sorts of things
can hang off. Including IOMMU and the drivers themselves that may
change operation in ADVERSARY mode.
That means they need simple general definitions for what they are, and
I really like this ADVERSARY naming vs trust as it makes it very clear
what the device disposition intention is.
> Not essential, but it is useful to have the "operate device" intent,
> IOMMU default domain configuration, trust level, and private DMA enable
> all in the same bus operation (->dma_configure). Which is why "accept"
> was demoted to just set RUN state and leave the rest to be finalized
> later.
I agree it is nice, and I think this can work fine, the bus code when
it does the dma configure step can reach out to the TSM and have it
enable DMA.
We also want the IOMMU to change to a default-disabled state, so we
also want its hook in dma configure to switch to non-default too.
It makes alot of sense to me that *all* DMA enable knobs will be
disabled until right before probe starts and then disabled again once
remove completes. TSM, IOMMU, PCI, etc.
Jason
^ permalink raw reply
* Re: [PATCH 02/15] tools: ynl: Teach pyynl to handle blobs
From: Donald Hunter @ 2026-07-08 13:48 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-3-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> The generic "device evidence" facility wants to convey large objects
> (certificate chains, measurements) and support various security commands
> relative to that evidence (like validate) [1].
>
> Update pyynl to understand the "bloblen" property indicates:
> * the associated attribute is a variable length byte array
> * it is multi-attr where each message with the same attribute represents
> chunks of the byte array
> * the byte array's size is conveyed in a scalar attribute named by the
> "bloblen" property.
> * the attribute identified by "bloblen" always arrives in a message before
> the first chunk of the associated blob
>
> A "multi-attr" without "bloblen" retains the legacy behavior where repeated
> reception of the same attribute across messages results in a new instance
> of the object. The new behavior causes repeated reception of an
> attribute to be treated as filling consecutive bytes of an array.
>
> Assisted-by: Codex:GPT-5
> [codex: drafted initial python changes]
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Link: http://lore.kernel.org/20260318170014.6650d2bf@kernel.org [1]
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> tools/net/ynl/pyynl/lib/nlspec.py | 12 ++++++
> tools/net/ynl/pyynl/lib/ynl.py | 62 ++++++++++++++++++++++++++++++-
> 2 files changed, 72 insertions(+), 2 deletions(-)
I am mid-review of this but I think it could be simplified. See initial
thoughts below. Can you show me how to configure a test env so I can try
it out?
>
> diff --git a/tools/net/ynl/pyynl/lib/nlspec.py b/tools/net/ynl/pyynl/lib/nlspec.py
> index 0469a0e270d0..4764263968ea 100644
> --- a/tools/net/ynl/pyynl/lib/nlspec.py
> +++ b/tools/net/ynl/pyynl/lib/nlspec.py
> @@ -172,6 +172,7 @@ class SpecAttr(SpecElement):
> selector string, name of attribute used to select
> sub-message type
>
> + bloblen string, name of attr which reports the blob length
> is_auto_scalar bool, attr is a variable-size scalar
> """
> def __init__(self, family, attr_set, yaml, value):
> @@ -181,6 +182,7 @@ class SpecAttr(SpecElement):
> self.value = value
> self.attr_set = attr_set
> self.is_multi = yaml.get('multi-attr', False)
> + self.bloblen = yaml.get('bloblen')
> self.struct_name = yaml.get('struct')
> self.sub_type = yaml.get('sub-type')
> self.byte_order = yaml.get('byte-order')
> @@ -191,6 +193,10 @@ class SpecAttr(SpecElement):
>
> self.is_auto_scalar = self.type in ("sint", "uint")
>
> + if self.bloblen and not self.is_multi:
> + raise SpecException(
> + f"Attribute '{attr_set.name}.{self.name}' has bloblen without multi-attr")
> +
>
> class SpecAttrSet(SpecElement):
> """ Netlink Attribute Set class.
> @@ -355,6 +361,8 @@ class SpecOperation(SpecElement):
> attr_set attribute set name
> fixed_header string, optional name of fixed header struct
>
> + dump_blob_attrs set, reply blob attribute names which may span messages
> +
> yaml raw spec as loaded from the spec file
> """
> def __init__(self, family, yaml, req_value, rsp_value):
> @@ -369,6 +377,7 @@ class SpecOperation(SpecElement):
> self.is_async = 'notify' in yaml or 'event' in yaml
> self.is_resv = not self.is_async and not self.is_call
> self.fixed_header = self.yaml.get('fixed-header', family.fixed_header)
> + self.dump_blob_attrs = frozenset()
Initialising to None would avoid some other logic.
>
> # Added by resolve:
> self.attr_set = None
> @@ -388,6 +397,9 @@ class SpecOperation(SpecElement):
> raise SpecException(f"Can't resolve attribute set for op '{self.name}'")
> if attr_set_name:
> self.attr_set = self.family.attr_sets[attr_set_name]
> + reply_attrs = self.yaml.get('dump', {}).get('reply', {}).get('attributes', ())
> + self.dump_blob_attrs = frozenset(
> + name for name in reply_attrs if self.attr_set[name].bloblen)
>
>
> class SpecMcastGroup(SpecElement):
> diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
> index 092d132edec1..f21b68043bd3 100644
> --- a/tools/net/ynl/pyynl/lib/ynl.py
> +++ b/tools/net/ynl/pyynl/lib/ynl.py
> @@ -998,6 +998,47 @@ class YnlFamily(SpecFamily):
> else:
> rsp[name] = [decoded]
>
> + def _blob_fill(self, buf, off, chunks, name):
> + for chunk in chunks:
> + end = off + len(chunk)
> + if end > len(buf):
> + raise YnlException(f"Blob '{name}' data exceeds reported length")
> + buf[off:end] = chunk
> + off = end
> + return off
> +
> + def _blob_init(self, op, rsp, blob_attrs):
> + """
> + Start a new blob object. Preallocate each blob payload using the
> + length carried ahead of the data and copy in the first chunk(s).
> + Return the per-attribute write offsets for any following messages.
> + """
> + offsets = {}
> + for name in blob_attrs:
> + if name not in rsp:
> + continue
> + length = rsp.get(op.attr_set[name].bloblen, 0)
> + buf = bytearray(length)
> + offsets[name] = self._blob_fill(buf, 0, rsp[name], name)
> + rsp[name] = buf
> + return offsets
> +
> + def _blob_extend(self, rsp, update, blob_attrs, offsets):
> + """
> + Continue filling the previous object's blob(s) when the current
> + message carries only blob continuation attributes.
> + """
> + if any(name in rsp and name not in blob_attrs for name in update):
> + return False
> +
> + for name, value in update.items():
> + if name in blob_attrs and name in rsp:
> + offsets[name] = self._blob_fill(rsp[name], offsets.get(name, 0),
> + value, name)
> + else:
> + rsp[name] = value
> + return True
> +
> def _resolve_selector(self, attr_spec, search_attrs):
> sub_msg = attr_spec.sub_message
> if sub_msg not in self.sub_msgs:
> @@ -1356,7 +1397,10 @@ class YnlFamily(SpecFamily):
> for (method, vals, flags) in ops:
> op = self.ops[method]
> msg = self._encode_message(op, vals, flags, req_seq)
> - reqs_by_seq[req_seq] = (op, vals, msg, flags)
> + blob_attrs = None
> + if Netlink.NLM_F_DUMP in flags:
> + blob_attrs = op.dump_blob_attrs or None
> + reqs_by_seq[req_seq] = (op, vals, msg, flags, blob_attrs)
I think this hunk can be dropped and instead use op.dump_blob_attrs directly
> payload += msg
> req_seq += 1
>
> @@ -1365,6 +1409,7 @@ class YnlFamily(SpecFamily):
> done = False
> rsp = []
> op_rsp = []
> + blob_off = {}
> while not done:
> reply, ancdata = self._recvmsg()
> nsid = self._decode_nsid(ancdata)
> @@ -1372,13 +1417,14 @@ class YnlFamily(SpecFamily):
> self._recv_dbg_print(reply, nms)
> for nl_msg in nms:
> if nl_msg.nl_seq in reqs_by_seq:
> - (op, vals, req_msg, req_flags) = reqs_by_seq[nl_msg.nl_seq]
> + (op, vals, req_msg, req_flags, blob_attrs) = reqs_by_seq[nl_msg.nl_seq]
> if nl_msg.extack:
> nl_msg.annotate_extack(op.attr_set)
> self._decode_extack(req_msg, op, nl_msg.extack, vals)
> else:
> op = None
> req_flags = []
> + blob_attrs = None
>
> if nl_msg.error:
> raise NlError(nl_msg)
> @@ -1388,6 +1434,11 @@ class YnlFamily(SpecFamily):
> print(nl_msg)
>
> if Netlink.NLM_F_DUMP in req_flags:
> + if blob_attrs:
> + for obj in op_rsp:
> + for name in blob_attrs:
> + if isinstance(obj.get(name), bytearray):
> + obj[name] = bytes(obj[name])
> rsp.append(op_rsp)
> elif not op_rsp:
> rsp.append(None)
> @@ -1414,6 +1465,13 @@ class YnlFamily(SpecFamily):
> rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name)
> if op.fixed_header:
> rsp_msg.update(self._decode_struct(decoded.raw, op.fixed_header))
> +
> + if blob_attrs:
> + if op_rsp and self._blob_extend(op_rsp[-1], rsp_msg,
> + blob_attrs, blob_off):
> + continue
> + blob_off = self._blob_init(op, rsp_msg, blob_attrs)
> +
This approach seems unnecessary for pyynl. Just let multi-attr do its
thing and then concatenate them at the end.
> op_rsp.append(rsp_msg)
>
> return rsp
^ permalink raw reply
* Re: [PATCH 01/15] netlink: specs: Introduce multi-message blobs for SPDM
From: Donald Hunter @ 2026-07-08 13:23 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-2-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> diff --git a/Documentation/netlink/genetlink.yaml b/Documentation/netlink/genetlink.yaml
> index d3f3f3399ddf..4caee7d55303 100644
> --- a/Documentation/netlink/genetlink.yaml
> +++ b/Documentation/netlink/genetlink.yaml
> @@ -153,6 +153,13 @@ properties:
> enum: [ little-endian, big-endian ]
> multi-attr:
> type: boolean
> + bloblen:
> + description: |
> + Marks this attribute as a variable-length blob that may be split across
> + multiple messages. The value names a separate scalar attribute that carries
> + the total blob length, and which is sent ahead of the blob data so a consumer
> + can preallocate the full payload.
> + type: string
> nested-attributes:
> description: Name of the space (sub-space) used inside the attribute.
> type: string
Can you add a schema constraint so that bloblen allowed only when
multi-attr == true and type == binary
^ permalink raw reply
* Re: [PATCH 04/15] device core: Introduce "device evidence" over netlink
From: Donald Hunter @ 2026-07-08 13:22 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski, Bjorn Helgaas, Xu Yilun,
Aneesh Kumar K.V (Arm), Alexey Kardashevskiy
In-Reply-To: <20260705220819.2472765-5-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> Multiple device security mechanisms are built on top of the SPDM protocol
> from the DMTF. The protocol arranges for devices to verify their identity
> and establish secure sessions between requesters and responders. In turn,
> device security protocols like PCIe IDE use SPDM sessions to program link
> encryption keys, and protocols like PCIe TDISP use exclusive secure
> sessions for managing the device's security state.
>
> All of these uses share a need to retrieve the device's installed
> certificates, its measurements, and other protocol specific objects like
> interface reports. Unify all of those object retrieval and regeneration
> cases into a device-scoped ABI that buses can leverage.
>
> When PCI CMA was presented at Linux Plumbers the consensus reaction was
> that it was not suitable to be driven via sysfs. Given the varied use
> cases of the same fundamental objects the consensus converged on
> netlink. With the new 'blob' extensions to netlink, it can support up to
> the 16MB signed transcripts that SPDM generates.
>
> A bus opts in to evidence gathering via device_evidence_register() and
> passing device_evidence_ops. The ops resolve a device handle to its
> corresponding 'struct device_evidence' context.
>
> See inline documentation in
> Documentation/netlink/specs/device-evidence.yaml for the object types and
> options. This includes objects like TDISP interface reports and options
> like digests instead of full blobs.
Can you include sample pyynl output; it helps immensely during review.
>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
You probably need to cc the netdev list so that the series gets picked
up by patchwork.
> ---
> drivers/base/Kconfig | 4 +
> drivers/base/Makefile | 1 +
> .../netlink/specs/device-evidence.yaml | 186 +++++++
> drivers/base/device-evidence-netlink.h | 23 +
> include/linux/device/evidence.h | 85 ++++
> include/uapi/linux/device-evidence.h | 110 +++++
> drivers/base/device-evidence-netlink.c | 44 ++
> drivers/base/evidence.c | 453 ++++++++++++++++++
> MAINTAINERS | 4 +
> 9 files changed, 910 insertions(+)
> create mode 100644 Documentation/netlink/specs/device-evidence.yaml
> create mode 100644 drivers/base/device-evidence-netlink.h
> create mode 100644 include/linux/device/evidence.h
> create mode 100644 include/uapi/linux/device-evidence.h
> create mode 100644 drivers/base/device-evidence-netlink.c
> create mode 100644 drivers/base/evidence.c
>
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index f7d385cbd3ba..e3929fe6b240 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -190,6 +190,10 @@ config HMEM_REPORTING
> Enable reporting for heterogeneous memory access attributes under
> their non-uniform memory nodes.
>
> +config DEVICE_EVIDENCE
> + bool
> + depends on NET
> +
> source "drivers/base/test/Kconfig"
>
> config SYS_HYPERVISOR
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 8074a10183dc..02bdc4f74019 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_DEV_COREDUMP) += devcoredump.o
> obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> +obj-$(CONFIG_DEVICE_EVIDENCE) += evidence.o device-evidence-netlink.o
> obj-$(CONFIG_ACPI) += physical_location.o
>
> obj-y += test/
> diff --git a/Documentation/netlink/specs/device-evidence.yaml b/Documentation/netlink/specs/device-evidence.yaml
> new file mode 100644
> index 000000000000..44b278bc20e1
> --- /dev/null
> +++ b/Documentation/netlink/specs/device-evidence.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
> +#
> +---
> +name: device-evidence
The naming seems to be too narrow - if the scope expands at all then the
name would need to change. A subsystem name like spdm, tsm or tsm-device
seems better?
> +protocol: genetlink
> +uapi-header: linux/device-evidence.h
> +doc: Device evidence retrieval over generic netlink
> +
> +definitions:
> + -
> + type: const
> + name: max-object-size
> + value: 0x01000000
> + -
> + type: const
> + name: max-nonce-size
> + value: 32
> + -
> + name: type
> + type: enum
> + doc: Device security evidence objects
> + render-max: true
> + entries:
> + -
> + name: cert0
> + doc: SPDM certificate chain from device slot0
> + -
> + name: cert1
> + doc: SPDM certificate chain from device slot1
> + -
> + name: cert2
> + doc: SPDM certificate chain from device slot2
> + -
> + name: cert3
> + doc: SPDM certificate chain from device slot3
> + -
> + name: cert4
> + doc: SPDM certificate chain from device slot4
> + -
> + name: cert5
> + doc: SPDM certificate chain from device slot5
> + -
> + name: cert6
> + doc: SPDM certificate chain from device slot6
> + -
> + name: cert7
> + doc: SPDM certificate chain from device slot7
> + -
> + name: vca
> + doc: |
> + SPDM version, capabilities, and algorithms transcript
> + negotiated at session establishment. An implementation may not
> + provide this separately and instead include it in the
> + measurements transcript.
> + -
> + name: measurements
> + doc: SPDM GET_MEASUREMENTS response
> + -
> + name: report
> + doc: |
> + A bus that implements a device interface security protocol
> + like TDISP may convey an interface report that details
> + interface settings and capabilities.
> + -
> + name: type-flag
> + type: flags
> + doc: Device security evidence request flags
> + render-max: true
> + # NOTE! these values must match the type enum name and order
> + entries:
> + -
> + name: cert0
> + -
> + name: cert1
> + -
> + name: cert2
> + -
> + name: cert3
> + -
> + name: cert4
> + -
> + name: cert5
> + -
> + name: cert6
> + -
> + name: cert7
> + -
> + name: vca
> + -
> + name: measurements
> + -
> + name: report
> + -
> + name: flag
> + type: flags
> + render-max: true
> + doc: Flags to control evidence retrieval
> + entries:
> + -
> + name: digest
> + doc: |
> + Request a secure hash of objects like vca and measurements.
> + The expectation is that this digest is produced by a responder
> + within the TCB like a platform TSM. It validates a blob that
> + may have traversed a transport without integrity protections.
> +
> +attribute-sets:
> + -
> + name: object
> + attributes:
> + -
> + name: type
> + type: u32
Is this an instance of the type enum? If so then:
enum: type
> + doc: evidence type-id
> + -
> + name: type-mask
> + type: u32
Likewise:
enum: type-flag
> + doc: evidence types as a flag mask
> + -
> + name: flags
> + type: u32
And:
enum: flags
> + doc: evidence modifier flags like 'request object digest'
> + -
> + name: subsys
> + type: string
> + doc: device subsystem name (e.g. bus or class name)
> + -
> + name: dev-name
> + type: string
> + doc: device name
> + -
> + name: nonce
> + type: binary
> + checks:
> + max-len: max-nonce-size
> + -
> + name: generation
> + type: u32
> + doc: detect local threads racing evidence refresh
> + -
> + name: count
> + type: u64
> + doc: TSM, if present, observed count of evidence refresh events
> + -
> + name: length
> + type: u32
> + checks:
> + max: max-object-size
> + doc: |
> + Final size of 'val' blob after all messages received.
> + -
> + name: val
Any reason for the abbreviated name?
> + type: binary
> + multi-attr: true
> + bloblen: length
> + doc: |
> + Ordered evidence object value chunk. Large evidence objects
> + may be split over several dump reply messages up to 'length'.
> + 'length' is transmitted before the first chunk.
> +
> +operations:
> + list:
> + -
> + name: read
> + doc: |
> + Read device evidence objects of a given type mask. The dump reply
> + reports the total value length before val attributes fill the evidence
> + object, possibly across multiple messages.
> + attribute-set: object
> + flags: [admin-perm]
> + dump:
> + pre: device-evidence-nl-read-pre
> + post: device-evidence-nl-read-post
> + request:
> + attributes:
> + - type-mask
> + - flags
> + - subsys
> + - dev-name
> + - nonce
> + reply:
> + attributes:
> + - type
> + - generation
> + - length
> + - val
Is the intention that the dump filter must narrow to a single device? Is
the user expected to enumerate the devices in subsystem then read device
evidence for each in turn?
^ permalink raw reply
* Re: [PATCH 01/15] netlink: specs: Introduce multi-message blobs for SPDM
From: Donald Hunter @ 2026-07-08 11:13 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, driver-core, ankita, Alistair Francis,
Lukas Wunner, Jakub Kicinski
In-Reply-To: <20260705220819.2472765-2-djbw@kernel.org>
Dan Williams <djbw@kernel.org> writes:
> The SPDM, Security Protocol and Data Model, underpins PCI device security
> and other use cases. It defines objects that allow for verification of
> device identity and configuration. These objects can be large in size 16MB.
> Netlink is otherwise suitable to define the operations, with optional
> parameters, and notifications for working with these objects. For example,
> operations like "regenerate evidence with nonce", "mark evidence
> validated", and "broadcast evidence / security state change events".
>
> A netlink 'blob' is introduced as a way to teach YNL that one instance of a
> attribute may span multiple messages. It enables netlink to convey all the
> data needed for verification and manipulation of SPDM transported evidence.
>
> The schema change to allows YNL to infer that an attribute may span
change to allows -> change allows
> multiple messages and interrogate its length to preallocate an
> appropriately sized receive buffer.
>
> The design direction to extend the netlink schema for a "multi-message
> object receive" case was the result of this discussion [1].
>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Donald Hunter <donald.hunter@gmail.com>
> Link: http://lore.kernel.org/20260318170014.6650d2bf@kernel.org [1]
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> Documentation/netlink/genetlink-legacy.yaml | 6 ++++++
> Documentation/netlink/genetlink.yaml | 7 +++++++
> Documentation/netlink/netlink-raw.yaml | 7 +++++++
I suggest you drop the changes to genetlink-legacy and netlink-raw
because I don't think we want to support the blob functionality for
legacy families.
Can you also update Documentation/userspace-api/netlink/specs.rst to
describe the multi-attr / bloblen API behaviour.
^ permalink raw reply
* Re: [PATCH v7 16/22] dma-direct: make dma_direct_map_phys() honor DMA_ATTR_CC_SHARED
From: Suzuki K Poulose @ 2026-07-08 12:11 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco
Cc: Robin Murphy, Marek Szyprowski, Will Deacon, Marc Zyngier,
Steven Price, 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, Jiri Pirko,
Michael Kelley
In-Reply-To: <20260701054926.825925-17-aneesh.kumar@kernel.org>
On 01/07/2026 06:49, Aneesh Kumar K.V (Arm) wrote:
> Teach dma_direct_map_phys() to select the DMA address encoding based on
> DMA_ATTR_CC_SHARED.
>
> Use phys_to_dma_unencrypted() for decrypted mappings and
> phys_to_dma_encrypted() otherwise. If a device requires unencrypted DMA
> but the source physical address is still encrypted, force the mapping
> through swiotlb so the DMA address and backing memory attributes remain
> consistent.
>
> Update the arm64, x86, s390 and powerpc secure-guest setup to not use
> swiotlb force option
>
> Tested-by: Jiri Pirko <jiri@nvidia.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Mostafa Saleh <smostafa@google.com>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> Changes from v3:
> * Handle DMA_ATTR_MMIO
> ---
> arch/arm64/mm/init.c | 4 +--
> arch/powerpc/platforms/pseries/svm.c | 2 +-
> arch/s390/mm/init.c | 2 +-
> arch/x86/kernel/pci-dma.c | 4 +--
> kernel/dma/direct.c | 4 ++-
> kernel/dma/direct.h | 45 +++++++++++++++-------------
> 6 files changed, 31 insertions(+), 30 deletions(-)
>
> 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;
> - }
>
> if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) && !swiotlb) {
> /*
For arm64 CCA bits:
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
^ 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-08 11:35 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
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: <20260701054926.825925-17-aneesh.kumar@kernel.org>
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.
In the other case, for an arm CCA guest, if the MMIO is shared we end up
setting the shared attribute but that's fine, it's only an IPA address.
--
Catalin
^ permalink raw reply
* Re: [PATCH 14/15] PCI/TSM: Create MMIO descriptors via TDISP Report
From: Alexey Kardashevskiy @ 2026-07-08 9:49 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Arnd Bergmann, Xu Yilun
In-Reply-To: <20260705220819.2472765-15-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> After pci_tsm_bind() and pci_tsm_lock() the low level TSM driver is
> expected to populate PCI_TSM_EVIDENCE_TYPE_REPORT in its evidence store.
> This report is defined by the TDISP GET_DEVICE_INTERFACE_REPORT response
> payload.
>
> Add a helper to create encrypted MMIO descriptors from that report
> data. With those descriptors the TSM driver can use pci_tsm_mmio_setup() to
> inform ioremap() how to map the device per the device's expectations. The
> VM is expected to validate the interface with the relying party before
> accepting the device for operation.
>
> The helper also provides the obfuscated starting address for each encrypted
> MMIO range as the VM is never disclosed on the hpa that correlates to the
> gpa of the device's mmio. The obfuscated address is BAR aligned.
>
> Based on an original patch by Aneesh [1]
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@kernel.org/ [1]
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> include/linux/ioport.h | 2 +
> include/linux/pci-tsm.h | 35 +++++++
> drivers/pci/tsm/core.c | 224 ++++++++++++++++++++++++++++++++++++++++
> kernel/resource.c | 8 ++
> 4 files changed, 269 insertions(+)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index f7930b3dfd0a..122f1eefb4b9 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -144,6 +144,7 @@ enum {
> IORES_DESC_RESERVED = 7,
> IORES_DESC_SOFT_RESERVED = 8,
> IORES_DESC_CXL = 9,
> + IORES_DESC_ENCRYPTED = 10,
> };
>
> /*
> @@ -240,6 +241,7 @@ struct resource_constraint {
> extern struct resource ioport_resource;
> extern struct resource iomem_resource;
> extern struct resource soft_reserve_resource;
> +extern struct resource encrypted_iomem_resource;
>
> extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
> extern int request_resource(struct resource *root, struct resource *new);
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index 397e5d8459cb..6d5fadd79360 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -156,12 +156,42 @@ struct pci_tsm_pf0 {
> struct pci_doe_mb *doe_mb;
> };
>
> +/**
> + * struct pci_tsm_mmio_entry - an encrypted MMIO range
> + * @res: MMIO address range (typically Guest Physical Address, GPA)
> + * @tsm_offset: Host Physical Address, HPA obfuscation offset added by the TSM.
> + * Translates report addresses to GPA.
> + */
> +struct pci_tsm_mmio_entry {
> + struct resource res;
> + u64 tsm_offset;
> +};
> +
> +struct pci_tsm_mmio {
> + int nr;
> + struct pci_tsm_mmio_entry mmio[];
> +};
> +
> +static inline struct pci_tsm_mmio_entry *
> +pci_tsm_mmio_entry(struct pci_tsm_mmio *mmio, int idx)
> +{
> + return &mmio->mmio[idx];
> +}
> +
> +static inline struct resource *pci_tsm_mmio_resource(struct pci_tsm_mmio *mmio,
> + int idx)
> +{
> + return &mmio->mmio[idx].res;
> +}
> +
> /**
> * struct pci_tsm_devsec - context for tracking private/accepted PCI resources
> * @base_tsm: generic core "tsm" context
> + * @mmio: encrypted MMIO resources for this assigned device
> */
> struct pci_tsm_devsec {
> struct pci_tsm base_tsm;
> + struct pci_tsm_mmio *mmio;
> };
>
> /* physical function0 and capable of 'connect' */
> @@ -263,6 +293,11 @@ static inline const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> return tsm->tsm_dev->pci_ops;
> }
> struct pci_tsm_devsec *to_pci_tsm_devsec(struct pci_tsm *tsm);
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio);
> +
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev);
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio);
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> {
> diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
> index 372363a39a44..9ac216ad896d 100644
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c
> @@ -15,6 +15,7 @@
> #include <linux/pci-tsm.h>
> #include <linux/sysfs.h>
> #include <linux/tsm.h>
> +#include <linux/unaligned.h>
> #include <linux/xarray.h>
> #include "../pci.h"
>
> @@ -552,6 +553,229 @@ static ssize_t dsm_show(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RO(dsm);
>
> +static void mmio_teardown(struct pci_tsm_mmio *mmio, int nr)
> +{
> + while (nr--)
> + remove_resource(pci_tsm_mmio_resource(mmio, nr));
> +}
> +
> +/**
> + * pci_tsm_mmio_setup() - mark device MMIO as encrypted in iomem
> + * @pdev: device owner of MMIO resources
> + * @mmio: container of an array of resources to mark encrypted
> + */
> +int pci_tsm_mmio_setup(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + int i;
> +
> + device_lock_assert(&pdev->dev);
> + if (pdev->dev.driver)
> + return -EBUSY;
> +
> + for (i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> + int j;
> +
> + if (resource_size(res) == 0 || !res->end)
> + break;
> +
> + /* Only require the caller to set the range, init remainder */
> + *res = DEFINE_RES_NAMED_DESC(res->start, resource_size(res),
> + pci_name(pdev), IORESOURCE_MEM,
> + IORES_DESC_ENCRYPTED);
> +
> + for (j = 0; j < PCI_NUM_RESOURCES; j++)
> + if (resource_contains(pci_resource_n(pdev, j), res))
> + break;
> +
> + /* Request is outside of device MMIO */
> + if (j >= PCI_NUM_RESOURCES)
> + break;
> +
> + if (insert_resource(&encrypted_iomem_resource, res) != 0)
> + break;
> + }
> +
> + if (i >= mmio->nr)
> + return 0;
> +
> + mmio_teardown(mmio, i);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_setup);
> +
> +void pci_tsm_mmio_teardown(struct pci_tsm_mmio *mmio)
> +{
> + mmio_teardown(mmio, mmio->nr);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_teardown);
> +
> +/*
> + * PCIe ECN TEE Device Interface Security Protocol (TDISP)
> + *
> + * Device Interface Report data object layout as defined by PCIe r7.0 section
> + * 11.3.11
> + */
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_TABLE BIT(0)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_MSIX_PBA BIT(1)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE BIT(2)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
> +#define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
> +
> +/* An interface report 'pfn' is 4K in size */
> +struct pci_tsm_devif_mmio {
> + __le64 phys;
> + __le32 nr_pfns;
> + __le32 attributes;
> +};
> +
> +struct pci_tsm_devif_report {
> + __le16 interface_info;
> + __le16 reserved;
> + __le16 msi_x_message_control;
> + __le16 lnr_control;
> + __le32 tph_control;
> + __le32 mmio_range_count;
> + struct pci_tsm_devif_mmio mmio[];
> +};
> +
> +/**
> + * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
> + * @pdev: device owner of MMIO ranges
> + *
> + * Return: the encrypted MMIO range descriptor on success, NULL on failure
> + *
> + * Assumes that this is called within the live lifetime of a PCI device's
> + * association with a low level TSM.
> + */
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +{
> + struct device_evidence *evidence = pdev->tsm->evidence;
> + u64 reporting_bar_base, last_reporting_end;
> + struct device_evidence_object *report_obj;
> + const struct pci_tsm_devif_report *report;
> + u32 mmio_range_count;
> + int last_bar = -1;
> + int i;
> +
> + if (!evidence)
> + return NULL;
> +
> + report_obj = &evidence->obj[DEVICE_EVIDENCE_TYPE_REPORT];
> +
> + guard(rwsem_read)(&evidence->lock);
> + if (report_obj->len < sizeof(struct pci_tsm_devif_report))
> + return NULL;
> +
> + report = report_obj->data;
> + mmio_range_count = __le32_to_cpu(report->mmio_range_count);
> +
> + /* check that the report object is self-consistent on mmio entries */
> + if (report_obj->len < struct_size(report, mmio, mmio_range_count))
> + return NULL;
> +
> + /* create pci_tsm_mmio descriptors from the report data */
> + struct pci_tsm_mmio *mmio __free(kfree) =
> + kzalloc_flex(*mmio, mmio, mmio_range_count);
> + if (!mmio)
> + return NULL;
> +
> + for (i = 0; i < mmio_range_count; i++) {
> + u64 range_off;
> + struct range range;
> + const struct pci_tsm_devif_mmio *mmio_data = &report->mmio[i];
> + struct pci_tsm_mmio_entry *entry =
> + pci_tsm_mmio_entry(mmio, mmio->nr);
> + u64 tsm_offset = __le64_to_cpu(mmio_data->phys);
* SZ_4K is missing.
> + u64 size = __le32_to_cpu(mmio_data->nr_pfns) * SZ_4K;
> + u32 attr = __le32_to_cpu(mmio_data->attributes);
> + int bar = FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID,
> + attr);
> +
> + if (bar >= PCI_STD_NUM_BARS ||
> + !(pci_resource_flags(pdev, bar) & IORESOURCE_MEM) ||
> + (pci_resource_flags(pdev, bar) & IORESOURCE_UNSET)) {
> + pci_dbg(pdev, "Invalid reporting bar ID %d\n", bar);
> + return NULL;
> + }
> +
> + if (last_bar > bar) {
> + pci_dbg(pdev, "Reporting bar ID not in ascending order\n");
> + return NULL;
> + }
> +
> + if (last_bar < bar) {
> + unsigned long mask = pci_resource_len(pdev, bar) - 1;
> +
> + /* Transition to a new bar */
> + last_bar = bar;
> +
> + /*
> + * Determine the obfuscated base of the BAR. BAR
> + * offsets are never obfuscated.
> + */
> + reporting_bar_base = tsm_offset & ~mask;
> + } else if (tsm_offset < last_reporting_end) {
> + pci_dbg(pdev, "Reporting ranges within BAR not in ascending order\n");
> + return NULL;
> + }
> +
> + /* Per spec the tsm_offset never results in overflow / underflow */
> + last_reporting_end = tsm_offset + size;
> + if (last_reporting_end < tsm_offset) {
> + pci_dbg(pdev, "Reporting range overflow\n");
> + return NULL;
> + }
> +
> + range_off = tsm_offset - reporting_bar_base;
If we all do the mmio_reporting_offset in the same way (use the bar size low bits), then this can be simplified and should be as it does not account for the non-locked (==not reported) MSIX anyway. Thanks,
> + if (pci_resource_len(pdev, bar) < range_off + size) {
> + pci_dbg(pdev, "Reporting range larger than BAR size\n");
> + return NULL;
> + }
> +
> + range.start = pci_resource_start(pdev, bar) + range_off;
> + range.end = range.start + size - 1;
> +
> + /* Only record the TEE ranges for later consideration by ioremap() */
> + if (FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_NON_TEE,
> + attr)) {
> + pci_dbg(pdev, "Skipping non-TEE range, BAR%d %pra\n",
> + bar, &range);
> + continue;
> + }
> +
> + entry->res.start = range.start;
> + entry->res.end = range.end;
> + entry->tsm_offset = tsm_offset;
> + mmio->nr++;
> + }
> +
> + return_ptr(mmio);
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_alloc);
> +
> +/**
> + * pci_tsm_mmio_free() - free a pci_tsm_mmio instance
> + * @pdev: device owner of MMIO ranges
> + * @mmio: instance to free
> + *
> + * Returns 0 if @mmio was idle on entry, -EBUSY otherwise
> + */
> +int pci_tsm_mmio_free(struct pci_dev *pdev, struct pci_tsm_mmio *mmio)
> +{
> + for (int i = 0; i < mmio->nr; i++) {
> + struct resource *res = pci_tsm_mmio_resource(mmio, i);
> +
> + if (dev_WARN_ONCE(&pdev->dev, resource_assigned(res),
> + "MMIO resource still assigned %pr\n", res))
> + return -EBUSY;
> + }
> + kfree(mmio);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_mmio_free);
> +
> /**
> * pci_tsm_accept() - accept a device for private MMIO operation
> * @pdev: PCI device to accept
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 3d17e3196a3e..ba91ce43ff7a 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -56,6 +56,14 @@ struct resource soft_reserve_resource = {
> .flags = IORESOURCE_MEM,
> };
>
> +struct resource encrypted_iomem_resource = {
> + .name = "Encrypted MMIO",
> + .start = 0,
> + .end = -1,
> + .desc = IORES_DESC_ENCRYPTED,
> + .flags = IORESOURCE_MEM,
> +};
> +
> static DEFINE_RWLOCK(resource_lock);
>
> /*
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Peter Fang @ 2026-07-08 9:25 UTC (permalink / raw)
To: Dave Hansen
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, seanjc
In-Reply-To: <e06806e8-7334-4533-b1a3-e1efbe3fab8a@intel.com>
On Wed, Jul 01, 2026 at 12:56:19PM -0700, Dave Hansen wrote:
> On 6/18/26 01:13, Xu Yilun wrote:
> > For simplicity, let all guests share a global buffer. Build the buffer's
> > HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> > va-to-pa conversions at runtime.
>
> va-to-pa conversions are not expensive. Even for vmalloc() memory.If
>
> I don't like the global buffer. It just generally seems like a bad idea.
> It needs locking, it wastes memory when nobody is doing quotes, etc..
>
> Is there a way to do this without vmalloc()?
>
> Could userspace provide the memory and then the kernel just does a gup
> to keep it in place while the TDX module is writing to it? Or, could it
> just be allocated:
Hi Dave,
The GUP idea makes a lot of sense at a high level I think. However, the
shared buffer used by the GHCI call is not 4K-aligned because it has a
header in front of it, while the SEAMCALL expects 4K-aligned HPAs
(lesson learned...). This memory does come from userspace though:
struct tdx_quote_req { <-- 4K-aligned
u64 version;
u64 status;
u32 in_len;
u32 out_len;
u8 data[]; <-- shared buffer
};
So perhaps the simplest thing here is to just allocate a kernel buffer
before the SEAMCALL. There's also another subtle wrinkle here: KVM is
shifting from a FOLL_PIN-based API to an MMU-notifier-based API for
guest page accesses [1], so I think that means more complex code to talk
to the TDX module.
>
> void *buffer = vmalloc(size);
> mutex_lock();
> // talk to TDX module
> mutex_unlock();
> vfree(buffer);
>
> return ret;
>
> That actually has some nice properties. It doesn't require locking for
> the buffer. It also penalizes folks doing lots of quotes. If you hammer
> on quoting, you hammer on the memory allocator and TLB flushing code and
> slow yourself down more. If you hammer on it from multiple threads, it
> gets even worse.
>
> That seems like a feature not a bug.
This is a really good point... I'm a bit worried about the global impact
of vfree() though. IIUC it eventually triggers an all-CPU TLB shootdown,
so a guest abusing this path would not only throttle itself, but could
also hurt everyone else. This might be more useful if the buffer was
allocated in userspace? A free() in userspace usually triggers a more
limited TLB shootdown.
I think I can use alloc_pages_bulk() instead to avoid the TLB shootdown
cost altogether. It's a bit more work in KVM, but maybe worth it...
Thanks,
Peter
[1] https://lore.kernel.org/kvm/20211115165030.7422-8-dwmw2@infradead.org/
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Yan Zhao @ 2026-07-08 9:13 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, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-8-rick.p.edgecombe@intel.com>
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 ?
> 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" ?
> 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." ?
> TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> TDCX) are all handed to the TDX module at 4KB page size and are therefore
> subject to this requirement. Replace the raw alloc_page()/__free_page()
> calls for these pages with tdx_alloc/free_control_page().
>
> Switching between special Dynamic PAMT operations or normal page
> alloc/free operations is handled internally in
> tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> calls. Just call them unconditionally. Similarly, drop the NULL checks
> before freeing, as tdx_free_control_page() handles NULL internally.
>
> No functional change intended when Dynamic PAMT is not in use.
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
^ permalink raw reply
* Re: [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put()
From: Yan Zhao @ 2026-07-08 8:46 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, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-7-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:10PM -0700, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> The Dynamic PAMT get/put helpers use a global spinlock to serialize all
> refcount updates and SEAMCALL invocations. This gives correct behavior for
> concurrent callers, but leads to contention. It is especially bad from the
> KVM side, which is designed to allow faulting in EPT under a shared lock.
> With the global spinlock, not only is the lock an exclusive one, but it is
> for all TDs instead of just a single one.
>
> But taking the global lock each time is actually unnecessary. Only the 0->1
> and 1->0 refcount transitions actually need the lock (to pair with
> SEAMCALLs that actually add and remove with the Dynamic PAMT pages). The
> common case of incrementing or decrementing a non-zero refcount can be
> done locklessly.
>
> So create a fast and slow path. Check the refcount outside the lock and
> only take it for the slowpath (0->1 and 1->0 transitions).
>
> On the put side make the refcount adjustment and lock taking atomic so if
> a 'get' happens between them, it doesn't cause the Dynamic PAMT to be
> freed incorrectly. On the get side there is no technique for doing the
> refcount adjustment and lock atomically, so check the refcount again
> inside the lock.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
The optimization LGTM.
^ permalink raw reply
* Re: [PATCH v6 09/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory
From: Chao Gao @ 2026-07-08 8:35 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, yan.y.zhao, kai.huang, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-10-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:13PM -0700, Rick Edgecombe wrote:
>From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
>Add Dynamic PAMT support to KVM's S-EPT MMU by "getting" a PAMT page when
>adding guest memory (PAGE.ADD or PAGE.AUG), and "putting" the page when
>removing guest memory (PAGE.REMOVE).
>
>To access the per-vCPU PAMT caches without plumbing @vcpu throughout the
>TDP MMU, begrudgingly use kvm_get_running_vcpu() to get the vCPU, and bug
>the VM if KVM attempts to set an S-EPT leaf without an active vCPU. KVM
>only supports creating _new_ mappings in page (pre)fault paths, all of
>which require an active vCPU.
>
>The PAMT memory holds metadata for TDX-protected memory. With Dynamic
>PAMT, PAMT_4K is allocated on demand. The kernel supplies the TDX module
>with a few pages that cover 2M of host physical memory.
>
>Releases are balanced via tdx_pamt_put(): every control-page free goes
>through tdx_free_control_page(), and guest data pages are put directly on
>the successful tdh_mem_page_remove() path and in the
>tdx_mem_page_add/aug() error path.
>
>Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
>Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>Co-developed-by: Sean Christopherson <seanjc@google.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>---
>v6:
> - Don't have topup op take a min param (Yan, Sean)
The topup_external_cache x86 op still takes int min_nr_spts. Did you
forget to remove it, or am I misreading this changelog?
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Nikolay Borisov @ 2026-07-08 7:52 UTC (permalink / raw)
To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
Cc: djbw, kas, rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, peter.fang, baolu.lu,
zhenzhong.duan, dave.hansen, dave.hansen, seanjc
In-Reply-To: <20260618081355.3253581-9-yilun.xu@linux.intel.com>
On 6/18/26 11:13, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
>
> During TDX attestation, the TDX guest asks the host to generate a
> signed, verifiable structure (a "Quote"). With the Quoting extension,
> the TDX module returns the Quote in pages that the host shares via an
> Extension-SEAMCALL.
nit: Just say the host provides the pages to the TDX module, no need to
specify if it's via an ext seamcall or some otherway. One can infer this
from the code itself.
>
> The SEAMCALL accepts the host buffer pages as a linked list of 4KB
> "HPA_LINKED_LIST" nodes. Each entry holds the physical address of a 4KB
> data page, except for the last entry, which points to the next node. The
> TDX module reports the required Quote buffer size through a global
> metadata field. See [1] for details.
That HPA_LINKED_LIST is basically a linked list of arrays, each node
into the linked list being the array, and each entry of that array being
the HPA witht he last being a pointer to the next HPA_LINKED_LIST, so in
a way a 2 level data structure. Can you be more explicit and just say
something along the lines of :
"SEAMCALL accepts host buffer pages arranged in a custom data structure,
consisting of nodes containing HPAs, arranged linearly, and each node is
linked to the next one via the last entry"
>
> For simplicity, let all guests share a global buffer. Build the buffer's
> HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> va-to-pa conversions at runtime.
>
> [1] Intel TDX Module ABI specification, Section "Physical Memory
> Management Types"
>
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx_global_metadata.h | 4 +
> arch/x86/virt/vmx/tdx/tdx.c | 115 +++++++++++++++++++-
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 14 +++
> 3 files changed, 129 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index b3442b7c88bb..17cb13a1bb40 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -57,4 +57,8 @@ struct tdx_sys_info_ext {
> bool ext_required;
> };
>
> +struct tdx_sys_info_quote {
> + u32 max_quote_size;
> +};
> +
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 06c42b86b05e..9716424a301f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -32,6 +32,7 @@
> #include <linux/idr.h>
> #include <linux/kvm_types.h>
> #include <linux/bitfield.h>
> +#include <linux/vmalloc.h>
> #include <asm/page.h>
> #include <asm/special_insns.h>
> #include <asm/msr-index.h>
> @@ -71,6 +72,24 @@ static LIST_HEAD(tdx_memlist);
>
> static struct tdx_sys_info tdx_sysinfo;
>
> +/*
> + * Quote buffer shared with the TDX module for quote generation, in HPA linked
> + * list format.
> + *
> + * @buf: Virtual address of the quote buffer.
> + * @buf_len: Size of @buf in bytes.
> + * @hpa_entries: HPA entries, starting at the first list node.
> + * @hpa_entries_pa: Physical address for @hpa_entries.
> + */
> +struct tdx_quote_data {
> + void *buf;
> + u64 buf_len;
> + u64 *hpa_entries;
> + phys_addr_t hpa_entries_pa;
> +};
> +
> +static struct tdx_quote_data tdx_quote;
> +
> static DEFINE_RAW_SPINLOCK(sysinit_lock);
>
> /*
> @@ -1167,6 +1186,81 @@ static __init int init_tdmrs(struct tdmr_info_list *tdmr_list)
> return 0;
> }
>
> +static inline phys_addr_t tdx_vmalloc_to_pa(const void *addr)
> +{
> + unsigned long pfn = vmalloc_to_pfn(addr);
> +
> + return PFN_PHYS(pfn);
> +}
> +
> +#define HPAS_PER_NODE (PAGE_SIZE / sizeof(u64))
> +
> +/*
> + * Pass the quote buffer to the TDX module as an HPA linked list, where each
> + * node holds 4KB page HPAs and the last entry points to the next node.
> + */
> +static __init int tdx_quote_create_buf(unsigned int npages,
> + struct tdx_quote_data *qdata)
> +{
> + unsigned int nnodes;
> + u64 *hpas;
> + void *qbuf;
> + int i, j;
> +
> + if (!npages)
> + return -EINVAL;
> +
> + /*
> + * Each node holds up to (HPAS_PER_NODE - 1) 4KB page HPAs.
> + * The last entry of the node points to the next node.
> + */
> + nnodes = DIV_ROUND_UP(npages, HPAS_PER_NODE - 1);
nit: It's somewhat arbitrary but num_nodes seems more explicit, saving 3
chars is not worth it.
> +
> + hpas = vmalloc_array(nnodes, PAGE_SIZE);
hpas is basically the starting node, so name it "nodes". Also I'm
slightly confused why you use vmalloc to allocate a contiguous address
space when the nodes are linked in a list? I.e you keep referring to
list in the changelog and the code seems to be working in an array chunks?
> + if (!hpas)
> + return -ENOMEM;
> +
> + /*
> + * ~0ULL is the list terminator for HPA_LINKED_LIST.
> + *
> + * Pre-fill the last node with 0xff bytes so that unused entries are
> + * terminators. Overwrite populated entries later.
> + */
> + memset((u8 *)hpas + (nnodes - 1) * PAGE_SIZE, 0xff, PAGE_SIZE);
> +
> + qbuf = vcalloc(npages, PAGE_SIZE);
> + if (!qbuf)
> + goto out_nomem;
'qbuf' is rather arbitrary here. You simply pre-allocate a bunch of
pages which you then initialize into the nodes. Can't this allocation be
moved inside the loop itself, of course it will increase the number of
allocs happening but this is during initialization so it's not
performance critical, yes?
> +
> + /* Populate the linked list */
> + for (i = 0, j = 0; j < npages; i++) {
nit: This feedback was given before, but both variables can be defined
in this loop.
> + if ((i % HPAS_PER_NODE) == HPAS_PER_NODE - 1) {
> + /*
> + * The last node entry always points to the next node.
> + * The address of the following entry must be on next
> + * node's page boundary.
> + */
> + hpas[i] = tdx_vmalloc_to_pa(&hpas[i + 1]);
> + continue;
> + }
> +
> + hpas[i] = tdx_vmalloc_to_pa((u8 *)qbuf + j * PAGE_SIZE);
> + j++;
> + }
> +> + qdata->buf = qbuf;
> + qdata->buf_len = (u64)npages * PAGE_SIZE;
> + qdata->hpa_entries = hpas;
> + qdata->hpa_entries_pa = tdx_vmalloc_to_pa(hpas);
> +
> + return 0;
> +
> +out_nomem:
> + vfree(hpas);
> +
> + return -ENOMEM;
> +}
> +
> /* Initialize quoting extension */
> static __init int tdx_quote_init(void)
> {
> @@ -1185,12 +1279,25 @@ static __init int tdx_quote_init(void)
>
> static __init void init_tdx_quoting_extension(void)
> {
> - int ret;
> + struct tdx_sys_info_quote sysinfo_quote;
> + unsigned int nr_quote_pages;
> +
> + if (!(tdx_addon_feature0 & TDX_FEATURES0_QUOTE))
> + return;
>
> - if (tdx_addon_feature0 & TDX_FEATURES0_QUOTE) {
> - ret = tdx_quote_init();
> - WARN_ON_ONCE(ret);
> + if (tdx_quote_init()) {
> + WARN_ON_ONCE(1);
> + return;
> }
> +
> + /* Quoting metadata is valid only after initialization */
> + if (get_tdx_sys_info_quote(&sysinfo_quote))
> + return;
> +
> + nr_quote_pages = PAGE_ALIGN(sysinfo_quote.max_quote_size) /
> + PAGE_SIZE;
> + if (tdx_quote_create_buf(nr_quote_pages, &tdx_quote))
> + pr_err("Failed to create quote buffer\n");
> }
>
> /* Initialize TDX module extensions for extension SEAMCALLs */
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 84364da89649..1eb2985307c6 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -151,3 +151,17 @@ static int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
>
> return 0;
> }
> +
> +static __init int get_tdx_sys_info_quote(struct tdx_sys_info_quote *sysinfo_quote)
> +{
> + int ret;
> + u64 val;
> +
> + ret = read_sys_metadata_field(0x2300000200000002, &val);
> + if (ret)
> + return ret;
> +
> + sysinfo_quote->max_quote_size = val;
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Yan Zhao @ 2026-07-08 6:46 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, Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-6-rick.p.edgecombe@intel.com>
On Mon, May 25, 2026 at 07:35:09PM -0700, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> 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".
> installed to cover the 2MB range. And when one page is freed, the Dynamic
> PAMT backing cannot be freed until all pages in the range are no longer in
> use. Make the helpers handle these races internally.
>
> Use the per-2MB refcounts from previous changes to track how many 4KB
> pages are in use within each region. 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 Dynamic PAMT add
> and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
"an internal TDX module locks" --> "internal TDX module locks"?
> so simultaneous attempts on the same region would conflict.
How about:
"This also avoids TDX module BUSY errors, as 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 conflict." ?
> The lock is global and heavyweight. Use simple conditional logic to keep
> correctness obvious. This will be optimized in a later change.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> v6:
> - Split from "x86/virt/tdx: Add tdx_alloc/free_control_page() helpers"
> - Return 0 instead of ret to be clearer (Binbin)
> - Clarify log (Nikolay)
> - Justify why the patch is not optimized in response to comments by
> (Nikolay)
> - Move tdx_find_pamt_refcount() to faciliate patch re-order
> - Adjustments from dropping error helper patches
> - Log tweaks
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 72 ++++++++++++++++++++++++++++---------
> 1 file changed, 56 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 6658a6be6697c..50333eb96efa6 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -2043,10 +2043,14 @@ static u64 tdh_phymem_pamt_remove(kvm_pfn_t pfn, struct page **pamt_pages)
> return 0;
> }
>
> -/* Allocate PAMT memory for the given page */
> +/* Serializes adding/removing PAMT memory */
> +static DEFINE_SPINLOCK(pamt_lock);
> +
> +/* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
How about
"Bump the refcount of the PAMT page pair for the given PFN and add the PAMT
page pair on the first reference." ?
> static int tdx_pamt_get(kvm_pfn_t pfn)
> {
> struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> + atomic_t *pamt_refcount;
> u64 tdx_status;
> int ret;
>
> @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> if (ret)
> return ret;
>
> - tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> - if (tdx_status != TDX_SUCCESS) {
> - ret = -EIO;
> - goto out_free;
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
> +
> + scoped_guard(spinlock, &pamt_lock) {
> + /*
> + * If the pamt page is already added (i.e. refcount >= 1),
> + * then just increment the refcount.
> + */
> + if (atomic_read(pamt_refcount)) {
> + atomic_inc(pamt_refcount);
> + goto out_free;
> + }
> +
> + /* Try to add the pamt page and take the refcount 0->1. */
> + tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> + if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS)) {
> + ret = -EIO;
> + goto out_free;
> + }
> +
> + atomic_set(pamt_refcount, 1);
> }
>
> return 0;
> @@ -2069,26 +2089,46 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> return ret;
> }
>
> -/* Free PAMT memory for the given page */
> +/*
> + * Drop PAMT refcount for the given page and free PAMT memory if it is no
> + * longer needed.
How about:
"Drop the refcount of the PAMT page pair for the given PFN, and remove the
PAMT page pair if it is no longer needed." ?
> + */
> static void tdx_pamt_put(kvm_pfn_t pfn)
> {
> struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT] = {};
> + atomic_t *pamt_refcount;
> u64 tdx_status;
>
> if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> return;
>
> - tdx_status = tdh_phymem_pamt_remove(pfn, pamt_pages);
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
>
> - /*
> - * Don't free pamt_pages as it could hold garbage when
> - * tdh_phymem_pamt_remove() fails. Don't panic/BUG_ON(), as
> - * there is no risk of data corruption, but do yell loudly as
> - * failure indicates a kernel bug, memory is being leaked, and
> - * the dangling PAMT entry may cause future operations to fail.
> - */
> - if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS))
> - return;
> + scoped_guard(spinlock, &pamt_lock) {
> + /*
> + * If the there are more than 1 references on the pamt page,
s/If the/If
s/references/reference
> + * don't remove it yet. Just decrement the refcount.
> + */
> + if (atomic_read(pamt_refcount) > 1) {
> + atomic_dec(pamt_refcount);
> + return;
> + }
> +
> + /* Try to remove the pamt page and take the refcount 1->0. */
> + tdx_status = tdh_phymem_pamt_remove(pfn, pamt_pages);
> +
> + /*
> + * Don't free pamt_pages as it could hold garbage when
> + * tdh_phymem_pamt_remove() fails. Don't panic/BUG_ON(), as
> + * there is no risk of data corruption, but do yell loudly as
> + * failure indicates a kernel bug, memory is being leaked, and
> + * the dangling PAMT entry may cause future operations to fail.
> + */
> + if (WARN_ON_ONCE(tdx_status != TDX_SUCCESS))
> + return;
> +
> + atomic_set(pamt_refcount, 0);
> + }
>
> free_pamt_array(pamt_pages);
> }
Another nit:
How about renaming the title from
"Handle concurrent callers in tdx_pamt_get/put()" to
"Handle concurrent calls to tdx_pamt_get/put()" ?
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
^ permalink raw reply
* Re: [PATCH v5 47/51] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: Nikunj A. Dadhania @ 2026-07-08 5:00 UTC (permalink / raw)
To: Sean Christopherson, Jonathan Corbet, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Rick Edgecombe, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Andy Lutomirski, Peter Zijlstra, Juergen Gross,
Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
David Woodhouse, David Woodhouse, Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-48-seanjc@google.com>
On 7/2/2026 1:02 AM, Sean Christopherson wrote:
> Silently ignore attempts to switch to a paravirt sched_clock when running
> as a CoCo guest with trusted TSC. In hand-wavy theory, a misbehaving
> hypervisor could attack the guest by manipulating the PV clock to affect
> guest scheduling in some weird and/or predictable way. More importantly,
> reading TSC on such platforms is faster than any PV clock, and sched_clock
> is all about speed.
>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kernel/tsc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 012321fed5e5..a146fc7b5e74 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
> int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
> void (*save)(void), void (*restore)(void))
> {
> + /*
> + * Don't replace TSC with a PV clock when running as a CoCo guest and
> + * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
> + * which isn't in the guest's TCB.
> + */
> + if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
> + boot_cpu_has(X86_FEATURE_TDX_GUEST))
> + return -EPERM;
> +
> if (!stable)
> clear_sched_clock_stable();
>
^ permalink raw reply
* Re: [PATCH 06/15] PCI/TSM: Add device evidence support
From: Alexey Kardashevskiy @ 2026-07-08 5:00 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: linux-pci, driver-core, ankita, Bjorn Helgaas, Alistair Francis,
Lukas Wunner, Xu Yilun, Aneesh Kumar K.V (Arm)
In-Reply-To: <20260705220819.2472765-7-djbw@kernel.org>
On 6/7/26 08:08, Dan Williams wrote:
> Register the PCI Trusted Execution Environment Security Manager (TSM)
> framework with the device-evidence netlink ABI. The security operations
> that PCI core coordinates with a device security manager (DSM) through a
> platform TEE security manager (TSM), builds upon the SPDM protocol.
>
> A TSM owns an SPDM session and publishes the corresponding evidence through
> TSM firwmware ABIs. A low level TSM driver is responsible for creating a
> 'struct device_evidence' context and carrying out refresh_evidence()
a nit: so it is really refresh_measurements() as it won't refresh the report? may be call it so?
> requests for regenerating measurement transcripts with a nonce.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alistair Francis <alistair.francis@wdc.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
> drivers/pci/Kconfig | 1 +
> drivers/pci/Makefile | 2 +-
> drivers/pci/tsm/Makefile | 8 +++
> include/linux/pci-tsm.h | 14 ++++
> drivers/pci/{tsm.c => tsm/core.c} | 9 +--
> drivers/pci/tsm/evidence.c | 110 ++++++++++++++++++++++++++++++
> MAINTAINERS | 2 +-
> 7 files changed, 137 insertions(+), 9 deletions(-)
> create mode 100644 drivers/pci/tsm/Makefile
> rename drivers/pci/{tsm.c => tsm/core.c} (99%)
> create mode 100644 drivers/pci/tsm/evidence.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 0c7408509ba2..ed17b5d2d5ae 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -126,6 +126,7 @@ config PCI_IDE
>
> config PCI_TSM
> bool "PCI TSM: Device security protocol support"
> + select DEVICE_EVIDENCE if NET
> select PCI_IDE
> select PCI_DOE
> select TSM
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 41ebc3b9a518..211f195ff2c9 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -35,7 +35,7 @@ obj-$(CONFIG_XEN_PCIDEV_FRONTEND) += xen-pcifront.o
> obj-$(CONFIG_VGA_ARB) += vgaarb.o
> obj-$(CONFIG_PCI_DOE) += doe.o
> obj-$(CONFIG_PCI_IDE) += ide.o
> -obj-$(CONFIG_PCI_TSM) += tsm.o
> +obj-$(CONFIG_PCI_TSM) += tsm/
> obj-$(CONFIG_PCI_DYNAMIC_OF_NODES) += of_property.o
> obj-$(CONFIG_PCI_NPEM) += npem.o
> obj-$(CONFIG_PCIE_TPH) += tph.o
> diff --git a/drivers/pci/tsm/Makefile b/drivers/pci/tsm/Makefile
> new file mode 100644
> index 000000000000..fd7ac2e862f1
> --- /dev/null
> +++ b/drivers/pci/tsm/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the PCI/TSM infrastructure
> +
> +obj-$(CONFIG_PCI_TSM) += tsm.o
> +
> +tsm-y := core.o
> +tsm-$(CONFIG_DEVICE_EVIDENCE) += evidence.o
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index a6435aba03f9..8bc16029d31e 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -3,7 +3,10 @@
> #define __PCI_TSM_H
> #include <linux/mutex.h>
> #include <linux/pci.h>
> +#include <linux/rwsem.h>
then this file is missing extern struct rw_semaphore pci_tsm_rwsem;
> #include <linux/sockptr.h>
> +#include <linux/tsm.h>
> +#include <linux/device/evidence.h>
>
> struct pci_tsm;
> struct tsm_dev;
> @@ -18,6 +21,7 @@ enum pci_tsm_req_scope;
> * @devsec_ops: Lock, unlock, and interrogate the security state of the
> * function via the platform TSM (typically virtual function
> * operations).
> + * @refresh_evidence: Common operation to regenerate attestation objects
> *
> * This operations are mutually exclusive either a tsm_dev instance
> * manages physical link properties or it manages function security
> @@ -75,6 +79,9 @@ struct pci_tsm_ops {
> struct pci_dev *pdev);
> void (*unlock)(struct pci_tsm *tsm);
> );
> +
> + int (*refresh_evidence)(struct pci_tsm *tsm, const void *nonce,
> + size_t nonce_len);
> };
>
> /**
> @@ -96,6 +103,8 @@ struct pci_tdi {
> * @tsm_dev: PCI TEE Security Manager device for Link Confidentiality or Device
> * Function Security operations
> * @tdi: TDI context established by the @bind link operation
> + * @evidence: cached evidence from SPDM session establishment (connect), or
> + * TDISP bind (lock)
> *
> * This structure is wrapped by low level TSM driver data and returned by
> * probe()/lock(), it is freed by the corresponding remove()/unlock().
> @@ -112,6 +121,7 @@ struct pci_tsm {
> struct pci_dev *dsm_dev;
> struct tsm_dev *tsm_dev;
> struct pci_tdi *tdi;
> + struct device_evidence *evidence;
> };
>
> /**
> @@ -216,6 +226,10 @@ void pci_tsm_tdi_constructor(struct pci_dev *pdev, struct pci_tdi *tdi,
> ssize_t pci_tsm_guest_req(struct pci_dev *pdev, enum pci_tsm_req_scope scope,
> sockptr_t req_in, size_t in_len, sockptr_t req_out,
> size_t out_len, u64 *tsm_code);
> +static inline const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> +{
> + return tsm->tsm_dev->pci_ops;
> +}
> #else
> static inline int pci_tsm_register(struct tsm_dev *tsm_dev)
> {
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm/core.c
> similarity index 99%
> rename from drivers/pci/tsm.c
> rename to drivers/pci/tsm/core.c
This move + related makefile changes + exposing pci_tsm_rwsem are better be in a separate patch. Thanks,
> index 5fdcd7f2e820..220842df42bc 100644
> --- a/drivers/pci/tsm.c
> +++ b/drivers/pci/tsm/core.c
> @@ -15,13 +15,13 @@
> #include <linux/sysfs.h>
> #include <linux/tsm.h>
> #include <linux/xarray.h>
> -#include "pci.h"
> +#include "../pci.h"
>
> /*
> * Provide a read/write lock against the init / exit of pdev tsm
> * capabilities and arrival/departure of a TSM instance
> */
> -static DECLARE_RWSEM(pci_tsm_rwsem);
> +DECLARE_RWSEM(pci_tsm_rwsem);
>
> /*
> * Count of TSMs registered that support physical link operations vs device
> @@ -30,11 +30,6 @@ static DECLARE_RWSEM(pci_tsm_rwsem);
> static int pci_tsm_link_count;
> static int pci_tsm_devsec_count;
>
> -static const struct pci_tsm_ops *to_pci_tsm_ops(struct pci_tsm *tsm)
> -{
> - return tsm->tsm_dev->pci_ops;
> -}
> -
> static inline bool is_dsm(struct pci_dev *pdev)
> {
> return pdev->tsm && pdev->tsm->dsm_dev == pdev;
> diff --git a/drivers/pci/tsm/evidence.c b/drivers/pci/tsm/evidence.c
> new file mode 100644
> index 000000000000..ffb08208f0c0
> --- /dev/null
> +++ b/drivers/pci/tsm/evidence.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2026 NVIDIA Corporation & Affiliates */
> +
> +#include <linux/device/evidence.h>
> +#include <linux/init.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/pci-tsm.h>
> +#include <linux/tsm.h>
> +
> +extern struct rw_semaphore pci_tsm_rwsem;
> +
> +static bool evidence_available(struct pci_dev *pdev)
> +{
> + return pdev->tsm && pdev->tsm->evidence;
> +}
> +
> +static struct device *pci_tsm_evidence_find_device(const char *name)
> +{
> + ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
> + if (ACQUIRE_ERR(rwsem_read_intr, &lock))
> + return NULL;
> +
> + struct device *dev __free(put_device) =
> + bus_find_device_by_name(&pci_bus_type, NULL, name);
> +
> + /*
> + * Bail evidence gathering early if we know at this point that
> + * the device has no valid evidence provider, but still need to
> + * revalidate the same in pci_tsm_evidence_read_begin().
> + */
> + if (!dev || !evidence_available(to_pci_dev(dev)))
> + return NULL;
> +
> + return no_free_ptr(dev);
> +}
> +
> +static struct device_evidence *pci_tsm_evidence_read_begin(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct device_evidence *evidence;
> + int rc;
> +
> + rc = down_read_interruptible(&pci_tsm_rwsem);
> + if (rc)
> + return ERR_PTR(rc);
> +
> + if (!evidence_available(pdev))
> + goto err;
> +
> + /* Hold the evidence stable against conflicting refresh updates */
> + evidence = pdev->tsm->evidence;
> + rc = down_read_interruptible(&evidence->lock);
> + if (rc)
> + goto err;
> +
> + return evidence;
> +err:
> + up_read(&pci_tsm_rwsem);
> + return ERR_PTR(-ENXIO);
> +}
> +
> +static void pci_tsm_evidence_read_end(struct device_evidence *evidence)
> +{
> + up_read(&evidence->lock);
> + up_read(&pci_tsm_rwsem);
> +}
> +
> +static int pci_tsm_refresh_evidence(struct device *dev, const void *nonce,
> + size_t nonce_len)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct device_evidence *evidence;
> + const struct pci_tsm_ops *ops;
> + int rc;
> +
> + /* Sync against disconnect */
> + ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
> + if ((rc = ACQUIRE_ERR(rwsem_read_intr, &lock)))
> + return rc;
> +
> + if (!pdev->tsm)
> + return -ENXIO;
> +
> + ops = to_pci_tsm_ops(pdev->tsm);
> + if (!ops->refresh_evidence)
> + return -EOPNOTSUPP;
> +
> + /* Sync against pci_tsm_evidence_read_begin */
> + evidence = pdev->tsm->evidence;
> + ACQUIRE(rwsem_write_kill, elock)(&evidence->lock);
> + if ((rc = ACQUIRE_ERR(rwsem_write_kill, &elock)))
> + return rc;
> +
> + return ops->refresh_evidence(pdev->tsm, nonce, nonce_len);
> +}
> +
> +static const struct device_evidence_ops pci_tsm_evidence_ops = {
> + .subsys_name = "pci",
> + .find_device = pci_tsm_evidence_find_device,
> + .evidence_read_begin = pci_tsm_evidence_read_begin,
> + .evidence_read_end = pci_tsm_evidence_read_end,
> + .refresh_evidence = pci_tsm_refresh_evidence,
> +};
> +
> +static int __init pci_tsm_evidence_init(void)
> +{
> + return device_evidence_register(&pci_tsm_evidence_ops);
> +}
> +subsys_initcall(pci_tsm_evidence_init);
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb4f74957f69..6cefaddc1120 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -27472,7 +27472,7 @@ F: Documentation/driver-api/coco/
> F: Documentation/driver-api/pci/tsm.rst
> F: Documentation/netlink/specs/device-evidence.yaml
> F: drivers/base/*evidence*
> -F: drivers/pci/tsm.c
> +F: drivers/pci/tsm/
> F: drivers/virt/coco/guest/
> F: include/uapi/linux/device-evidence.h
> F: include/linux/device/evidence.h
--
Alexey
^ 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