* [PATCH 2/2] x86/tdx: Fix zero-extension for 32-bit port I/O
From: Kiryl Shutsemau (Meta) @ 2026-03-31 11:24 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: Kiryl Shutsemau, H . Peter Anvin, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Borys Tsyrulnikov, linux-kernel,
linux-coco, kvm, stable
In-Reply-To: <20260331112430.71425-1-kas@kernel.org>
According to x86 architecture rules, 32-bit operations zero-extend the
result to 64 bits. The current implementation of handle_in() only masks
the lower 32 bits, which preserves the upper 32 bits of RAX when a
32-bit port IN instruction is emulated.
Update handle_in() to zero out the entire RAX register when the I/O size
is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2
bytes), continue to preserve the unaffected upper bits.
Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Cc: stable@vger.kernel.org
---
arch/x86/coco/tdx/tdx.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 4d7f71d50122..b9b9a2d75119 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
*/
success = !__tdx_hypercall(&args);
- /* Update part of the register affected by the emulated instruction */
- regs->ax &= ~mask;
+ /*
+ * Update part of the register affected by the emulated instruction.
+ *
+ * 32-bit operands generate a 32-bit result, zero-extended to a 64-bit
+ * result.
+ */
+ if (size < 4)
+ regs->ax &= ~mask;
+ else
+ regs->ax = 0;
+
if (success)
regs->ax |= args.r11 & mask;
--
2.51.2
^ permalink raw reply related
* [PATCH 1/2] x86/tdx: Fix off-by-one in port I/O handling
From: Kiryl Shutsemau (Meta) @ 2026-03-31 11:24 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: Kiryl Shutsemau, H . Peter Anvin, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Borys Tsyrulnikov, linux-kernel,
linux-coco, kvm, stable
In-Reply-To: <20260331112430.71425-1-kas@kernel.org>
handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:
u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l) includes bit h. For size=1 (INB), this produces
GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8
bits). The mask is one bit too wide for all I/O sizes.
Fix the mask calculation.
Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Cc: stable@vger.kernel.org
---
arch/x86/coco/tdx/tdx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 7b2833705d47..4d7f71d50122 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -693,7 +693,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
.r13 = PORT_READ,
.r14 = port,
};
- u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+ u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
bool success;
/*
@@ -713,7 +713,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
static bool handle_out(struct pt_regs *regs, int size, int port)
{
- u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+ u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
/*
* Emulate the I/O write via hypercall. More info about ABI can be found
--
2.51.2
^ permalink raw reply related
* [PATCH 0/2] x86/tdx: Port I/O emulation fixes
From: Kiryl Shutsemau (Meta) @ 2026-03-31 11:24 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: Kiryl Shutsemau, H . Peter Anvin, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Borys Tsyrulnikov, linux-kernel,
linux-coco, kvm, stable
This series addresses two technical inaccuracies in the TDX guest port
I/O emulation code reported by Borys Tsyrulnikov.
The first patch fixes an off-by-one error in the GENMASK() macro usage
where the mask was being calculated as one bit too wide (e.g., 9 bits for
an 8-bit operation).
The second patch ensures that 32-bit port I/O operations (INL) correctly
zero-extend the result to the full 64-bit RAX register, as required by
the x86 architecture. Currently, the emulation preserves the upper 32
bits of RAX during such operations.
Both issues were introduced in the initial implementation of the runtime
hypercalls for port I/O.
Kiryl Shutsemau (Meta) (2):
x86/tdx: Fix off-by-one in port I/O handling
x86/tdx: Fix zero-extension for 32-bit port I/O
arch/x86/coco/tdx/tdx.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
--
2.51.2
^ permalink raw reply
* Re: [PATCH v13 10/48] arm64: RMI: Ensure that the RMM has GPT entries for memory
From: Suzuki K Poulose @ 2026-03-31 11:05 UTC (permalink / raw)
To: Mathieu Poirier, Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Gavin Shan,
Shanker Donthineni, Alper Gun, Aneesh Kumar K . V, Emi Kisanuki,
Vishal Annapurve
In-Reply-To: <acrj-cKphy4hJsEG@p14s>
Hi Mathieu,
On 30/03/2026 21:58, Mathieu Poirier wrote:
> Hi,
>
> On Wed, Mar 18, 2026 at 03:53:34PM +0000, Steven Price wrote:
>> The RMM may not be tracking all the memory of the system at boot. Create
>> the necessary tracking state and GPTs within the RMM so that all boot
>> memory can be delegated to the RMM as needed during runtime.
>>
>> Note: support is currently missing for SROs which means that if the RMM
>> needs memory donating this will fail (and render CCA unusable in Linux).
>>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> New patch for v13
>> ---
>> arch/arm64/kvm/rmi.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>>
>> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
>> index 9590dff9a2c1..80aedc85e94a 100644
>> --- a/arch/arm64/kvm/rmi.c
>> +++ b/arch/arm64/kvm/rmi.c
>> @@ -4,6 +4,7 @@
>> */
>>
>> #include <linux/kvm_host.h>
>> +#include <linux/memblock.h>
>>
>> #include <asm/kvm_pgtable.h>
>> #include <asm/rmi_cmds.h>
>> @@ -56,6 +57,18 @@ static int rmi_check_version(void)
>> return 0;
>> }
>>
>> +/*
>> + * These are the 'default' sizes when passing 0 as the tracking_region_size.
>> + * TODO: Support other granule sizes
>> + */
>> +#ifdef CONFIG_PAGE_SIZE_4KB
>> +#define RMM_GRANULE_TRACKING_SIZE SZ_1G
>> +#elif defined(CONFIG_PAGE_SIZE_16KB)
>> +#define RMM_GRANULE_TRACKING_SIZE SZ_32M
>> +#elif defined(CONFIG_PAGE_SIZE_64KB)
>> +#define RMM_GRANULE_TRACKING_SIZE SZ_512M
>> +#endif
>> +
>> static int rmi_configure(void)
>> {
>> struct rmm_config *config __free(free_page) = NULL;
>> @@ -95,6 +108,80 @@ static int rmi_configure(void)
>> return 0;
>> }
>>
>> +static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
>> +{
>> + start = ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
>
> This will produce an error on systems where the start of system memory is not
> aligned to RMM_GRANULE_TRACKING_SIZE. For instance, on QEMU-SBSA the system
> memory starts at 0x100_4300_0000. With the above and RMM_GRANULE_TRACKING_SIZE
> set to SZ_1G, @start becomes 0x100_4000_0000, which falls outside the memory map
> known to the TF-A. I fixed it with these modifications:
Thanks for raising this. This would need to be addressed in the RMM
spec, I have raised it with the team and will be addressed soon.
>
> LINUX:
>
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index 10ff1c3bddaf..21bfbbe2f047 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -424,7 +424,9 @@ static int rmi_configure(void)
>
> static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> {
> - start = ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
> + phys_addr_t offset;
> +
> + offset = start - ALIGN_DOWN(start, RMM_GRANULE_TRACKING_SIZE);
> end = ALIGN(end, RMM_GRANULE_TRACKING_SIZE);
>
> while (start < end) {
> @@ -439,7 +441,13 @@ static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> start);
> return -ENODEV;
> }
> - start += RMM_GRANULE_TRACKING_SIZE;
> +
> + if (offset) {
> + start += (RMM_GRANULE_TRACKING_SIZE - offset);
> + offset = 0;
> + } else {
> + start += RMM_GRANULE_TRACKING_SIZE;
> + }
> }
>
> return 0;
>
> RMM:
>
> diff --git a/runtime/rmi/granule.c b/runtime/rmi/granule.c
> index cef521fc0869..60358d9ee81e 100644
> --- a/runtime/rmi/granule.c
> +++ b/runtime/rmi/granule.c
> @@ -209,9 +209,11 @@ void smc_granule_tracking_get(unsigned long addr,
> return;
> }
>
> +#if 0
> if (!ALIGNED(addr, RMM_INTERNAL_TRACKING_REGION_SIZE)) {
> return;
> }
> +#endif
>
> g = find_granule(addr);
> if (g != NULL) {
>
> This is likely not the right fix but hopefully provides some guidance. Send me
> your patches when you have an idea and I'll test them.
We will send you the update once it is fixed in the RMM spec. The rough
idea is to remove the ALIGNMENT restrictions and return a Range that
the host can iterate over to find "regions" with the same type of
memory.
Cheers
Suzuki
>
> Thanks,
> Mathieu
>
>
>> + end = ALIGN(end, RMM_GRANULE_TRACKING_SIZE);
>> +
>> + while (start < end) {
>> + unsigned long ret, category, state;
>> +
>> + ret = rmi_granule_tracking_get(start, &category, &state);
>> + if (ret != RMI_SUCCESS ||
>> + state != RMI_TRACKING_FINE ||
>> + category != RMI_MEM_CATEGORY_CONVENTIONAL) {
>> + /* TODO: Set granule tracking in this case */
>> + kvm_err("Granule tracking for region isn't fine/conventional: %llx",
>> + start);
>> + return -ENODEV;
>> + }
>> + start += RMM_GRANULE_TRACKING_SIZE;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long rmi_l0gpt_size(void)
>> +{
>> + return 1UL << (30 + FIELD_GET(RMI_FEATURE_REGISTER_1_L0GPTSZ,
>> + rmm_feat_reg1));
>> +}
>> +
>> +static int rmi_create_gpts(phys_addr_t start, phys_addr_t end)
>> +{
>> + unsigned long l0gpt_sz = rmi_l0gpt_size();
>> +
>> + start = ALIGN_DOWN(start, l0gpt_sz);
>> + end = ALIGN(end, l0gpt_sz);
>> +
>> + while (start < end) {
>> + int ret = rmi_gpt_l1_create(start);
>> +
>> + if (ret && ret != RMI_ERROR_GPT) {
>> + /*
>> + * FIXME: Handle SRO so that memory can be donated for
>> + * the tables.
>> + */
>> + kvm_err("GPT Level1 table missing for %llx\n", start);
>> + return -ENOMEM;
>> + }
>> + start += l0gpt_sz;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int rmi_init_metadata(void)
>> +{
>> + phys_addr_t start, end;
>> + const struct memblock_region *r;
>> +
>> + for_each_mem_region(r) {
>> + int ret;
>> +
>> + start = memblock_region_memory_base_pfn(r) << PAGE_SHIFT;
>> + end = memblock_region_memory_end_pfn(r) << PAGE_SHIFT;
>> + ret = rmi_verify_memory_tracking(start, end);
>> + if (ret)
>> + return ret;
>> + ret = rmi_create_gpts(start, end);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int rmm_check_features(void)
>> {
>> if (kvm_lpa2_is_enabled() && !rmi_has_feature(RMI_FEATURE_REGISTER_0_LPA2)) {
>> @@ -120,6 +207,8 @@ void kvm_init_rmi(void)
>> return;
>> if (rmi_configure())
>> return;
>> + if (rmi_init_metadata())
>> + return;
>>
>> /* Future patch will enable static branch kvm_rmi_is_available */
>> }
>> --
>> 2.43.0
>>
>>
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Nikolay Borisov @ 2026-03-31 11:00 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-11-yilun.xu@linux.intel.com>
On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> Adding more memory to TDX Module is the first step to enable Extensions.
>
> Currently, TDX Module memory use is relatively static. But, some new
> features (called "TDX Module Extensions") need to use memory more
> dynamically. While 'static' here means the kernel provides necessary
> amount of memory to TDX Module for its basic functionalities, 'dynamic'
> means extra memory is needed only if new optional features are to be
> enabled. So add a new memory feeding process backed by a new SEAMCALL
> TDH.EXT.MEM.ADD.
>
> The process is mostly the same as adding PAMT. The kernel queries TDX
> Module how much memory needed, allocates it, hands it over, and never
> gets it back.
>
> TDH.EXT.MEM.ADD uses tdx_page_array to provide control (private) pages
> to TDX Module. Introduce a tdx_clflush_page_array() helper to flush
> shared cache before SEAMCALL, to avoid shared cache write back damages
> these private pages.
>
> For now, TDX Module Extensions consume relatively large amount of
> memory (~50MB). Use contiguous page allocation to avoid permanently
> fragment too much memory. Print this readout value on TDX Module
> Extensions initialization for visibility.
>
> Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 92 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 870bb75da3ba..31ccdfcf518c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -60,6 +60,7 @@
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG_V0 45
> #define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
> +#define TDH_EXT_MEM_ADD 61
>
> /* TDX page types */
> #define PT_NDA 0x0
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 4fb56bb442f0..5fae17c13191 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -560,7 +560,7 @@ static int tdx_alloc_pages_contig(unsigned int nr_pages, struct page **pages,
> * Similar to tdx_page_array_alloc(), after allocating with this
> * function, call tdx_page_array_populate() to populate the tdx_page_array.
> */
> -static __maybe_unused struct tdx_page_array *
> +static struct tdx_page_array *
> tdx_page_array_alloc_contig(unsigned int nr_pages)
> {
> return tdx_page_array_alloc(nr_pages, tdx_alloc_pages_contig, NULL);
> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(tdx_page_array_create_iommu_mt);
> #define HPA_LIST_INFO_PFN GENMASK_U64(51, 12)
> #define HPA_LIST_INFO_LAST_ENTRY GENMASK_U64(63, 55)
>
> -static u64 __maybe_unused hpa_list_info_assign_raw(struct tdx_page_array *array)
> +static u64 hpa_list_info_assign_raw(struct tdx_page_array *array)
> {
> return FIELD_PREP(HPA_LIST_INFO_FIRST_ENTRY, 0) |
> FIELD_PREP(HPA_LIST_INFO_PFN,
> @@ -1513,6 +1513,94 @@ static void tdx_clflush_page(struct page *page)
> clflush_cache_range(page_to_virt(page), PAGE_SIZE);
> }
>
> +static void tdx_clflush_page_array(struct tdx_page_array *array)
> +{
> + for (int i = 0; i < array->nents; i++)
shouldn't the actual number of entries be adjusted as per offset,
similarly to how 'nents' in tdx_page_array_validate_release is calculated?
<snip>
^ permalink raw reply
* Re: [PATCH v2 08/31] x86/virt/tdx: Configure TDX Module with optional TDX Connect feature
From: Nikolay Borisov @ 2026-03-31 10:38 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-9-yilun.xu@linux.intel.com>
On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> TDX Module supports optional TDX features (e.g. TDX Connect & TDX Module
> Extensions) that won't be enabled by default. It extends TDH.SYS.CONFIG
> for host to choose to enable them on bootup.
>
> Call TDH.SYS.CONFIG with a new bitmap input parameter to specify which
> features to enable. The bitmap uses the same definitions as
> TDX_FEATURES0. But note not all bits in TDX_FEATURES0 are valid for
> configuration, e.g. TDX Module Extensions is a service that supports TDX
> Connect, it is implicitly enabled when TDX Connect is enabled. Setting
> TDX_FEATURES0_EXT in the bitmap has no effect.
>
> TDX Module advances the version of TDH.SYS.CONFIG for the change, so
> use the latest version (v1) for optional feature enabling. But
> supporting existing Modules which only support v0 is still necessary
> until they are deprecated, enumerate via TDX_FEATURES0 to decide which
> version to use.
>
> TDX Module updates global metadata when optional features are enabled.
> Host should update the cached tdx_sysinfo to reflect these changes.
>
> Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.h | 3 ++-
> arch/x86/virt/vmx/tdx/tdx.c | 16 +++++++++++++++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index e5a9331df451..870bb75da3ba 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -58,7 +58,8 @@
> #define TDH_PHYMEM_CACHE_WB 40
> #define TDH_PHYMEM_PAGE_WBINVD 41
> #define TDH_VP_WR 43
> -#define TDH_SYS_CONFIG 45
> +#define TDH_SYS_CONFIG_V0 45
> +#define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
Since newer versions of tdx module apis are backwards compatible with
older ones, and v0 are actually deprecated why have both definitions?
<snip>
^ permalink raw reply
* Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects
From: Xu Yilun @ 2026-03-31 10:11 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: Gao, Chao, Xu, Yilun, x86@kernel.org, kas@kernel.org,
baolu.lu@linux.intel.com, dave.hansen@linux.intel.com,
Li, Xiaoyao, Williams, Dan J, Jiang, Dave,
linux-pci@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Duan, Zhenzhong, Verma, Vishal L,
kvm@vger.kernel.org
In-Reply-To: <f163ab3b7c939a7d19864dfde9897ca3b424c60b.camel@intel.com>
On Mon, Mar 30, 2026 at 11:57:11PM +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-03-30 at 23:47 +0800, Xu Yilun wrote:
> > > pages is going to be an array of struct pointers, and root is a single page
> > > of
> > > PA's that gets re-used to copy and pass the PA's to the TDX module. Why do
> > > we
> > > need both? Like just keep an array of PA's that would be the same size as
> > > the
> > > struct page array. And not need the populate loop?
> >
> > We need Linux language, struct page *, for alloc and free. Also need
> > TDX Module language - PA list - for SEAMCALLs. So IIUC, the page to PA
> > populating won't disappear on allocation, the PA to page populating
> > would appear on free.
>
> Not sure what you mean by this.
I mean host use struct page * for memory allocation and free. If we only
keep the PA list, we still need to convert PAs back to struct page * and
free.
>
> >
> > Besides, host may need to vmap and access the (shared) pages.
>
> Some code someday may need to convert a PA to another format? Is that it?
No, I don't convert a PA to something else. PA list is only for
SEAMCALLs.
Now we use vm_map_ram(array->pages, ...) to map the shared pages,
that's another reason I want to keep the struct page ** in
struct tdx_page_array.
Anyway we use struct page ** for kernel memory management in several
cases, keeping the struct page ** avoids PA -> page populating.
> Doesn't seem like big problem.
>
> But I'm not sure about this idea yet.
^ permalink raw reply
* Re: [PATCH v6 05/22] x86/virt/seamldr: Add a helper to retrieve P-SEAMLDR information
From: Xiaoyao Li @ 2026-03-31 10:25 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260326084448.29947-6-chao.gao@intel.com>
On 3/26/2026 4:43 PM, Chao Gao wrote:
> P-SEAMLDR returns its information such as version number, in response to
> the SEAMLDR.INFO SEAMCALL.
>
> This information is useful for userspace. For example, the admin can decide
> which TDX module versions are compatible with the P-SEAMLDR according to
> the P-SEAMLDR version.
>
> Add a helper to retrieve P-SEAMLDR information in preparation for
> exposing P-SEAMLDR version and other necessary information to userspace.
> Export the new kAPI for use by tdx-host.ko.
>
> Note that there are two distinct P-SEAMLDR APIs with similar names:
>
> SEAMLDR.INFO: Returns a SEAMLDR_INFO structure containing SEAMLDR
> information such as version and remaining updates.
>
> SEAMLDR.SEAMINFO: Returns a SEAMLDR_SEAMINFO structure containing SEAM
> and system information such as Convertible Memory
> Regions (CMRs) and number of CPUs and sockets.
>
> The former is used here.
>
> For details, see "Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
> Interface Specification" revision 343755-003.
>
> Signed-off-by: Chao Gao<chao.gao@intel.com>
> Reviewed-by: Kai Huang<kai.huang@intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta)<kas@kernel.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v6 04/22] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Xiaoyao Li @ 2026-03-31 10:23 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm, linux-rt-devel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt
In-Reply-To: <20260326084448.29947-5-chao.gao@intel.com>
On 3/26/2026 4:43 PM, 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.
>
> Use seamcall_prerr() (not '_ret') because current P-SEAMLDR calls do not
> use any output registers other than RAX.
>
> 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 need to interact with it for runtime update.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v6 03/22] coco/tdx-host: Expose TDX module version
From: Xiaoyao Li @ 2026-03-31 10:21 UTC (permalink / raw)
To: Chao Gao, x86, linux-coco, kvm, linux-kernel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, yan.y.zhao
In-Reply-To: <20260326084448.29947-4-chao.gao@intel.com>
On 3/26/2026 4:43 PM, 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:
>
> 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.
>
> 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
>
> 3. ARM's CCA implementation isn't in-tree yet, but will likely follow a
> similar faux device approach, though it's unclear whether they need
> to expose firmware version information
>
> 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>
> Reviewed-by: Kai Huang<kai.huang@intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta)<kas@kernel.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v6 02/22] coco/tdx-host: Introduce a "tdx_host" device
From: Xiaoyao Li @ 2026-03-31 10:07 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, yan.y.zhao, Jonathan Cameron, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260326084448.29947-3-chao.gao@intel.com>
On 3/26/2026 4:43 PM, Chao Gao wrote:
> TDX depends on a platform firmware module that is invoked via instructions
> similar to vmenter (i.e. enter into a new privileged "root-mode" context to
> manage private memory and private device mechanisms). It is a software
> construct that depends on the CPU vmxon state to enable invocation of
> TDX module ABIs. Unlike other Trusted Execution Environment (TEE) platform
> implementations that employ a firmware module running on a PCI device with
> an MMIO mailbox for communication, TDX has no hardware device to point to
> as the TEE Secure Manager (TSM).
>
> 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
>
> - implement firmware uploader APIs which are tied to a device. This is
> needed to support TDX module runtime updates
>
> - enable TDX Connect which will share a common infrastructure with other
> platform implementations. In the TDX Connect context, every
> architecture has a TSM, represented by a PCIe or virtual device. The
> new "tdx_host" device will serve the TSM role.
>
> A faux device is used for TDX because the TDX module is singular within
> 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>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
^ permalink raw reply
* Re: [PATCH v6 01/22] x86/virt/tdx: Move low level SEAMCALL helpers out of <asm/tdx.h>
From: Xiaoyao Li @ 2026-03-31 9:51 UTC (permalink / raw)
To: Chao Gao, linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, yan.y.zhao, Zhenzhong Duan, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260326084448.29947-2-chao.gao@intel.com>
On 3/26/2026 4:43 PM, Chao Gao wrote:
> From: Kai Huang <kai.huang@intel.com>
>
> TDX host core code implements three seamcall*() helpers to make SEAMCALL
> to the TDX module. Currently, they are implemented in <asm/tdx.h> and
> are exposed to other kernel code which includes <asm/tdx.h>.
>
> However, other than the TDX host core, seamcall*() are not expected to
> be used by other kernel code directly. For instance, for all SEAMCALLs
> that are used by KVM, the TDX host core exports a wrapper function for
> each of them.
>
> Move seamcall*() and related code out of <asm/tdx.h> and make them only
> visible to TDX host core.
>
> Since TDX host core tdx.c is already very heavy, don't put low level
> seamcall*() code there but to a new dedicated "seamcall_internal.h". Also,
> currently tdx.c has seamcall_prerr*() helpers which additionally print
> error message when calling seamcall*() fails. Move them to
> "seamcall_internal.h" as well. In such way all low level SEAMCALL helpers
> are in a dedicated place, which is much more readable.
>
> Copy the copyright notice from the original files and consolidate the
> date ranges to:
>
> Copyright (C) 2021-2023 Intel Corporation
>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
[...]
> -static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
> - struct tdx_module_args *args)
Nit: Maybe take the chance to align the second line?
^ permalink raw reply
* Re: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
From: Baolu Lu @ 2026-03-31 7:20 UTC (permalink / raw)
To: kernel test robot, Xu Yilun, linux-coco, linux-pci,
dan.j.williams, x86
Cc: oe-kbuild-all, chao.gao, dave.jiang, yilun.xu, zhenzhong.duan,
kvm, rick.p.edgecombe, dave.hansen, kas, xiaoyao.li,
vishal.l.verma, linux-kernel
In-Reply-To: <202603290006.za7iiDgF-lkp@intel.com>
On 3/29/26 00:57, kernel test robot wrote:
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 11439c4635edd669ae435eec308f4ab8a0804808]
>
> url:https://github.com/intel-lab-lkp/linux/commits/Xu-Yilun/x86-tdx-Move-
> all-TDX-error-defines-into-asm-shared-tdx_errno-h/20260328-151524
> base: 11439c4635edd669ae435eec308f4ab8a0804808
> patch link:https://lore.kernel.org/r/20260327160132.2946114-20-
> yilun.xu%40linux.intel.com
> patch subject: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
> config: i386-randconfig-141-20260328 (https://download.01.org/0day-ci/archive/20260329/202603290006.za7iiDgF-
> lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> smatch: v0.5.0-9004-gb810ac53
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603290006.za7iiDgF-
> lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot<lkp@intel.com>
> | Closes:https://lore.kernel.org/oe-kbuild-all/202603290006.za7iiDgF-lkp@intel.com/
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
>>> WARNING: modpost: vmlinux: section mismatch in reference: iommu_max_domain_id+0x55 (section: .text.iommu_max_domain_id) -> acpi_table_parse_keyp (section: .init.text)
acpi_table_parse_keyp() is marked as __init. But this patch causes the
intel iommu driver to call it from a runtime function.
int __init_or_acpilib
acpi_table_parse_keyp(enum acpi_keyp_type id,
acpi_tbl_entry_handler_arg handler_arg, void *arg)
{
return __acpi_table_parse_entries(ACPI_SIG_KEYP,
sizeof(struct
acpi_table_keyp), id,
NULL, handler_arg, arg, 0);
}
One way to solve this might be parsing the table once in the __init
context and store the result in variable that could be used after boot.
How about the following additional change (untested)?
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 097c4a90302f..0b384a58a3a0 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -63,6 +63,7 @@ LIST_HEAD(dmar_drhd_units);
struct acpi_table_header * __initdata dmar_tbl;
static int dmar_dev_scope_status = 1;
static DEFINE_IDA(dmar_seq_ids);
+static bool tdx_tvm_usable __ro_after_init;
static int alloc_iommu(struct dmar_drhd_unit *drhd);
static void free_iommu(struct intel_iommu *iommu);
@@ -915,6 +916,17 @@ dmar_validate_one_drhd(struct acpi_dmar_header
*entry, void *arg)
return 0;
}
+static void __init intel_iommu_check_tdxc_enhancement(void)
+{
+ int tvm_usable = 0;
+ int ret;
+
+ ret = acpi_table_parse_keyp(ACPI_KEYP_TYPE_CONFIG_UNIT,
+ keyp_config_unit_tvm_usable, &tvm_usable);
+ if (ret >= 0)
+ tdx_tvm_usable = !!tvm_usable;
+}
+
void __init detect_intel_iommu(void)
{
int ret;
@@ -923,6 +935,8 @@ void __init detect_intel_iommu(void)
.ignore_unhandled = true,
};
+ intel_iommu_check_tdxc_enhancement();
+
down_write(&dmar_global_lock);
ret = dmar_table_detect();
if (!ret)
@@ -1046,24 +1060,6 @@ static int keyp_config_unit_tvm_usable(union
acpi_subtable_headers *header,
return 0;
}
-static bool platform_is_tdxc_enhanced(void)
-{
- static int tvm_usable = -1;
- int ret;
-
- /* only need to parse once */
- if (tvm_usable != -1)
- return !!tvm_usable;
-
- tvm_usable = 0;
- ret = acpi_table_parse_keyp(ACPI_KEYP_TYPE_CONFIG_UNIT,
- keyp_config_unit_tvm_usable, &tvm_usable);
- if (ret < 0)
- tvm_usable = 0;
-
- return !!tvm_usable;
-}
-
static unsigned long iommu_max_domain_id(struct intel_iommu *iommu)
{
unsigned long ndoms = cap_ndoms(iommu->cap);
@@ -1075,7 +1071,7 @@ static unsigned long iommu_max_domain_id(struct
intel_iommu *iommu)
* the VMM’s DID setting, reserving the MSB bit for the TDX module. The
* TDX module always sets this reserved bit on the trusted DMA table.
*/
- if (ecap_tdxc(iommu->ecap) && platform_is_tdxc_enhanced()) {
+ if (ecap_tdxc(iommu->ecap) && tdx_tvm_usable) {
pr_info_once("Most Significant Bit of domain ID reserved.\n");
return ndoms >> 1;
}
Thanks,
baolu
^ permalink raw reply related
* Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects
From: Tony Lindgren @ 2026-03-31 6:25 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: yilun.xu@linux.intel.com, Gao, Chao, Xu, Yilun, x86@kernel.org,
kas@kernel.org, baolu.lu@linux.intel.com,
dave.hansen@linux.intel.com, Li, Xiaoyao, Williams, Dan J,
Jiang, Dave, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org,
Duan, Zhenzhong, Verma, Vishal L, kvm@vger.kernel.org
In-Reply-To: <23b92a154c859450d106de6c9badbe284e3aa19f.camel@intel.com>
On Mon, Mar 30, 2026 at 11:25:08PM +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-03-30 at 18:25 +0800, Xu Yilun wrote:
> > Yes in POC code we do write dedicated code for each type, but it ends up
> > with plenty of similar logics on caller side about root page
> > manipulation. By now, the differences are not much, but I think we
> > should not write copies for every type, we should stop new types.
>
> Hmm, doesn't it seem like this is quickly becoming complicated though, to
> combine all the different types together? And it seems there are more coming
> that people want to add to this.
Most of the list entry types are the same with just some bits unused.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v2 5/7] KVM: guest_memfd: Add cleanup interface for guest teardown
From: Kalra, Ashish @ 2026-03-31 4:45 UTC (permalink / raw)
To: Ackerley Tng, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <CAEvNRgFJ8csUW0fXGB3cimjP=jev7mzaexUnfyj0p1ptFdPvCA@mail.gmail.com>
Hello Ackerley,
On 3/27/2026 12:16 PM, Ackerley Tng wrote:
> "Kalra, Ashish" <ashish.kalra@amd.com> writes:
>
>> Hello Ackerley,
>>
>> On 3/11/2026 1:00 AM, Ackerley Tng wrote:
>>> "Kalra, Ashish" <ashish.kalra@amd.com> writes:
>>>
>>>> Hello Ackerley,
>>>>
>>>> On 3/9/2026 4:01 AM, Ackerley Tng wrote:
>>>>> Ashish Kalra <Ashish.Kalra@amd.com> writes:
>>>>>
>>>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>>>
>>>>>> Introduce kvm_arch_gmem_cleanup() to perform architecture-specific
>>>>>> cleanups when the last file descriptor for the guest_memfd inode is
>>>>>> closed. This typically occurs during guest shutdown and termination
>>>>>> and allows for final resource release.
>>>>>>
>>>>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>>>>> ---
>>>>>>
>>>>>> [...snip...]
>>>>>>
>>>>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>>>>> index 017d84a7adf3..2724dd1099f2 100644
>>>>>> --- a/virt/kvm/guest_memfd.c
>>>>>> +++ b/virt/kvm/guest_memfd.c
>>>>>> @@ -955,6 +955,14 @@ static void kvm_gmem_destroy_inode(struct inode *inode)
>>>>>>
>>>>>> static void kvm_gmem_free_inode(struct inode *inode)
>>>>>> {
>>>>>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
>>>>>> + /*
>>>>>> + * Finalize cleanup for the inode once the last guest_memfd
>>>>>> + * reference is released. This usually occurs after guest
>>>>>> + * termination.
>>>>>> + */
>>>>>> + kvm_arch_gmem_cleanup();
>>>>>> +#endif
>>>>>
>>>>> Folks have already talked about the performance implications of doing
>>>>> the scan and rmpopt, I just want to call out that one VM could have more
>>>>> than one associated guest_memfd too.
>>>>
>>>> Yes, i have observed that kvm_gmem_free_inode() gets invoked multiple times
>>>> at SNP guest shutdown.
>>>>
>>>> And the same is true for kvm_gmem_destroy_inode() too.
>>>>
>>>>>
>>>>> I think the cleanup function should be thought of as cleanup for the
>>>>> inode (even if it doesn't take an inode pointer since it's not (yet)
>>>>> required).
>>>>>
>>>>> So, the gmem cleanup function should not handle deduplicating cleanup
>>>>> requests, but the arch function should, if the cleanup needs
>>>>> deduplicating.
>>>>
>>>> I agree, the arch function will have to handle deduplicating, and for that
>>>> the arch function will probably need to be passed the inode pointer,
>>>> to have a parameter to assist with deduplicating.
>>>>
>>>
>>> By the time .free_folio() is called, folio->mapping may no longer exist,
>>> so if we definitely want to deduplicate using something in the inode,
>>> .free_folio() won't be the right callback to use.
>>
>> Ok.
>>
>>>
>>> I was thinking that deduplicating using something in the folio would be
>>> better. Can rmpopt take a PFN range? Then there's really no
>>> deduplication, the cleanup would be nicely narrowed to whatever was just
>>> freed. Perhaps the PFNs could be aligned up to the nearest PMD or PUD
>>> size for rmpopt to do the right thing.
>>>
>>
>> It will really be ideal if the cleanup can be narrowed down to whatever was just freed.
>>
>> RMPOPT takes a SPA which is GB aligned, so if the PFNs are aligned to the nearest
>> PUD, then RMPOPT will be perfectly aligned to optimize the 1G regions that contained
>> memory associated with that guest being freed.
>>
>> This will also be the most optimal way to use RMPOPT, as we only optimize the 1G regions
>> that contains memory associated with that guest, which should be much smaller than
>> optimizing the whole 2TB RAM.
>>
>> And that's what the actual plans for RMPOPT are.
>>
>> We had planned for a phased RMPOPT implementation.
>>
>> In the first phase, we were planning to do RMP re-optimizations for entire 2TB
>> RAM.
>>
>> Once 1GB hugetlb guest_memfd support is merged, we planned to support re-enabling
>> RMPOPT optimizations during 1GB page cleanup as a follow-on series.
>>
>> But i believe this support is dependent on:
>> 1). in-place conversion for guest_memfd,
>> 2). 2M hugepage support for guest_memfd.
>>
>
> You're right about this dependency. Do you meant guest_memfd THP support
> for "2M hugepage"?
>
Yes.
>> Another alternative we are considering is implementing a bitmap of 1GB regions in guest_memfd
>> that tracks when they are being freed and then issue RMPOPT on those 1GB regions.
>> (and this will be independent of the 1GB hugeTLB support for guest_memfd).
>>
>>> Or perhaps some more tracking is required to check that the entire
>>> aligned range is freed before doing the rmpopt.
>>>
>>> I need to implement some of this tracking for guest_memfd HugeTLB
>>> support, so if the tracking is useful for you, we should discuss!
>>
>> Yes, this tracking is going to be useful for RMPOPT.
>>
>> Is this going to be implemented as part of the 1GB hugeTLB support for guest_memfd ?
>>
>
> Yes, this is going to be implemented as part of the HugeTLB support
> for guest_memfd. HugeTLB support for guest_memfd extends to any HugeTLB
> page size the host supports, so not just 1G, 2M as well. :)
>
Ok.
>>>
>>>>>
>>>>> Also, .free_inode() is called through RCU, so it could be called after
>>>>> some delay. Could it be possible that .free_inode() ends up being called
>>>>> way after the associated VM gets torn down, or after KVM the module gets
>>>>> unloaded? Does rmpopt still work fine if KVM the module got unloaded?
>>>>
>>>> Yes, .free_inode() can probably get called after the associated VM has
>>>> been torn down and which should be fine for issuing RMPOPT to do
>>>> RMP re-optimizations.
>>>>
>>>> As far as about KVM module getting unloaded, then as part of the forthcoming patch-series,
>>>> during KVM module unload, X86_SNP_SHUTDOWN would be issued which means SNP would get
>>>> disabled and therefore, RMP checks are also disabled.
>>>>
>>>> And as CC_ATTR_HOST_SEV_SNP would then be cleared, therefore, snp_perform_rmp_optimization()
>>>> will simply return.
>>>>
>>>
>>> I think relying on CC_ATTR_HOST_SEV_SNP to skip optimization should be
>>> best as long as there are no races (like the .free_inode() will
>>> definitely not try to optimize when SNP is half shut down or something
>>> like that.
>>
>> Yeah, i will have to take a look at such races.
>>
>>>
>>>> Another option is to add a new guest_memfd superblock operation, and then do the
>>>> final guest_memfd cleanup using the .evict_inode() callback. This will then ensure
>>>> that the cleanup is not called through RCU and avoids any kind of delays, as following:
>>>>
>>>> +static void kvm_gmem_evict_inode(struct inode *inode)
>>>> +{
>>>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_CLEANUP
>>>> + kvm_arch_gmem_cleanup();
>>>> +#endif
>>>> + truncate_inode_pages_final(&inode->i_data);
>>>> + clear_inode(inode);
>>>> +}
>>>> +
>>>>
>>>
>>> At the point of .evict_inode(), CoCo-shared guest_memfd pages could
>>> still be pinned (for DMA or whatever, accidentally or maliciously), can
>>> rmpopt work on shared pages that might still be used for DMA?
>>>
>>
>> Yes, RMPOPT should be safe to work here, as it checks the RMP table for assigned
>> or private pages in the 1GB range specified. For a 1GB range full of shared pages,
>> it will mark that range to be RMP optimized.
>>
>> If all RMPUPDATE's for all private->shared pages conversion have been completed at
>> the point of .evict_inode(), then RMPOPT re-optimizations will work nicely.
>>
>
> Ah okay. The kvm_arch_gmem_invalidate() call in .free_folio is the part
> that updates the RMP table to make anything private become shared.
>
> So the RMPOPT probably needs to happen after the invalidate in .free_folio
>
> The RMPOPT stuff is still useful even if the host never uses huge pages
> for guest_memfd, right? If so, I think we still need a solution
> regardless of when huge page support for guest_memfd lands.
>
Yes.
> What if we do it this way: in .free_folio, after doing the invalidate,
> take the pfn of the folio being freed, align that to the GB containing
> that pfn, then RMPOPT that? This way there is no dependency on the inode
> being around.
The issue with doing it in .free_folio after doing the invalidate OR after P->S translation,
and aligning it to the GB containing that pfn, the chances of that being the only
private page in the range is small and there is a high probability of multiple pages
still existing in this range which are still assigned.
So doing RMPOPT for such page(s) (aligned to 1GB) will most likely fail, as there are
still private/assigned pages in this 1GB range.
RMPOPT will succeed or work optimally when we issue it when RMPOPT-able ranges or
full 1GB regions are actually becoming available/freed from guest_memfd.
>
> RMPOPT looks up the shared/private-ness of the page in the RMP table
> anyway so as long as the RMP table is updated, we should be good?
>
Yes.
> The awkward part is if RMPOPT is run twice when the RMP table state
> hasn't changed. Is my understanding right that there will be no
> correctness issues, just performance?
>
Yes.
> We can perhaps optimize (away or otherwise) unnecessary RMPOPTs later?
>
> With this aligning-up-to-the-GB, at least we're not iterating the entire
> host memory.
>
Until we have full 1GB regions available on which we can issue RMPOPT it does
not make sense to issue RMPOPT as most likely it will fail.
That's why until hugeTLB support for guest_memfd is available, for the initial
series, iterating all system RAM for RMPOPT after SNP VM teardown is an option.
As this is being scheduled like 10 seconds later, then if other VMs terminate
there is already work scheduled and we skip scheduling another RMPOPT work.
Thanks,
Ashish
>>> .invalidate_folio() and .free_folio() both actually happen on removal
>>> from guest_memfd ownership, though both are not exactly when the folio
>>> is completely not in use.
>>>
>>> Is the best time to optimize when the pages are truly freed?
>>>
>>
>> Yes.
>>
>> Thanks,
>> Ashish
>>
>
> Thank you!
>
>>>> @@ -971,6 +979,7 @@ static const struct super_operations kvm_gmem_super_operations = {
>>>> .alloc_inode = kvm_gmem_alloc_inode,
>>>> .destroy_inode = kvm_gmem_destroy_inode,
>>>> .free_inode = kvm_gmem_free_inode,
>>>> + .evict_inode = kvm_gmem_evict_inode,
>>>> };
>>>>
>>>>
>>>> Thanks,
>>>> Ashish
>>>>
>>>>>
>>>>> IIUC the current kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>>>>> is fine because in kvm_gmem_exit(), there is a rcu_barrier() before
>>>>> kmem_cache_destroy(kvm_gmem_inode_cachep);.
>>>>>
>>>>>> kmem_cache_free(kvm_gmem_inode_cachep, GMEM_I(inode));
>>>>>> }
>>>>>>
>>>>>> --
>>>>>> 2.43.0
^ permalink raw reply
* Re: [PATCH v6 17/22] x86/virt/tdx: Avoid updates during update-sensitive operations
From: Chao Gao @ 2026-03-31 2:34 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: linux-kernel, linux-coco, kvm, binbin.wu, dan.j.williams,
dave.hansen, ira.weiny, kai.huang, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vannapurve, vishal.l.verma, yilun.xu, xiaoyao.li, yan.y.zhao,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin
In-Reply-To: <acp1YAMOCiKpcZaz@thinkstation>
On Mon, Mar 30, 2026 at 01:07:27PM +0000, Kiryl Shutsemau wrote:
>On Thu, Mar 26, 2026 at 01:44:08AM -0700, Chao Gao wrote:
>> + if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_UPDATE_COMPAT)
>> + args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
>
>I think you need to explain what would happen if the feature is not
>supported.
I included this explanation in the changelog:
When the "avoid update-sensitive" feature isn't supported by the TDX
module, proceed with updates and let userspace update at their own risk.
...
Do you mean making it more explicit:
When the "avoid update-sensitive" feature isn't supported, proceed with
updates. If a race occurs between module update and update-sensitive
operations, failures happen at a later stage (e.g., incorrect TD
measurements in attestation reports for TD build). Effectively, this
means "let userspace update at their own risk." ...
^ permalink raw reply
* Re: [PATCH v3 4/6] x86/sev: Add interface to re-enable RMP optimizations.
From: Kalra, Ashish @ 2026-03-31 0:54 UTC (permalink / raw)
To: Dave Hansen, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <23267200-9fed-43a9-a28b-a6daa701159b@intel.com>
On 3/30/2026 6:33 PM, Dave Hansen wrote:
> The subject seems rather imprecise. This both adds a function to
> "re-enable RMP optimizations" *AND* calls it.
>
>> RMPOPT table is a per-processor table which indicates if 1GB regions of
>> physical memory are entirely hypervisor-owned or not.
>
> It's per-core, right? Why not just be precise about it?
>
Ok.
>> When performing host memory accesses in hypervisor mode as well as
>> non-SNP guest mode, the processor may consult the RMPOPT table to
>> potentially skip an RMP access and improve performance.
>>
>> Events such as RMPUPDATE or SNP_INIT can clear RMP optimizations. Add
>> an interface to re-enable those optimizations.
>
>
>> +int snp_perform_rmp_optimization(void)
>> +{
>> + if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
>> + return -EINVAL;
>> +
>> + if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
>> + return -EINVAL;
>> +
>> + if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED))
>> + return -EINVAL;
>
> This seems wrong. How about we just make 'X86_FEATURE_RMPOPT' the one
> true source of RMP support?
>
Ok, i will work on that.
> If you don't have CC_ATTR_HOST_SEV_SNP you:
>
> setup_clear_cpu_cap(X86_FEATURE_RMPOPT)
>
> Ditto for MSR_AMD64_SEG_RMP_ENABLED.
>
> It could also potentially replace the 'rmpopt_wq' checks.
>
>> + rmpopt_all_physmem(FALSE);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(snp_perform_rmp_optimization);
>> +
>> void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp)
>> {
>> struct page *page = pfn_to_page(pfn);
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index aebf4dad545e..0cbe828d204c 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -1476,6 +1476,10 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>> }
>>
>> snp_hv_fixed_pages_state_update(sev, HV_FIXED);
>> +
>> + /* SNP_INIT clears the RMPOPT table, re-enable RMP optimizations */
>> + snp_perform_rmp_optimization();
>
> Ahhh, so this isn't happening at boot, it happens when kvm_amd.ko gets
> loaded? That escaped me until now. It would be nice to mention
> somewhere, please.
Well, as of now, it is happening at both boot time and here after SNP_INIT_EX,
as SNP_INIT clears the RMPOPT table contents to 0, but eventually, when SNP enable
is moved completely out of snp_rmptable_init() and only done when kvm_amd.ko
gets loaded, then it will only happen here.
Eventually, call to "configure_and_enable_rmpopt()" (setup_rmpopt()) will move out
of snp_rmptable_init() and get called from here.
>
> There is basically no naming difference between
> snp_perform_rmp_optimization() and rmpopt_all_physmem(). Can you just
> get this all down to a single function, please?
>
> If you really have a reason to have a scan now and scan later mode, just
> do this:
>
> rmpopt_all_physmem(RMPOPT_SCAN_NOW);
>
> and:
>
> rmpopt_all_physmem(RMPOPT_SCAN_LATER);
>
> *That* function can do the X86_FEATURE_RMPOPT check.
The only thing is that this is an exported function out of the x86 platform
code, so will probably need to be renamed as snp_rmpopt_all_physmem().
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v3 3/6] x86/sev: Add support to perform RMP optimizations asynchronously
From: Kalra, Ashish @ 2026-03-31 0:46 UTC (permalink / raw)
To: Dave Hansen, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <ab41b1d8-e464-4ad6-ac07-7318686db10e@intel.com>
On 3/30/2026 6:22 PM, Dave Hansen wrote:
> On 3/30/26 15:26, Ashish Kalra wrote:
> ...
>> As SEV-SNP is enabled by default on boot when an RMP table is
>> allocated by BIOS, the hypervisor and non-SNP guests are subject to
>> RMP write checks to provide integrity of SNP guest memory.
>
> This is a long-winded way of saying:
>
> When SEV-SNP is enabled, all writes to memory are checked to
> ensure integrity of SNP guest memory. This imposes performance
> overhead on the whole system.
>
>> RMPOPT is a new instruction that minimizes the performance overhead of
>> RMP checks on the hypervisor and on non-SNP guests by allowing RMP
>> checks to be skipped for 1GB regions of memory that are known not to
>> contain any SEV-SNP guest memory.
>>
>> Add support for performing RMP optimizations asynchronously using a
>> dedicated workqueue, scheduling delayed work to perform RMP
>> optimizations every 10 seconds.
>
> Gah, does it really do this _every_ 10 seconds? Whether or not any
> guests are running or if the SEV-SNP state has changed at *all*? This
> code doesn't implement that, right? If so, why mention it here?
>
>> +static void rmpopt_work_handler(struct work_struct *work)
>> +{
>> + phys_addr_t pa;
>> +
>> + pr_info("Attempt RMP optimizations on physical address range @1GB alignment [0x%016llx - 0x%016llx]\n",
>> + rmpopt_pa_start, rmpopt_pa_end);
>> +
>> + /*
>> + * RMPOPT optimizations skip RMP checks at 1GB granularity if this
>> + * range of memory does not contain any SNP guest memory. Optimize
>> + * each range on one CPU first, then let other CPUs execute RMPOPT
>> + * in parallel so they can skip most work as the range has already
>> + * been optimized.
>> + */
>
> This comment could be much more clear.
>
> First, the granularity has *zero* to do with this optimization.
>
> Second, the optimization this code is doing only makes sense if the RMP
> itself is caching the RMPOPT result in a global, single place. That's
> not explained. It needs something like:
>
> RMPOPT does three things: It scans the RMP table, stores the
> result of the scan in the global RMP table and copies that
> result to a per-CPU table. The scan is the most expensive part.
> If a second RMPOPT occurs, it can skip the expensive scan if it
> sees the "cached" scan result in the RMP.
>
> Do RMPOPT on one CPU alone. Then, follow that up with RMPOPT
> on every other primary thread. This potentially allows the
> followers to use the "cached" scan results to avoid repeating
> full scans.
>
>> + cpumask_clear_cpu(smp_processor_id(), &primary_threads_cpumask);
>
> How do you know that the current CPU is *in* 'primary_threads_cpumask'
> in the first place? I guess it doesn't hurt to do RMPOPT in two places,
> but why not just be careful about it?
>
> Also, logically, 'primary_threads_cpumask' never changes (modulo CPU
> hotplug). The thing you're tracking here is "primary CPUs that need to
> have RMPOPT executed on them". That's a far different thing than the
> name for the variable.
Ok, i will name it more appropriately.
>
>> + /* current CPU */
>> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
>> + rmpopt((void *)pa);
>
> This _looks_ rather wonky because it's casting a 'pa' to a virtual
> address for no apparent reason.
This is re-using the rmpopt() wrapper, which exists for using the
on_each_cpu_mask() call below, and that needs the "void *' parameter.
The on_each_cpu_mask() is needed to execute RMPOPT instructions in
parallel, as that is part of the bare minimum optimizations needed
for the RMPOPT loop.
I will add another wrapper for calling rmpopt() with the 'pa'
parameter directly.
>
> Also, rmpopt() itself does 1G alignment. This code ^ also aligns the
> start and end. Why?
Yes, RMPOPT instruction does 1G alignment on the specified PA, this code
alignment is to ensure the PFNs - min_low_pfn and max_pfn are aligned
appropriately.
>
>> + for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G) {
>> + on_each_cpu_mask(&primary_threads_cpumask, rmpopt,
>> + (void *)pa, true);
>> +
>> + /* Give a chance for other threads to run */
>> + cond_resched();
>> +
>> + }
>> +
>> + cpumask_set_cpu(smp_processor_id(), &primary_threads_cpumask);
>> +}
>
> Honestly, I _really_ wish this series would dispense with *all* the
> optimizations in the first version. This looks really wonky because
> 'primary_threads_cpumask' is a global variable and is initialized before
> the work function when it could probably be done within the work function.
>
It is static and will never change, so i was doing it here outside the work function,
but i can move it inside the work function.
> It's also *really* generically and non-descriptively named for a
> global-scope variable.
>
I can name it appropriately.
I definitely want to have the bare minimum set of optimizations for the RMPOPT loop
in place for the first series of patches, which is:
1). Issuing RMPOPT on one only one CPU first, before doing it in parallel on
all other CPUs.
2). Issuing RMPOPT on only one thread per core.
This is the bare minimum set of optimizations for RMPOPT loop, for which
i had the performance numbers computed before:
Best runtime for the RMPOPT loop:
When RMPOPT exits early as it finds an assigned page on the first RMP entry it checks in the 1GB -> ~95ms.
Worst runtime for the RMPOPT loop:
When RMPOPT does not find any assigned page in the full 1GB range it is checking -> ~311ms.
The following series will have support for system RAM > 2TB and other optimizations, but
these bare minimum set of optimizations for RMPOPT should definitely be part of initial
patch series.
>> +static void rmpopt_all_physmem(bool early)
>> +{
>> + if (!rmpopt_wq)
>> + return;
>> +
>> + if (early)
>> + queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
>> + msecs_to_jiffies(1));
>> + else
>> + queue_delayed_work(rmpopt_wq, &rmpopt_delayed_work,
>> + msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
>> +}
>
> This is rather unfortunate on several levels.
>
> First, even if the 'bool early' thing was a good idea, this should be
> written:
>
> unsigned long timeout = RMPOPT_WORK_TIMEOUT;
>
> if (early)
> timeout = 1;
>
> queue_delayed_work(rmpopt_wq,
> &rmpopt_delayed_work,
> msecs_to_jiffies(timeout));
>
> But, really, why does it even *need* a bool for early/late? Just do a
> late_initcall() if you want this done near boot time.
>
Ok.
>
>> static __init void configure_and_enable_rmpopt(void)
>> {
>> phys_addr_t pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
>> @@ -499,6 +582,37 @@ static __init void configure_and_enable_rmpopt(void)
>> */
>> for_each_online_cpu(cpu)
>> wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
>
> What is the scope of MSR_AMD64_RMPOPT_BASE? Can you have it enabled on
> one thread and not the other? Could they be different values both for
> enabling and the rmpopt_base value?
>
> If it's not per-thread, then why is it being initialized for each thread?
>
Only one logical thread per core needs to set RMPOPT_BASE MSR as it is per-core,
so i will use the "primary_threads_cpumask" here to use it for programming this
MSR.
Just another reason, to set the "primary_threads_cpumask" here in this function
and then re-use it for the RMPOPT worker.
>> + /*
>> + * Create an RMPOPT-specific workqueue to avoid scheduling
>> + * RMPOPT workitem on the global system workqueue.
>> + */
>> + rmpopt_wq = alloc_workqueue("rmpopt_wq", WQ_UNBOUND, 1);
>> + if (!rmpopt_wq)
>> + return;
>
> I'd probably just put this first. Then if the allocation fails, you
> don't even bother doing the WRMSRs. Heck if you did that, you could even
> use the MSR bit for the indicator of if RMPOPT is supported.
Ok.
>
>> + INIT_DELAYED_WORK(&rmpopt_delayed_work, rmpopt_work_handler);
>> +
>> + rmpopt_pa_start = pa_start;
>
> Why is there a 'rmpopt_pa_start' and 'pa_start'?
Again, just statically setting the 'rmpopt_pa_start' for the RMPOPT worker,
but i can just use 'rmpopt_pa_start' here.
Thanks,
Ashish
>
>> + rmpopt_pa_end = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
>> +
>> + /* Limit memory scanning to the first 2 TB of RAM */
>> + if ((rmpopt_pa_end - rmpopt_pa_start) > SZ_2T)
>> + rmpopt_pa_end = rmpopt_pa_start + SZ_2T;
>> +
>> + /* Only one thread per core needs to issue RMPOPT instruction */
>> + for_each_online_cpu(cpu) {
>> + if (!topology_is_primary_thread(cpu))
>> + continue;
>> +
>> + cpumask_set_cpu(cpu, &primary_threads_cpumask);
>> + }
>> +
>> + /*
>> + * Once all per-CPU RMPOPT tables have been configured, enable RMPOPT
>> + * optimizations on all physical memory.
>> + */
>> + rmpopt_all_physmem(TRUE);
>> }
^ permalink raw reply
* Re: [PATCH v2 01/31] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Dave Hansen @ 2026-03-31 0:01 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: chao.gao, dave.jiang, baolu.lu, yilun.xu, zhenzhong.duan, kvm,
rick.p.edgecombe, dave.hansen, kas, xiaoyao.li, vishal.l.verma,
linux-kernel
In-Reply-To: <20260327160132.2946114-2-yilun.xu@linux.intel.com>
On 3/27/26 09:01, Xu Yilun wrote:
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
This is missing a SoB from the person sending the patch.
Honestly, for something coming all from one company, I'd just prefer
that folks simplify the tale of woe about how a patch came to be
written. Just axe the SoB's and put one on it. Credit folks with a blurb
in the changelog if you have to.
^ permalink raw reply
* Re: [PATCH v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects
From: Edgecombe, Rick P @ 2026-03-30 23:57 UTC (permalink / raw)
To: yilun.xu@linux.intel.com
Cc: Gao, Chao, Xu, Yilun, x86@kernel.org, kas@kernel.org,
baolu.lu@linux.intel.com, dave.hansen@linux.intel.com,
Li, Xiaoyao, Williams, Dan J, Jiang, Dave,
linux-pci@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, Duan, Zhenzhong, Verma, Vishal L,
kvm@vger.kernel.org
In-Reply-To: <acqbDVnd6gNJWmBi@yilunxu-OptiPlex-7050>
On Mon, 2026-03-30 at 23:47 +0800, Xu Yilun wrote:
> > pages is going to be an array of struct pointers, and root is a single page
> > of
> > PA's that gets re-used to copy and pass the PA's to the TDX module. Why do
> > we
> > need both? Like just keep an array of PA's that would be the same size as
> > the
> > struct page array. And not need the populate loop?
>
> We need Linux language, struct page *, for alloc and free. Also need
> TDX Module language - PA list - for SEAMCALLs. So IIUC, the page to PA
> populating won't disappear on allocation, the PA to page populating
> would appear on free.
Not sure what you mean by this.
>
> Besides, host may need to vmap and access the (shared) pages.
Some code someday may need to convert a PA to another format? Is that it?
Doesn't seem like big problem.
But I'm not sure about this idea yet.
^ permalink raw reply
* Re: [PATCH v2 05/31] x86/virt/tdx: Extend tdx_page_array to support IOMMU_MT
From: Edgecombe, Rick P @ 2026-03-30 23:54 UTC (permalink / raw)
To: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com, kas@kernel.org,
baolu.lu@linux.intel.com, Jiang, Dave, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260327160132.2946114-6-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> IOMMU_MT is another TDX Module defined structure similar to HPA_ARRAY_T
> and HPA_LIST_INFO. The difference is it requires multi-order contiguous
> pages for some entries. It adds an additional NUM_PAGES field for every
> multi-order page entry.
>
> Add a dedicated allocation helper for IOMMU_MT. Fortunately put_page()
> works well for both single pages and multi-order folios, simplifying the
> cleanup logic for all allocation methods.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx.h | 2 +
> arch/x86/virt/vmx/tdx/tdx.c | 90 +++++++++++++++++++++++++++++++++++--
> 2 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 9173a432b312..d5f1d7b7d1e7 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -175,6 +175,8 @@ void tdx_page_array_ctrl_leak(struct tdx_page_array *array);
> int tdx_page_array_ctrl_release(struct tdx_page_array *array,
> unsigned int nr_released,
> u64 released_hpa);
> +struct tdx_page_array *
> +tdx_page_array_create_iommu_mt(unsigned int iq_order, unsigned int nr_mt_pages);
>
> struct tdx_td {
> /* TD root structure: */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 6c4ed80e8e5a..2b17e0f73dac 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -275,8 +275,15 @@ static int tdx_page_array_populate(struct tdx_page_array *array,
> TDX_PAGE_ARRAY_MAX_NENTS);
>
> entries = array->root;
> - for (i = 0; i < array->nents; i++)
> - entries[i] = page_to_phys(array->pages[offset + i]);
> + for (i = 0; i < array->nents; i++) {
> + struct page *page = array->pages[offset + i];
> +
> + entries[i] = page_to_phys(page);
> +
> + /* Now only for iommu_mt */
> + if (compound_nr(page) > 1)
> + entries[i] |= compound_nr(page);
> + }
>
> return array->nents;
> }
> @@ -286,7 +293,7 @@ static void tdx_free_pages_bulk(unsigned int nr_pages, struct page **pages)
> int i;
>
> for (i = 0; i < nr_pages; i++)
> - __free_page(pages[i]);
> + put_page(pages[i]);
> }
>
> static int tdx_alloc_pages_bulk(unsigned int nr_pages, struct page **pages,
> @@ -463,6 +470,10 @@ static bool tdx_page_array_validate_release(struct tdx_page_array *array,
> struct page *page = array->pages[offset + i];
> u64 val = page_to_phys(page);
>
> + /* Now only for iommu_mt */
> + if (compound_nr(page) > 1)
> + val |= compound_nr(page);
> +
> if (val != entries[i]) {
> pr_err("%s entry[%d] [0x%llx] doesn't match page hpa [0x%llx]\n",
> __func__, i, entries[i], val);
> @@ -555,6 +566,79 @@ tdx_page_array_alloc_contig(unsigned int nr_pages)
> return tdx_page_array_alloc(nr_pages, tdx_alloc_pages_contig, NULL);
> }
>
> +static int tdx_alloc_pages_iommu_mt(unsigned int nr_pages, struct page **pages,
> + void *data)
> +{
> + unsigned int iq_order = (unsigned int)(long)data;
> + struct folio *t_iq, *t_ctxiq;
> + int ret;
> +
> + /* TODO: folio_alloc_node() is preferred, but need numa info */
> + t_iq = folio_alloc(GFP_KERNEL | __GFP_ZERO, iq_order);
> + if (!t_iq)
> + return -ENOMEM;
> +
> + t_ctxiq = folio_alloc(GFP_KERNEL | __GFP_ZERO, iq_order);
> + if (!t_ctxiq) {
> + ret = -ENOMEM;
> + goto out_t_iq;
> + }
> +
> + ret = tdx_alloc_pages_bulk(nr_pages - 2, pages + 2, NULL);
> + if (ret)
> + goto out_t_ctxiq;
> +
> + pages[0] = folio_page(t_iq, 0);
> + pages[1] = folio_page(t_ctxiq, 0);
To me it seems like this can't really be called a page array any more. The first
two u64's are too special. Instead it's a special one-off ABI format passed via
a page.
BTW, I can't find TDH.IOMMU.SETUP in the docs. Any pointers?
> +
> + return 0;
> +
> +out_t_ctxiq:
> + folio_put(t_ctxiq);
> +out_t_iq:
> + folio_put(t_iq);
> +
> + return ret;
> +}
> +
> +/**
> + * tdx_page_array_create_iommu_mt() - Create a page array for IOMMU Memory Tables
> + * @iq_order: The allocation order for the IOMMU Invalidation Queue.
> + * @nr_mt_pages: Number of additional order-0 pages for the MT.
> + *
> + * Allocate and populate a specialized tdx_page_array for IOMMU_MT structures.
> + * The resulting array consists of two multi-order folios (at index 0 and 1)
> + * followed by the requested number of order-0 pages.
> + *
> + * Return: Fully populated tdx_page_array or NULL on failure.
> + */
> +struct tdx_page_array *
> +tdx_page_array_create_iommu_mt(unsigned int iq_order, unsigned int nr_mt_pages)
> +{
> + unsigned int nr_pages = nr_mt_pages + 2;
Consider the amount of tricks that are needed to coax the tdx_page_array to
populate the handoff page as needed. It adds 2 pages here, then subtracts them
later in the callback. Then tweaks the pa in tdx_page_array_populate() to add
the length...
> + struct tdx_page_array *array;
> + int populated;
> +
> + if (nr_pages > TDX_PAGE_ARRAY_MAX_NENTS)
> + return NULL;
> +
> + array = tdx_page_array_alloc(nr_pages, tdx_alloc_pages_iommu_mt,
> + (void *)(long)iq_order);
> + if (!array)
> + return NULL;
> +
> + populated = tdx_page_array_populate(array, 0);
> + if (populated != nr_pages)
> + goto out_free;
> +
> + return array;
> +
> +out_free:
> + tdx_page_array_free(array);
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(tdx_page_array_create_iommu_mt);
> +
> #define HPA_LIST_INFO_FIRST_ENTRY GENMASK_U64(11, 3)
> #define HPA_LIST_INFO_PFN GENMASK_U64(51, 12)
> #define HPA_LIST_INFO_LAST_ENTRY GENMASK_U64(63, 55)
^ permalink raw reply
* Re: [PATCH v3 5/6] KVM: SEV: Perform RMP optimizations on SNP guest shutdown
From: Dave Hansen @ 2026-03-30 23:47 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <b3566eb53194f1262dd74c930e0ee2f835733a38.1774755884.git.ashish.kalra@amd.com>
On 3/30/26 15:27, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> As SNP guests are launched, pages converted to private cause RMPUPDATE
> to disable the corresponding RMPOPT optimizations.
>
> Conversely, during SNP guest termination, when guest pages are
> converted back to shared and are not assigned, RMPOPT will be used
> to re-enable RMP optimizations.
This is all super passive. It makes it impossible to tell what is
background versus imperative voice on what the patch is doing.
== Background / Problem ==
Pages are converted from shared to private as SNP guests are
launched. This destroys existing RMPOPT optimizations in the
regions where pages are converted.
Conversely, guest pages are converted back to shared during SNP
guest termination and their region may become eligible for
RMPOPT optimization.
== Solution ==
To take advantage of this, perform RMPOPT after guest
termination. Do it after a delay so that a single RMPOPT pass
can be done if multiple guests terminate in a short period of
time.
With a fixed changelog:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
I'm just saying "ack" on this one because it's a pretty KVM-specific
thing about when the guest is destroyed to the point of being good for
RMPOPT. This needs many more eyeballs from the KVM folks than me.
^ permalink raw reply
* Re: [PATCH v2 10/31] x86/virt/tdx: Add extra memory to TDX Module for Extensions
From: Edgecombe, Rick P @ 2026-03-30 23:36 UTC (permalink / raw)
To: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com, kas@kernel.org,
baolu.lu@linux.intel.com, Jiang, Dave, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260327160132.2946114-11-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> Adding more memory to TDX Module is the first step to enable Extensions.
>
> Currently, TDX Module memory use is relatively static. But, some new
> features (called "TDX Module Extensions") need to use memory more
> dynamically. While 'static' here means the kernel provides necessary
> amount of memory to TDX Module for its basic functionalities, 'dynamic'
> means extra memory is needed only if new optional features are to be
> enabled. So add a new memory feeding process backed by a new SEAMCALL
> TDH.EXT.MEM.ADD.
>
> The process is mostly the same as adding PAMT. The kernel queries TDX
> Module how much memory needed, allocates it, hands it over, and never
> gets it back.
>
> TDH.EXT.MEM.ADD uses tdx_page_array to provide control (private) pages
> to TDX Module. Introduce a tdx_clflush_page_array() helper to flush
> shared cache before SEAMCALL, to avoid shared cache write back damages
> these private pages.
>
> For now, TDX Module Extensions consume relatively large amount of
> memory (~50MB). Use contiguous page allocation to avoid permanently
> fragment too much memory. Print this readout value on TDX Module
> Extensions initialization for visibility.
>
> Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 92 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 870bb75da3ba..31ccdfcf518c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -60,6 +60,7 @@
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG_V0 45
> #define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
> +#define TDH_EXT_MEM_ADD 61
>
> /* TDX page types */
> #define PT_NDA 0x0
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 4fb56bb442f0..5fae17c13191 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -560,7 +560,7 @@ static int tdx_alloc_pages_contig(unsigned int nr_pages, struct page **pages,
> * Similar to tdx_page_array_alloc(), after allocating with this
> * function, call tdx_page_array_populate() to populate the tdx_page_array.
> */
> -static __maybe_unused struct tdx_page_array *
> +static struct tdx_page_array *
> tdx_page_array_alloc_contig(unsigned int nr_pages)
> {
> return tdx_page_array_alloc(nr_pages, tdx_alloc_pages_contig, NULL);
> @@ -643,7 +643,7 @@ EXPORT_SYMBOL_GPL(tdx_page_array_create_iommu_mt);
> #define HPA_LIST_INFO_PFN GENMASK_U64(51, 12)
> #define HPA_LIST_INFO_LAST_ENTRY GENMASK_U64(63, 55)
>
> -static u64 __maybe_unused hpa_list_info_assign_raw(struct tdx_page_array *array)
> +static u64 hpa_list_info_assign_raw(struct tdx_page_array *array)
> {
> return FIELD_PREP(HPA_LIST_INFO_FIRST_ENTRY, 0) |
> FIELD_PREP(HPA_LIST_INFO_PFN,
> @@ -1513,6 +1513,94 @@ static void tdx_clflush_page(struct page *page)
> clflush_cache_range(page_to_virt(page), PAGE_SIZE);
> }
>
> +static void tdx_clflush_page_array(struct tdx_page_array *array)
It doesn't clflush the page array, it clflushes the current populate chunk. Hmm.
Does it suggest that the page array and the format for handing them to the TDX
module are two different things?
> +{
> + for (int i = 0; i < array->nents; i++)
> + tdx_clflush_page(array->pages[array->offset + i]);
> +}
> +
> +static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> +{
> + struct tdx_module_args args = {
> + .rcx = hpa_list_info_assign_raw(ext_mem),
> + };
> + u64 r;
> +
> + tdx_clflush_page_array(ext_mem);
> +
> + do {
> + r = seamcall_ret(TDH_EXT_MEM_ADD, &args);
> + cond_resched();
> + } while (r == TDX_INTERRUPTED_RESUMABLE);
> +
> + if (r != TDX_SUCCESS)
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +static int tdx_ext_mem_setup(struct tdx_page_array *ext_mem)
> +{
> + unsigned int populated, offset = 0;
> + int ret;
> +
> + /*
> + * tdx_page_array's root page can hold 512 HPAs at most. We have ~50MB
> + * memory to add, re-populate the array and add pages bulk by bulk.
> + */
> + while (1) {
> + populated = tdx_page_array_populate(ext_mem, offset);
> + if (!populated)
> + break;
For this case of populate it seems like it would be ok to keep an array of PA's
instead of an array of struct pages. Not sure on it yet.
> +
> + ret = tdx_ext_mem_add(ext_mem);
> + if (ret)
> + return ret;
> +
> + offset += populated;
> + }
> +
> + return 0;
> +}
> +
> +static int __maybe_unused init_tdx_ext(void)
> +{
> + struct tdx_page_array *ext_mem = NULL;
> + unsigned int nr_pages;
> + int ret;
> +
> + if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> + return 0;
> +
> + nr_pages = tdx_sysinfo.ext.memory_pool_required_pages;
> + /*
> + * memory_pool_required_pages == 0 means no need to add more pages,
> + * skip the memory setup.
> + */
Is this ever expected? Extensions are supported, but require no pages?
> + if (nr_pages) {
> + ext_mem = tdx_page_array_alloc_contig(nr_pages);
> + if (!ext_mem)
> + return -ENOMEM;
> +
> + ret = tdx_ext_mem_setup(ext_mem);
> + if (ret)
> + goto out_ext_mem;
> + }
> +
> + /* Extension memory is never reclaimed once assigned */
> + tdx_page_array_ctrl_leak(ext_mem);
This looks very weird to call "leak" in the success path.
> +
> + pr_info("%lu KB allocated for TDX Module Extensions\n",
> + nr_pages * PAGE_SIZE / 1024);
> +
> + return 0;
> +
> +out_ext_mem:
> + tdx_page_array_free(ext_mem);
> +
> + return ret;
> +}
> +
> static int init_tdx_module(void)
> {
> int ret;
^ permalink raw reply
* Re: [PATCH v3 4/6] x86/sev: Add interface to re-enable RMP optimizations.
From: Dave Hansen @ 2026-03-30 23:33 UTC (permalink / raw)
To: Ashish Kalra, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <a30809d43368d6ddeb82e6717be83327282ee52e.1774755884.git.ashish.kalra@amd.com>
The subject seems rather imprecise. This both adds a function to
"re-enable RMP optimizations" *AND* calls it.
> RMPOPT table is a per-processor table which indicates if 1GB regions of
> physical memory are entirely hypervisor-owned or not.
It's per-core, right? Why not just be precise about it?
> When performing host memory accesses in hypervisor mode as well as
> non-SNP guest mode, the processor may consult the RMPOPT table to
> potentially skip an RMP access and improve performance.
>
> Events such as RMPUPDATE or SNP_INIT can clear RMP optimizations. Add
> an interface to re-enable those optimizations.
> +int snp_perform_rmp_optimization(void)
> +{
> + if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
> + return -EINVAL;
> +
> + if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
> + return -EINVAL;
> +
> + if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED))
> + return -EINVAL;
This seems wrong. How about we just make 'X86_FEATURE_RMPOPT' the one
true source of RMP support?
If you don't have CC_ATTR_HOST_SEV_SNP you:
setup_clear_cpu_cap(X86_FEATURE_RMPOPT)
Ditto for MSR_AMD64_SEG_RMP_ENABLED.
It could also potentially replace the 'rmpopt_wq' checks.
> + rmpopt_all_physmem(FALSE);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(snp_perform_rmp_optimization);
> +
> void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp)
> {
> struct page *page = pfn_to_page(pfn);
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index aebf4dad545e..0cbe828d204c 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1476,6 +1476,10 @@ static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
> }
>
> snp_hv_fixed_pages_state_update(sev, HV_FIXED);
> +
> + /* SNP_INIT clears the RMPOPT table, re-enable RMP optimizations */
> + snp_perform_rmp_optimization();
Ahhh, so this isn't happening at boot, it happens when kvm_amd.ko gets
loaded? That escaped me until now. It would be nice to mention
somewhere, please.
There is basically no naming difference between
snp_perform_rmp_optimization() and rmpopt_all_physmem(). Can you just
get this all down to a single function, please?
If you really have a reason to have a scan now and scan later mode, just
do this:
rmpopt_all_physmem(RMPOPT_SCAN_NOW);
and:
rmpopt_all_physmem(RMPOPT_SCAN_LATER);
*That* function can do the X86_FEATURE_RMPOPT check.
^ permalink raw reply
* Re: [PATCH v2 11/31] x86/virt/tdx: Make TDX Module initialize Extensions
From: Edgecombe, Rick P @ 2026-03-30 23:25 UTC (permalink / raw)
To: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, yilun.xu@linux.intel.com,
x86@kernel.org
Cc: Gao, Chao, Xu, Yilun, dave.hansen@linux.intel.com, kas@kernel.org,
baolu.lu@linux.intel.com, Jiang, Dave, Li, Xiaoyao,
Verma, Vishal L, Duan, Zhenzhong, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260327160132.2946114-12-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> After providing all required memory to TDX Module, initialize the
> Extensions via TDH.EXT.INIT, and then Extension-SEAMCALLs can be used.
>
> The initialization of Extensions touches the required memory (previously
> provided by TDH.EXT.MEM.ADD) in private manner. If failed, flush cache
> before freeing these memory, to avoid private cache write back damages
> the shared pages.
>
> TDX should use movdir64b to clear private pages when reclaiming them on
> older platforms with the X86_BUG_TDX_PW_MCE erratum. For simplicity,
> don't expect this errata on any TDX Extensions supported platform. So
> TDX Extensions & all features that require TDX Extensions (e.g. TDX
> Connect) will not call the clearing helpers.
>
> Note the "ext_required" global metadata specifies if TDH.EXT.INIT call
> is needed. If 0, the Extensions are already working, so skip the SEAMCALL.
>
> Co-developed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 45 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 31ccdfcf518c..a26fe94c07ff 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -60,6 +60,7 @@
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG_V0 45
> #define TDH_SYS_CONFIG SEAMCALL_LEAF_VER(TDH_SYS_CONFIG_V0, 1)
> +#define TDH_EXT_INIT 60
> #define TDH_EXT_MEM_ADD 61
>
> /* TDX page types */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 5fae17c13191..4134f92425da 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1519,6 +1519,23 @@ static void tdx_clflush_page_array(struct tdx_page_array *array)
> tdx_clflush_page(array->pages[array->offset + i]);
> }
>
> +/* Initialize the TDX Module Extensions then Extension-SEAMCALLs can be used */
> +static int tdx_ext_init(void)
> +{
> + struct tdx_module_args args = {};
> + u64 r;
> +
> + do {
> + r = seamcall(TDH_EXT_INIT, &args);
> + cond_resched();
> + } while (r == TDX_INTERRUPTED_RESUMABLE);
> +
> + if (r != TDX_SUCCESS)
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int tdx_ext_mem_add(struct tdx_page_array *ext_mem)
> {
> struct tdx_module_args args = {
> @@ -1572,6 +1589,17 @@ static int __maybe_unused init_tdx_ext(void)
> if (!(tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_EXT))
> return 0;
>
> + /*
> + * With this errata, TDX should use movdir64b to clear private pages
> + * when reclaiming them. See tdx_quirk_reset_paddr().
> + *
> + * Don't expect this errata on any TDX Extensions supported platform.
> + * All features require TDX Extensions (including TDX Extensions
> + * itself) will never call tdx_quirk_reset_paddr().
> + */
> + if (boot_cpu_has_bug(X86_BUG_TDX_PW_MCE))
> + return -ENXIO;
I don't know if we are going to want to sprinkle these over every new feature
until the end of time. If this feature will only show up on the platforms with
this erratum, then I say we just drop the check.
> +
> nr_pages = tdx_sysinfo.ext.memory_pool_required_pages;
> /*
> * memory_pool_required_pages == 0 means no need to add more pages,
> @@ -1587,6 +1615,20 @@ static int __maybe_unused init_tdx_ext(void)
> goto out_ext_mem;
> }
>
> + /*
> + * ext_required == 0 means no need to call TDH.EXT.INIT, the Extensions
> + * are already working.
How does this scenario happen exactly? And why not check it above at the
beginning? Before the allocation, so it doesn't need to free.
Is there a scenario where the memory needs to be given, but the extension is
already inited?
> + */
> + if (tdx_sysinfo.ext.ext_required) {
> + ret = tdx_ext_init();
> + /*
> + * Some pages may have been touched by the TDX module.
> + * Flush cache before returning these pages to kernel.
> + */
> + if (ret)
> + goto out_flush;
> + }
> +
> /* Extension memory is never reclaimed once assigned */
> tdx_page_array_ctrl_leak(ext_mem);
>
> @@ -1595,6 +1637,9 @@ static int __maybe_unused init_tdx_ext(void)
>
> return 0;
>
> +out_flush:
> + if (ext_mem)
For the error path we don't need to be efficient. But also why does it assume
tdx_ext_init() can touch the pages, but tdx_ext_mem_add() can't?
> + wbinvd_on_all_cpus();
> out_ext_mem:
> tdx_page_array_free(ext_mem);
>
^ 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