Linux Confidential Computing Development
 help / color / mirror / Atom feed
* 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

* Re: [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
From: Nikunj A. Dadhania @ 2026-07-08  4:54 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-7-seanjc@google.com>



On 7/2/2026 1:01 AM, Sean Christopherson wrote:
> Don't override the kernel's CPU frequency calibration routine when
> registering SNP's Secure TSC calibration routine.  SNP (the architecture)
> provides zero guarantees that the CPU runs at the same frequency as the
> TSC.  The justification for clobbering the CPU routine was:
> 
>   Since the difference between CPU base and TSC frequency does not apply
>   in this case, the same callback is being used.
> 
> but that's simply not true.  E.g. if APERF/MPERF is exposed to the VM, then
> the CPU frequency absolutely does matter.
> 
> While relying on heuristics and/or the untrusted hypervisor to provide the
> CPU frequency isn't ideal, it's at least not outright wrong.
> 
> Fixes: 73bbf3b0fbba ("x86/tsc: Init the TSC for Secure TSC guests")
> Cc: Nikunj A Dadhania <nikunj@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>

> ---
>  arch/x86/coco/sev/core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index ed0ac52a765e..665de1aea0ee 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
>  
>  	snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
>  
> -	x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
>  	x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
>  
>  	early_memunmap(mem, PAGE_SIZE);


^ permalink raw reply

* Re: [PATCH v5 08/51] x86/sev: Shove SNP's secure/trusted TSC frequency directly into "calibration"
From: Nikunj A. Dadhania @ 2026-07-08  4:52 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-9-seanjc@google.com>



On 7/2/2026 1:01 AM, Sean Christopherson wrote:
> As a first step towards dropping .calibrate_{cpu,tsc}() and explicitly
> defining precedence/priority for "calibration" routines, pass the secure
> TSC frequency obtained from SNP firmware directly to
> determine_cpu_tsc_frequencies() instead of overriding the .calibrate_tsc()
> hook.
> 
> Unlike the native calibration routines, all of the paravirtual overrides,
> including SNP and TDX, are constant in the sense that the frequency
> provided by the hypervisor or trusted firmware is fixed, known, and always
> available during early boot.  More importantly, for CoCo (SNP and TDX) VMs,
> it's imperative that the kernel uses the frequency provided by the trusted
> firmware, not by the untrusted hypervisor.  Enforcing the priority between
> sources by carefully ordering seemingly unrelated init calls, so that the
> trusted override "wins", is brittle and all but impossible to follow.
> 
> Explicitly ignore tsc_early_khz if the exact TSC frequency was obtained
> from trusted firmware, as per commit bd35c77e32e4 ("x86/tsc: Add
> tsc_early_khz command line parameter"), the goal of the param is to play
> nice with setups that provide partial frequency information in CPUID, i.e.
> is NOT intended to be a hard override.  Neither SNP's secure TSC nor TDX
> was supported when commit bd35c77e32e4 landed back in 2020, i.e. lack of
> consideration for the interaction was purely due to oversight when SNP and
> TDX support came along.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  4 +++
>  arch/x86/coco/sev/core.c                      | 14 +++--------
>  arch/x86/include/asm/sev.h                    |  4 +--
>  arch/x86/kernel/tsc.c                         | 25 ++++++++++++++-----
>  4 files changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b5493a7f8f22..181149f633c3 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7946,6 +7946,10 @@ Kernel parameters
>  			with CPUID.16h support and partial CPUID.15h support.
>  			Format: <unsigned int>
>  
> +			Note, tsc_early_khz is ignored if the TSC frequency is
> +			provided by trusted firmware when running as an SNP
> +			guest.
> +
>  	tsx=		[X86] Control Transactional Synchronization
>  			Extensions (TSX) feature in Intel processors that
>  			support TSX control.
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 403dcea86452..bc5ae9ef74da 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -99,7 +99,6 @@ static const char * const sev_status_feat_names[] = {
>   */
>  static u64 snp_tsc_scale __ro_after_init;
>  static u64 snp_tsc_offset __ro_after_init;
> -static unsigned long snp_tsc_freq_khz __ro_after_init;
>  
>  DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
>  DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
> @@ -2014,15 +2013,10 @@ void __init snp_secure_tsc_prepare(void)
>  	pr_debug("SecureTSC enabled");
>  }
>  
> -static unsigned long securetsc_get_tsc_khz(void)
> -{
> -	return snp_tsc_freq_khz;
> -}
> -
> -void __init snp_secure_tsc_init(void)
> +unsigned int __init snp_secure_tsc_init(void)
>  {
> +	unsigned long snp_tsc_freq_khz, tsc_freq_mhz;
>  	struct snp_secrets_page *secrets;
> -	unsigned long tsc_freq_mhz;
>  	void *mem;
>  
>  	mem = early_memremap_encrypted(sev_secrets_pa, PAGE_SIZE);
> @@ -2043,7 +2037,7 @@ void __init snp_secure_tsc_init(void)
>  
>  	snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
>  
> -	x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
> -
>  	early_memunmap(mem, PAGE_SIZE);
> +
> +	return snp_tsc_freq_khz;
>  }
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 594cfa19cbd4..05ebf0b73ef4 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -530,7 +530,7 @@ int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req
>  int snp_svsm_vtpm_send_command(u8 *buffer);
>  
>  void __init snp_secure_tsc_prepare(void);
> -void __init snp_secure_tsc_init(void);
> +unsigned int snp_secure_tsc_init(void);

It seems __init got dropped here accidentally?

Apart from this:

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Tested-by: Nikunj A Dadhania <nikunj@amd.com>

>  enum es_result savic_register_gpa(u64 gpa);
>  enum es_result savic_unregister_gpa(u64 *gpa);
>  u64 savic_ghcb_msr_read(u32 reg);
> @@ -637,7 +637,7 @@ static inline int snp_send_guest_request(struct snp_msg_desc *mdesc,
>  					 struct snp_guest_req *req) { return -ENODEV; }
>  static inline int snp_svsm_vtpm_send_command(u8 *buffer) { return -ENODEV; }
>  static inline void __init snp_secure_tsc_prepare(void) { }
> -static inline void __init snp_secure_tsc_init(void) { }
> +static inline unsigned int __init snp_secure_tsc_init(void) { return 0; }
>  static inline void sev_evict_cache(void *va, int npages) {}
>  static inline enum es_result savic_register_gpa(u64 gpa) { return ES_UNSUPPORTED; }
>  static inline enum es_result savic_unregister_gpa(u64 *gpa) { return ES_UNSUPPORTED; }
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 8f1604ffe986..f049c126e47c 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1440,15 +1440,16 @@ static int __init init_tsc_clocksource(void)
>   */
>  device_initcall(init_tsc_clocksource);
>  
> -static bool __init determine_cpu_tsc_frequencies(bool early)
> +static bool __init determine_cpu_tsc_frequencies(bool early,
> +						 unsigned int known_tsc_khz)
>  {
>  	/* Make sure that cpu and tsc are not already calibrated */
>  	WARN_ON(cpu_khz || tsc_khz);
>  
>  	if (early) {
>  		cpu_khz = x86_platform.calibrate_cpu();
> -		if (tsc_early_khz)
> -			tsc_khz = tsc_early_khz;
> +		if (known_tsc_khz)
> +			tsc_khz = known_tsc_khz;
>  		else
>  			tsc_khz = x86_platform.calibrate_tsc();
>  	} else {
> @@ -1503,6 +1504,8 @@ static void __init tsc_enable_sched_clock(void)
>  
>  void __init tsc_early_init(void)
>  {
> +	unsigned int known_tsc_khz = 0;
> +
>  	if (!boot_cpu_has(X86_FEATURE_TSC))
>  		return;
>  	/* Don't change UV TSC multi-chassis synchronization */
> @@ -1510,9 +1513,19 @@ void __init tsc_early_init(void)
>  		return;
>  
>  	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC))
> -		snp_secure_tsc_init();
> +		known_tsc_khz = snp_secure_tsc_init();
>  
> -	if (!determine_cpu_tsc_frequencies(true))
> +	/*
> +	 * Ignore the user-provided TSC frequency if the exact frequency was
> +	 * obtained from trusted firmware, as the user-provided frequency is
> +	 * intended as a "starting point", not a known, guaranteed frequency.
> +	 */
> +	if (!known_tsc_khz)
> +		known_tsc_khz = tsc_early_khz;
> +	else if (tsc_early_khz)
> +		pr_err("Ignoring 'tsc_early_khz' in favor of trusted firmware.\n");
> +
> +	if (!determine_cpu_tsc_frequencies(true, known_tsc_khz))
>  		return;
>  	tsc_enable_sched_clock();
>  }
> @@ -1533,7 +1546,7 @@ void __init tsc_init(void)
>  
>  	if (!tsc_khz) {
>  		/* We failed to determine frequencies earlier, try again */
> -		if (!determine_cpu_tsc_frequencies(false)) {
> +		if (!determine_cpu_tsc_frequencies(false, 0)) {
>  			mark_tsc_unstable("could not calculate TSC khz");
>  			setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE_TIMER);
>  			return;


^ permalink raw reply

* Re: [PATCH v6 03/11] x86/virt/tdx: Add tdx_alloc/free_control_page() helpers
From: Sohil Mehta @ 2026-07-08  3:50 UTC (permalink / raw)
  To: Rick Edgecombe, bp, dave.hansen, hpa, kas, kvm, linux-coco,
	linux-doc, linux-kernel, mingo, nik.borisov, pbonzini, seanjc,
	tglx, vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
  Cc: Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-4-rick.p.edgecombe@intel.com>

How about?

x86/virt/tdx: Add tdx_{alloc,free}_control_page() helpers

On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> Add helpers to use when allocating or preparing pages that are handed to
> the TDX-Module for use as control/S-EPT pages, and thus need Dynamic PAMT
> adjustments.
> 
> 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
> 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 for pages that TDX will use as 2MB physically
> contiguous pages, Dynamic PAMT backing is not needed.

I lost the continuation in the last sentence. Why does it only talk
about 2MB if only 4KB is dynamically allocated. What about 1GB?

(Probably due to my lack of TDX knowledge)
Similarly, why do these functions only refer to 2MB only and not 1GB?

pamt_2mb_arg(), tdh_phymem_pamt_add(), tdh_phymem_pamt_remove().

> 
> KVM will need to hand pages to the TDX module that it will use at 4KB
> granularity. So these pages will need Dynamic PAMT backing added before
> they are used by the TDX module, and removed afterwards.
> 

...

> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 82dc27aecf297..74e75db5728c7 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -37,6 +37,7 @@
>  
>  #include <uapi/asm/mce.h>
>  #include <asm/tdx_global_metadata.h>
> +#include <linux/mm.h>
>  #include <linux/pgtable.h>
>  
>  /*
> @@ -160,6 +161,12 @@ void tdx_guest_keyid_free(unsigned int keyid);
>  
>  void tdx_quirk_reset_paddr(unsigned long base, unsigned long size);
>  
> +/* Number PAMT pages to be provided to TDX module per 2MB region of PA */

	^^^ of PAMT pages


> +#define TDX_DPAMT_ENTRY_PAGE_CNT 2
> +
> +struct page *tdx_alloc_control_page(void);
> +void tdx_free_control_page(struct page *page);
> +
>  struct tdx_td {
>  	/* TD root structure: */
>  	struct page *tdr_page;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 9ebd192cb5c17..9e0812d87ab06 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1919,6 +1919,165 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn)
>  }
>  EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
>  
> +static int alloc_pamt_array(struct page **pamt_pages)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < TDX_DPAMT_ENTRY_PAGE_CNT; i++) {
> +		pamt_pages[i] = alloc_page(GFP_KERNEL_ACCOUNT);
> +		if (!pamt_pages[i])
> +			goto err;
> +	}
> +
> +	return 0;
> +err:
> +	for (j = 0; j < i; j++)
> +		__free_page(pamt_pages[j]);
> +	return -ENOMEM;

Add a blank line before the return to separate from the for loop?

> +}
> +
> +static void free_pamt_array(struct page **pamt_pages)
> +{
> +	for (int i = 0; i < TDX_DPAMT_ENTRY_PAGE_CNT; i++) {
> +		/*
> +		 * Reset pages unconditionally to cover cases
> +		 * where they were passed to the TDX module.
> +		 */
> +		tdx_quirk_reset_paddr(page_to_phys(pamt_pages[i]), PAGE_SIZE);
> +
> +		__free_page(pamt_pages[i]);
> +	}
> +}
> +
> +/*
> + * Calculate the arg needed for operating on the DPAMT backing for
> + * a given 4KB page.
> + */
> +static u64 pamt_2mb_arg(kvm_pfn_t pfn)
> +{
> +	unsigned long hpa_2mb = ALIGN_DOWN(pfn << PAGE_SHIFT, PMD_SIZE);
> +
> +	return hpa_2mb | TDX_PS_2M;
> +}
> +
> +/* Add PAMT backing for the given page. */
> +static u64 tdh_phymem_pamt_add(kvm_pfn_t pfn, struct page **pamt_pages)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = pamt_2mb_arg(pfn),
> +		.rdx = page_to_phys(pamt_pages[0]),
> +		.r8 = page_to_phys(pamt_pages[1]),
> +	};
> +
> +	return seamcall(TDH_PHYMEM_PAMT_ADD, &args);
> +}
> +
> +/* Remove PAMT backing for the given page. */
> +static u64 tdh_phymem_pamt_remove(kvm_pfn_t pfn, struct page **pamt_pages)
> +{
> +	struct tdx_module_args args = {
> +		.rcx = pamt_2mb_arg(pfn),
> +	};
> +	u64 ret;
> +
> +	ret = seamcall_ret(TDH_PHYMEM_PAMT_REMOVE, &args);
> +	if (ret)
> +		return ret;
> +
> +	/* Copy PAMT pages out of the struct per the TDX ABI */
> +	pamt_pages[0] = phys_to_page(args.rdx);
> +	pamt_pages[1] = phys_to_page(args.r8);
> +
> +	return 0;
> +}
> +
> +/* Allocate PAMT memory for the given page */
> +static int tdx_pamt_get(kvm_pfn_t pfn)
> +{
> +	struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
> +	u64 tdx_status;
> +	int ret;
> +
> +	if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> +		return 0;
> +
> +	ret = alloc_pamt_array(pamt_pages);
> +	if (ret)
> +		return ret;
> +
> +	tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> +	if (tdx_status != TDX_SUCCESS) {
> +		ret = -EIO;
> +		goto out_free;
> +	}
> +
> +	return 0;

Blank line here as well.
> +out_free:
> +	free_pamt_array(pamt_pages);
> +	return ret;
> +}
> +


^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Sohil Mehta @ 2026-07-08  3:35 UTC (permalink / raw)
  To: Edgecombe, Rick P, kvm@vger.kernel.org,
	linux-coco@lists.linux.dev, Huang, Kai, Hansen, Dave, Zhao, Yan Y,
	kas@kernel.org, seanjc@google.com, mingo@redhat.com,
	pbonzini@redhat.com, linux-kernel@vger.kernel.org,
	nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
	Annapurve, Vishal, tglx@kernel.org, bp@alien8.de, Gao, Chao,
	x86@kernel.org
  Cc: binbin.wu@linux.intel.com
In-Reply-To: <818e0de368588836810718106e3b592323aaa0f1.camel@intel.com>

On 7/7/2026 7:10 PM, Edgecombe, Rick P wrote:
> On Tue, 2026-07-07 at 19:07 -0700, Rick Edgecombe wrote:
>>> I think you can avoid the comment altogether in that case.
>>
>> I don't think it removes the need for a comment. The point is if
>> tdx_supports_dynamic_pamt() is not supported. The comment should be more about
>> "why", than what the code does, right?
>>
>> How about:
>>
>> 	if (ret)
>> 		return ret;
>>
>> 	/*
>> 	 * The kernel supports using TDX without Dynamic PAMT, so
>> 	 * avoid reporting failure if it's not supported.
>> 	 */
>> 	if (tdx_supports_dynamic_pamt(sysinfo))
>> 		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>>
>> 	return ret;
> 
> Actually this really stands out from the autogenerated code now, how about:
> 

I thought that's a good thing. Eventually, we want to make this file
conform to the standard kernel style, right? The autogenerated pattern
in this file to "check any previous failure" before doing the current
step is odd.


> 	/*
> 	 * The kernel supports using TDX without Dynamic PAMT, so
> 	 * avoid reporting failure if it's not supported.
> 	 */
> 	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> 		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
> 
> It why-ifys the comment more, but only gently moves from the auto-generated
> patterns.

Sure, that's fine if you want to continue the style in this file.

BTW, do you also want to clarify why we fail the init if
get_tdx_sys_info_tdmr_dpamt() fails?

Is it because:

If reading the DPAMT metadata fails, something is really wrong with the
TDX module. It is better to fail the initialization in that case.

^ permalink raw reply

* Re: [PATCH 15/15] PCI/TSM: Add relative MMIO offset support?
From: Alexey Kardashevskiy @ 2026-07-08  2:25 UTC (permalink / raw)
  To: Dan Williams, linux-coco
  Cc: linux-pci, driver-core, ankita, Xu Yilun, Aneesh Kumar K.V
In-Reply-To: <20260705220819.2472765-16-djbw@kernel.org>

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,


> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
>   include/linux/pci-tsm.h | 15 ++++++++++++++-
>   drivers/pci/tsm/core.c  | 14 ++++++++++----
>   2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> index 6d5fadd79360..be9f78ca2c1a 100644
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
> @@ -296,7 +296,20 @@ 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);
> +/**
> + * enum tdisp_offset_scheme - MMIO_REPORTING_OFFSET assumptions
> + * @TDISP_OFFSET_BAR_ALIGN: mask by bar size to recover offset
> + * @TDISP_OFFSET_RELATIVE: first mmio report per bar is bar-offset-0
> + *
> + * A TSM driver may know that the default TDISP_OFFSET_BAR_ALIGN
> + * assumption is being violated.
> + */
> +enum tdisp_offset_scheme {
> +	TDISP_OFFSET_BAR_ALIGN,
> +	TDISP_OFFSET_RELATIVE,
> +};
> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev,
> +					enum tdisp_offset_scheme scheme);
>   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 9ac216ad896d..19ad35f2da4a 100644
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c
> @@ -643,13 +643,15 @@ struct pci_tsm_devif_report {
>   /**
>    * pci_tsm_mmio_alloc() - allocate encrypted MMIO range descriptor
>    * @pdev: device owner of MMIO ranges
> + * @scheme: allow the low level TSM driver to hint the offset calc scheme
>    *
>    * 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 pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev,
> +					enum tdisp_offset_scheme scheme)
>   {
>   	struct device_evidence *evidence = pdev->tsm->evidence;
>   	u64 reporting_bar_base, last_reporting_end;
> @@ -712,10 +714,14 @@ struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
>   			last_bar = bar;
>   
>   			/*
> -			 * Determine the obfuscated base of the BAR. BAR
> -			 * offsets are never obfuscated.
> +			 * Either the first range per bar always maps
> +			 * the start of the BAR, or the reporting_offset
> +			 * is BAR size aligned.
>   			 */
> -			reporting_bar_base = tsm_offset & ~mask;
> +			if (scheme == TDISP_OFFSET_RELATIVE)
> +				reporting_bar_base = tsm_offset;
> +			else
> +				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;

-- 
Alexey


^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-08  2:10 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
	mingo@redhat.com, pbonzini@redhat.com,
	linux-kernel@vger.kernel.org, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
	tglx@kernel.org, Mehta, Sohil, bp@alien8.de, Gao, Chao,
	x86@kernel.org
  Cc: kirill.shutemov@linux.intel.com, binbin.wu@linux.intel.com
In-Reply-To: <a724bce3e92e8a8dfb04798bc21860ba9be786d9.camel@intel.com>

On Tue, 2026-07-07 at 19:07 -0700, Rick Edgecombe wrote:
> > I think you can avoid the comment altogether in that case.
> 
> I don't think it removes the need for a comment. The point is if
> tdx_supports_dynamic_pamt() is not supported. The comment should be more about
> "why", than what the code does, right?
> 
> How about:
> 
> 	if (ret)
> 		return ret;
> 
> 	/*
> 	 * The kernel supports using TDX without Dynamic PAMT, so
> 	 * avoid reporting failure if it's not supported.
> 	 */
> 	if (tdx_supports_dynamic_pamt(sysinfo))
> 		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
> 
> 	return ret;

Actually this really stands out from the autogenerated code now, how about:

	/*
	 * The kernel supports using TDX without Dynamic PAMT, so
	 * avoid reporting failure if it's not supported.
	 */
	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);

It why-ifys the comment more, but only gently moves from the auto-generated
patterns.

^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-08  2:07 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
	mingo@redhat.com, pbonzini@redhat.com,
	linux-kernel@vger.kernel.org, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
	tglx@kernel.org, Mehta, Sohil, bp@alien8.de, Gao, Chao,
	x86@kernel.org
  Cc: kirill.shutemov@linux.intel.com, binbin.wu@linux.intel.com
In-Reply-To: <efbff823-7b4e-4349-bdc4-72a244699d76@intel.com>

On Tue, 2026-07-07 at 17:49 -0700, Sohil Mehta wrote:
> On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > 
> > The TDX Physical Address Metadata Table (PAMT) holds data about the
> > physical memory used by TDX, and must be allocated by the kernel during
> > TDX module initialization.
> > 
> > The exact size of the required PAMT memory is determined by the TDX module
> > and may vary between TDX module versions. Currently it is approximately
> > 0.4% of the system memory. This is a significant commitment, especially if
> > it is not known upfront whether the machine will run any TDX guests.
> > 
> > Each memory region that the TDX module might use needs three separate PAMT
> > allocations. One for each supported page size (1GB, 2MB, 4KB). The
> > TDX module supports a new feature designed to reduce PAMT overhead called
> > Dynamic PAMT. At a high level, Dynamic PAMT still has the 1GB and 2MB
> > levels allocated on TDX module initialization, but the 4KB level is
> > allocated dynamically during runtime.
> 
> The last statement is slightly confusing to me. Is it trying to say that
> the "dynamic" part is only applicable to 4KB allocations?

Yea. Can you explain more about what is confusing? I guess this is more compact:

Under Dynamic PAMT the 4KB level is allocated dynamically during runtime, while
the 1GB and 2MB levels remain allocated on TDX module initialization.

> 
> 
> > 
> > However, in the details, Dynamic PAMT still needs some smaller per 4KB
> > page scoped data (currently it is 1 bit per page). The TDX module exposes
> > the number of bits as a separate piece of metadata than the 4KB static
> > allocation for regular PAMT. Although the size is enumerated differently,
> > it is handed to the TDX module in the same way the 4KB page size PAMT
> > allocation is for regular, non-dynamic PAMT.
> > 
> > Begin to implement Dynamic PAMT in the kernel by reading the bits-per-page
> > needed for Dynamic PAMT. Calculate the size needed for the bitmap,
> > and use it instead of the 4KB size determined for normal PAMT, in the case
> > of Dynamic PAMT.
> > 
> > Unlike the existing metadata reading code, this code is not generated by a
> > script. 
> 
> 
> It might be useful to say that this file was auto-generated in the past
> but going forward it is going to be manually updated.
> 
> > So adjust the comment to be more generic. Also, start to adopt a
> > more normal kernel code style without the tenary statements and if
> 
> s/a more/
> s/tenary/ternary

How about this?

The existing metadata reading code was generated by a script, but the current
plan is to stop generating this code, as the script has continued to need
adjustments. So add manually written code and adjust the comment about it being
autogenerated to be more generic. Start to adopt a more normal kernel code style
without the ternary statements and if conditionals assignments that the auto
generated code has.

> 
> 
> > conditionals assignments that the auto generated code has.
> > 
> > Assisted-by: Sashiko:claude-opus-4-6
> > Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> 
> The review tags goes after the SOBs.
Yep, I have been fixing those across the series.

> 
> > 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>
> > ---
> 
> > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> > index 503f9a3f46d61..82dc27aecf297 100644
> > --- a/arch/x86/include/asm/tdx.h
> > +++ b/arch/x86/include/asm/tdx.h
> > @@ -149,6 +149,11 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> >  const char *tdx_dump_mce_info(struct mce *m);
> >  const struct tdx_sys_info *tdx_get_sysinfo(void);
> >  
> > +static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> > +{
> > +	return false; /* To be enabled when kernel is ready */
> 
> I would avoid the tail comment even if it is temporary.

Yep, Yan commented the same thing.

> 
> > +}
> > +
> >  int tdx_guest_keyid_alloc(void);
> >  u32 tdx_get_nr_guest_keyids(void);
> >  void tdx_guest_keyid_free(unsigned int keyid);
> 
> 
> 
> > @@ -33,6 +33,18 @@ static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinf
> >  	return ret;
> >  }
> >  
> > +static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> > +{
> > +	int ret;
> > +	u64 val;
> > +
> > +	ret = read_sys_metadata_field(0x9100000100000013, &val);
> 
> Should this be a #define now that the file is being manually updated? Or
> is the plan to do it all together? A #define would make it easier to
> read this patch.

I think switching to defines would overlap too much into the ultimate metadata
reading solution that gets discussed. Chao is currently working on a series for
this, so I'd think to leave that part for later.

> 
> > +	if (!ret)
> > +		sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;
> > +
> > +	return ret;
> > +}
> > +
> >  static __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> >  {
> >  	int ret = 0;
> > @@ -116,5 +128,12 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
> >  	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> >  	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
> >  
> > +	/*
> > +	 * Don't treat a module that doesn't support Dynamic PAMT
> > +	 * as a failure. Only read the metadata optionally.
> > +	 */
> > +	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> > +		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
> 
> There is a need for the comment because it combines two checks:
> 
> 1) Did any of the previous stages fail?
> 2) Does the TDX module support Dynamic PAMT?
> 
> Should these be separated for readability and to follow the typical
> kernel style?
> 
> 	if (ret)
> 		return ret;
> 
> 	if (tdx_supports_dynamic_pamt(sysinfo))
> 		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
> 
> 	return ret;
> 
> I think you can avoid the comment altogether in that case.

I don't think it removes the need for a comment. The point is if
tdx_supports_dynamic_pamt() is not supported. The comment should be more about
"why", than what the code does, right?

How about:

	if (ret)
		return ret;

	/*
	 * The kernel supports using TDX without Dynamic PAMT, so
	 * avoid reporting failure if it's not supported.
	 */
	if (tdx_supports_dynamic_pamt(sysinfo))
		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);

	return ret;

> 
> > +
> >  	return ret;
> >  }
> 


^ permalink raw reply

* Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()
From: Edgecombe, Rick P @ 2026-07-08  1:23 UTC (permalink / raw)
  To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, Zhao, Yan Y, kas@kernel.org, seanjc@google.com,
	mingo@redhat.com, pbonzini@redhat.com,
	linux-kernel@vger.kernel.org, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
	tglx@kernel.org, Mehta, Sohil, bp@alien8.de, Gao, Chao,
	x86@kernel.org
  Cc: binbin.wu@linux.intel.com
In-Reply-To: <a4b596d1-c0c7-4674-b5c4-dd565a6fd3ec@intel.com>

Thanks for the review!

On Tue, 2026-07-07 at 16:22 -0700, Sohil Mehta wrote:
> On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> > For each memory region that the TDX module might use (called TDMR), three
> > separate traditional PAMT allocations are needed. 
> 
> > One for each supported page size (1GB, 2MB, 4KB).
> Missing verb in this sentence. Maybe use a '-' to merge it with the
> previous sentence.

Hmm, yea I'll update it to:

There is one for each supported page size (1GB, 2MB, 4KB).

> 
> 
> > There are some commonalities in the math needed to calculate the base and
> > size for each smaller allocation, and so an effort was made to share logic
> > across the three. Unfortunately doing this turned out unnaturally tortured,
> > with a loop iterating over the three page sizes, only to call into a
> > function with cases statement for each page size. 
> 
> Did you mean "..with a case statement for each.."?

yep.

> 
> 
> 
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 967482ae3c801..487f389f52f4b 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -516,31 +516,21 @@ static __init int fill_out_tdmrs(struct list_head *tmb_list,
> >    * Calculate PAMT size given a TDMR and a page size.  The returned
> >    * PAMT size is always aligned up to 4K page boundary.
> >    */
> > -static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz,
> > -					     u16 pamt_entry_size)
> > +static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz)
> >   {
> >   	unsigned long pamt_sz, nr_pamt_entries;
> > +	const int tdx_pg_size_shift[] = { PAGE_SHIFT, PMD_SHIFT, PUD_SHIFT };
> 
> Both of these consts go hand-in-hand, right? I would write it as
> tdx_pg_size_shift[TDX_PS_NR] to make the connection obvious.

Yea that seems reasonable.

> 
> Just curious, why is TDX_PS_NR defined as (TDX_PS_1G + 1)? I don't think
> we are planning to add TDX_PS_256G anytime soon. But, should
> TDX_PS_4K..TDX_PS_NR be an enum?

Yea probably. The point of this patch is to improve the code such the following
patch can not make it worse. So I'd think it's best not to add extra cleanups to
it. I'll add this to the list. But we probably have more pressing cleanups.

^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Sohil Mehta @ 2026-07-08  0:49 UTC (permalink / raw)
  To: Rick Edgecombe, bp, dave.hansen, hpa, kas, kvm, linux-coco,
	linux-doc, linux-kernel, mingo, nik.borisov, pbonzini, seanjc,
	tglx, vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
  Cc: Kirill A. Shutemov, Binbin Wu
In-Reply-To: <20260526023515.288829-3-rick.p.edgecombe@intel.com>

On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> 
> The TDX Physical Address Metadata Table (PAMT) holds data about the
> physical memory used by TDX, and must be allocated by the kernel during
> TDX module initialization.
> 
> The exact size of the required PAMT memory is determined by the TDX module
> and may vary between TDX module versions. Currently it is approximately
> 0.4% of the system memory. This is a significant commitment, especially if
> it is not known upfront whether the machine will run any TDX guests.
> 
> Each memory region that the TDX module might use needs three separate PAMT
> allocations. One for each supported page size (1GB, 2MB, 4KB). The
> TDX module supports a new feature designed to reduce PAMT overhead called
> Dynamic PAMT. At a high level, Dynamic PAMT still has the 1GB and 2MB
> levels allocated on TDX module initialization, but the 4KB level is
> allocated dynamically during runtime.

The last statement is slightly confusing to me. Is it trying to say that
the "dynamic" part is only applicable to 4KB allocations?


> 
> However, in the details, Dynamic PAMT still needs some smaller per 4KB
> page scoped data (currently it is 1 bit per page). The TDX module exposes
> the number of bits as a separate piece of metadata than the 4KB static
> allocation for regular PAMT. Although the size is enumerated differently,
> it is handed to the TDX module in the same way the 4KB page size PAMT
> allocation is for regular, non-dynamic PAMT.
> 
> Begin to implement Dynamic PAMT in the kernel by reading the bits-per-page
> needed for Dynamic PAMT. Calculate the size needed for the bitmap,
> and use it instead of the 4KB size determined for normal PAMT, in the case
> of Dynamic PAMT.
> 
> Unlike the existing metadata reading code, this code is not generated by a
> script. 


It might be useful to say that this file was auto-generated in the past
but going forward it is going to be manually updated.

> So adjust the comment to be more generic. Also, start to adopt a
> more normal kernel code style without the tenary statements and if

s/a more/
s/tenary/ternary


> conditionals assignments that the auto generated code has.
> 
> Assisted-by: Sashiko:claude-opus-4-6
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

The review tags goes after the SOBs.

> 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>
> ---

> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 503f9a3f46d61..82dc27aecf297 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -149,6 +149,11 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>  const char *tdx_dump_mce_info(struct mce *m);
>  const struct tdx_sys_info *tdx_get_sysinfo(void);
>  
> +static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> +{
> +	return false; /* To be enabled when kernel is ready */

I would avoid the tail comment even if it is temporary.

> +}
> +
>  int tdx_guest_keyid_alloc(void);
>  u32 tdx_get_nr_guest_keyids(void);
>  void tdx_guest_keyid_free(unsigned int keyid);



> @@ -33,6 +33,18 @@ static __init int get_tdx_sys_info_features(struct tdx_sys_info_features *sysinf
>  	return ret;
>  }
>  
> +static __init int get_tdx_sys_info_tdmr_dpamt(struct tdx_sys_info_tdmr *sysinfo_tdmr)
> +{
> +	int ret;
> +	u64 val;
> +
> +	ret = read_sys_metadata_field(0x9100000100000013, &val);

Should this be a #define now that the file is being manually updated? Or
is the plan to do it all together? A #define would make it easier to
read this patch.

> +	if (!ret)
> +		sysinfo_tdmr->pamt_page_bitmap_entry_bits = val;
> +
> +	return ret;
> +}
> +
>  static __init int get_tdx_sys_info_tdmr(struct tdx_sys_info_tdmr *sysinfo_tdmr)
>  {
>  	int ret = 0;
> @@ -116,5 +128,12 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
>  	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
>  
> +	/*
> +	 * Don't treat a module that doesn't support Dynamic PAMT
> +	 * as a failure. Only read the metadata optionally.
> +	 */
> +	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> +		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);

There is a need for the comment because it combines two checks:

1) Did any of the previous stages fail?
2) Does the TDX module support Dynamic PAMT?

Should these be separated for readability and to follow the typical
kernel style?

	if (ret)
		return ret;

	if (tdx_supports_dynamic_pamt(sysinfo))
		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);

	return ret;

I think you can avoid the comment altogether in that case.

> +
>  	return ret;
>  }


^ permalink raw reply

* Re: [PATCH 00/15] Device Evidence and Trust for PCI Security Protocol (TDISP)
From: Dan Williams (nvidia) @ 2026-07-08  0:12 UTC (permalink / raw)
  To: Jason Gunthorpe, 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: <20260707124321.GF118978@ziepe.ca>

Jason Gunthorpe wrote:
> On Mon, Jul 06, 2026 at 01:55:12PM -0700, Dan Williams (nvidia) wrote:
> > Jason Gunthorpe wrote:
> > > On Sun, Jul 05, 2026 at 03:08:04PM -0700, Dan Williams wrote:
> > > > * NONE: no usage of the device unless the trust is explicitly overridden
> > > >   by user policy specified via a driver flag, module flag, or uapi (TBD).
> > > > 
> > > > * ADVERSARY: needs acknowledgement from the bus and IOMMU / DMA layers
> > > >   that the device is limited to strict IOMMU translation behavior. Drivers
> > > >   can use this as a signal to limit functionality. This designation
> > > >   implies follow-on IOMMU and bus enabling work for features like
> > > >   arranging for the device to attach to a blocked IOMMU domain when
> > > >   detached from a driver.
> > > > 
> > > > * AUTO: typical / historical Linux driver model.
> > > > 
> > > > * TCB: a trust level that only exists in Confidential Computing
> > > >   environments. When acked by the IOMMU / DMA layer it enables the device
> > > >   to issue direct-DMA to private/encrypted addresses or otherwise attach to
> > > >   a secure vIOMMU within the TCB.
> > > 
> > > I'm not sure I entirely like this one, certainly it needs to be
> > > possible to have both T=1 and ADVERSARY together.
> > 
> > T=1 and ADVERSARY are independent for link encryption and private MMIO.
> > In other words the device is placed into the TDISP RUN state independent
> > of its trust level. 
> 
> That's the right thing
> 
> > Downstream accesses to the device must have T=1, and
> > its upstream accesses will have T=1, but with force_dma_unencrypted() ==
> > true.
> 
> That should never happen. Once in RUN force_dma_unencrypyted() ==
> false, it has nothing to do with the trust level.

2 concerns:

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.

[1]: https://lore.kernel.org/all/20260326120046.GG67624@nvidia.com/

If the private-MMIO-no-private-DMA case exists then ->dma_configure()
needs a "trusted to access private-DMA" signal not a "required to access
private-DMA" signal to determine how to set force_dma_unencrypyted()
while the device is in the RUN state. I am ok to reconsider that use
case, but that was part of what lead to the proposal for
force_dma_unencrypyted() to use the trust level.

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.

echo tsm0 > $pdev/tsm/lock
cat $nonce | device-evidence dump $pdev
device-evidence validate $pdev $generation
echo full > $pdev/trust
echo 1 > $pdev/tsm/accept
<--- T=1 and force_dma_unencrypted() remains true in this window
echo $pdev > $driver/bind

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.


Another factor that lead to the assumption that "operational trust level
configures force_dma_unencrypted()" is taken from a comment from Greg
[2].

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.

My answer, given the concerns of drivers dangerously flipping the
force_dma_unencrypted() result at runtime was to place it in 'struct
device_private' and make private memory access a result of the trust
level. 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+.

[2]: http://lore.kernel.org/2026031230-mastiff-create-7593@gregkh

> Even if you set ADVERSARY it should still be bouncing partial page
> DMAs into private memory. The point of running something like this is
> to remove the shared memory attack surface - ie the hypervisor SW. The
> attack surface is reduced to the device itself by remaining in shared
> memory.

Yes.

> 
> > > I'd also argue this list is missing "FULL" trust, which is the
> > > historical Linux behavior for a normal device. AUTO should be
> > > selecting between FULL/ADVERSARY based on things like the ACPI/etc as
> > > it does today.
> > 
> > 1/ that is effectively how the UNSET level behaves. If the
> > bus has not set ADVERSARY before device_add() then the default behavior
> > is the AUTO level. Where AUTO means all of the automatic privileges a
> > device can be offered without needing any other coordination.
> 
> I think my other remark about two enums is some of the issue, the
> policy can have things like UNSET or AUTO, but once the driver starts
> to probe an in-effect mode should be computed and be concrete. Having
> a driver run with a trust mode of AUTO or UNSET is just confusing.

UNSET never escapes to something a driver would see, but yes, appears
AUTO is too confusing a term compared to FULL.

> > 2/ The ambiguity and conflict occurs at ->dma_configure() time when the
> > bus and IOMMU layer want to reject the device's access to some privilege
> > by failing. When FULL is defined as !ADVERSARY then it is difficult to
> > describe the semantics when FULL trust honors rejections to private DMA
> > and when it falls back to shared operation.
> 
> Given that the trust level shouldn't impact force_dma_unencrypted(),
> the only thing left is to setup the IOMMU differently, and maybe
> operate a driver in a hardened mode or something like that.
> 
> I don't see what the TCB is supposed to be changing here.
> 
> That leaves it just as a policy gate to check that T=1, I'm not sure
> if that is worthwhile enough for dedicated UAPI?

If we ditch the private-MMIO-no-private-DMA case then yes I can
see T=1 being a uniform requirement. 

> > The above more points to a need to have an explicit trust level for
> > adversarial private memory access. The address spaces are distinct
> > assets with different levels of trust.
> > 
> > UNSET: bus picks initial level, or leaves it to the device_core().
> > NONE:
> > ADVERSARY: Device can be in T=0, or T=1 mode (UNLOCKED, or RUN).
> > AUTO: Could rename this to be FULL or ALL or DEFAULT, I still keep
> >       coming back to the "AUTO" name because the privileges are not
> >       uniform based on the IOMMU / DMA topology and device capability.
> >       Again, the TDISP state is independent. The TSM driver does
> >       not get called to gatekeep and verify access in this mode.
> > TCB_ADVERSARY: or PRIVATE_ADVERSARY. Device can access private platform
> > 	       resources iff an enforcing IOMMU is present.
> > TCB: or PRIVATE_FULL, automatically enable all access privileges
> >      including private memory access.
> 
> Yeah, we can keep adding more modes to make a big cross product, but
> I do wonder if this is going to get too big..

The bus specific requirements like private MMIO + link encryption are
pushed out to the TSM driver. Userspace policy that wants to require
that the TSM driver is consulted on enable private DMA events needs to
arrange for it to be loaded otherwise ->dma_configure() can just proceed
with what the bus pre-configured.

> IDK, maybe it should be a bitmap instead of a level?

...not sure

> bit 0 = Force Disable
> bit 1 = Device is adversarial:
>            - Enable strong IOMMU protections
>            - Enable driver protections
> bit 2 = Require T=1
> bit 3 = Require IOMMU
> bit 4 = Require DMA/MMIO security (eg Link IDE)
>
> Where value 0 means the current level of full trust.
> 
> It is a little easier to explain what each thing is doing and easier
> to add new things

The question is whether these requirements belong on a central device
trust mechanism bitmap or should be pushed out to other ABI. 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. If T=1 is
required then either a TSM driver is present to validate it can enable
DMA, or the bus/paravisor arranges for that T=1 capability to be
pre-enabled.

Perhaps the levels are indeed:
DISABLED
ADVERSARY
FULL

...with a T=1 flag on the side that leaf drivers can not set on their
own.

> Then from a sysfs perpsective the policy would have special string
> values like 'use bus default'
> 
> > > If the trust level is reduced to just be a command to the kernel how
> > > it should operate the device then it would be up to userspace to
> > > confirm things like T=1 before setting the trust.
> > 
> > This discussion gets strained for me when T=1 is used to mean both
> > "device is in TDISP RUN (with link encryption and private MMIO)" and
> > "device is in TDISP RUN + force_dma_unencrypted() == false".
> 
> It means both things though, we really must not run with
> force_dma_unencrypted() = true when T=1, that's pointless and harmful.

Like I said above, I am ok to ditch the consideration of the
private-MMIO-no-private-DMA case.

> > Otherwise, full bi-directional T=1 before setting the trust would
> > require an IOMMU to be blocking the device until that final confirmation
> > point. Given that is not always available the proposal is to defer
> > acknowledging the trust level with the TSM until ->dma_configure().
> 
> ?? If you have no iommu the instance you set T=1 and do the platform
> step to activate DMA the device has 100% acess to all
> memory. force_dma_unencrypted() does nothing to constrain device
> access, it is all about accommodating an addressing limitation.

Right, this is my point above about why "echo 1 > $pdev/accept" does not
immediately set force_dma_unencrypted to false nor enable private DMA.

> ARM at least has a dedicated call to enable DMA. It would be nice to
> place that call right before the driver probes so DMA remains off
> until we commit to using the device. Maybe other platforms have the
> same - but I'm not sure it is *essential* as the point of setting RUN
> can reasonably be the acceptance point.

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.

> > NONE: Device core rejects device operation
> > ADVERSARY: reject device operation if an IOMMU to set IOMMU_DOMAIN_DMA
> >            not available (not in current patches)
> > AUTO: no rejections, but no private memory access either
> > TCB_ADVERSARY: reject device operation if IOMMU_DOMAIN_DMA not
> > 	       available, or TSM rejects the evidence used to enable
> >                private memory access.
> > TCB: reject device operation if TSM rejects chosen evidence
> 
> What does "TSM rejectes chosen evidence" mean? Kernel isn't supposed
> to be evaluating evidence?

The host / platform TSM is protecting the VM against evidence that has
changed since the last time the VM asked. I.e. the TSM is tracking if
the untrusted VMM relocked the device without the VM having re-read the
evidence.

^ permalink raw reply

* Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()
From: Sohil Mehta @ 2026-07-07 23:22 UTC (permalink / raw)
  To: Rick Edgecombe, bp, dave.hansen, hpa, kas, kvm, linux-coco,
	linux-doc, linux-kernel, mingo, nik.borisov, pbonzini, seanjc,
	tglx, vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
  Cc: Binbin Wu
In-Reply-To: <20260526023515.288829-2-rick.p.edgecombe@intel.com>

On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> For each memory region that the TDX module might use (called TDMR), three
> separate traditional PAMT allocations are needed. 

> One for each supported page size (1GB, 2MB, 4KB).
Missing verb in this sentence. Maybe use a '-' to merge it with the
previous sentence.


> There are some commonalities in the math needed to calculate the base and
> size for each smaller allocation, and so an effort was made to share logic
> across the three. Unfortunately doing this turned out unnaturally tortured,
> with a loop iterating over the three page sizes, only to call into a
> function with cases statement for each page size. 

Did you mean "..with a case statement for each.."?



> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 967482ae3c801..487f389f52f4b 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -516,31 +516,21 @@ static __init int fill_out_tdmrs(struct list_head *tmb_list,
>   * Calculate PAMT size given a TDMR and a page size.  The returned
>   * PAMT size is always aligned up to 4K page boundary.
>   */
> -static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz,
> -					     u16 pamt_entry_size)
> +static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz)
>  {
>  	unsigned long pamt_sz, nr_pamt_entries;
> +	const int tdx_pg_size_shift[] = { PAGE_SHIFT, PMD_SHIFT, PUD_SHIFT };

Both of these consts go hand-in-hand, right? I would write it as
tdx_pg_size_shift[TDX_PS_NR] to make the connection obvious.

Just curious, why is TDX_PS_NR defined as (TDX_PS_1G + 1)? I don't think
we are planning to add TDX_PS_256G anytime soon. But, should
TDX_PS_4K..TDX_PS_NR be an enum?

> +	const u16 pamt_entry_size[TDX_PS_NR] = {
> +		tdx_sysinfo.tdmr.pamt_4k_entry_size,
> +		tdx_sysinfo.tdmr.pamt_2m_entry_size,
> +		tdx_sysinfo.tdmr.pamt_1g_entry_size,
> +	};
>  

^ permalink raw reply

* Re: [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory
From: Edgecombe, Rick P @ 2026-07-07 22:32 UTC (permalink / raw)
  To: Zhao, Yan Y
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, kirill.shutemov@linux.intel.com, kas@kernel.org,
	seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
	pbonzini@redhat.com, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
	Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <akyGVOzkHBkST4hK@yzhao56-desk.sh.intel.com>

On Tue, 2026-07-07 at 12:53 +0800, Yan Zhao wrote:
> On Mon, May 25, 2026 at 07:35:08PM -0700, Rick Edgecombe wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> > 
> > The PAMT memory holds metadata for all possible TDX protected memory. Each
> > physical address range is covered by PAMT entries at three levels (1GB,
> > 2MB, 4KB). With Dynamic PAMT, the 4KB range of PAMT is allocated on
>                                          ^level ?
> > demand. The kernel supplies the TDX module with page pairs to store the
> > 4KB entries, which cover 2MB of host physical memory. The kernel must
> 4KB-level entries ?

Sure.

> 
> > provide this page pair before using pages from the range for TDX. If this
> > is not done, SEAMCALLs that give the pages to be protected by the TDX module
> Nit: > 75 chars per line.

Oh yea, must have been a late change.

> 
> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>


^ permalink raw reply

* Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-07 21:25 UTC (permalink / raw)
  To: Zhao, Yan Y
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, kirill.shutemov@linux.intel.com, kas@kernel.org,
	seanjc@google.com, mingo@redhat.com, linux-kernel@vger.kernel.org,
	pbonzini@redhat.com, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, tglx@kernel.org,
	Annapurve, Vishal, bp@alien8.de, Gao, Chao,
	binbin.wu@linux.intel.com, x86@kernel.org
In-Reply-To: <akx5pdOgoaGzsw/u@yzhao56-desk.sh.intel.com>

On Tue, 2026-07-07 at 11:59 +0800, Yan Zhao wrote:
> On Mon, May 25, 2026 at 07:35:06PM -0700, Rick Edgecombe wrote:
> > +static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> > +{
> > +	return false; /* To be enabled when kernel is ready */
> > +}
> > +
> Nit:
> Would the following style be better, though the comment will soon be removed?
> 
> static inline bool tdx_supports_dynamic_pamt(const struct tdx_sys_info *sysinfo)
> {
> 	 /* To be enabled when kernel is ready */
> 	return false;
> }
> 
> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>

Sure.

^ permalink raw reply

* Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()
From: Edgecombe, Rick P @ 2026-07-07 19:08 UTC (permalink / raw)
  To: Zhao, Yan Y
  Cc: linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
	Huang, Kai, kvm@vger.kernel.org, Hansen, Dave, kas@kernel.org,
	seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
	binbin.wu@linux.intel.com, nik.borisov@suse.com,
	linux-doc@vger.kernel.org, hpa@zytor.com, Annapurve, Vishal,
	tglx@kernel.org, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <akxxWF5nMbFQWlD3@yzhao56-desk.sh.intel.com>

On Tue, 2026-07-07 at 11:24 +0800, Yan Zhao wrote:
> On Tue, Jul 07, 2026 at 04:18:25AM +0800, Edgecombe, Rick P wrote:
> > On Fri, 2026-07-03 at 13:48 +0800, Chao Gao wrote:
> > > On Mon, May 25, 2026 at 07:35:05PM -0700, Rick Edgecombe wrote:
> > > > Since the loop that iterates over it is gone, further simplify the code by
> Nit:
> The patch is titled "Simplify tdmr_get_pamt_sz()" but the log mainly talks about
> removing the loop in tdmr_set_up_pamt().
> So, how about renaming the patch to "Simplify setting up pamt calculation for
> TDMRs"?

Hmm, yea. I'll change it to:

x86/virt/tdx: Simplify PAMT layout calculation

> 
> > > > dropping the array of intermediate size and base storage. Just store the
> > > > values to their final locations.
> > > 
> > > > Accept the small complication of having
> > > > to clear tdmr->pamt_4k_base in the error path, so that tdmr_do_pamt_func()
> > > > will not try to operate on the TDMR struct when attempting to free it.
> > > 
> > > The clearing of tdmr->pamt_4k_base was dropped, so this section is a bit
> > > stale. Apart from this nit,
> > 
> > Oh, good point.
> 
> With those nits fixed,
> 
> Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>

Thanks.

^ permalink raw reply

* Re: [PATCH v6 00/11] Dynamic PAMT
From: Edgecombe, Rick P @ 2026-07-07 17:25 UTC (permalink / raw)
  To: seanjc@google.com
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
	Hansen, Dave, Zhao, Yan Y, kas@kernel.org, mingo@redhat.com,
	linux-kernel@vger.kernel.org, pbonzini@redhat.com,
	nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
	tglx@kernel.org, Annapurve, Vishal, bp@alien8.de, Gao, Chao,
	x86@kernel.org
In-Reply-To: <ak0mnZKoxK5Y04Dw@google.com>

On Tue, 2026-07-07 at 09:17 -0700, Sean Christopherson wrote:
> On Mon, Jul 06, 2026, Rick P Edgecombe wrote:
> > Sean,
> > 
> > On Mon, 2026-05-25 at 19:35 -0700, Rick Edgecombe wrote:
> > >   KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
> > >   KVM: TDX: Get/put PAMT pages when (un)mapping private memory
> > 
> > Would you be willing to take a look at these two patches that need ack's from
> > the KVM side? I'm hoping to make the next version of this the last one. So it
> > would be great to get any remaining comments before then.
> 
> LGTM beyond the to_tdx() and Assited-by nits.

Thanks Sean!

^ permalink raw reply

* Re: [PATCH v7 08/22] coco: arm64: s390: powerpc: Mark secure guests with CC_ATTR_GUEST_MEM_ENCRYPT
From: Catalin Marinas @ 2026-07-07 16:58 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
In-Reply-To: <20260701054926.825925-9-aneesh.kumar@kernel.org>

On Wed, Jul 01, 2026 at 11:19:12AM +0530, Aneesh Kumar K.V (Arm) wrote:
> arm64 Realms, powerpc secure guests and s390 protected virtualization
> guests currently report CC_ATTR_MEM_ENCRYPT, but not
> CC_ATTR_GUEST_MEM_ENCRYPT.
> 
> These environments are all secure guest configurations. Their
> force_dma_unencrypted() implementations also treat the same secure guest
> state as requiring unencrypted DMA. Report CC_ATTR_GUEST_MEM_ENCRYPT for
> the same condition so generic confidential-computing code can distinguish
> guest memory encryption from host memory encryption instead of relying only
> on CC_ATTR_MEM_ENCRYPT.
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>

For arm64:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* SVSM Development Call July 8th, 2026
From: Jörg Rödel @ 2026-07-07 16:19 UTC (permalink / raw)
  To: coconut-svsm, linux-coco

Hi,

Here is the call for agenda items for this weeks SVSM development call.  Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.

We will use the LF Zoom instance. Details of the meeting  can be found in our
governance repository at:

	https://github.com/coconut-svsm/governance

The link to the COCONUT-SVSM calendar is:

	https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week

The meeting will be recorded and the recording eventually published.

Regards,

	Jörg

^ permalink raw reply

* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-07 16:18 UTC (permalink / raw)
  To: Edgecombe, Rick P
  Cc: linux-kernel@vger.kernel.org, x86@kernel.org, Gao, Chao,
	Xu, Yilun, Hansen, Dave, 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 Mon, Jul 06, 2026 at 09:44:29PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2026-07-02 at 22:46 +0800, Xu Yilun wrote:
> > TDX uses the SEAMCALL instruction to invoke various TDX module
> > functions. Just like the syscall, a SEAMCALL specifies the operation
> > using a function number and parameters. Moreover, TDX also uses SEAMCALL
> > versions to extend the functionalities of existing SEAMCALLs while
> > keeping backward compatibility. Unlike syscall versions that assign
> > brand new numbers, TDX segments the function number into a basic
> > function number field and a version field. Together, they encode the new
> > function number.
> 
> So isn't this pretty much like numbered seamcalls, except there is a special
> format for generating seamcall2, seamcall3, etc? In the end you just use a
> different number for a different version of the call. So it's just like
> syscalls, except there is a pattern in the specific number for calls of the same
> family.

OK. I will refactored this paragraph:

  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 for the same family: a base
  function number and a version together encode the full function number.

> 
> > 
> > An existing SEAMCALL (TDH.VP.INIT) helper is already using the version
>                                     ^ maybe swap the order of these two?
> > field. However, having the caller pack the version into the function
>           ^Can drop this?
> > number open-codes the ABI layout, making the SEAMCALL helper definition
> > obscure and error prone.
> 
> Do we need the second part of the sentence?
> 
> > 
> > Add a version field in struct tdx_module_args, 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.
> > 
> > As an internal implementation detail,
> 
> ^ Is this needed to make it clear?

I think these are also about "make every word count", I'll drop all
3 places above.

> 
> >  encode the
> > tdx_module_args.version in the function number before calling into
> > assembly code.
> > 
> > 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.

OK. I'll delete these adverbs.

> 
> > Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> 
> How about a link to the thread where this was suggested.
> 
> > ---
> >  arch/x86/include/asm/shared/tdx.h         |  2 ++
> >  arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 ++++++++++++++++++-
> >  arch/x86/virt/vmx/tdx/tdx.h               |  8 --------
> >  arch/x86/virt/vmx/tdx/tdx.c               |  5 +++--
> >  4 files changed, 23 insertions(+), 11 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..7002e41cddad 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,22 @@ 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)
> 
> The annoying thing is that the path touched here is also used for seamldr calls
> now, which afaict has no concept of version. Underscoring how much of a mess the
> wrapper stack is.

OK, I'll drop the wrapper.

> 
> > +
> > +static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
> > +						struct tdx_module_args *args)
> > +{
> 
> I have the same question as Xiaoyao. We have so many wrappers already.
> 
> > +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> > +
> > +	return func(fn, args);
> > +}
> > +
> >  static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  						  struct tdx_module_args *args)
> >  {
> > @@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >  	 */
> >  	this_cpu_write(cache_state_incoherent, true);
> >  
> > -	return func(fn, args);
> > +	return __seamcall_encode_fn(func, fn, args);
> >  }

^ permalink raw reply

* Re: [PATCH v6 00/11] Dynamic PAMT
From: Sean Christopherson @ 2026-07-07 16:17 UTC (permalink / raw)
  To: Rick P Edgecombe
  Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Kai Huang,
	Dave Hansen, Yan Y Zhao, kas@kernel.org, mingo@redhat.com,
	linux-kernel@vger.kernel.org, pbonzini@redhat.com,
	nik.borisov@suse.com, linux-doc@vger.kernel.org, hpa@zytor.com,
	tglx@kernel.org, Vishal Annapurve, bp@alien8.de, Chao Gao,
	x86@kernel.org
In-Reply-To: <f2b2a3a048ad6429305e0e21319a8c27bedf39db.camel@intel.com>

On Mon, Jul 06, 2026, Rick P Edgecombe wrote:
> Sean,
> 
> On Mon, 2026-05-25 at 19:35 -0700, Rick Edgecombe wrote:
> >   KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
> >   KVM: TDX: Get/put PAMT pages when (un)mapping private memory
> 
> Would you be willing to take a look at these two patches that need ack's from
> the KVM side? I'm hoping to make the next version of this the last one. So it
> would be great to get any remaining comments before then.

LGTM beyond the to_tdx() and Assited-by nits.

^ permalink raw reply

* Re: [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-07 15:45 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: x86, linux-kernel, kas, rick.p.edgecombe, dave.hansen,
	dave.hansen, yilun.xu, chao.gao, djbw, linux-coco, peter.fang
In-Reply-To: <ede965a3-0fd6-46b4-906d-716bce163036@intel.com>

On Fri, Jul 03, 2026 at 08:00:38AM +0800, Xiaoyao Li wrote:
> On 7/2/2026 10:46 PM, 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_encode_fn(sc_func_t func, u64 fn,
> > +						struct tdx_module_args *args)
> > +{
> > +	FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
> > +
> > +	return func(fn, args);
> > +}
> > +
> >   static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >   						  struct tdx_module_args *args)
> >   {
> > @@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
> >   	 */
> >   	this_cpu_write(cache_state_incoherent, true);
> > -	return func(fn, args);
> > +	return __seamcall_encode_fn(func, fn, args);
> >   }
> 
> Can we drop the new wrapper? I don't see why we need it. The wrapper makes
> the code harder to read.

OK, will call FIELD_MODIFY() directly in __seamcall_dirty_cache()
> 
> 

^ permalink raw reply

* Re: [PATCH v7 08/22] coco: arm64: s390: powerpc: Mark secure guests with CC_ATTR_GUEST_MEM_ENCRYPT
From: Suzuki K Poulose @ 2026-07-07 15:41 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
In-Reply-To: <20260701054926.825925-9-aneesh.kumar@kernel.org>

On 01/07/2026 06:49, Aneesh Kumar K.V (Arm) wrote:
> arm64 Realms, powerpc secure guests and s390 protected virtualization
> guests currently report CC_ATTR_MEM_ENCRYPT, but not
> CC_ATTR_GUEST_MEM_ENCRYPT.
> 
> These environments are all secure guest configurations. Their
> force_dma_unencrypted() implementations also treat the same secure guest
> state as requiring unencrypted DMA. Report CC_ATTR_GUEST_MEM_ENCRYPT for
> the same condition so generic confidential-computing code can distinguish
> guest memory encryption from host memory encryption instead of relying only
> on CC_ATTR_MEM_ENCRYPT.
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>


For arm64 bits:



> ---
>   arch/arm64/kernel/rsi.c                      | 1 +
>   arch/powerpc/platforms/pseries/cc_platform.c | 1 +
>   arch/s390/mm/init.c                          | 1 +
>   3 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
> index 92160f2e57ff..207e36db1e7a 100644
> --- a/arch/arm64/kernel/rsi.c
> +++ b/arch/arm64/kernel/rsi.c
> @@ -27,6 +27,7 @@ bool cc_platform_has(enum cc_attr attr)
>   {
>   	switch (attr) {
>   	case CC_ATTR_MEM_ENCRYPT:
> +	case CC_ATTR_GUEST_MEM_ENCRYPT:
>   		return is_realm_world();
>   	default:
>   		return false;

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>


Suzuki

^ permalink raw reply

* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Sean Christopherson @ 2026-07-07 14:46 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: David Laight, Dave Hansen, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, x86, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
	Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, Andi Kleen,
	Dan Williams, Borys Tsyrulnikov, kvm, linux-coco, linux-kernel,
	stable
In-Reply-To: <akaCzNRGVy5Xr-bG@thinkstation>

On Thu, Jul 02, 2026, Kiryl Shutsemau wrote:
> On Wed, Jul 01, 2026 at 06:00:33PM +0100, David Laight wrote:
> > Or be even more specific and use '& 0xffffffff' rather than a cast.
> > Particularly since the casts of the RHS in the byte/short cases aren't
> > needed at all.
> 
> I'd rather keep the body exactly as KVM has it today.

+1.  My main argument for casting in the 1-byte and 2-byte cases is consistency
above all else, using a mask for the 4-byte case defeats that goal.

> This is now a straight move + rename with no functional change, and the v4
> attempt to rewrite it with arithmetic is precisely what introduced the
> AH/CH/DH/BH clobber Sashiko flagged.  Tidying the casts turns it back into a
> rewrite and diverges from the form KVM has shipped for years.  Feel free to
> submit a separate cleanup on top if you feel strongly.
> 
> Updated patch below; I'll fold it into v6.
> 
> -- >8 --
> Subject: [PATCH] x86/insn-eval: Move assign_register() out of KVM as insn_assign_reg()
> 
> KVM's instruction emulator has a small helper, assign_register(), that
> writes a value into a register following the x86 rules for writes to
> general-purpose registers: an 8- or 16-bit write leaves the rest of the
> register untouched, a 32-bit write zero-extends the result to 64 bits,
> and a 64-bit write replaces the whole register.
> 
> The TDX guest #VE handler needs the same logic for port I/O emulation
> to get 32-bit zero-extension right.  Rather than add a third copy of
> the same switch, move the helper verbatim to <asm/insn-eval.h>, rename
> it to insn_assign_reg(), and route KVM's callers through it.
> 
> Add <asm/insn.h> to the header's includes so it builds standalone in
> callers that have not pulled it in transitively.
> 
> No functional change.
> 
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> ---

Acked-by: Sean Christopherson <seanjc@google.com>

^ permalink raw reply

* Re: [PATCH 09/15] PCI, device core: Move "untrusted" concept to DEVICE_TRUST_ADVERSARY
From: Robin Murphy @ 2026-07-07 13:04 UTC (permalink / raw)
  To: Dan Williams, linux-coco
  Cc: linux-pci, driver-core, ankita, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Will Deacon, Jason Gunthorpe,
	Bjorn Helgaas, Alexey Kardashevskiy, Xu Yilun, Aneesh Kumar K.V,
	Dexuan Cui
In-Reply-To: <20260705220819.2472765-10-djbw@kernel.org>

On 05/07/2026 11:08 pm, Dan Williams wrote:
> An "adversary" device is one that the kernel is allowed to operate, but
> with extra safety / paranoia added. It aims to keep the device bounded in
> both address space and time (no batched invalidation) for DMA accesses. The
> concept of adversarial devices already exists in the PCI core. The
> categorization is applied to externally attached PCI devices by default
> (like Thunderbolt attached devices). Unlike DEVICE_TRUST_NONE that says "do
> not allow driver bind", DEVICE_TRUST_ADVERSARY says "allow driver bind as
> long as the driver and/or IOMMU are taking precautions." Uplevel this trust
> mechanism from a PCI device "untrusted" boolean flag to the core device
> trust level.
> 
> The rationale for dev->bus_trust separate from dev->p->trust is to give
> buses the ability to establish a trust level before device_add(). Then,
> after device_add(), the dependency on 'struct device_private' mandates that
> the device-core own and coordinate changes to the device's trust level.
> 
> There are implications of the bus expressing less than full trust in a
> device and a module later expressing higher levels of trust. For PCI, the
> "adversary" / pci_untrusted() determination is made prior to initial
> iommu_probe_device(). The default IOMMU domain determination is latched
> from bus's specified trust value. Later changes to the trust level, for
> example by "modprobe $module trust=auto" skip that IOMMU probing. The
> result is that moving a device to DEVICE_TRUST_AUTO may lift translation
> blocking (PCI_ACS_TB), but not change the default IOMMU domain from
> IOMMU_DOMAIN_DMA.
> 
> The default domain can be validated / changed via iommu_group sysfs. The
> longer term goal is to dynamically adjust iommu domain at the next
> $bus_dma_configure() event, but that needs more surgery and consideration
> for multi-device iommu groups.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> Cc: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> ---
>   drivers/base/Kconfig         | 13 +++++++++++++
>   drivers/pci/Kconfig          |  1 +
>   include/linux/device.h       |  3 +++
>   include/linux/device/trust.h | 20 +++++++++++++++-----
>   include/linux/pci.h          | 13 +++++--------
>   drivers/base/trust.c         | 20 +++++++++++++++++++-
>   drivers/iommu/amd/iommu.c    |  2 +-
>   drivers/iommu/dma-iommu.c    | 13 ++++---------
>   drivers/iommu/intel/iommu.c  |  2 +-
>   drivers/iommu/iommu.c        |  2 +-

For the straightforward drivers/iommu changes:

Acked-by: Robin Murphy <robin.murphy@arm.com>

I don't have time to have a strong opinion about the internal 
implementation details, but I agree a driver-core-level abstraction of 
hierarchical bus/device trust seems like the right step to take in general.

Cheers,
Robin.

>   drivers/pci/ats.c            |  2 +-
>   drivers/pci/pci.c            |  2 +-
>   drivers/pci/probe.c          | 10 +++++++---
>   drivers/pci/quirks.c         |  4 ++--
>   14 files changed, 74 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index a08523d348d8..a4233bdf9804 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -291,6 +291,14 @@ config DEVICE_TRUST_NONE
>   	  possible, the device is blocked by an IOMMU from accessing
>   	  assets.
>   
> +config DEVICE_TRUST_ADVERSARY
> +	bool "Adversary"
> +	help
> +	  Device is allowed to bind. Bus, IOMMU, and driver layers may
> +	  react to this trust level by disabling access validation
> +	  bypass mechanisms like PCI ATS. When device is unbound from a
> +	  driver the device is blocked by an IOMMU where possible.
> +
>   config DEVICE_TRUST_AUTO
>   	bool "Auto"
>   	help
> @@ -317,6 +325,11 @@ config BUILTIN_DEVICE_TRUST_AUTO
>   	  a driver and deploy all available mechanisms to allow performant
>   	  direct memory access This trust level does not grant TCB privileges.
>   
> +config BUILTIN_DEVICE_TRUST_ADVERSARY
> +	bool "Adversary"
> +	help
> +	  Deploy mitigations in the IOMMU layer and driver to limit access.
> +
>   endchoice
>   
>   endmenu
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index ed17b5d2d5ae..185b42cefe20 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -22,6 +22,7 @@ menuconfig PCI
>   	bool "PCI support"
>   	depends on HAVE_PCI
>   	depends on MMU
> +	select DEVICE_TRUST
>   	help
>   	  This option enables support for the PCI local bus, including
>   	  support for PCI-X and the foundations for PCI Express support.
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 7b2baffdd2f5..3e203d573a58 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -688,6 +688,8 @@ enum struct_device_flags {
>    * @removable:  Whether the device can be removed from the system. This
>    *              should be set by the subsystem / bus driver that discovered
>    *              the device.
> + * @bus_trust: Device's initial / prior to device_add() trust level.
> + *
>    * @flags:	DEV_FLAG_XXX flags. Use atomic bitfield operations to modify.
>    *
>    * At the lowest level, every device in a Linux system is represented by an
> @@ -791,6 +793,7 @@ struct device {
>   	struct device_physical_location *physical_location;
>   
>   	enum device_removable	removable;
> +	enum device_trust	bus_trust;
>   
>   	DECLARE_BITMAP(flags, DEV_FLAG_COUNT);
>   };
> diff --git a/include/linux/device/trust.h b/include/linux/device/trust.h
> index 3377d26dc485..283d3196e5e6 100644
> --- a/include/linux/device/trust.h
> +++ b/include/linux/device/trust.h
> @@ -11,30 +11,40 @@
>    *
>    * @DEVICE_TRUST_UNSET: Unregistered device object with no current bus
>    * @DEVICE_TRUST_NONE: Blocked when idle, cannot bind
> + * @DEVICE_TRUST_ADVERSARY: Blocked when idle, constrained when active.
>    * @DEVICE_TRUST_AUTO: All typical privileges granted
> + *
> + * Devices flagged as adversarial are the ones that can potentially
> + * execute DMA attacks and similar. They are typically connected through
> + * external ports such as Thunderbolt but not limited to that. When an
> + * IOMMU is enabled they should be getting full mappings to make sure
> + * they cannot access arbitrary memory.
>    */
>   enum device_trust {
>   	DEVICE_TRUST_UNSET,
>   	DEVICE_TRUST_NONE,
> +	DEVICE_TRUST_ADVERSARY,
>   	DEVICE_TRUST_AUTO,
>   };
>   
> -#define DEVICE_DEFAULT_TRUST                                        \
> -	(IS_ENABLED(CONFIG_DEVICE_TRUST_NONE) ? DEVICE_TRUST_NONE : \
> -						DEVICE_TRUST_AUTO)
> +#define DEVICE_DEFAULT_TRUST \
> +	(IS_ENABLED(CONFIG_DEVICE_TRUST_NONE)      ? DEVICE_TRUST_NONE      : \
> +	 IS_ENABLED(CONFIG_DEVICE_TRUST_ADVERSARY) ? DEVICE_TRUST_ADVERSARY : \
> +	 DEVICE_TRUST_AUTO)
>   
>   struct device;
>   struct device_driver;
>   
>   #ifdef CONFIG_DEVICE_TRUST
> +bool device_untrusted(struct device *dev);
>   void module_driver_trust(struct module *mod, const char *val);
> -void module_driver_trust_init(struct module *mod, bool distrust);
> +void module_driver_trust_init(struct module *mod, bool require_trust);
>   #else
>   static inline void module_driver_trust(struct module *mod, const char *val)
>   {
>   	pr_warn("module: %s: trust= support disabled\n", mod->name);
>   }
> -static inline void module_driver_trust_init(struct module *mod, bool distrust)
> +static inline void module_driver_trust_init(struct module *mod, bool require_trust)
>   {
>   }
>   #endif
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index ebb5b9d76360..095ea37fc6d0 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -485,14 +485,6 @@ struct pci_dev {
>   	unsigned int	shpc_managed:1;		/* SHPC owned by shpchp */
>   	unsigned int	is_thunderbolt:1;	/* Thunderbolt controller */
>   	unsigned int	is_cxl:1;               /* Compute Express Link (CXL) */
> -	/*
> -	 * Devices marked being untrusted are the ones that can potentially
> -	 * execute DMA attacks and similar. They are typically connected
> -	 * through external ports such as Thunderbolt but not limited to
> -	 * that. When an IOMMU is enabled they should be getting full
> -	 * mappings to make sure they cannot access arbitrary memory.
> -	 */
> -	unsigned int	untrusted:1;
>   	/*
>   	 * Info from the platform, e.g., ACPI or device tree, may mark a
>   	 * device as "external-facing".  An external-facing device is
> @@ -812,6 +804,11 @@ static inline bool pcie_is_cxl(struct pci_dev *pci_dev)
>   	return pci_dev->is_cxl;
>   }
>   
> +static inline bool pci_untrusted(struct pci_dev *pdev)
> +{
> +	return device_untrusted(&pdev->dev);
> +}
> +
>   #define for_each_pci_bridge(dev, bus)				\
>   	list_for_each_entry(dev, &bus->devices, bus_list)	\
>   		if (!pci_is_bridge(dev)) {} else
> diff --git a/drivers/base/trust.c b/drivers/base/trust.c
> index 0fd494e1557d..8efbe5c51250 100644
> --- a/drivers/base/trust.c
> +++ b/drivers/base/trust.c
> @@ -6,15 +6,28 @@
>   #include <linux/module.h>
>   #include "base.h"
>   
> +/* If the bus did not initialize trust, set a default */
>   void device_initialize_trust(struct device *dev)
>   {
> +	dev->p->trust = dev->bus_trust;
>   	if (dev->p->trust == DEVICE_TRUST_UNSET)
>   		dev->p->trust = DEVICE_DEFAULT_TRUST;
>   }
>   
> +/*
> + * ->bus_trust is evaluated / manipulated prior to device_add() and
> + *  synced with dev->p->trust post device_add() under device_lock().
> + */
> +bool device_untrusted(struct device *dev)
> +{
> +	return dev->bus_trust && dev->bus_trust <= DEVICE_TRUST_ADVERSARY;
> +}
> +
>   /* Driver trust policy requires modules, builtin drivers always attach */
>   static enum device_trust builtin_driver_trust(void)
>   {
> +	if (IS_ENABLED(CONFIG_BUILTIN_DEVICE_TRUST_ADVERSARY))
> +		return DEVICE_TRUST_ADVERSARY;
>   	return DEVICE_TRUST_AUTO;
>   }
>   
> @@ -30,18 +43,23 @@ static enum device_trust driver_trust(struct module *mod)
>    * policy on trusting devices it attaches, update the device's trust
>    * level from that policy. Trust privileges beyond driver bind are
>    * realized in a bus's ->dma_configure().
> + *
> + * Reflect the operational trust level back to the public indicator.
>    */
>   bool device_trust_bind(const struct device_driver *drv, struct device *dev)
>   {
>   	enum device_trust drv_trust = driver_trust(drv->owner);
>   
> -	if (drv_trust != DEVICE_TRUST_UNSET)
> +	if (drv_trust != DEVICE_TRUST_UNSET) {
>   		dev->p->trust = drv_trust;
> +		dev->bus_trust = drv_trust;
> +	}
>   	return dev->p->trust > DEVICE_TRUST_NONE;
>   }
>   
>   static const char * const device_trust_names[] = {
>   	[DEVICE_TRUST_NONE]	 = "none",
> +	[DEVICE_TRUST_ADVERSARY] = "adversary",
>   	[DEVICE_TRUST_AUTO]	 = "auto",
>   };
>   
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 563f9c2672d5..ef663e3efc70 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3121,7 +3121,7 @@ static int amd_iommu_def_domain_type(struct device *dev)
>   		return 0;
>   
>   	/* Always use DMA domain for untrusted device */
> -	if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
> +	if (device_untrusted(dev))
>   		return IOMMU_DOMAIN_DMA;
>   
>   	/*
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 9abaec0703ef..957ef77911a9 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -588,16 +588,11 @@ static int iova_reserve_iommu_regions(struct device *dev,
>   	return ret;
>   }
>   
> -static bool dev_is_untrusted(struct device *dev)
> -{
> -	return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
> -}
> -
>   static bool dev_use_swiotlb(struct device *dev, size_t size,
>   			    enum dma_data_direction dir)
>   {
>   	return IS_ENABLED(CONFIG_SWIOTLB) &&
> -		(dev_is_untrusted(dev) ||
> +		(device_untrusted(dev) ||
>   		 dma_kmalloc_needs_bounce(dev, size, dir));
>   }
>   
> @@ -610,7 +605,7 @@ static bool dev_use_sg_swiotlb(struct device *dev, struct scatterlist *sg,
>   	if (!IS_ENABLED(CONFIG_SWIOTLB))
>   		return false;
>   
> -	if (dev_is_untrusted(dev))
> +	if (device_untrusted(dev))
>   		return true;
>   
>   	/*
> @@ -1188,7 +1183,7 @@ static phys_addr_t iommu_dma_map_swiotlb(struct device *dev, phys_addr_t phys,
>   	 * swiotlb_tbl_map_single() has initialized the bounce buffer proper to
>   	 * the contents of the original memory buffer.
>   	 */
> -	if (phys != (phys_addr_t)DMA_MAPPING_ERROR && dev_is_untrusted(dev)) {
> +	if (phys != (phys_addr_t)DMA_MAPPING_ERROR && device_untrusted(dev)) {
>   		size_t start, virt = (size_t)phys_to_virt(phys);
>   
>   		/* Pre-padding */
> @@ -1761,7 +1756,7 @@ size_t iommu_dma_opt_mapping_size(void)
>   
>   size_t iommu_dma_max_mapping_size(struct device *dev)
>   {
> -	if (dev_is_untrusted(dev))
> +	if (device_untrusted(dev))
>   		return swiotlb_max_mapping_size(dev);
>   
>   	return SIZE_MAX;
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 849d06dfe1ae..82d9abe87d99 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3501,7 +3501,7 @@ static bool intel_iommu_is_attach_deferred(struct device *dev)
>    */
>   static bool risky_device(struct pci_dev *pdev)
>   {
> -	if (pdev->untrusted) {
> +	if (pci_untrusted(pdev)) {
>   		pci_info(pdev,
>   			 "Skipping IOMMU quirk for dev [%04X:%04X] on untrusted PCI link\n",
>   			 pdev->vendor, pdev->device);
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e8f13dcebbde..4769d2b548d6 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1901,7 +1901,7 @@ static int iommu_get_default_domain_type(struct iommu_group *group,
>   		driver_type = iommu_get_def_domain_type(group, gdev->dev,
>   							driver_type);
>   
> -		if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->untrusted) {
> +		if (device_untrusted(gdev->dev)) {
>   			/*
>   			 * No ARM32 using systems will set untrusted, it cannot
>   			 * work.
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 96efa00d9743..7296f0ce7530 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -43,7 +43,7 @@ bool pci_ats_supported(struct pci_dev *dev)
>   	if (!dev->ats_cap)
>   		return false;
>   
> -	return (dev->untrusted == 0);
> +	return !pci_untrusted(dev);
>   }
>   EXPORT_SYMBOL_GPL(pci_ats_supported);
>   
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee61..ff1ca74f30b1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -994,7 +994,7 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
>   	caps->ctrl |= (dev->acs_capabilities & PCI_ACS_UF);
>   
>   	/* Enable Translation Blocking for external devices and noats */
> -	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
> +	if (pci_ats_disabled() || dev->external_facing || pci_untrusted(dev))
>   		caps->ctrl |= (dev->acs_capabilities & PCI_ACS_TB);
>   }
>   
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dd0abbc63e18..91a7bbcfdecb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -6,6 +6,7 @@
>   #include <linux/array_size.h>
>   #include <linux/kernel.h>
>   #include <linux/delay.h>
> +#include <linux/device/trust.h>
>   #include <linux/init.h>
>   #include <linux/pci.h>
>   #include <linux/msi.h>
> @@ -1737,20 +1738,23 @@ static void set_pcie_untrusted(struct pci_dev *dev)
>   {
>   	struct pci_dev *parent = pci_upstream_bridge(dev);
>   
> +	dev->dev.bus_trust = DEVICE_DEFAULT_TRUST;
>   	if (!parent)
>   		return;
>   	/*
>   	 * If the upstream bridge is untrusted we treat this device as
>   	 * untrusted as well.
>   	 */
> -	if (parent->untrusted) {
> -		dev->untrusted = true;
> +	if (pci_untrusted(parent)) {
> +		dev->dev.bus_trust =
> +			min(dev->dev.bus_trust, parent->dev.bus_trust);
>   		return;
>   	}
>   
>   	if (arch_pci_dev_is_removable(dev)) {
>   		pci_dbg(dev, "marking as untrusted\n");
> -		dev->untrusted = true;
> +		dev->dev.bus_trust =
> +			min(dev->dev.bus_trust, DEVICE_TRUST_ADVERSARY);
>   	}
>   }
>   
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b09f27f7846f..73e473856999 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5370,7 +5370,7 @@ static void pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
>    * PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF
>    *
>    * TODO: This quirk also needs to do equivalent of PCI_ACS_TB,
> - * if dev->external_facing || dev->untrusted
> + * if dev->external_facing || pci_untrusted(dev)
>    */
>   static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev)
>   {
> @@ -5411,7 +5411,7 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
>   	ctrl |= (cap & PCI_ACS_CR);
>   	ctrl |= (cap & PCI_ACS_UF);
>   
> -	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
> +	if (pci_ats_disabled() || dev->external_facing || pci_untrusted(dev))
>   		ctrl |= (cap & PCI_ACS_TB);
>   
>   	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);


^ permalink raw reply

* Re: [PATCH v7 00/22] dma-mapping: Track shared DMA state through direct, pool and swiotlb paths
From: Marek Szyprowski @ 2026-07-07 13:03 UTC (permalink / raw)
  To: Aneesh Kumar K.V, iommu, linux-arm-kernel, linux-kernel,
	linux-coco
  Cc: Robin Murphy, Will Deacon, Marc Zyngier, Steven Price,
	Suzuki K Poulose, Catalin Marinas, Jiri Pirko, Jason Gunthorpe,
	Mostafa Saleh, Petr Tesarik, Alexey Kardashevskiy, Dan Williams,
	Xu Yilun, linuxppc-dev, linux-s390, Madhavan Srinivasan,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
	Alexander Gordeev, Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
	Christian Borntraeger, Sven Schnelle, x86
In-Reply-To: <yq5av7argr65.fsf@kernel.org>

On 07.07.2026 10:06, Aneesh Kumar K.V wrote:
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> writes:
>
>> This series tracks confidential-computing shared DMA state through the
>> dma-direct, dma-pool, and swiotlb paths so that encrypted and decrypted
>> DMA buffers are handled consistently.
>>
>> Today, the direct DMA path mostly relies on force_dma_unencrypted() for
>> shared/decrypted buffer handling. This series consolidates the
>> force_dma_unencrypted() checks in the top-level functions and ensures
>> that the remaining DMA interfaces use DMA attributes to make the correct
>> decisions.
>>
>> The series separates mapping and allocation state:
>> - DMA_ATTR_CC_SHARED describes the DMA address attribute requested for a
>>   mapping. It tells the DMA mapping path that the DMA address must target
>>   shared/decrypted memory.
>> - __DMA_ATTR_ALLOC_CC_SHARED is an internal DMA-mapping attribute used only
>>   by allocation paths after the DMA core decides that the backing pages
>>   must be allocated as shared/decrypted memory.
>>
>> The series:
>> - moves swiotlb-backed allocations out of __dma_direct_alloc_pages(),
>> - uses __DMA_ATTR_ALLOC_CC_SHARED through the dma-direct alloc/free paths
>> - teaches the atomic DMA pools to track encrypted versus decrypted
>>   state
>> - tracks swiotlb pool encryption state and enforces strict pool
>>   selection
>> - centralizes encrypted/decrypted pgprot handling in dma_pgprot() using
>>   DMA attributes
>> - passes DMA attributes down to dma_capable() so capability checks can
>>   validate whether the selected DMA address encoding matches
>>   DMA_ATTR_CC_SHARED
>> - makes dma_direct_map_phys() choose the DMA address encoding from
>>   DMA_ATTR_CC_SHARED and fall back to swiotlb when a shared DMA request
>>   cannot use the direct mapping, which lets arm64 and x86 CCA guests stop
>>   relying on SWIOTLB_FORCE for DMA mappings
>> - use the selected swiotlb pool state to derive the returned DMA
>>   address
>> - reports CC_ATTR_GUEST_MEM_ENCRYPT for arm64 Realms, powerpc secure
>>   guests, and s390 protected virtualization guests.
>>
>> Dependency:
>> This series depends on the pKVM changes posted at:
>> https://lore.kernel.org/all/20260603110522.3331819-1-smostafa@google.com
>>
>> Please merge this series only after the pKVM changes above are merged.
>> Otherwise pKVM will be broken.
>>
> A rebased tree on top of the dependent pKVM changes can be found at:
> https://gitlab.arm.com/linux-arm/linux-cca/-/tree/scratch/pkvm/testing?ref_type=heads
>
> The patches had minor conflicts. I am not sure how we want to get this
> merged.
>
> Should we ask the pKVM maintainers for a topic branch, and then I can
> repost the updated series on top of that?
I'm fine with merging on top of the topic branch and I assume that this
patchset is mature enough to give it a try in linux-next, but first I
would like to get a review or at least acks from others with good CC
knowledge or experience.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox