* Re: [PATCH 1/1] virt: tdx-guest: Optimize the get-quote polling interval time
From: Kuppuswamy Sathyanarayanan @ 2026-02-20 18:45 UTC (permalink / raw)
To: Jun Miao, kas, dave.hansen, rick.p.edgecombe
Cc: linux-coco, kvm, linux-kernel
In-Reply-To: <20260211085801.4036464-2-jun.miao@intel.com>
Hi Miao,
On 2/11/2026 12:58 AM, Jun Miao wrote:
> The TD guest sends TDREPORT to the TD Quoting Enclave via a vsock or
> a tdvmcall. In general, vsock is indeed much faster than tdvmcall,
> and Quote requests usually take a few millisecond to complete rather
> than seconds based on actual measurements.
>
> The following get quote time via tdvmcall were obtained on the GNR:
>
> | msleep_interruptible(time) | 1s | 5ms | 1ms |
> | ------------------------------ | -------- | -------- | ---------- |
> | Duration | 1.004 s | 1.005 s | 1.036 s |
> | Total(Get Quote) | 167 | 142 | 167 |
> | Success: | 167 | 142 | 167 |
> | Failure: | 0 | 0 | 0 |
> | Avg total / 1s | 0.97 | 141.31 | 166.35 |
> | Avg success / 1s | 0.97 | 141.31 | 166.35 |
> | Avg total / 1s / thread | 0.97 | 141.31 | 166.35 |
> | Avg success / 1s / thread | 0.97 | 141.31 | 166.35 |
> | Min elapsed_time | 1025.95ms| 6.85 ms | 2.99 ms |
> | Max elapsed_time | 1025.95ms| 10.93 ms | 10.76 ms |
>
Thanks for sharing the data!
> According to trace analysis, the typical execution tdvmcall get the
> quote time is 4 ms. Therefore, 5 ms is a reasonable balance between
> performance efficiency and CPU overhead.
Since the average is 4 ms, why choose 5ms?
>
> And compared to the previous throughput of one request per second,
> the current 5ms can get 142 requests per second delivers a
> 142× performance improvement, which is critical for high-frequency
> use cases without vsock.
Is this addressing a real customer issue or a theoretical improvement?
If this is solving a real problem, could you share more details about
the use case and Quoting Service implementation you're testing against?
I ask because the Quote completion time depends heavily on the Quoting
Service implementation, which varies by deployment. Since we're optimizing
for performance, I'm wondering if we should consider an interrupt-based
approach using the SetupEventNotifyInterrupt TDVMCALL instead of polling.
>
> So, change the 1s (MSEC_PER_SEC) -> 5ms (MSEC_PER_SEC / 200)
>
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> ---
> drivers/virt/coco/tdx-guest/tdx-guest.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 4e239ec960c9..71d2d7304b1a 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -251,11 +251,11 @@ static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeou
> int i = 0;
>
> /*
> - * Quote requests usually take a few seconds to complete, so waking up
> - * once per second to recheck the status is fine for this use case.
> + * Quote requests usually take a few milliseconds to complete, so waking up
> + * once per 5 milliseconds to recheck the status is fine for this use case.
> */
> - while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
> - if (msleep_interruptible(MSEC_PER_SEC))
> + while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < 200 * timeout) {
> + if (msleep_interruptible(MSEC_PER_SEC / 200))
> return -EINTR;
> }
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Trilok Soni @ 2026-02-20 21:48 UTC (permalink / raw)
To: Andre Przywara, Salman Nabi
Cc: mark.rutland, lpieralisi, linux-kernel, vwadekar, sdonthineni,
vsethi, linux-coco, sudeep.holla, ardb, linux-arm-kernel,
chao.gao
In-Reply-To: <20260220173929.02bc2cf2@orionap.fritz.box>
On 2/20/2026 9:39 AM, Andre Przywara wrote:
> On Mon, 19 Jan 2026 12:27:29 +0000
> Salman Nabi <salman.nabi@arm.com> wrote:
>
> Hey,
>
> for the records: while working on improving the patch and during internal
> review, we found some bugs and issues in there. I will mark them below for
> the benefit of others. I will send a v2 after -rc1, with those things
> fixed and some improvements, and will include Vedashree's patches, so
> that we have everything in one series.
>
>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>> activating firmware components without a reboot. Those components
>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>> at system runtime, without requiring a reboot.
>> The specification covers the SMCCC interface to list and query available
>> components and eventually trigger the activation.
>>
>> Add a new directory under /sys/firmware to present firmware components
>> capable of live activation. Each of them is a directory under lfa/,
>> and is identified via its GUID. The activation will be triggered by echoing
>> "1" into the "activate" file:
>> ==========================================
>> /sys/firmware/lfa # ls -l . 6c*
>> .:
>> total 0
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 47d4086d-4cfe-9846-9b95-2950cbbd5a00
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 6c0762a6-12f2-4b56-92cb-ba8f633606d9
>> drwxr-xr-x 2 0 0 0 Jan 19 11:33 d6d0eea7-fcea-d54b-9782-9934f234b6e4
>>
>> 6c0762a6-12f2-4b56-92cb-ba8f633606d9:
>> total 0
>> --w------- 1 0 0 4096 Jan 19 11:33 activate
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_capable
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 activation_pending
>> --w------- 1 0 0 4096 Jan 19 11:33 cancel
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 cpu_rendezvous
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 current_version
>> -rw-r--r-- 1 0 0 4096 Jan 19 11:33 force_cpu_rendezvous
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 may_reset_cpu
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 name
>> -r--r--r-- 1 0 0 4096 Jan 19 11:33 pending_version
>> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # grep . *
>> grep: activate: Permission denied
>> activation_capable:1
>> activation_pending:1
>> grep: cancel: Permission denied
>> cpu_rendezvous:1
>> current_version:0.0
>> force_cpu_rendezvous:1
>> may_reset_cpu:0
>> name:TF-RMM
>> pending_version:0.0
>> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 # echo 1 > activate
>> [ 2825.797871] Arm LFA: firmware activation succeeded.
>> /sys/firmware/lfa/6c0762a6-12f2-4b56-92cb-ba8f633606d9 #
>> ==========================================
>>
>> [1] https://developer.arm.com/documentation/den0147/latest/
>>
>> Signed-off-by: Salman Nabi <salman.nabi@arm.com>
>> ---
>> drivers/firmware/smccc/Kconfig | 8 +
>> drivers/firmware/smccc/Makefile | 1 +
>> drivers/firmware/smccc/lfa_fw.c | 668 ++++++++++++++++++++++++++++++++
>> 3 files changed, 677 insertions(+)
>> create mode 100644 drivers/firmware/smccc/lfa_fw.c
>>
>> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
>> index 15e7466179a6..ff7ca49486b0 100644
>> --- a/drivers/firmware/smccc/Kconfig
>> +++ b/drivers/firmware/smccc/Kconfig
>> @@ -23,3 +23,11 @@ config ARM_SMCCC_SOC_ID
>> help
>> Include support for the SoC bus on the ARM SMCCC firmware based
>> platforms providing some sysfs information about the SoC variant.
>> +
>> +config ARM_LFA
>> + tristate "Arm Live Firmware activation support"
>> + depends on HAVE_ARM_SMCCC_DISCOVERY
>
> As the kernel test robot correctly pointed out, this only works on ARM64.
> SMCCC v1.2 is only defined for AArch64, and the LFA spec documents
> actually explicitly mentions AArch64-only at the beginning.
Thank you for the update. How to test these patches outside of NVIDIA devices?
Is it possible to test them on the QEMU or FVP? Any instructions on doing
these tests will be helpful w/ the virtual platforms.
---Trilok Soni
^ permalink raw reply
* RE: [PATCH 1/1] virt: tdx-guest: Optimize the get-quote polling interval time
From: Miao, Jun @ 2026-02-22 2:17 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan, kas@kernel.org,
dave.hansen@linux.intel.com, Edgecombe, Rick P
Cc: linux-coco@lists.linux.dev, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Miao, Jun
In-Reply-To: <1cc137df-0940-4eb4-b7c3-2e5e8948d9f5@linux.intel.com>
>On 2/11/2026 12:58 AM, Jun Miao wrote:
>> The TD guest sends TDREPORT to the TD Quoting Enclave via a vsock or a
>> tdvmcall. In general, vsock is indeed much faster than tdvmcall, and
>> Quote requests usually take a few millisecond to complete rather than
>> seconds based on actual measurements.
>>
>> The following get quote time via tdvmcall were obtained on the GNR:
>>
>> | msleep_interruptible(time) | 1s | 5ms | 1ms |
>> | ------------------------------ | -------- | -------- | ---------- |
>> | Duration | 1.004 s | 1.005 s | 1.036 s |
>> | Total(Get Quote) | 167 | 142 | 167 |
>> | Success: | 167 | 142 | 167 |
>> | Failure: | 0 | 0 | 0 |
>> | Avg total / 1s | 0.97 | 141.31 | 166.35 |
>> | Avg success / 1s | 0.97 | 141.31 | 166.35 |
>> | Avg total / 1s / thread | 0.97 | 141.31 | 166.35 |
>> | Avg success / 1s / thread | 0.97 | 141.31 | 166.35 |
>> | Min elapsed_time | 1025.95ms| 6.85 ms | 2.99 ms |
>> | Max elapsed_time | 1025.95ms| 10.93 ms | 10.76 ms |
>>
>
>Thanks for sharing the data!
>
>> According to trace analysis, the typical execution tdvmcall get the
>> quote time is 4 ms. Therefore, 5 ms is a reasonable balance between
>> performance efficiency and CPU overhead.
>
>Since the average is 4 ms, why choose 5ms?
>
>>
>> And compared to the previous throughput of one request per second, the
>> current 5ms can get 142 requests per second delivers a 142×
>> performance improvement, which is critical for high-frequency use
>> cases without vsock.
>
>Is this addressing a real customer issue or a theoretical improvement?
Hi Kuppuswamy,
From the customer issue, the more detail "Test Report"
[PATCH 0/1] [Test Report] get qutoe time via tdvmcall
[Background]
Currently, many mobile device vendors (such as OPPO and Xiaomi) use TDVM for security management.
Each mobile terminal must perform remote attestation before it can access the TDVM confidential container.
As a result, there are a large number of remote attestation get-quote requests, especially in cases where vsock
is not configured or misconfigured and cannot be used.
>If this is solving a real problem, could you share more details about the use case
>and Quoting Service implementation you're testing against?
>
Version Service chooses v1.22 DCAP:
https://download.01.org/intel-sgx/sgx-dcap/1.22/
Which includes the test case tdx-quote-generation-sample.
And the test case which I have shared all the test examples and the complete test environment with you through the team.
I’m curious about how the 1-second figure was obtained.
Was it based on actual test data, or was it just an estimate?
Warm regards
Jun Miao
>I ask because the Quote completion time depends heavily on the Quoting Service
>implementation, which varies by deployment. Since we're optimizing for
>performance, I'm wondering if we should consider an interrupt-based approach
>using the SetupEventNotifyInterrupt TDVMCALL instead of polling.
>
>>
>> So, change the 1s (MSEC_PER_SEC) -> 5ms (MSEC_PER_SEC / 200)
>>
>> Signed-off-by: Jun Miao <jun.miao@intel.com>
>> ---
>> drivers/virt/coco/tdx-guest/tdx-guest.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> index 4e239ec960c9..71d2d7304b1a 100644
>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
>> @@ -251,11 +251,11 @@ static int wait_for_quote_completion(struct
>tdx_quote_buf *quote_buf, u32 timeou
>> int i = 0;
>>
>> /*
>> - * Quote requests usually take a few seconds to complete, so waking up
>> - * once per second to recheck the status is fine for this use case.
>> + * Quote requests usually take a few milliseconds to complete, so waking
>up
>> + * once per 5 milliseconds to recheck the status is fine for this use case.
>> */
>> - while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
>> - if (msleep_interruptible(MSEC_PER_SEC))
>> + while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < 200 *
>timeout) {
>> + if (msleep_interruptible(MSEC_PER_SEC / 200))
>> return -EINTR;
>> }
>>
>
>--
>Sathyanarayanan Kuppuswamy
>Linux Kernel Developer
^ permalink raw reply
* Re: [PATCH v4 21/24] x86/virt/tdx: Avoid updates during update-sensitive operations
From: Huang, Kai @ 2026-02-23 4:58 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, pbonzini@redhat.com, hpa@zytor.com,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
Edgecombe, Rick P, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-22-chao.gao@intel.com>
>
> The TDX Module offers two solutions:
>
> 1. Avoid updates during update-sensitive times
>
> The host VMM can instruct TDH.SYS.SHUTDOWN to fail if any of the TDs
> are currently in any update-sensitive cases.
>
> 2. Detect incompatibility after updates
>
> On TDH.SYS.UPDATE, the host VMM can configure the TDX Module to detect
> actual incompatibility cases. The TDX Module will then return a special
> error to signal the incompatibility, allowing the host VMM to restart
> the update-sensitive operations.
>
> Implement option #1 to fail updates if the feature is available. Also,
> distinguish this update failure from other failures by returning -EBUSY,
> which will be converted to a firmware update error code indicating that the
> firmware is busy.
>
> Options like "do nothing" or option #2 are not viable [1] because the
> former allows damage to propagate to multiple, potentially unknown
> components (adding significant complexity to the whole ecosystem), while
> the latter may make existing KVM ioctls unstable.
>
[...]
>
> +#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE BIT(16)
> +
> int tdx_module_shutdown(void)
> {
> struct tdx_module_args args = {};
> - int ret, cpu;
> + u64 ret;
> + int cpu;
>
> /*
> * Shut down the TDX Module and prepare handoff data for the next
> @@ -1189,9 +1192,21 @@ int tdx_module_shutdown(void)
> * modules as new modules likely have higher handoff version.
> */
> args.rcx = tdx_sysinfo.handoff.module_hv;
> - ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> - if (ret)
> - return ret;
> +
> + if (tdx_supports_update_compatibility(&tdx_sysinfo))
> + args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
> +
> + ret = seamcall(TDH_SYS_SHUTDOWN, &args);
> +
> + /*
> + * Return -EBUSY to signal that there is one or more ongoing flows
> + * which may not be compatible with an updated TDX module, so that
> + * userspace can retry on this error.
> + */
> + if ((ret & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
> + return -EBUSY;
> + else if (ret)
> + return -EIO;
>
The changelog says "doing nothing" isn't an option, and we need to depend on
TDH.SYS.SHUTDOWN to catch such incompatibilities.
To me this means we cannot support module update if TDH.SYS.SHUTDOWN doesn't
support this "AVOID_COMPAT_SENSITIVE" feature, because w/o it we cannot tell
whether the update is happening during any sensitive operation.
But the code above proceeds to TDH.SYS.SHUTDOWN anyway when this feature
isn't supported. I don't think we should do that?
^ permalink raw reply
* Re: [PATCH v4 20/24] x86/virt/tdx: Enable TDX Module runtime updates
From: Huang, Kai @ 2026-02-23 5:09 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-21-chao.gao@intel.com>
On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> All pieces of TDX Module runtime updates are in place. Enable it if it
> is supported.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> ---
> v4:
> - s/BIT/BIT_ULL [Tony]
> ---
> arch/x86/include/asm/tdx.h | 5 ++++-
> arch/x86/virt/vmx/tdx/tdx.h | 3 ---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index ffadbf64d0c1..ad62a7be0443 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -32,6 +32,9 @@
> #define TDX_SUCCESS 0ULL
> #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
>
> +/* Bit definitions of TDX_FEATURES0 metadata field */
> +#define TDX_FEATURES0_TD_PRESERVING BIT_ULL(1)
> +#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
> #ifndef __ASSEMBLER__
>
> #include <uapi/asm/mce.h>
> @@ -105,7 +108,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
>
> static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
> {
> - return false; /* To be enabled when kernel is ready */
> + return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
> }
>
> int tdx_guest_keyid_alloc(void);
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index d1807a476d3b..749f4d74cb2c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -88,9 +88,6 @@ struct tdmr_info {
> DECLARE_FLEX_ARRAY(struct tdmr_reserved_area, reserved_areas);
> } __packed __aligned(TDMR_INFO_ALIGNMENT);
>
> -/* Bit definitions of TDX_FEATURES0 metadata field */
> -#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
> -
>
Nit:
Strictly speaking, moving this "NO_RBP_MOD" isn't required to "enable TDX
module runtime updates". So I think it's better to call out in changelog
that this is trying to centralize the bit definitions.
Anyway, I think we have multiple series doing this so I guess things will
just sort out eventually.
^ permalink raw reply
* Re: [PATCH 1/5] dma-mapping: avoid random addr value print out on error path
From: Marek Szyprowski @ 2026-02-23 7:28 UTC (permalink / raw)
To: Jiri Pirko, dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, robin.murphy, jgg, leon,
sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260209153809.250835-2-jiri@resnulli.us>
On 09.02.2026 16:38, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> dma_addr is unitialized in dma_direct_map_phys() when swiotlb is forced
> and DMA_ATTR_MMIO is set which leads to random value print out in
> warning. Fix that by just returning DMA_MAPPING_ERROR.
>
> Fixes: e53d29f957b3 ("dma-mapping: convert dma_direct_*map_page to be phys_addr_t based")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Applied to dma-mapping-fixes, thanks!
> ---
> kernel/dma/direct.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
> index da2fadf45bcd..62f0d9d0ba02 100644
> --- a/kernel/dma/direct.h
> +++ b/kernel/dma/direct.h
> @@ -88,7 +88,7 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>
> if (is_swiotlb_force_bounce(dev)) {
> if (attrs & DMA_ATTR_MMIO)
> - goto err_overflow;
> + return DMA_MAPPING_ERROR;
>
> return swiotlb_map(dev, phys, size, dir, attrs);
> }
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Huang, Kai @ 2026-02-23 9:25 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Gao, Chao, x86@kernel.org
Cc: dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <20260212143606.534586-12-chao.gao@intel.com>
>
> +/*
> + * During a TDX Module update, all CPUs start from TDP_START and progress
Nit: start from TDP_START or TDP_START + 1 ?
The code below says:
+ set_target_state(TDP_START + 1);
+ ret = stop_machine_cpuslocked(do_seamldr_install_module, params,
cpu_online_mask);
> + * to TDP_DONE. Each state is associated with certain work. For some
> + * states, just one CPU needs to perform the work, while other CPUs just
> + * wait during those states.
> + */
> +enum tdp_state {
> + TDP_START,
> + TDP_DONE,
> +};
Nit: just curious, what does "TDP" mean?
Maybe something more obvious?
> +
> +static struct {
> + enum tdp_state state;
> + atomic_t thread_ack;
> +} tdp_data;
> +
> +static void set_target_state(enum tdp_state state)
> +{
> + /* Reset ack counter. */
> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
> + /* Ensure thread_ack is updated before the new state */
Nit: perhaps add "so that ..." part to the comment?
> + smp_wmb();
> + WRITE_ONCE(tdp_data.state, state);
> +}
> +
> +/* Last one to ack a state moves to the next state. */
> +static void ack_state(void)
> +{
> + if (atomic_dec_and_test(&tdp_data.thread_ack))
> + set_target_state(tdp_data.state + 1);
> +}
> +
> +/*
> + * See multi_cpu_stop() from where this multi-cpu state-machine was
> + * adopted, and the rationale for touch_nmi_watchdog()
> + */
Nit: add a period to the end of the sentence.
(btw, I found using period or not isn't consistent even among the 'one-line-
sentence' comments, maybe you want to make that consistent.)
^ permalink raw reply
* [PATCH v2 0/2] dma-buf: heaps: system: add an option to allocate explicitly decrypted memory
From: Jiri Pirko @ 2026-02-23 9:51 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
From: Jiri Pirko <jiri@nvidia.com>
Confidential computing (CoCo) VMs/guests, such as AMD SEV and Intel TDX,
run with encrypted/protected memory which creates a challenge
for devices that do not support DMA to it (no TDISP support).
For kernel-only DMA operations, swiotlb bounce buffering provides a
transparent solution by copying data through decrypted memory.
However, the only way to get this memory into userspace is via the DMA
API's dma_alloc_pages()/dma_mmap_pages() type interfaces which limits
the use of the memory to a single DMA device, and is incompatible with
pin_user_pages().
These limitations are particularly problematic for the RDMA subsystem
which makes heavy use of pin_user_pages() and expects flexible memory
usage between many different DMA devices.
This patch series enables userspace to explicitly request decrypted
(shared) memory allocations from the dma-buf system heap.
Userspace can mmap this memory and pass the dma-buf fd to other
existing importers such as RDMA or DRM devices to access the
memory. The DMA API is improved to allow the dma heap exporter to DMA
map the shared memory to each importing device.
---
v1->v2:
patch1:
- rebased on top of recent dma-mapping-fixes
patch2:
- fixed build errors on s390 by including mem_encrypt.h
- converted system heap flag implementation to a separate heap
Based on dma-mapping-fixes HEAD d5b5e8149af0f5efed58653cbebf1cb3258ce49a
Jiri Pirko (2):
dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
dma-buf: heaps: system: add system_cc_decrypted heap for explicitly
decrypted memory
drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
include/linux/dma-heap.h | 1 +
include/linux/dma-mapping.h | 6 ++
include/trace/events/dma.h | 3 +-
include/uapi/linux/dma-heap.h | 3 +-
kernel/dma/direct.h | 14 +++-
6 files changed, 119 insertions(+), 11 deletions(-)
--
2.51.1
^ permalink raw reply
* [PATCH v2 1/2] dma-mapping: introduce DMA_ATTR_CC_DECRYPTED for pre-decrypted memory
From: Jiri Pirko @ 2026-02-23 9:51 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260223095136.225277-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Current CC designs don't place a vIOMMU in front of untrusted devices.
Instead, the DMA API forces all untrusted device DMA through swiotlb
bounce buffers (is_swiotlb_force_bounce()) which copies data into
decrypted memory on behalf of the device.
When a caller has already arranged for the memory to be decrypted
via set_memory_decrypted(), the DMA API needs to know so it can map
directly using the unencrypted physical address rather than bounce
buffering. Following the pattern of DMA_ATTR_MMIO, add
DMA_ATTR_CC_DECRYPTED for this purpose. Like the MMIO case, only the
caller knows what kind of memory it has and must inform the DMA API
for it to work correctly.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- rebased on top of recent dma-mapping-fixes
---
include/linux/dma-mapping.h | 6 ++++++
include/trace/events/dma.h | 3 ++-
kernel/dma/direct.h | 14 +++++++++++---
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 29973baa0581..ae3d85e494ec 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -85,6 +85,12 @@
* a cacheline must have this attribute for this to be considered safe.
*/
#define DMA_ATTR_CPU_CACHE_CLEAN (1UL << 11)
+/*
+ * DMA_ATTR_CC_DECRYPTED: Indicates memory that has been explicitly decrypted
+ * (shared) for confidential computing guests. The caller must have
+ * called set_memory_decrypted(). A struct page is required.
+ */
+#define DMA_ATTR_CC_DECRYPTED (1UL << 12)
/*
* A dma_addr_t can hold any valid DMA or bus address for the platform. It can
diff --git a/include/trace/events/dma.h b/include/trace/events/dma.h
index 33e99e792f1a..b8082d5177c4 100644
--- a/include/trace/events/dma.h
+++ b/include/trace/events/dma.h
@@ -32,7 +32,8 @@ TRACE_DEFINE_ENUM(DMA_NONE);
{ DMA_ATTR_ALLOC_SINGLE_PAGES, "ALLOC_SINGLE_PAGES" }, \
{ DMA_ATTR_NO_WARN, "NO_WARN" }, \
{ DMA_ATTR_PRIVILEGED, "PRIVILEGED" }, \
- { DMA_ATTR_MMIO, "MMIO" })
+ { DMA_ATTR_MMIO, "MMIO" }, \
+ { DMA_ATTR_CC_DECRYPTED, "CC_DECRYPTED" })
DECLARE_EVENT_CLASS(dma_map,
TP_PROTO(struct device *dev, phys_addr_t phys_addr, dma_addr_t dma_addr,
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index e89f175e9c2d..c047a9d0fda3 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -84,16 +84,24 @@ static inline dma_addr_t dma_direct_map_phys(struct device *dev,
dma_addr_t dma_addr;
if (is_swiotlb_force_bounce(dev)) {
- if (attrs & DMA_ATTR_MMIO)
- return DMA_MAPPING_ERROR;
+ if (!(attrs & DMA_ATTR_CC_DECRYPTED)) {
+ if (attrs & DMA_ATTR_MMIO)
+ return DMA_MAPPING_ERROR;
- return swiotlb_map(dev, phys, size, dir, attrs);
+ return swiotlb_map(dev, phys, size, dir, attrs);
+ }
+ } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
+ return DMA_MAPPING_ERROR;
}
if (attrs & DMA_ATTR_MMIO) {
dma_addr = phys;
if (unlikely(!dma_capable(dev, dma_addr, size, false)))
goto err_overflow;
+ } else if (attrs & DMA_ATTR_CC_DECRYPTED) {
+ dma_addr = phys_to_dma_unencrypted(dev, phys);
+ if (unlikely(!dma_capable(dev, dma_addr, size, false)))
+ goto err_overflow;
} else {
dma_addr = phys_to_dma(dev, phys);
if (unlikely(!dma_capable(dev, dma_addr, size, true)) ||
--
2.51.1
^ permalink raw reply related
* [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: Jiri Pirko @ 2026-02-23 9:51 UTC (permalink / raw)
To: dri-devel, linaro-mm-sig, iommu, linux-media
Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
tjmercier, christian.koenig, m.szyprowski, robin.murphy, jgg,
leon, sean.anderson, ptesarik, catalin.marinas, aneesh.kumar,
suzuki.poulose, steven.price, thomas.lendacky, john.allen,
ashish.kalra, suravee.suthikulpanit, linux-coco
In-Reply-To: <20260223095136.225277-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@nvidia.com>
Add a new "system_cc_decrypted" dma-buf heap to allow userspace to
allocate decrypted (shared) memory for confidential computing (CoCo)
VMs.
On CoCo VMs, guest memory is encrypted by default. The hardware uses an
encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
Intel TDX) to control whether a given memory access is encrypted or
decrypted. The kernel's direct map is set up with encryption enabled,
so pages returned by alloc_pages() are encrypted in the direct map
by default. To make this memory usable for devices that do not support
DMA to encrypted memory (no TDISP support), it has to be explicitly
decrypted. A couple of things are needed to properly handle
decrypted memory for the dma-buf use case:
- set_memory_decrypted() on the direct map after allocation:
Besides clearing the encryption bit in the direct map PTEs, this
also notifies the hypervisor about the page state change. On free,
the inverse set_memory_encrypted() must be called before returning
pages to the allocator. If re-encryption fails, pages
are intentionally leaked to prevent decrypted memory from being
reused as private.
- pgprot_decrypted() for userspace and kernel virtual mappings:
Any new mapping of the decrypted pages, be it to userspace via
mmap or to kernel vmalloc space via vmap, creates PTEs independent
of the direct map. These must also have the encryption bit cleared,
otherwise accesses through them would see encrypted (garbage) data.
- DMA_ATTR_CC_DECRYPTED for DMA mapping:
Since the pages are already decrypted, the DMA API needs to be
informed via DMA_ATTR_CC_DECRYPTED so it can map them correctly
as unencrypted for device access.
On non-CoCo VMs, the system_cc_decrypted heap is not registered
to prevent misuse by userspace that does not understand
the security implications of explicitly decrypted memory.
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- fixed build errors on s390 by including mem_encrypt.h
- converted system heap flag implementation to a separate heap
---
drivers/dma-buf/heaps/system_heap.c | 103 ++++++++++++++++++++++++++--
include/linux/dma-heap.h | 1 +
include/uapi/linux/dma-heap.h | 3 +-
3 files changed, 100 insertions(+), 7 deletions(-)
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index b3650d8fd651..a525e9aaaffa 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -10,17 +10,25 @@
* Andrew F. Davis <afd@ti.com>
*/
+#include <linux/cc_platform.h>
#include <linux/dma-buf.h>
#include <linux/dma-mapping.h>
#include <linux/dma-heap.h>
#include <linux/err.h>
#include <linux/highmem.h>
+#include <linux/mem_encrypt.h>
#include <linux/mm.h>
+#include <linux/set_memory.h>
#include <linux/module.h>
+#include <linux/pgtable.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
+struct system_heap_priv {
+ bool decrypted;
+};
+
struct system_heap_buffer {
struct dma_heap *heap;
struct list_head attachments;
@@ -29,6 +37,7 @@ struct system_heap_buffer {
struct sg_table sg_table;
int vmap_cnt;
void *vaddr;
+ bool decrypted;
};
struct dma_heap_attachment {
@@ -36,6 +45,7 @@ struct dma_heap_attachment {
struct sg_table table;
struct list_head list;
bool mapped;
+ bool decrypted;
};
#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO)
@@ -52,6 +62,34 @@ static gfp_t order_flags[] = {HIGH_ORDER_GFP, HIGH_ORDER_GFP, LOW_ORDER_GFP};
static const unsigned int orders[] = {8, 4, 0};
#define NUM_ORDERS ARRAY_SIZE(orders)
+static int system_heap_set_page_decrypted(struct page *page)
+{
+ unsigned long addr = (unsigned long)page_address(page);
+ unsigned int nr_pages = 1 << compound_order(page);
+ int ret;
+
+ ret = set_memory_decrypted(addr, nr_pages);
+ if (ret)
+ pr_warn_ratelimited("dma-buf system heap: failed to decrypt page at %p\n",
+ page_address(page));
+
+ return ret;
+}
+
+static int system_heap_set_page_encrypted(struct page *page)
+{
+ unsigned long addr = (unsigned long)page_address(page);
+ unsigned int nr_pages = 1 << compound_order(page);
+ int ret;
+
+ ret = set_memory_encrypted(addr, nr_pages);
+ if (ret)
+ pr_warn_ratelimited("dma-buf system heap: failed to re-encrypt page at %p, leaking memory\n",
+ page_address(page));
+
+ return ret;
+}
+
static int dup_sg_table(struct sg_table *from, struct sg_table *to)
{
struct scatterlist *sg, *new_sg;
@@ -90,6 +128,7 @@ static int system_heap_attach(struct dma_buf *dmabuf,
a->dev = attachment->dev;
INIT_LIST_HEAD(&a->list);
a->mapped = false;
+ a->decrypted = buffer->decrypted;
attachment->priv = a;
@@ -119,9 +158,11 @@ static struct sg_table *system_heap_map_dma_buf(struct dma_buf_attachment *attac
{
struct dma_heap_attachment *a = attachment->priv;
struct sg_table *table = &a->table;
+ unsigned long attrs;
int ret;
- ret = dma_map_sgtable(attachment->dev, table, direction, 0);
+ attrs = a->decrypted ? DMA_ATTR_CC_DECRYPTED : 0;
+ ret = dma_map_sgtable(attachment->dev, table, direction, attrs);
if (ret)
return ERR_PTR(ret);
@@ -188,8 +229,13 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
unsigned long addr = vma->vm_start;
unsigned long pgoff = vma->vm_pgoff;
struct scatterlist *sg;
+ pgprot_t prot;
int i, ret;
+ prot = vma->vm_page_prot;
+ if (buffer->decrypted)
+ prot = pgprot_decrypted(prot);
+
for_each_sgtable_sg(table, sg, i) {
unsigned long n = sg->length >> PAGE_SHIFT;
@@ -206,8 +252,7 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
if (addr + size > vma->vm_end)
size = vma->vm_end - addr;
- ret = remap_pfn_range(vma, addr, page_to_pfn(page),
- size, vma->vm_page_prot);
+ ret = remap_pfn_range(vma, addr, page_to_pfn(page), size, prot);
if (ret)
return ret;
@@ -225,6 +270,7 @@ static void *system_heap_do_vmap(struct system_heap_buffer *buffer)
struct page **pages = vmalloc(sizeof(struct page *) * npages);
struct page **tmp = pages;
struct sg_page_iter piter;
+ pgprot_t prot;
void *vaddr;
if (!pages)
@@ -235,7 +281,10 @@ static void *system_heap_do_vmap(struct system_heap_buffer *buffer)
*tmp++ = sg_page_iter_page(&piter);
}
- vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
+ prot = PAGE_KERNEL;
+ if (buffer->decrypted)
+ prot = pgprot_decrypted(prot);
+ vaddr = vmap(pages, npages, VM_MAP, prot);
vfree(pages);
if (!vaddr)
@@ -296,6 +345,14 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
for_each_sgtable_sg(table, sg, i) {
struct page *page = sg_page(sg);
+ /*
+ * Intentionally leak pages that cannot be re-encrypted
+ * to prevent decrypted memory from being reused.
+ */
+ if (buffer->decrypted &&
+ system_heap_set_page_encrypted(page))
+ continue;
+
__free_pages(page, compound_order(page));
}
sg_free_table(table);
@@ -347,6 +404,8 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
unsigned long size_remaining = len;
unsigned int max_order = orders[0];
+ struct system_heap_priv *priv = dma_heap_get_drvdata(heap);
+ bool decrypted = priv->decrypted;
struct dma_buf *dmabuf;
struct sg_table *table;
struct scatterlist *sg;
@@ -362,6 +421,7 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
mutex_init(&buffer->lock);
buffer->heap = heap;
buffer->len = len;
+ buffer->decrypted = decrypted;
INIT_LIST_HEAD(&pages);
i = 0;
@@ -396,6 +456,14 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
list_del(&page->lru);
}
+ if (decrypted) {
+ for_each_sgtable_sg(table, sg, i) {
+ ret = system_heap_set_page_decrypted(sg_page(sg));
+ if (ret)
+ goto free_pages;
+ }
+ }
+
/* create the dmabuf */
exp_info.exp_name = dma_heap_get_name(heap);
exp_info.ops = &system_heap_buf_ops;
@@ -413,6 +481,13 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
for_each_sgtable_sg(table, sg, i) {
struct page *p = sg_page(sg);
+ /*
+ * Intentionally leak pages that cannot be re-encrypted
+ * to prevent decrypted memory from being reused.
+ */
+ if (buffer->decrypted &&
+ system_heap_set_page_encrypted(p))
+ continue;
__free_pages(p, compound_order(p));
}
sg_free_table(table);
@@ -428,6 +503,14 @@ static const struct dma_heap_ops system_heap_ops = {
.allocate = system_heap_allocate,
};
+static struct system_heap_priv system_heap_priv = {
+ .decrypted = false,
+};
+
+static struct system_heap_priv system_heap_cc_decrypted_priv = {
+ .decrypted = true,
+};
+
static int __init system_heap_create(void)
{
struct dma_heap_export_info exp_info;
@@ -435,8 +518,18 @@ static int __init system_heap_create(void)
exp_info.name = "system";
exp_info.ops = &system_heap_ops;
- exp_info.priv = NULL;
+ exp_info.priv = &system_heap_priv;
+
+ sys_heap = dma_heap_add(&exp_info);
+ if (IS_ERR(sys_heap))
+ return PTR_ERR(sys_heap);
+
+ if (IS_ENABLED(CONFIG_HIGHMEM) ||
+ !cc_platform_has(CC_ATTR_MEM_ENCRYPT))
+ return 0;
+ exp_info.name = "system_cc_decrypted";
+ exp_info.priv = &system_heap_cc_decrypted_priv;
sys_heap = dma_heap_add(&exp_info);
if (IS_ERR(sys_heap))
return PTR_ERR(sys_heap);
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 648328a64b27..d97b668413c1 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -10,6 +10,7 @@
#define _DMA_HEAPS_H
#include <linux/types.h>
+#include <uapi/linux/dma-heap.h>
struct dma_heap;
diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
index a4cf716a49fa..ab95bb355ed5 100644
--- a/include/uapi/linux/dma-heap.h
+++ b/include/uapi/linux/dma-heap.h
@@ -18,8 +18,7 @@
/* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
#define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
-/* Currently no heap flags */
-#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
+#define DMA_HEAP_VALID_HEAP_FLAGS (0)
/**
* struct dma_heap_allocation_data - metadata passed from userspace for
--
2.51.1
^ permalink raw reply related
* SVSM Development Call Meeting Minutes
From: Jörg Rödel @ 2026-02-23 13:55 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
I pushed a PR with the meeting minutes for the SVSM development calls on
February 11th and 18th:
https://github.com/coconut-svsm/governance/pull/96
Please review and sorry for the delay.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: John Stultz @ 2026-02-23 18:33 UTC (permalink / raw)
To: Jiri Pirko
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, tjmercier, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <20260223095136.225277-3-jiri@resnulli.us>
On Mon, Feb 23, 2026 at 1:51 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@nvidia.com>
>
> Add a new "system_cc_decrypted" dma-buf heap to allow userspace to
> allocate decrypted (shared) memory for confidential computing (CoCo)
> VMs.
>
> On CoCo VMs, guest memory is encrypted by default. The hardware uses an
> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
> Intel TDX) to control whether a given memory access is encrypted or
> decrypted. The kernel's direct map is set up with encryption enabled,
> so pages returned by alloc_pages() are encrypted in the direct map
> by default. To make this memory usable for devices that do not support
> DMA to encrypted memory (no TDISP support), it has to be explicitly
> decrypted. A couple of things are needed to properly handle
> decrypted memory for the dma-buf use case:
>
> - set_memory_decrypted() on the direct map after allocation:
> Besides clearing the encryption bit in the direct map PTEs, this
> also notifies the hypervisor about the page state change. On free,
> the inverse set_memory_encrypted() must be called before returning
> pages to the allocator. If re-encryption fails, pages
> are intentionally leaked to prevent decrypted memory from being
> reused as private.
>
> - pgprot_decrypted() for userspace and kernel virtual mappings:
> Any new mapping of the decrypted pages, be it to userspace via
> mmap or to kernel vmalloc space via vmap, creates PTEs independent
> of the direct map. These must also have the encryption bit cleared,
> otherwise accesses through them would see encrypted (garbage) data.
>
> - DMA_ATTR_CC_DECRYPTED for DMA mapping:
> Since the pages are already decrypted, the DMA API needs to be
> informed via DMA_ATTR_CC_DECRYPTED so it can map them correctly
> as unencrypted for device access.
>
> On non-CoCo VMs, the system_cc_decrypted heap is not registered
> to prevent misuse by userspace that does not understand
> the security implications of explicitly decrypted memory.
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Thanks for reworking this! I've not reviewed it super closely, but I
believe it resolves my objection on your first version.
Few nits/questions below.
> @@ -296,6 +345,14 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
> for_each_sgtable_sg(table, sg, i) {
> struct page *page = sg_page(sg);
>
> + /*
> + * Intentionally leak pages that cannot be re-encrypted
> + * to prevent decrypted memory from being reused.
> + */
> + if (buffer->decrypted &&
> + system_heap_set_page_encrypted(page))
> + continue;
> +
What are the conditions where this would fail? How much of an edge
case is this? I fret this opens a DoS vector if one is able to
allocate from this heap and then stress the system when doing the
free.
Should there be some global list of leaked decrypted pages such that
the mm subsystem could try again later to recover these?
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index 648328a64b27..d97b668413c1 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -10,6 +10,7 @@
> #define _DMA_HEAPS_H
>
> #include <linux/types.h>
> +#include <uapi/linux/dma-heap.h>
>
> struct dma_heap;
>
> diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
> index a4cf716a49fa..ab95bb355ed5 100644
> --- a/include/uapi/linux/dma-heap.h
> +++ b/include/uapi/linux/dma-heap.h
> @@ -18,8 +18,7 @@
> /* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
> #define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
>
> -/* Currently no heap flags */
> -#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
> +#define DMA_HEAP_VALID_HEAP_FLAGS (0)
>
> /**
> * struct dma_heap_allocation_data - metadata passed from userspace for
Are these header changes still necessary?
thanks
-john
^ permalink raw reply
* [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Changyuan Lyu @ 2026-02-23 21:43 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Isaku Yamahata, Xiaoyao Li
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, kvm,
linux-kernel, linux-coco, Changyuan Lyu
Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
This ensures consistency with the CPUID entries returned by
KVM_GET_SUPPORTED_CPUID.
Additionally, add a WARN_ON_ONCE() to verify that the TDX module's
reported entries align with KVM's expectations regarding indexed
CPUID functions.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
---
arch/x86/kvm/vmx/tdx.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2d7a4d52ccfb4..0c524f9a94a6c 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
- if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
+ if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
entry->index = 0;
+ entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ } else {
+ entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ }
+ WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
+ !!(entry->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
/*
* The TDX module doesn't allow configuring the guest phys addr bits
* (EAX[23:16]). However, KVM uses it as an interface to the userspace
--
2.53.0.371.g1d285c8824-goog
^ permalink raw reply related
* Re: [PATCH v4 02/24] coco/tdx-host: Introduce a "tdx_host" device
From: Chao Gao @ 2026-02-24 1:11 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, jonathan.cameron@huawei.com,
Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
Edgecombe, Rick P, paulmck@kernel.org, hpa@zytor.com,
tglx@kernel.org, yilun.xu@linux.intel.com, Williams, Dan J,
bp@alien8.de
In-Reply-To: <3881637de6fc6dc5561e0dcf42c536ae57f6eef9.camel@intel.com>
On Fri, Feb 20, 2026 at 08:15:20AM +0800, Huang, Kai wrote:
>>
>> A faux device is used as for TDX because the TDX module is singular within
> ^
>"as" should be removed.
Sure. Will fix this.
>
>> the system and lacks associated platform resources. Using a faux device
>> eliminates the need to create a stub bus.
>>
>> The call to tdx_get_sysinfo() ensures that the TDX Module is ready to
>> provide services.
>>
>> Note that AMD has a PCI device for the PSP for SEV and ARM CCA will
>> likely have a faux device [1].
>>
>> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
>
>Reviewed-by: Kai Huang <kai.huang@intel.com>
Thanks.
>
>A nit below ..
>
>
>[...]
>
>> +config TDX_HOST_SERVICES
>> + tristate "TDX Host Services Driver"
>> + depends on INTEL_TDX_HOST
>> + default m
>> + help
>> + Enable access to TDX host services like module update and
>> + extensions (e.g. TDX Connect).
>> +
>> + Say y or m if enabling support for confidential virtual machine
>> + support (CONFIG_INTEL_TDX_HOST). The module is called tdx_host.ko
>
>.. Missing period at the end of the last sentence.
Will do and apply this to the whole series.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Edgecombe, Rick P @ 2026-02-24 1:57 UTC (permalink / raw)
To: Li, Xiaoyao, changyuanl@google.com, pbonzini@redhat.com,
seanjc@google.com, Wu, Binbin, Yamahata, Isaku
Cc: bp@alien8.de, x86@kernel.org, kas@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, kvm@vger.kernel.org,
linux-coco@lists.linux.dev
In-Reply-To: <20260223214336.722463-1-changyuanl@google.com>
+binbin
On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu wrote:
> Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
> structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
> This ensures consistency with the CPUID entries returned by
> KVM_GET_SUPPORTED_CPUID.
>
> Additionally, add a WARN_ON_ONCE() to verify that the TDX module's
> reported entries align with KVM's expectations regarding indexed
> CPUID functions.
>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
> ---
> arch/x86/kvm/vmx/tdx.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 2d7a4d52ccfb4..0c524f9a94a6c 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
> kvm_cpuid_entry2 *entry, unsigned char i
> entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
> entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
>
> - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
> + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
> entry->index = 0;
> + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
There are two callers of this. One is already zeroed, and the other has
stack garbage in flags. But that second caller doesn't look at the
flags so it is harmless. Maybe it would be simpler and clearer to just
zero init the entry struct in that caller. Then you don't need to clear
it here. Or alternatively set flags to zero above, and then add
KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
single bit in a field of garbage, which seems weird.
> + } else {
> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + }
>
> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
> + !!(entry->flags &
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
It warns on leaf 0x23 for me. Is it intentional?
This warning kind of begs the question of how how much consistency
there should be between KVM_TDX_CAPABILITIES and
KVM_GET_SUPPORTED_CPUID. There was quite a bit of debate on this and in
the end we moved forward with a solution that did the bare minimum
consistency checking.
We actually have been looking at some potential TDX module changes to
fix the deficiencies from not enforcing the consistency. But didn't
consider this pattern. Can you explain more about the failure mode?
> /*
> * The TDX module doesn't allow configuring the guest phys
> addr bits
> * (EAX[23:16]). However, KVM uses it as an interface to
> the userspace
> --
^ permalink raw reply
* Re: [PATCH v4 03/24] coco/tdx-host: Expose TDX Module version
From: Chao Gao @ 2026-02-24 2:02 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
Weiny, Ira, Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
Edgecombe, Rick P, paulmck@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J
In-Reply-To: <3a8feb5470bd964e421969918b5553259abdd493.camel@intel.com>
On Fri, Feb 20, 2026 at 08:40:13AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> For TDX Module updates, userspace needs to select compatible update
>> versions based on the current module version. This design delegates
>> module selection complexity to userspace because TDX Module update
>> policies are complex and version series are platform-specific.
>>
>> For example, the 1.5.x series is for certain platform generations, while
>> the 2.0.x series is intended for others. And TDX Module 1.5.x may be
>> updated to 1.5.y but not to 1.5.y+1.
>>
>> Expose the TDX Module version to userspace via sysfs to aid module
>> selection. Since the TDX faux device will drive module updates, expose
>> the version as its attribute.
>>
>> One bonus of exposing TDX Module version via sysfs is: TDX Module
>> version information remains available even after dmesg logs are cleared.
>>
>> == Background ==
>>
>> The "faux device + device attribute" approach compares to other update
>> mechanisms as follows:
>
>This "faux device + device attribute" approach seems to be a wider design
>choice instead of how to expose module version (which is the scope of this
>patch). Overall, shouldn't this be in the changelog of the previous patch
>which actually introduces "faux device" (albeit no attribute is introduced
>in that patch)?
Yes, it's mentioned briefly in the previous patch:
"""
Create a virtual device not only to align with other implementations but
also to make it easier to
- expose metadata (e.g., TDX module version, seamldr version etc) to
the userspace as device attributes
...
"""
The previous patch doesn't provide details for version information
exposure, as version attributes are just one of several purposes for the
virtual device.
>
>>
>> 1. AMD SEV leverages an existing PCI device for the PSP to expose
>> metadata. TDX uses a faux device as it doesn't have PCI device
>> in its architecture.
>
>E.g., this sounds to justify "why to use faux device for TDX", but not "to
>expose module version via faux device attributes".
This provides additional context as suggested by Dave:
https://lore.kernel.org/kvm/aa3f026b-ad69-4070-8433-8950e5250edb@intel.com/
Dave asked:
"""
What are other CPU vendors doing for this? SEV? CCA? S390? How are their
firmware versions exposed? What about other things in the Intel world
like CPU microcode or the billion other chunks of firmware? ...
"""
>
>>
>> 2. Microcode uses per-CPU virtual devices to report microcode revisions
>> because CPUs can have different revisions. But, there is only a
>> single TDX Module, so exposing the TDX Module version through a global
>> TDX faux device is appropriate
>
>This is related to exposing module version, but to me "there's only a single
>TDX module" is also more like a justification to use "one faux device",
>which should belong to changelog of previous patch too.
The previous patch already includes this justification:
"""
A faux device is used as for TDX because the TDX module is singular within
the system ...
"""
>
>With "there's only a single TDX module" being said in previous patch
>changelog, I think we can safely deduce that there's only "one module
>version" but not per-cpu (thus I don't think we even need to call this out
>in _this_ patch).
>
>>
>> 3. ARM's CCA implementation isn't in-tree yet, but will likely follow a
>> similar faux device approach [1], though it's unclear whether they need
>> to expose firmware version information
>
>Again, I don't feel "follow a similar faux device approach" for ARM CCA
>should be a justification of "exposing module version via faux attributes".
>It should be a justification of "using faux device for TDX".
Agreed. I repeated this information here under "== Background ==" to give
broader context for the overall approach.
>
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
>>
>
>[...]
>
>> +Description: (RO) Report the version of the loaded TDX Module. The TDX Module
>> + version is formatted as x.y.z, where "x" is the major version,
>> + "y" is the minor version and "z" is the update version. Versions
>> + are used for bug reporting, TDX Module updates and etc.
> ^
>
>Nit: No need to use "and" before "etc".
Thanks. Will fix this.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-02-24 2:31 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org, Chen, Farrah,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <2683dff7a7950c57aa7a73584d86cf1b34bcfc07.camel@intel.com>
On Fri, Feb 20, 2026 at 09:12:29AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> The TDX architecture uses the "SEAMCALL" instruction to communicate with
>> SEAM mode software. Right now, the only SEAM mode software that the kernel
>> communicates with is the TDX module. But, there is actually another
>> component that runs in SEAM mode but it is separate from the TDX module:
>> the persistent SEAM loader or "P-SEAMLDR". Right now, the only component
>> that communicates with it is the BIOS which loads the TDX module itself at
>> boot. But, to support updating the TDX module, the kernel now needs to be
>> able to talk to it.
>>
>> P-SEAMLDR SEAMCALLs differ from TDX Module SEAMCALLs in areas such as
>> concurrency requirements. Add a P-SEAMLDR wrapper to handle these
>> differences and prepare for implementing concrete functions.
>>
>> Note that unlike P-SEAMLDR, there is also a non-persistent SEAM loader
>> ("NP-SEAMLDR"). This is an authenticated code module (ACM) that is not
>> callable at runtime. Only BIOS launches it to load P-SEAMLDR at boot;
>
>[...]
>
>> the kernel does not interact with it.
>
>Nit:
>
>Again, to me this only describes what does the kernel do today. It doesn't
>describe what the kernel needs to do for runtime updating.
>
>Maybe it can just be something like:
>
> The kernel does not need to interact with it for runtime update.
I am fine with this. Will do.
>
>But I don't know why do you even need to talk about NP-SEAMLDR.
I included this because Dave had some confusion about NP-SEAMLDR [1], so I
wanted to clarify it.
[1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/
And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
helpful to clarify the difference. This follows Dave's earlier suggestion to
explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].
[2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/
>
>>
>> For details of P-SEAMLDR SEAMCALLs, see Intel® Trust Domain CPU
>> Architectural Extensions, Revision 343754-002, Chapter 2.3 "INSTRUCTION
>> SET REFERENCE".
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Tested-by: Farrah Chen <farrah.chen@intel.com>
>> Link: https://cdrdv2.intel.com/v1/dl/getContent/733582 # [1]
>>
>
>[...]
>
>> + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
>> + * interact with P-SEAMLDR simultaneously.
>> + */
>> +static DEFINE_RAW_SPINLOCK(seamldr_lock);
>> +
>> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
>> +{
>> + /*
>> + * Serialize P-SEAMLDR calls and disable interrupts as the calls
>> + * can be made from IRQ context.
>> + */
>> + guard(raw_spinlock_irqsave)(&seamldr_lock);
>
>Why do you need to disable IRQ? A plain raw_spinlock should work with both
>cases where seamldr_call() is called from IRQ disabled context and normal
>task context?
No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
fires while a task context already holds the lock, and the interrupt handler
also tries to acquire the same lock.
>
>> + return seamcall_prerr(fn, args);
>> +}
^ permalink raw reply
* Re: [PATCH v4 05/24] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Chao Gao @ 2026-02-24 2:59 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org, Chen, Farrah,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <88141072be073896990f87b2b4c33bdd99f38b29.camel@intel.com>
On Fri, Feb 20, 2026 at 05:36:33PM +0800, Huang, Kai wrote:
>
>> +int seamldr_get_info(struct seamldr_info *seamldr_info)
>> +{
>> + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
>
>Should we have a comment for slow_virt_to_phys()? This patch alone doesn't
>really tell where is the memory from.
How about:
/*
* Use slow_virt_to_phys() since @seamldr_info may be allocated on
* the stack.
*/
I was hesitant to add a comment since most existing slow_virt_to_phys() usage
lacks comments.
>
>Btw, it it were me, I would just merge this patch with the next one. Then
>it's clear the memory comes from tdx-host module's stack. The merged patch
>won't be too big to review either (IMHO). You can then have this
>seamldr_get_info() and its user together in one patch, with one changelog to
>tell the full story.
>
>But just my 2cents, feel free to ignore.
I'm fine with this. But let's see what others think about merging the patches.
>
>> +
>> + return seamldr_call(P_SEAMLDR_INFO, &args);
>> +}
>> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>> --
>> 2.47.3
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-02-24 5:15 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <1aa733f9066dd85c1d4f880c5c48b40c76d518c7.camel@intel.com>
On Fri, Feb 20, 2026 at 06:31:24AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
>> update requests. This structure contains physical addresses pointing to
>> the module binary and its signature file (or sigstruct), along with an
>> update scenario field.
>>
>> TDX Modules are distributed in the tdx_blob format defined at [1]. A
>> tdx_blob contains a header, sigstruct, and module binary. This is also
>> the format supplied by the userspace to the kernel.
>>
>> Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
>> accordingly. This structure will be passed to P-SEAMLDR to initiate the
>> update.
>>
>> Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
>> a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
>> Specification" will be published separately. The kernel does not
>> validate P-SEAMLDR compatibility (for example, whether it supports 4KB
>> or 16KB sigstruct);
>>
>
>Nit:
>
>This sounds like the kernel can validate but chooses not to. But I thought
>the fact is the kernel cannot validate because there's no P-SEAMLDR ABI to
>enumerate such compatibility?
Emm, the kernel could validate this by parsing mapping_file.json, but the
complexity wouldn't be worth it.
>
>> userspace must ensure the P-SEAMLDR version is
>> compatible with the selected TDX Module by checking the minimum
>> P-SEAMLDR version requirements at [2].
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
>>
>
>Nit:
>
>As mentioned in v3, can the link be considered as "stable", e.g., won't
>disappear couple of years later?
I'm not sure when this link will be outdated, but we'll definitely have a TDX
Module release repository with a blob_structure.txt file describing the format.
>
>Not sure we should just have a documentation patch for 'tdx_blob' layout. I
>suspect the content won't be changed in the future anyway, at least for
>foreseeable future, given you have already updated the sigstruct part.
>
>We can include the links to the actual doc too, and if necessarily, point
>out the links may get updated in the future. We can actually update the
>links if they are in some doc.
Regarding the documentation patch, I don't see the value in adding one. It
would just mirror the code and become outdated when 'tdx_blob' layout is
updated.
If the concern is that tdx_blob layout changes could cause incompatibilities,
that's not the kernel's responsibility to prevent; the kernel has no control
over external format changes.
If the issue is simply that links may become outdated, that's a common problem.
We can address this by referring to blob_structure.txt in the "Intel TDX Module
Binaries Repository" and dropping the specific link. For example:
TDX Modules are distributed in the tdx_blob format defined in
blob_structure.txt from the "Intel TDX Module Binaries Repository". A
tdx_blob contains a header, sigstruct, and module binary. This is also the
format supplied by the userspace to the kernel.
>
>[...]
>
>> +/*
>> + * Intel TDX Module blob. Its format is defined at:
>> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
I will drop this link as well.
>> + *
>> + * Note this structure differs from the reference above: the two variable-length
>> + * fields "@sigstruct" and "@module" are represented as a single "@data" field
>> + * here and split programmatically using the offset_of_module value.
>> + */
>> +struct tdx_blob {
>> + u16 version;
>> + u16 checksum;
>> + u32 offset_of_module;
>> + u8 signature[8];
>> + u32 length;
>> + u32 resv0;
>> + u64 resv1[509];
>> + u8 data[];
>> +} __packed;
>
>Nit:
>
>It appeared you said you will s/resv/rsvd in v3.
>
>I don't quite mind if other people are fine with 'resv'. Or you can spell
>out 'reserved' in full to match the one in 'struct seamldr_params' above.
Sorry, I missed this feedback. I'll use "reserved".
I even updated "len" to "length" and changed the index to start from 0 (to match
blob_structure.txt) but somehow missed updating "resv."
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao @ 2026-02-24 6:00 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <14ee337df2983edb3677e3929d31e54374a1762e.camel@intel.com>
On Mon, Feb 23, 2026 at 05:25:53PM +0800, Huang, Kai wrote:
>
>>
>> +/*
>> + * During a TDX Module update, all CPUs start from TDP_START and progress
>
>Nit: start from TDP_START or TDP_START + 1 ?
TDP_START. See:
+static int do_seamldr_install_module(void *params)
+{
+ enum tdp_state newstate, curstate = TDP_START;
^^^^^^^^^^^^^^^^^^^^
>
>The code below says:
>
>+ set_target_state(TDP_START + 1);
set_target_state() sets a global target (or next) state for all CPUs. Each CPU
compares its current state to the target. If they don't match, the CPU performs
the required task and then acks the state.
The global target state must be reset at the start of each update to trigger
the do-while loop in do_seamldr_install_module().
>+ ret = stop_machine_cpuslocked(do_seamldr_install_module, params,
>cpu_online_mask);
>
>> + * to TDP_DONE. Each state is associated with certain work. For some
>> + * states, just one CPU needs to perform the work, while other CPUs just
>> + * wait during those states.
>> + */
>> +enum tdp_state {
>> + TDP_START,
>> + TDP_DONE,
>> +};
>
>Nit: just curious, what does "TDP" mean?
>
>Maybe something more obvious?
It stands for TD Preserving. Since this term isn't commonly used outside
Intel, "TDX Module updates" is clearer. I'll change this enum to:
enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_CPU_INIT,
MODULE_UPDATE_RUN_UPDATE,
MODULE_UPDATE_DONE,
};
>
>> +
>> +static struct {
>> + enum tdp_state state;
>> + atomic_t thread_ack;
>> +} tdp_data;
>> +
>> +static void set_target_state(enum tdp_state state)
>> +{
>> + /* Reset ack counter. */
>> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
>> + /* Ensure thread_ack is updated before the new state */
>
>Nit: perhaps add "so that ..." part to the comment?
how about:
/*
* Ensure thread_ack is updated before the new state.
* Otherwise, other CPUs may see the new state and ack
* it before thread_ack is reset. An ack before reset
* is effectively lost, causing the system to wait
* forever for thread_ack to become zero.
*/
>
>> + smp_wmb();
>> + WRITE_ONCE(tdp_data.state, state);
>> +}
>> +
>> +/* Last one to ack a state moves to the next state. */
>> +static void ack_state(void)
>> +{
>> + if (atomic_dec_and_test(&tdp_data.thread_ack))
>> + set_target_state(tdp_data.state + 1);
>> +}
>> +
>> +/*
>> + * See multi_cpu_stop() from where this multi-cpu state-machine was
>> + * adopted, and the rationale for touch_nmi_watchdog()
>> + */
>
>Nit: add a period to the end of the sentence.
>
>(btw, I found using period or not isn't consistent even among the 'one-line-
>sentence' comments, maybe you want to make that consistent.)
Will do. Thanks for this suggestion.
^ permalink raw reply
* Re: [PATCH v4 20/24] x86/virt/tdx: Enable TDX Module runtime updates
From: Chao Gao @ 2026-02-24 6:02 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <5bd8500eba9a8e83491c02ae84f81b55ac09dacb.camel@intel.com>
On Mon, Feb 23, 2026 at 01:09:10PM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> All pieces of TDX Module runtime updates are in place. Enable it if it
>> is supported.
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> ---
>> v4:
>> - s/BIT/BIT_ULL [Tony]
>> ---
>> arch/x86/include/asm/tdx.h | 5 ++++-
>> arch/x86/virt/vmx/tdx/tdx.h | 3 ---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
>> index ffadbf64d0c1..ad62a7be0443 100644
>> --- a/arch/x86/include/asm/tdx.h
>> +++ b/arch/x86/include/asm/tdx.h
>> @@ -32,6 +32,9 @@
>> #define TDX_SUCCESS 0ULL
>> #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
>>
>> +/* Bit definitions of TDX_FEATURES0 metadata field */
>> +#define TDX_FEATURES0_TD_PRESERVING BIT_ULL(1)
>> +#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
>> #ifndef __ASSEMBLER__
>>
>> #include <uapi/asm/mce.h>
>> @@ -105,7 +108,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
>>
>> static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
>> {
>> - return false; /* To be enabled when kernel is ready */
>> + return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
>> }
>>
>> int tdx_guest_keyid_alloc(void);
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
>> index d1807a476d3b..749f4d74cb2c 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.h
>> +++ b/arch/x86/virt/vmx/tdx/tdx.h
>> @@ -88,9 +88,6 @@ struct tdmr_info {
>> DECLARE_FLEX_ARRAY(struct tdmr_reserved_area, reserved_areas);
>> } __packed __aligned(TDMR_INFO_ALIGNMENT);
>>
>> -/* Bit definitions of TDX_FEATURES0 metadata field */
>> -#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
>> -
>>
>
>Nit:
>
>Strictly speaking, moving this "NO_RBP_MOD" isn't required to "enable TDX
>module runtime updates". So I think it's better to call out in changelog
>that this is trying to centralize the bit definitions.
Sure. Will do.
^ permalink raw reply
* Re: [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: Jiri Pirko @ 2026-02-24 8:32 UTC (permalink / raw)
To: John Stultz
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, tjmercier, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <CANDhNCp94KG06P_7ivMTNA27qEM9g8ox-h3b_tZ=v6e-25xJ3g@mail.gmail.com>
Mon, Feb 23, 2026 at 07:33:07PM +0100, jstultz@google.com wrote:
>On Mon, Feb 23, 2026 at 1:51 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Add a new "system_cc_decrypted" dma-buf heap to allow userspace to
>> allocate decrypted (shared) memory for confidential computing (CoCo)
>> VMs.
>>
>> On CoCo VMs, guest memory is encrypted by default. The hardware uses an
>> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
>> Intel TDX) to control whether a given memory access is encrypted or
>> decrypted. The kernel's direct map is set up with encryption enabled,
>> so pages returned by alloc_pages() are encrypted in the direct map
>> by default. To make this memory usable for devices that do not support
>> DMA to encrypted memory (no TDISP support), it has to be explicitly
>> decrypted. A couple of things are needed to properly handle
>> decrypted memory for the dma-buf use case:
>>
>> - set_memory_decrypted() on the direct map after allocation:
>> Besides clearing the encryption bit in the direct map PTEs, this
>> also notifies the hypervisor about the page state change. On free,
>> the inverse set_memory_encrypted() must be called before returning
>> pages to the allocator. If re-encryption fails, pages
>> are intentionally leaked to prevent decrypted memory from being
>> reused as private.
>>
>> - pgprot_decrypted() for userspace and kernel virtual mappings:
>> Any new mapping of the decrypted pages, be it to userspace via
>> mmap or to kernel vmalloc space via vmap, creates PTEs independent
>> of the direct map. These must also have the encryption bit cleared,
>> otherwise accesses through them would see encrypted (garbage) data.
>>
>> - DMA_ATTR_CC_DECRYPTED for DMA mapping:
>> Since the pages are already decrypted, the DMA API needs to be
>> informed via DMA_ATTR_CC_DECRYPTED so it can map them correctly
>> as unencrypted for device access.
>>
>> On non-CoCo VMs, the system_cc_decrypted heap is not registered
>> to prevent misuse by userspace that does not understand
>> the security implications of explicitly decrypted memory.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>Thanks for reworking this! I've not reviewed it super closely, but I
>believe it resolves my objection on your first version.
>
>Few nits/questions below.
>
>> @@ -296,6 +345,14 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
>> for_each_sgtable_sg(table, sg, i) {
>> struct page *page = sg_page(sg);
>>
>> + /*
>> + * Intentionally leak pages that cannot be re-encrypted
>> + * to prevent decrypted memory from being reused.
>> + */
>> + if (buffer->decrypted &&
>> + system_heap_set_page_encrypted(page))
>> + continue;
>> +
>
>What are the conditions where this would fail? How much of an edge
>case is this? I fret this opens a DoS vector if one is able to
>allocate from this heap and then stress the system when doing the
>free.
From what I can see, the failure of set_memory_encrypted() is quite
rare. Don't see any real DoS scenario for this. All the failures seems
to be either theoretical (sanity checks, malicious VMM) or concurrent
kexec execution in case of x86/pat.
>
>Should there be some global list of leaked decrypted pages such that
>the mm subsystem could try again later to recover these?
swiotlb does the same non-recovery leakage. I belive is it not worth
implementing this at this time,
>
>> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
>> index 648328a64b27..d97b668413c1 100644
>> --- a/include/linux/dma-heap.h
>> +++ b/include/linux/dma-heap.h
>> @@ -10,6 +10,7 @@
>> #define _DMA_HEAPS_H
>>
>> #include <linux/types.h>
>> +#include <uapi/linux/dma-heap.h>
>>
>> struct dma_heap;
>>
>> diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
>> index a4cf716a49fa..ab95bb355ed5 100644
>> --- a/include/uapi/linux/dma-heap.h
>> +++ b/include/uapi/linux/dma-heap.h
>> @@ -18,8 +18,7 @@
>> /* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
>> #define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
>>
>> -/* Currently no heap flags */
>> -#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
>> +#define DMA_HEAP_VALID_HEAP_FLAGS (0)
>>
>> /**
>> * struct dma_heap_allocation_data - metadata passed from userspace for
>
>Are these header changes still necessary?
Oops, leftovers. Will remove.
Thanks!
>
>thanks
>-john
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Binbin Wu @ 2026-02-24 8:50 UTC (permalink / raw)
To: Edgecombe, Rick P, Li, Xiaoyao, changyuanl@google.com,
pbonzini@redhat.com, seanjc@google.com, Wu, Binbin,
Yamahata, Isaku
Cc: bp@alien8.de, x86@kernel.org, kas@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, kvm@vger.kernel.org,
linux-coco@lists.linux.dev
In-Reply-To: <213d614fe73e183a230c8f4e0c8fa1cc3d45df39.camel@intel.com>
On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
> +binbin
>
> On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu wrote:
>> Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
>> structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
>> This ensures consistency with the CPUID entries returned by
>> KVM_GET_SUPPORTED_CPUID.
>>
>> Additionally, add a WARN_ON_ONCE() to verify that the TDX module's
>> reported entries align with KVM's expectations regarding indexed
>> CPUID functions.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
>> ---
>> arch/x86/kvm/vmx/tdx.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> index 2d7a4d52ccfb4..0c524f9a94a6c 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
>> kvm_cpuid_entry2 *entry, unsigned char i
>> entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
>> entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
>>
>> - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
>> + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
>> entry->index = 0;
>> + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>
> There are two callers of this. One is already zeroed, and the other has
> stack garbage in flags. But that second caller doesn't look at the
> flags so it is harmless. Maybe it would be simpler and clearer to just
> zero init the entry struct in that caller. Then you don't need to clear
> it here. Or alternatively set flags to zero above, and then add
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> single bit in a field of garbage, which seems weird.
>
>> + } else {
>> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>> + }
>>
>> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
>> + !!(entry->flags &
>> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
>
> It warns on leaf 0x23 for me. Is it intentional?
I guess because the list in cpuid_function_is_indexed() is hard-coded
and 0x23 is not added into the list yet.
It's fine for existing KVM code because cpuid_function_is_indexed() is
only used to check that if a CPUID entry is queried without index, it
shouldn't be included in the indexed list.
But adding the consistency check here would cause compatibility issue.
Generally, if a new CPUID indexed function is added for some new CPU and
the TDX module reports it, KVM versions without the CPUID function in
the list will trigger the warning.
>
> This warning kind of begs the question of how how much consistency
> there should be between KVM_TDX_CAPABILITIES and
> KVM_GET_SUPPORTED_CPUID. There was quite a bit of debate on this and in
> the end we moved forward with a solution that did the bare minimum
> consistency checking.
>
> We actually have been looking at some potential TDX module changes to
> fix the deficiencies from not enforcing the consistency. But didn't
> consider this pattern. Can you explain more about the failure mode?
>
>> /*
>> * The TDX module doesn't allow configuring the guest phys
>> addr bits
>> * (EAX[23:16]). However, KVM uses it as an interface to
>> the userspace
>> --
>
^ permalink raw reply
* Re: [PATCH v4 03/24] coco/tdx-host: Expose TDX Module version
From: Huang, Kai @ 2026-02-24 10:18 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com, kas@kernel.org, seanjc@google.com,
Chatre, Reinette, Weiny, Ira, binbin.wu@linux.intel.com,
Verma, Vishal L, nik.borisov@suse.com,
linux-kernel@vger.kernel.org, sagis@google.com, Annapurve, Vishal,
Duan, Zhenzhong, Edgecombe, Rick P, tony.lindgren@linux.intel.com,
paulmck@kernel.org, yilun.xu@linux.intel.com, x86@kernel.org,
Williams, Dan J
In-Reply-To: <aZ0Gm5/xpBnhOeod@intel.com>
On Tue, 2026-02-24 at 10:02 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 08:40:13AM +0800, Huang, Kai wrote:
> > On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > > For TDX Module updates, userspace needs to select compatible update
> > > versions based on the current module version. This design delegates
> > > module selection complexity to userspace because TDX Module update
> > > policies are complex and version series are platform-specific.
> > >
> > > For example, the 1.5.x series is for certain platform generations, while
> > > the 2.0.x series is intended for others. And TDX Module 1.5.x may be
> > > updated to 1.5.y but not to 1.5.y+1.
> > >
> > > Expose the TDX Module version to userspace via sysfs to aid module
> > > selection. Since the TDX faux device will drive module updates, expose
> > > the version as its attribute.
> > >
> > > One bonus of exposing TDX Module version via sysfs is: TDX Module
> > > version information remains available even after dmesg logs are cleared.
> > >
> > > == Background ==
> > >
> > > The "faux device + device attribute" approach compares to other update
> > > mechanisms as follows:
> >
> > This "faux device + device attribute" approach seems to be a wider design
> > choice instead of how to expose module version (which is the scope of this
> > patch). Overall, shouldn't this be in the changelog of the previous patch
> > which actually introduces "faux device" (albeit no attribute is introduced
> > in that patch)?
> ,
> Yes, it's mentioned briefly in the previous patch:
>
> """
> Create a virtual device not only to align with other implementations but
> also to make it easier to
>
> - expose metadata (e.g., TDX module version, seamldr version etc) to
> the userspace as device attributes
>
> ...
> """
>
> The previous patch doesn't provide details for version information
> exposure, as version attributes are just one of several purposes for the
> virtual device.
>
> >
> > >
> > > 1. AMD SEV leverages an existing PCI device for the PSP to expose
> > > metadata. TDX uses a faux device as it doesn't have PCI device
> > > in its architecture.
> >
> > E.g., this sounds to justify "why to use faux device for TDX", but not "to
> > expose module version via faux device attributes".
>
> This provides additional context as suggested by Dave:
>
> https://lore.kernel.org/kvm/aa3f026b-ad69-4070-8433-8950e5250edb@intel.com/
>
> Dave asked:
>
> """
> What are other CPU vendors doing for this? SEV? CCA? S390? How are their
> firmware versions exposed? What about other things in the Intel world
> like CPU microcode or the billion other chunks of firmware? ...
> """
I fully agree with this. We need justification of why we need to expose TDX
module version to somewhere in /sysfs, and the choice of that somewhere is
the faux device attributes.
But my interpretation is Dave is asking to provide such justification in
general, but not specifically in _this_ patch.
In this patch, you have already adequately put why to expose version info
via /sysfs. The "background" is really explaining why to choose "faux
device" as the /sysfs entry.
But you have already made the choice to use faux device (and mentioned
exposing version is one purpose) in the previous patch, so to me the
"background" part is a bit weird to be here, but not in previous patch.
But I also see there's some connection here -- and anyway this is just my
interpretation, so feel free to ignore :-)
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Huang, Kai @ 2026-02-24 10:25 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0Nnay7ygKeXmuC@intel.com>
>
> >
> > But I don't know why do you even need to talk about NP-SEAMLDR.
>
> I included this because Dave had some confusion about NP-SEAMLDR [1], so I
> wanted to clarify it.
>
> [1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/
I thought that was under assumption both NP-SEAMLDR and P-SEAMLDR are SEAM
software (which is why both of them are mentioned). But only P-SEAMLDR is,
so I thought we can skip NP-SEAMLDR.
>
> And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
> helpful to clarify the difference. This follows Dave's earlier suggestion to
> explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].
>
> [2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/
>
Sure. If you feel that helps.
[...]
>
> > > + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> > > + * interact with P-SEAMLDR simultaneously.
> > > + */
> > > +static DEFINE_RAW_SPINLOCK(seamldr_lock);
> > > +
> > > +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> > > +{
> > > + /*
> > > + * Serialize P-SEAMLDR calls and disable interrupts as the calls
> > > + * can be made from IRQ context.
> > > + */
> > > + guard(raw_spinlock_irqsave)(&seamldr_lock);
> >
> > Why do you need to disable IRQ? A plain raw_spinlock should work with both
> > cases where seamldr_call() is called from IRQ disabled context and normal
> > task context?
>
> No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
> fires while a task context already holds the lock, and the interrupt handler
> also tries to acquire the same lock.
I thought that's not possible to happen because during module update we have
a machine state to serialize these P-SEAMLDR SEAMCALLs.
But I agree making it IRQ safe is the simplest way so that we don't need to
worry about the deadlock.
Sorry about the noise.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox