* 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
* 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:25 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: <acpPsPivfllpWfv1@yilunxu-OptiPlex-7050>
On Mon, 2026-03-30 at 18:25 +0800, Xu Yilun wrote:
> > On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > > Add struct tdx_page_array definition for new TDX Module object
> > > types - HPA_ARRAY_T and HPA_LIST_INFO.
> >
> > This is unfortunate. I see you agree in the comments.
>
> Yes, basically they are defining the same concept, behave mostly the same
> but some differences...
>
> >
> > >
> > > They are used as input/output
> > > parameters in newly defined SEAMCALLs. Also define some helpers to
> > > allocate, setup and free tdx_page_array.
> > >
> > > HPA_ARRAY_T and HPA_LIST_INFO are similar in most aspects. They both
> > > represent a list of pages for TDX Module accessing. There are several
> > > use cases for these 2 structures:
> > >
> > > - As SEAMCALL inputs. They are claimed by TDX Module as control pages.
> > > Control pages are private pages for TDX Module to hold its internal
> > > control structures or private data. TDR, TDCS, TDVPR... are existing
> > > control pages, just not added via tdx_page_array.
> > > - As SEAMCALL outputs. They were TDX Module control pages and now are
> > > released.
> > > - As SEAMCALL inputs. They are just temporary buffers for exchanging
> > > data blobs in one SEAMCALL. TDX Module will not hold them for long
> > > time.
> >
> > This is kind of verbose for what it seems to be trying to say. It's just
> > that
>
> I assume if you feel the explanation of "what is control page" is off
> track. I added it cause the term firstly appears in x86 (only in KVM
> TDX previously), and people ask the definition:
>
> https://lore.kernel.org/all/cfcfb160-fcd2-4a75-9639-5f7f0894d14b@intel.com/
I meant it more generally.
>
> > these types can be input or output params. The TDX module could hold on to
> > the
> > pages for a long time, or just transiently.
>
> Mm.. I'm trying to ramp up on the kernel API level flow:
>
> For control pages, it would be hold by TDX Module long time, so host
> inputs the page array, later TDX Module outputs the page array back.
> Host need to verify the outputs.
>
> For shared pages, TDX Module's accessing is transient in one SEAMCALL,
> so only as input, TDX Module never needs to output the array.
>
> I think the verboseness makes the following pseudo code easier to
> understand.
>
> > For that latter part I think you are
> > trying to say sometimes they need flushing and sometimes they don't?
>
> Yeah.
> control pages => long term => host verifies and releases => flush on release
> shared pages => transient => no verify and releases => no flush
>
> Maybe I should mention the flushing is already covered by releasing
> kAPI.
I hear:
1. Long time vs short time
2. Accessed as private vs "shared"
I think (2) is the important point, right? Why does (1) matter?
> >
> > >
> > > The 2 structures both need a 'root page' which contains a list of HPAs.
> > > They collapse the HPA of the root page and the number of valid HPAs
> > > into a 64 bit raw value for SEAMCALL parameters. The root page is
> > > always a medium for passing data pages, TDX Module never keeps the
> > > root page.
> > >
> > > A main difference is HPA_ARRAY_T requires singleton mode when
> > > containing just 1 functional page (page0). In this mode the root page is
> > > not needed and the HPA field of the raw value directly points to the
> > > page0. But in this patch, root page is always allocated for user
> > > friendly kAPIs.
> >
> > "singleton mode"? What is it? If it's the case of not needing populate loop,
> > it
>
> It is the SEAMCALL level detail for HPA_ARRAY_T. It is literally as
> explained above - the HPA field should be filled by page0, not root page.
>
> > probably deserves more explanation. I'm not sure, but the populate loop
> > seems to
> > drive a lot of the struct design?
>
> The caller is not aware of singleton mode. Actually, I'm trying to make
> the tdx_page_array independent of HPA_ARRAY_T or HPA_LIST_INFO details
> when allocating/populating, root page is still populated even not needed
> for singleton mode. The differences only happen when collaping the struct
> into u64 SEAMCALL parameters.
It seems tdx_page_array combines two concepts. An array of pages, and the method
that the pages get handed to the TDX module. What if we broke apart these
concepts?
>
> >
> > >
> > > Another small difference is HPA_LIST_INFO contains a 'first entry' field
> > > which could be filled by TDX Module. This simplifies host by providing
> > > the same structure when re-invoke the interrupted SEAMCALL. No need for
> > > host to touch this field.
> >
> > Not clear what this is talking about. But I'm starting to wonder if we
> > should be
> > so bold to claim that the differences between the types really simplify the
> > host.
>
> I'm talking about another SEAMCALL level detail. Sometimes TDX Module
> got interrupted in the middle of page array processing, it needs an
> anchor to resuming from where it stops, TDX Module record the anchor
> in the 'first entry'.
>
> By illustrating these SEAMCALL level differences, I want to explain
> they don't impact the general SW flow and kAPI cares about them
> internally.
>
> 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.
>
> Please allow me to stop here, will continue later...
>
> Thanks.
^ permalink raw reply
* Re: [PATCH v3 3/6] x86/sev: Add support to perform RMP optimizations asynchronously
From: Dave Hansen @ 2026-03-30 23:22 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: <6345df31337125280f91ad8f37843aa865fd85fc.1774755884.git.ashish.kalra@amd.com>
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.
> + /* 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.
Also, rmpopt() itself does 1G alignment. This code ^ also aligns the
start and end. Why?
> + 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's also *really* generically and non-descriptively named for a
global-scope variable.
> +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.
> 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?
> + /*
> + * 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.
> + INIT_DELAYED_WORK(&rmpopt_delayed_work, rmpopt_work_handler);
> +
> + rmpopt_pa_start = pa_start;
Why is there a 'rmpopt_pa_start' and 'pa_start'?
> + 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 v3 2/6] x86/sev: Add support for enabling RMPOPT
From: Dave Hansen @ 2026-03-30 22:38 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: <0f6539b8fd4c4e7d40fb19e6b3255a120664bfb4.1774755884.git.ashish.kalra@amd.com>
On 3/30/26 15:26, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
>
> The new RMPOPT instruction helps manage per-CPU RMP optimization
> structures inside the CPU. It takes a 1GB-aligned physical address
> and either returns the status of the optimizations or tries to enable
> the optimizations.
>
> Per-CPU RMPOPT tables support at most 2 TB of addressable memory for
> RMP optimizations.
>
> Initialize the per-CPU RMPOPT table base to the starting physical
> address. This enables RMP optimization for up to 2 TB of system RAM on
> all CPUs.
Isn't the Subject: more appropriately something like:
x86/sev: Initialize RMPOPT configuration MSRs
The subject is too generic as-is.
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index be3e3cc963b2..9c8a6dfd7891 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -758,6 +758,9 @@
> #define MSR_AMD64_SEG_RMP_ENABLED_BIT 0
> #define MSR_AMD64_SEG_RMP_ENABLED BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
> #define MSR_AMD64_RMP_SEGMENT_SHIFT(x) (((x) & GENMASK_ULL(13, 8)) >> 8)
> +#define MSR_AMD64_RMPOPT_BASE 0xc0010139
> +#define MSR_AMD64_RMPOPT_ENABLE_BIT 0
> +#define MSR_AMD64_RMPOPT_ENABLE BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
>
> #define MSR_SVSM_CAA 0xc001f000
>
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index e35fac0a8a3d..dc6a8e102cdc 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -477,6 +477,30 @@ static bool __init setup_rmptable(void)
> return true;
> }
>
> +static __init void configure_and_enable_rmpopt(void)
> +{
Nit: just "setup_rmpopt()" would be fine, IMNHO. We have plenty of
"setup_foo()" functions that configure and enable CPU features.
> + phys_addr_t pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
> + u64 rmpopt_base = pa_start | MSR_AMD64_RMPOPT_ENABLE;
> + int cpu;
> +
> + if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
> + return;
> +
> + if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)) {
> + pr_notice("RMPOPT optimizations not enabled, segmented RMP required\n");
> + return;
> + }
> +
> + /*
> + * Per-CPU RMPOPT tables support at most 2 TB of addressable memory
> + * for RMP optimizations. Initialize the per-CPU RMPOPT table base
> + * to the starting physical address to enable RMP optimizations for
> + * up to 2 TB of system RAM on all CPUs.
> + */
> + for_each_online_cpu(cpu)
> + wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
> +}
> +
With those nits fixed:
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply
* [BUG] x86/tdx: handle_in()/handle_out() use one-bit-too-wide GENMASK for port I/O
From: Borys Tsyrulnikov @ 2026-03-30 22:33 UTC (permalink / raw)
To: x86; +Cc: linux-coco, kvm, kas, dave.hansen, rick.p.edgecombe, linux-kernel
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:
size=1 (INB): GENMASK(8,0) = 0x1FF vs correct 0xFF
(extra: bit 8)
size=2 (INW): GENMASK(16,0) = 0x1FFFF vs correct 0xFFFF
(extra: bit 16)
size=4 (INL): GENMASK(32,0) = 0x1FFFFFFFF vs correct 0xFFFFFFFF
(extra: bit 32)
In handle_in(), this causes the VMM response's extra bit to overwrite
one bit of RAX that should be preserved. For INB, bit 8 (AH bit 0) is
overwritten from the VMM response instead of being preserved from the
guest's prior AX. On native x86, INB only modifies AL -- AH is
preserved.
In handle_out(), one extra bit of regs->ax is sent to the VMM beyond
what the I/O width requires.
I did not find a broad exploit in common kernel callers under tested
GCC/Clang toolchains, because many typed u8/u16/u32 paths naturally
truncate or zero-extend the value before reuse. That is not a full
non-exploitability proof, though: some ordinary in-tree caller shapes
may still preserve the primitive, especially read/test/write-back
sequences such as inl(...) followed by a branch and outl(same_value,
...), or inb(...) followed by if (v) outb(v, ...). I am therefore
treating this as a correctness bug in the #VE emulation path with
guest-config-
dependent leak potential, not as a proven broad exploit.
Expected vs observed (INB, size=1)
----------------------------------
Native x86 INB: writes AL (bits 0-7). AH (bit 8+) is preserved.
TDX handle_in() with buggy mask: clears bits 0-8, then sets bits 0-8
from VMM response. AH bit 0 depends on VMM, not the guest's prior AX.
Live reproduction
-----------------
GCP c3-standard-4 Confidential VM (TDX), kernel 6.17.0-1009-gcp, 2026-03-30.
Kernel module sets AX to two values, does INB from CMOS port 0x71, checks AH:
Test A (AX=0x0000): result AX=0x0026 AL=0x26 AH=0x00
Test B (AX=0x0100): result AX=0x0026 AL=0x26 AH=0x00
*** AH same in both tests (0x00) -- bit 8 from VMM, not initial AX ***
*** GENMASK off-by-one CONFIRMED on live TDX ***
10x test: bit8 set with AX=0x0000: 0/10
10x test: bit8 set with AX=0x0100: 0/10
*** Bit 8 independent of initial AX -- CONFIRMED ***
In test B, AH should be 0x01 (preserved). Instead it is 0x00 --
overwritten from VMM data. 10/10 consistent. Reproduced twice on
2026-03-30, including a fresh rerun on a new GCP TDX VM.
Additional live write-side confirmation
---------------------------------------
Separate raw-asm kernel modules confirmed the write-side primitive
reaches the outbound sink for all widths:
[outb] before outb A = 0x005a
[outb] before outb B = 0x015a
[outw] before outw A = 0x00008086
[outw] before outw B = 0x00018086
[outl] before outl A = 0x0000000012378086
[outl] before outl B = 0x0000000112378086
For correct emulation, only the intended-width values should be
exposed to the VMM: 0x5a / 0x8086 / 0x12378086. With the current
handle_out() mask, the paired local payload simulator gives:
outb: 0x05a vs 0x15a
outw: 0x08086 vs 0x18086
outl: 0x012378086 vs 0x112378086
This is not a host-side capture on a controllable TDX VMM, so I am not
treating it as a full confidentiality exploit. It does strengthen the
claim that the extra-bit write-side primitive is real on live TDX for
outb, outw, and outl.
Secondary issue: missing zero-extension for INL
------------------------------------------------
Separately, handle_in() does not zero-extend bits 32-63 of RAX for
size=4 (INL). On native x86-64, INL writes EAX which zero-extends to
RAX (Intel SDM Vol. 1, 3.4.1.1). handle_in() preserves bits above the
mask instead.
handle_mmio() in the same file handles this correctly:
case INSN_MMIO_READ:
/* Zero-extend for 32-bit operation */
extend_size = size == 4 ? sizeof(*reg) : 0;
read_msr() also does it correctly:
regs->ax = lower_32_bits(args.r11);
handle_in() has no equivalent. Same compiler truncation caveat applies
-- no known practical impact via standard inl() callers.
A separate raw-asm kernel module reproduced this on the same GCP TDX
kernel. Preloading RAX=0xDEADBEEF00000000 before INL 0xCFC produced
RAX=0xDEADBEEE12378086, and preloading RAX=0x1234567800000000 produced
RAX=0x1234567812378086. Bits 33-63 remain stale from pre-INL RAX; bit
32 reflects VMM-controlled data; lower 32 bits contain the expected
PCI config data.
Fix
---
In both handle_in() and handle_out(), change:
u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
to:
u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
Reproduction modules available on request: portio_test.c (read-side
INB), portio_pure_out_test.c / portio_pure_outw_test.c (write-side),
portio_zeroext_test.c (INL zero-extension).
Tested on: GCP c3-standard-4 Confidential VM (TDX)
Kernel: 6.17.0-1009-gcp (Ubuntu 24.04)
Borys Tsyrulnikov
^ permalink raw reply
* [PATCH v3 6/6] x86/sev: Add debugfs support for RMPOPT
From: Ashish Kalra @ 2026-03-30 22:27 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
Add a debugfs interface to report per-CPU RMPOPT status across all
system RAM.
To dump the per-CPU RMPOPT status for all system RAM:
/sys/kernel/debug/rmpopt# cat rmpopt-table
Memory @ 0GB: CPU(s): none
Memory @ 1GB: CPU(s): none
Memory @ 2GB: CPU(s): 0-1023
Memory @ 3GB: CPU(s): 0-1023
Memory @ 4GB: CPU(s): none
Memory @ 5GB: CPU(s): 0-1023
Memory @ 6GB: CPU(s): 0-1023
Memory @ 7GB: CPU(s): 0-1023
...
Memory @1025GB: CPU(s): 0-1023
Memory @1026GB: CPU(s): 0-1023
Memory @1027GB: CPU(s): 0-1023
Memory @1028GB: CPU(s): 0-1023
Memory @1029GB: CPU(s): 0-1023
Memory @1030GB: CPU(s): 0-1023
Memory @1031GB: CPU(s): 0-1023
Memory @1032GB: CPU(s): 0-1023
Memory @1033GB: CPU(s): 0-1023
Memory @1034GB: CPU(s): 0-1023
Memory @1035GB: CPU(s): 0-1023
Memory @1036GB: CPU(s): 0-1023
Memory @1037GB: CPU(s): 0-1023
Memory @1038GB: CPU(s): none
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 107 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 784c0e79200e..04d905894408 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -20,6 +20,8 @@
#include <linux/amd-iommu.h>
#include <linux/nospec.h>
#include <linux/workqueue.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
#include <asm/sev.h>
#include <asm/processor.h>
@@ -143,6 +145,13 @@ static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
static unsigned long snp_nr_leaked_pages;
+static cpumask_t rmpopt_cpumask;
+static struct dentry *rmpopt_debugfs;
+
+struct seq_paddr {
+ phys_addr_t next_seq_paddr;
+};
+
#undef pr_fmt
#define pr_fmt(fmt) "SEV-SNP: " fmt
@@ -500,6 +509,8 @@ static inline bool __rmpopt(u64 rax, u64 rcx)
: "a" (rax), "c" (rcx)
: "memory", "cc");
+ assign_cpu(smp_processor_id(), &rmpopt_cpumask, optimized);
+
return optimized;
}
@@ -514,6 +525,17 @@ static void rmpopt(void *val)
__rmpopt(rax, rcx);
}
+/*
+ * 'val' is a system physical address.
+ */
+static void rmpopt_report_status(void *val)
+{
+ u64 rax = ALIGN_DOWN((u64)val, SZ_1G);
+ u64 rcx = RMPOPT_FUNC_REPORT_STATUS;
+
+ __rmpopt(rax, rcx);
+}
+
static void rmpopt_work_handler(struct work_struct *work)
{
phys_addr_t pa;
@@ -560,6 +582,89 @@ static void rmpopt_all_physmem(bool early)
msecs_to_jiffies(RMPOPT_WORK_TIMEOUT));
}
+/*
+ * start() can be called multiple times if allocated buffer has overflowed
+ * and bigger buffer is allocated.
+ */
+static void *rmpopt_table_seq_start(struct seq_file *seq, loff_t *pos)
+{
+ phys_addr_t end_paddr = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
+ struct seq_paddr *p = seq->private;
+
+ if (*pos == 0) {
+ p->next_seq_paddr = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
+ return &p->next_seq_paddr;
+ }
+
+ if (p->next_seq_paddr == end_paddr)
+ return NULL;
+
+ return &p->next_seq_paddr;
+}
+
+static void *rmpopt_table_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+ phys_addr_t end_paddr = ALIGN(PFN_PHYS(max_pfn), SZ_1G);
+ phys_addr_t *curr_paddr = v;
+
+ (*pos)++;
+ if (*curr_paddr == end_paddr)
+ return NULL;
+ *curr_paddr += SZ_1G;
+
+ return curr_paddr;
+}
+
+static void rmpopt_table_seq_stop(struct seq_file *seq, void *v)
+{
+}
+
+static int rmpopt_table_seq_show(struct seq_file *seq, void *v)
+{
+ phys_addr_t *curr_paddr = v;
+
+ seq_printf(seq, "Memory @%3lluGB: ",
+ *curr_paddr >> (get_order(SZ_1G) + PAGE_SHIFT));
+
+ cpumask_clear(&rmpopt_cpumask);
+ on_each_cpu_mask(cpu_online_mask, rmpopt_report_status,
+ (void *)*curr_paddr, true);
+
+ if (cpumask_empty(&rmpopt_cpumask))
+ seq_puts(seq, "CPU(s): none\n");
+ else
+ seq_printf(seq, "CPU(s): %*pbl\n", cpumask_pr_args(&rmpopt_cpumask));
+
+ return 0;
+}
+
+static const struct seq_operations rmpopt_table_seq_ops = {
+ .start = rmpopt_table_seq_start,
+ .next = rmpopt_table_seq_next,
+ .stop = rmpopt_table_seq_stop,
+ .show = rmpopt_table_seq_show
+};
+
+static int rmpopt_table_open(struct inode *inode, struct file *file)
+{
+ return seq_open_private(file, &rmpopt_table_seq_ops, sizeof(struct seq_paddr));
+}
+
+static const struct file_operations rmpopt_table_fops = {
+ .open = rmpopt_table_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release_private,
+};
+
+static void rmpopt_debugfs_setup(void)
+{
+ rmpopt_debugfs = debugfs_create_dir("rmpopt", arch_debugfs_dir);
+
+ debugfs_create_file("rmpopt-table", 0444, rmpopt_debugfs,
+ NULL, &rmpopt_table_fops);
+}
+
static __init void configure_and_enable_rmpopt(void)
{
phys_addr_t pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
@@ -613,6 +718,8 @@ static __init void configure_and_enable_rmpopt(void)
* optimizations on all physical memory.
*/
rmpopt_all_physmem(TRUE);
+
+ rmpopt_debugfs_setup();
}
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v3 5/6] KVM: SEV: Perform RMP optimizations on SNP guest shutdown
From: Ashish Kalra @ 2026-03-30 22:27 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
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.
RMP optimizations are performed asynchronously by queuing work on a
dedicated workqueue with a delay.
Delaying work allows batching of multiple SNP guest terminations.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/kvm/svm/sev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3f9c1aa39a0a..2ad4727c4177 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2942,6 +2942,8 @@ void sev_vm_destroy(struct kvm *kvm)
if (sev_snp_guest(kvm)) {
snp_guest_req_cleanup(kvm);
+ snp_perform_rmp_optimization();
+
/*
* Decomission handles unbinding of the ASID. If it fails for
* some unexpected reason, just leak the ASID.
--
2.43.0
^ permalink raw reply related
* [PATCH v3 4/6] x86/sev: Add interface to re-enable RMP optimizations.
From: Ashish Kalra @ 2026-03-30 22:26 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
RMPOPT table is a per-processor table which indicates if 1GB regions of
physical memory are entirely hypervisor-owned or not.
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.
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/sev.h | 2 ++
arch/x86/virt/svm/sev.c | 17 +++++++++++++++++
drivers/crypto/ccp/sev-dev.c | 4 ++++
3 files changed, 23 insertions(+)
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 0e6c0940100f..451fb2b2a0f7 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -657,6 +657,7 @@ int rmp_make_shared(u64 pfn, enum pg_level level);
void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp);
void kdump_sev_callback(void);
void snp_fixup_e820_tables(void);
+int snp_perform_rmp_optimization(void);
static inline void snp_leak_pages(u64 pfn, unsigned int pages)
{
__snp_leak_pages(pfn, pages, true);
@@ -677,6 +678,7 @@ static inline void __snp_leak_pages(u64 pfn, unsigned int npages, bool dump_rmp)
static inline void snp_leak_pages(u64 pfn, unsigned int npages) {}
static inline void kdump_sev_callback(void) { }
static inline void snp_fixup_e820_tables(void) {}
+static inline int snp_perform_rmp_optimization(void) { return 0; }
#endif
#endif
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 1644f8a9b2a2..784c0e79200e 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -1138,6 +1138,23 @@ int rmp_make_shared(u64 pfn, enum pg_level level)
}
EXPORT_SYMBOL_GPL(rmp_make_shared);
+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;
+
+ 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();
+
sev->snp_initialized = true;
dev_dbg(sev->dev, "SEV-SNP firmware initialized, SEV-TIO is %s\n",
data.tio_en ? "enabled" : "disabled");
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/6] x86/sev: Add support to perform RMP optimizations asynchronously
From: Ashish Kalra @ 2026-03-30 22:26 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
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.
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.
Enable RMPOPT optimizations globally for all system RAM up to 2TB at
RMP initialization time. RMP checks can initially be skipped for 1GB
memory ranges that do not contain SEV-SNP guest memory (excluding
preassigned pages such as the RMP table and firmware pages). As SNP
guests are launched, RMPUPDATE will disable the corresponding RMPOPT
optimizations.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/virt/svm/sev.c | 114 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index dc6a8e102cdc..1644f8a9b2a2 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -19,6 +19,7 @@
#include <linux/iommu.h>
#include <linux/amd-iommu.h>
#include <linux/nospec.h>
+#include <linux/workqueue.h>
#include <asm/sev.h>
#include <asm/processor.h>
@@ -124,6 +125,19 @@ static void *rmp_bookkeeping __ro_after_init;
static u64 probed_rmp_base, probed_rmp_size;
+enum rmpopt_function {
+ RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS,
+ RMPOPT_FUNC_REPORT_STATUS
+};
+
+#define RMPOPT_WORK_TIMEOUT 10000
+
+static struct workqueue_struct *rmpopt_wq;
+static struct delayed_work rmpopt_delayed_work;
+
+static cpumask_t primary_threads_cpumask;
+static phys_addr_t rmpopt_pa_start, rmpopt_pa_end;
+
static LIST_HEAD(snp_leaked_pages_list);
static DEFINE_SPINLOCK(snp_leaked_pages_list_lock);
@@ -477,6 +491,75 @@ static bool __init setup_rmptable(void)
return true;
}
+static inline bool __rmpopt(u64 rax, u64 rcx)
+{
+ bool optimized;
+
+ asm volatile(".byte 0xf2, 0x0f, 0x01, 0xfc"
+ : "=@ccc" (optimized)
+ : "a" (rax), "c" (rcx)
+ : "memory", "cc");
+
+ return optimized;
+}
+
+/*
+ * 'val' is a system physical address.
+ */
+static void rmpopt(void *val)
+{
+ u64 rax = ALIGN_DOWN((u64)val, SZ_1G);
+ u64 rcx = RMPOPT_FUNC_VERIFY_AND_REPORT_STATUS;
+
+ __rmpopt(rax, rcx);
+}
+
+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.
+ */
+
+ cpumask_clear_cpu(smp_processor_id(), &primary_threads_cpumask);
+
+ /* current CPU */
+ for (pa = rmpopt_pa_start; pa < rmpopt_pa_end; pa += SZ_1G)
+ rmpopt((void *)pa);
+
+ 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);
+}
+
+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));
+}
+
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);
+
+ /*
+ * 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;
+
+ INIT_DELAYED_WORK(&rmpopt_delayed_work, rmpopt_work_handler);
+
+ rmpopt_pa_start = pa_start;
+ 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);
}
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/6] x86/sev: Add support for enabling RMPOPT
From: Ashish Kalra @ 2026-03-30 22:26 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
The new RMPOPT instruction helps manage per-CPU RMP optimization
structures inside the CPU. It takes a 1GB-aligned physical address
and either returns the status of the optimizations or tries to enable
the optimizations.
Per-CPU RMPOPT tables support at most 2 TB of addressable memory for
RMP optimizations.
Initialize the per-CPU RMPOPT table base to the starting physical
address. This enables RMP optimization for up to 2 TB of system RAM on
all CPUs.
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/msr-index.h | 3 +++
arch/x86/virt/svm/sev.c | 26 ++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index be3e3cc963b2..9c8a6dfd7891 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -758,6 +758,9 @@
#define MSR_AMD64_SEG_RMP_ENABLED_BIT 0
#define MSR_AMD64_SEG_RMP_ENABLED BIT_ULL(MSR_AMD64_SEG_RMP_ENABLED_BIT)
#define MSR_AMD64_RMP_SEGMENT_SHIFT(x) (((x) & GENMASK_ULL(13, 8)) >> 8)
+#define MSR_AMD64_RMPOPT_BASE 0xc0010139
+#define MSR_AMD64_RMPOPT_ENABLE_BIT 0
+#define MSR_AMD64_RMPOPT_ENABLE BIT_ULL(MSR_AMD64_RMPOPT_ENABLE_BIT)
#define MSR_SVSM_CAA 0xc001f000
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index e35fac0a8a3d..dc6a8e102cdc 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -477,6 +477,30 @@ static bool __init setup_rmptable(void)
return true;
}
+static __init void configure_and_enable_rmpopt(void)
+{
+ phys_addr_t pa_start = ALIGN_DOWN(PFN_PHYS(min_low_pfn), SZ_1G);
+ u64 rmpopt_base = pa_start | MSR_AMD64_RMPOPT_ENABLE;
+ int cpu;
+
+ if (!cpu_feature_enabled(X86_FEATURE_RMPOPT))
+ return;
+
+ if (!(rmp_cfg & MSR_AMD64_SEG_RMP_ENABLED)) {
+ pr_notice("RMPOPT optimizations not enabled, segmented RMP required\n");
+ return;
+ }
+
+ /*
+ * Per-CPU RMPOPT tables support at most 2 TB of addressable memory
+ * for RMP optimizations. Initialize the per-CPU RMPOPT table base
+ * to the starting physical address to enable RMP optimizations for
+ * up to 2 TB of system RAM on all CPUs.
+ */
+ for_each_online_cpu(cpu)
+ wrmsrq_on_cpu(cpu, MSR_AMD64_RMPOPT_BASE, rmpopt_base);
+}
+
/*
* Do the necessary preparations which are verified by the firmware as
* described in the SNP_INIT_EX firmware command description in the SNP
@@ -530,6 +554,8 @@ int __init snp_rmptable_init(void)
*/
crash_kexec_post_notifiers = true;
+ configure_and_enable_rmpopt();
+
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/6] x86/cpufeatures: Add X86_FEATURE_AMD_RMPOPT feature flag
From: Ashish Kalra @ 2026-03-30 22:25 UTC (permalink / raw)
To: 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: <cover.1774755884.git.ashish.kalra@amd.com>
From: Ashish Kalra <ashish.kalra@amd.com>
Add a flag indicating whether RMPOPT instruction is supported.
RMPOPT is a new instruction designed to minimize the performance
overhead of RMP checks on the hypervisor and on non-SNP guests by
allowing RMP checks to be skipped when 1G regions of memory are known
not to contain any SEV-SNP guest memory.
For more information on the RMPOPT instruction, see the AMD64 RMPOPT
technical documentation.
Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
arch/x86/include/asm/cpufeatures.h | 2 +-
arch/x86/kernel/cpu/scattered.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index dbe104df339b..bce1b2e2a35c 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -76,7 +76,7 @@
#define X86_FEATURE_K8 ( 3*32+ 4) /* Opteron, Athlon64 */
#define X86_FEATURE_ZEN5 ( 3*32+ 5) /* CPU based on Zen5 microarchitecture */
#define X86_FEATURE_ZEN6 ( 3*32+ 6) /* CPU based on Zen6 microarchitecture */
-/* Free ( 3*32+ 7) */
+#define X86_FEATURE_RMPOPT ( 3*32+ 7) /* Support for AMD RMPOPT instruction */
#define X86_FEATURE_CONSTANT_TSC ( 3*32+ 8) /* "constant_tsc" TSC ticks at a constant rate */
#define X86_FEATURE_UP ( 3*32+ 9) /* "up" SMP kernel running on UP */
#define X86_FEATURE_ART ( 3*32+10) /* "art" Always running timer (ART) */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 42c7eac0c387..7ac3818c4502 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -65,6 +65,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_PERFMON_V2, CPUID_EAX, 0, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_V2, CPUID_EAX, 1, 0x80000022, 0 },
{ X86_FEATURE_AMD_LBR_PMC_FREEZE, CPUID_EAX, 2, 0x80000022, 0 },
+ { X86_FEATURE_RMPOPT, CPUID_EDX, 0, 0x80000025, 0 },
{ X86_FEATURE_AMD_HTR_CORES, CPUID_EAX, 30, 0x80000026, 0 },
{ 0, 0, 0, 0, 0 }
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/6] Add RMPOPT support.
From: Ashish Kalra @ 2026-03-30 22:25 UTC (permalink / raw)
To: 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
From: Ashish Kalra <ashish.kalra@amd.com>
In the SEV-SNP architecture, hypervisor and non-SNP guests are subject
to RMP checks on writes to provide integrity of SEV-SNP guest memory.
The RMPOPT architecture enables optimizations whereby the RMP checks
can be skipped if 1GB regions of memory are known to not contain any
SNP guest memory.
RMPOPT is a new instruction designed to minimize the performance
overhead of RMP checks for the hypervisor and non-SNP guests.
RMPOPT instruction currently supports two functions. In case of the
verify and report status function the CPU will read the RMP contents,
verify the entire 1GB region starting at the provided SPA is HV-owned.
For the entire 1GB region it checks that all RMP entries in this region
are HV-owned (i.e, not in assigned state) and then accordingly updates
the RMPOPT table to indicate if optimization has been enabled and
provide indication to software if the optimization was successful.
In case of report status function, the CPU returns the optimization
status for the 1GB region.
The RMPOPT table is managed by a combination of software and hardware.
Software uses the RMPOPT instruction to set bits in the table,
indicating that regions of memory are entirely HV-owned. Hardware
automatically clears bits in the RMPOPT table when RMP contents are
changed during RMPUPDATE instruction.
For more information on the RMPOPT instruction, see the AMD64 RMPOPT
technical documentation.
As SNP is enabled by default the hypervisor and non-SNP guests are
subject to RMP write checks to provide integrity of SNP guest memory.
This patch-series adds support to enable RMP optimizations for up to
2TB of system RAM across the system and allow RMPUPDATE to disable
those optimizations as SNP guests are launched.
Support for RAM larger than 2 TB will be added in follow-on series.
This series also introduces support to re-enable RMP optimizations
during SNP guest termination, after guest pages have been converted
back to shared.
RMP optimizations are performed asynchronously by queuing work on a
dedicated workqueue after a 10 second delay.
Delaying work allows batching of multiple SNP guest terminations.
Once 1GB hugetlb guest_memfd support is merged, support for
re-enabling RMPOPT optimizations during 1GB page cleanup will be added
in follow-on series.
Additionally add debugfs interface to report per-CPU RMPOPT status
across all system RAM.
v3:
- Drop all RMPOPT kthread support and introduce adding custom and
dedicated workqueue to schedule delayed and asynchronous RMPOPT work.
- Drop the guest_memfd inode cleanup interface and add support to
re-enable RMP optimizations during guest shutdown using the
asynchronous and delayed workqueue interface.
- Introduce new __rmpopt() helper and rmpopt() and
rmpopt_report_status() wrappers on top which use rax and rcx
parameters to closely match RMPOPT specs.
- Use new optimized RMPOPT loop to issue RMPOPT instructions on all
system RAM upto 2TB and all CPUs, by optimizing 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.
- Also add support for running the optimized RMPOPT loop only on
one thread per core.
- Replace all PUD_SIZE references with SZ_1G to conform to 1GB regions
as specified by RMPOPT specifications and not be dependent on PUD_SIZE
which makes the RMPOPT patch-set independent of x86 page table sizes.
- Use wrmsrq_on_cpu() to program the RMPOPT_BASE MSR registers on
all CPUs that removes all ugly casting to use on_each_cpu_mask().
- Fix inline commits and patch commit messages
v2:
- Drop all NUMA and Socket configuration and enablement support and
enable RMPOPT support for up to 2TB of system RAM.
- Drop get_cpumask_of_primary_threads() and enable per-core RMPOPT
base MSRs and issue RMPOPT instruction on all CPUs.
- Drop the configfs interface to manually re-enable RMP optimizations.
- Add new guest_memfd cleanup interface to automatically re-enable
RMP optimizations during guest shutdown.
- Include references to the public RMPOPT documentation.
- Move debugfs directory for RMPOPT under architecuture specific
parent directory.
Ashish Kalra (6):
x86/cpufeatures: Add X86_FEATURE_AMD_RMPOPT feature flag
x86/sev: Add support for enabling RMPOPT
x86/sev: Add support to perform RMP optimizations asynchronously
x86/sev: Add interface to re-enable RMP optimizations.
KVM: SEV: Perform RMP optimizations on SNP guest shutdown
x86/sev: Add debugfs support for RMPOPT
arch/x86/include/asm/cpufeatures.h | 2 +-
arch/x86/include/asm/msr-index.h | 3 +
arch/x86/include/asm/sev.h | 2 +
arch/x86/kernel/cpu/scattered.c | 1 +
arch/x86/kvm/svm/sev.c | 2 +
arch/x86/virt/svm/sev.c | 263 +++++++++++++++++++++++++++++
drivers/crypto/ccp/sev-dev.c | 4 +
7 files changed, 276 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v13 10/48] arm64: RMI: Ensure that the RMM has GPT entries for memory
From: Mathieu Poirier @ 2026-03-30 20:58 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, 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: <20260318155413.793430-11-steven.price@arm.com>
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:
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.
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 related
* Re: [PATCH v2 3/5] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Edgecombe, Rick P @ 2026-03-30 19:25 UTC (permalink / raw)
To: kas@kernel.org, Verma, Vishal L
Cc: seanjc@google.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, pbonzini@redhat.com,
linux-coco@lists.linux.dev, kvm@vger.kernel.org
In-Reply-To: <acpiGUKS-WRmLoxp@thinkstation>
On Mon, 2026-03-30 at 11:58 +0000, Kiryl Shutsemau wrote:
> > + * - TDX_SYS_BUSY could transiently contend with
> > TDH.SYS.* SEAMCALLs,
> > + * but will lock out future ones.
>
> Locked out by who? Is it TDX module contract? I don't see it
> documented in
> the spec.
Yea, by the TDX module.
We relayed that we need this specific behavior around TDX_SYS_BUSY
contention, but the implementation isn't done. That spec is actually
still in draft form. Which is refreshing, because we can actually tweak
things like this based on what the kernel needs.
>
> I assumed that if the SEAMCALL fails other SEAMCALLs suppose to be
> functional. Hm?
The behavior should be that once you make this seamcall (assuming it's
supported) that no other seamcalls can be made. They will return an
error. Do you think something else would be better? If it's an old TDX
module, nothing happens of course.
So let's change the module if we see a problem. What should it be?
^ permalink raw reply
* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Babu Moger @ 2026-03-30 18:46 UTC (permalink / raw)
To: Reinette Chatre, corbet, tony.luck, Dave.Martin, james.morse,
tglx, mingo, bp, dave.hansen
Cc: skhan, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <88eebfac-5286-4788-b244-911c659c0439@intel.com>
Hi Reinette,
On 3/27/26 17:11, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/26/26 10:12 AM, Babu Moger wrote:
>> Hi Reinette,
>>
>> Thanks for the review comments. Will address one by one.
>>
>> On 3/24/26 17:51, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 3/12/26 1:36 PM, Babu Moger wrote:
>>>> This series adds support for Privilege-Level Zero Association (PLZA) to the
>>>> resctrl subsystem. PLZA is an AMD feature that allows specifying a CLOSID
>>>> and/or RMID for execution in kernel mode (privilege level zero), so that
>>>> kernel work is not subject to the same resource constrains as the current
>>>> user-space task. This avoids kernel operations being aggressively throttled
>>>> when a task's memory bandwidth is heavily limited.
>>>>
>>>> The feature documentation is not yet publicly available, but it is expected
>>>> to be released in the next few weeks. In the meantime, a brief description
>>>> of the features is provided below.
>>>>
>>>> Privilege Level Zero Association (PLZA)
>>>>
>>>> Privilege Level Zero Association (PLZA) allows the hardware to
>>>> automatically associate execution in Privilege Level Zero (CPL=0) with a
>>>> specific COS (Class of Service) and/or RMID (Resource Monitoring
>>>> Identifier). The QoS feature set already has a mechanism to associate
>>>> execution on each logical processor with an RMID or COS. PLZA allows the
>>>> system to override this per-thread association for a thread that is
>>>> executing with CPL=0.
>>>> ------------------------------------------------------------------------
>>>>
>>>> The series introduces the feature in a way that supports the interface in
>>>> a generic manner to accomodate MPAM or other vendor specific implimentation.
>>>>
>>>> Below is the detailed requirements provided by Reinette:
>>>> https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
>>> Our discussion considered how resctrl could support PLZA in a generic way while
>>> also preparing to support MPAM's variants and how PLZA may evolve to have similar
>>> capabilities when considering the capabilities of its registers.
>>>
>>> This does not mean that your work needs to implement everything that was discussed.
>>> Instead, this work is expected to just support what PLZA is capable of today but
>>> do so in a way that the future enhancements could be added to.
>>>
>>> This series is quite difficult to follow since it appears to implement a full
>>> featured generic interface while PLZA cannot take advantage of it.
>>>
>>> Could you please simplify this work to focus on just enabling PLZA and only
>>> add interfaces needed to do so?
>> Sure. Will try. Lets continue the discussion.
>>>> Summary:
>>>> 1. Kernel-mode/PLZA controls and status should be exposed under the resctrl
>>>> info directory:/sys/fs/resctrl/info/, not as a separate or arch-specific path.
>>>>
>>>> 2. Add two info files
>>>>
>>>> a. kernel_mode
>>>> Purpose: Control how resource allocation and monitoring apply in kernel mode
>>>> (e.g. inherit from task vs global assign).
>>>>
>>>> Read: List supported modes and show current one (e.g. with [brackets]).
>>>> Write: Set current mode by name (e.g. inherit_ctrl_and_mon, global_assign_ctrl_assign_mon).
>>>>
>>>> b. kernel_mode_assignment
>>>>
>>>> Purpose: When a “global assign” kernel mode is active, specify which resctrl group
>>>> (CLOSID/RMID) is used for kernel work.
>>>>
>>>> Read: Show the assigned group in a path-like form (e.g. //, ctrl1//, ctrl1/mon1/).
>>>> Write: Assign or clear the group used for kernel mode (and optionally clear with an empty write).
>>>>
>>>> The patches are based on top of commit (v7.0.0-rc3)
>>>> 839e91ce3f41b (tip/master) Merge branch into tip/master: 'x86/tdx'
>>>> ------------------------------------------------------------------------
>>>>
>>>> Examples: kernel_mode and kernel_mode_assignment
>>>>
>>>> All paths below are under /sys/fs/resctrl/ (e.g. info/kernel_mode means
>>>> /sys/fs/resctrl/info/kernel_mode). Resctrl must be mounted and the platform
>>>> must support the relevant modes (e.g. AMD with PLZA).
>>>>
>>>> 1) kernel_mode — show and set the current kernel mode
>>>>
>>>> Read supported modes and which one is active (current in brackets):
>>>>
>>>> $ cat info/kernel_mode
>>>> [inherit_ctrl_and_mon]
>>>> global_assign_ctrl_inherit_mon
>>>> global_assign_ctrl_assign_mon
>>>>
>>>> Set the active mode (e.g. use one CLOSID+RMID for all kernel work):
>>>>
>>>> $ echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>>>> $ cat info/kernel_mode
>>>> inherit_ctrl_and_mon
>>>> global_assign_ctrl_inherit_mon
>>>> [global_assign_ctrl_assign_mon]
>>>>
>>>> Mode meanings:
>>>> - inherit_ctrl_and_mon: kernel uses same CLOSID/RMID as the current task (default).
>>>> - global_assign_ctrl_inherit_mon: one CLOSID for all kernel work; RMID inherited from user.
>>>> - global_assign_ctrl_assign_mon: one resource group (CLOSID+RMID) for all kernel work.
>>>>
>>>> 2) kernel_mode_assignment — show and set which group is used for kernel work
>>>>
>>>> Only relevant when kernel_mode is not "inherit_ctrl_and_mon". Read the
>>> To help with future usages please connect visibility of this file with the mode in
>>> info/kernel_mode. This helps us to support future modes with other resctrl files, possible
>>> within each resource group.
>>> Specifically, kernel_mode_assignment is not visible to user space if mode is "inherit_ctrl_and_mon",
>>> while it is visible when mode is global_assign_ctrl_inherit_mon or global_assign_ctrl_assign_mon.
>> Sure. Will do.
>>
>>>> currently assigned group (path format is "CTRL_MON/MON/"):
>>> The format depends on the mode, right? If the mode is "global_assign_ctrl_inherit_mon"
>>> then it should only contain a control group, alternatively, if the mode is
>>> "global_assign_ctrl_assign_mon" then it contains control and mon group. This gives
>>> resctrl future flexibility to change format for future modes.
>> This can be done both ways. Whole purpose of these groups is to get CLOSID and RMID to enable PLZA. User can echo CTRL_MON or MON group to kernel_mode_assignment in any of the modes. We can decide what needs to be updated in MSR (PQR_PLZA_ASSOC) based on what kernel mode is selected.
> The "both ways" are specific to one of the two active modes though.
> PLZA only needs the RMID when the mode is "global_assign_ctrl_assign_mon".
>
> Displaying and parsing monitor group when the mode is
> "global_assign_ctrl_inherit_mon" creates an inconsistent interface since the mode
> only uses a control group. The interface to user space should match the mode otherwise
> it becomes confusing.
Ok. That is fine. We can do that.
> ...
>
>
>>>> Tony suggested using global variables to store the kernel mode
>>>> CLOSID and RMID. However, the kernel mode CLOSID and RMID are
>>>> coming from rdtgroup structure with the new interface. Accessing
>>>> them requires holding the associated lock, which would make the
>>>> context switch path unnecessarily expensive. So, dropped the idea.
>>>> https://lore.kernel.org/lkml/aXuxVSbk1GR2ttzF@agluck-desk3/
>>>> Let me know if there are other ways to optimize this.
>>> I do not see why the context switch path needs to be touched at all with this
>>> implementation. Since PLZA only supports global assignment does it not mean that resctrl
>>> only needs to update PQR_PLZA_ASSOC when user writes to info/kernel_mode and
>>> info/kernel_mode_assignment?
>> Each thread has an MSR to configure whether to associate privilege level zero execution with a separate COS and/or RMID, and the value of the COS and/or RMID. PLZA may be enabled or disabled on a per-thread basis. However, the COS and RMID association and configuration must be the same for all threads in the QOS Domain.
> Based on previous comment in https://lore.kernel.org/lkml/abb049fa-3a3d-4601-9ae3-61eeb7fd8fcf@amd.com/
> and this implementation all fields of PQR_PLZA_ASSOC except PQR_PLZA_ASSOC.plza_en must be the
> same for all CPUs on the system, not just per QoS domain. Could you please confirm?
Sorry for the confusion. It is "per QoS domain".
All the fields of PQR_PLZA_ASSOC except PQR_PLZA_ASSOC.plza_enmust be set to the same value for all HW threads in the QOS domain for
consistent operation (Per-QosDomain).
>
>> So, PQR_PLZA_ASSOC is a per thread MSR just like PQR_ASSOC.
>>
>> Privilege-Level Zero Association (PLZA) allows the user to specify a COS and/or RMID associated with execution in Privilege-Level Zero. When enabled on a HW thread, when that thread enters Privilige-Level Zero, transactions associated with that thread will be associated with the PLZA COS and/or RMID. Otherwise, the HW thread will be associated with the COS and RMID identified by PQR_ASSOC.
>>
>> More below.
>>
>>> Consider some of the scenarios:
>>>
>>> resctrl mount with default state:
>>>
>>> # cat info/kernel_mode
>>> [inherit_ctrl_and_mon]
>>> global_assign_ctrl_inherit_mon
>>> global_assign_ctrl_assign_mon
>>> # ls info/kernel_mode_assignment
>>> ls: cannot access 'info/kernel_mode_assignment': No such file or directory
>>>
>>> enable global_assign_ctrl_assign_mon mode:
>>> # echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>>>
>>> Expectation here is that when user space sets this mode as above then resctrl would
>>> in turn program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
>>> MSR_IA32_PQR_PLZA_ASSOC.rmid=0
>>> MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
>>> MSR_IA32_PQR_PLZA_ASSOC.closid=0
>>> MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
>>> MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>>>
>>> I do not see why it is necessary to maintain any per-CPU or per-task state or needing
>>> to touch the context switch code. Since PLZA only supports global could it not
>>> just set MSR_IA32_PQR_PLZA_ASSOC on all online CPUs and be done with it?
>>> Only caveat is that if a CPU is offline then this setting needs to be stashed
>>> so that MSR_IA32_PQR_PLZA_ASSOC can be set when new CPU comes online.
>>>
>>> The way that rdtgroup_config_kmode() introduced in patch #11 assumes it is dealing
>>> with RDT_RESOURCE_L3 and traverses the resource domain list and resource group
>>> CPU mask seems unnecessary to me as well as error prone since the system may only
>>> have, for example, RDT_RESOURCE_MBA enabled or even just monitoring. Why not just set
>>> MSR_IA32_PQR_PLZA_ASSOC on all CPUs and be done?
>>>
>>> To continue the scenarios ...
>>>
>>> After user's setting above related files read:
>>> # cat info/kernel_mode
>>> inherit_ctrl_and_mon
>>> global_assign_ctrl_inherit_mon
>>> [global_assign_ctrl_assign_mon]
>>> # cat info/kernel_mode_assignment
>>> //
>>>
>>> Modify group used by global_assign_ctrl_assign_mon mode:
>>> # echo 'ctrl1/mon1/' > info/kernel_mode_assignment
>>>
>>> Expectation here is that when user space sets this then resctrl would
>>> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
>>> MSR_IA32_PQR_PLZA_ASSOC.rmid=<rmid of mon1>
>>> MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
>>> MSR_IA32_PQR_PLZA_ASSOC.closid=<closid of ctrl1>
>>> MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
>>> MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>>
>> This works correctly when PLZA associations are defined by per CPU. For example, lets assume that *ctrl1* is assigned *CLOSID 1*.
>>
>> In this scenario, every task in the system running on a any CPU will use the limits associated with *CLOSID 1* whenever it enters Privilege-Level Zero, because the CPU's *PQR_PLZA_ASSOC* register has PLZA enabled and CLOSID is 1.
>>
>> Now consider task-based association:
>>
>> We have two resctrl groups:
>>
>> * *ctrl1 -> CLOSID 1 -> task1.plza = 1 : *User wants PLZA be enabled
>> for this task.
>> * *ctrl2 -> CLOSID 2 -> task2.plza = 0 : *User wants PLZA
>> disabled for this task.
>>
>> Suppose *task1* is first scheduled on *CPU 0*. This behaves as expected: since CPU 0 's *PQR_PLZA_ASSOC* contains *CLOSID 1, plza_en =1*, task1 will use the limits from CLOSID 1 when it enters Privilege-Level Zero.
>>
>> However, if *task2* later runs on *CPU 0*, we expect it to use *CLOSID 2* in both user mode and kernel mode, because user has PLZA disabled for this task. But CPU 0 still has *CLOSID 1, **plza_en =1* in its PQR_PLZA_ASSOC register.
>>
>> As a result, task2 will incorrectly run with *CLOSID 1* when entering Privilege-Level Zero something we explicitly want to avoid.
>>
>> At that point, PLZA must be disabled on CPU 0 to prevent the unintended association. Hope this explanation makes the issue clear.
>>
> A couple of points:
> - Looks like we still need to come to agreement what is meant by "global" when it
> comes to kernel mode.
>
> In your description there is a "global" configuration, but the assignment is "per-task".
> To me this sounds like a new and distinct kernel_mode from the "global" modes
> considered so far. This seems to move to the "per_task" mode mentioned in but
> the implementation does not take into account any of the earlier discussions
> surrounding it:
> https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
>
> We only learned about one use case in https://lore.kernel.org/lkml/CABPqkBSq=cgn-am4qorA_VN0vsbpbfDePSi7gubicpROB1=djw@mail.gmail.com/
> As I understand this use case requires PLZA globally enabled for all tasks. Thus
> I consider task assignment to be "global" when in the "global_*" kernel modes.
> If this is indeed a common use case then supporting only global configuration
> but then requiring user space to manually assign all tasks afterwards sounds
> cumbersome for user space and also detrimental to system performance with all
> the churn to modify all the task_structs involved. The accompanying documentation
> does not mention all this additional user space interactions required by user
> space to use this implementation.
>
> I find this implementation difficult and inefficient to use in the one use case
> we know of. I would suggest that resctrl optimizes for the one known use case.
>
> - This implementation ignores discussion on how existing resctrl files should
> not be repurposed.
>
> This implementation allows user space to set a resource group in
> kernel_mode_assignment with the consequence that this resource group's
> "tasks" file changes behavior. I consider this a break of resctrl interface.
> We did briefly consider per-task configuration/assignment in previous discussion
> and the proposal was for it to use a new file (only when and if needed!).
>
> - Now a user is required to write the task id of every task that participates
> in PLZA. Apart from the churn already mentioned this also breaks existing
> usage since it is no longer possible for new tasks to be added to this
> resource group. This creates an awkward interface where all tasks belonging
> to a resource group inherits the allocations/monitoring for their user space
> work and will get PLZA enabled whether user requested it or not while
> tasks from other resource groups need to be explicitly enabled. This creates
> an inconsistency when it comes to task assignment. The only way to "remove"
> PLZA from such a task would be to assign it to another resource group which
> may not have the user space allocations ... and once this is done the task
> cannot be moved back.
> There is no requirement that CLOSID/RMID should be dedicated to kernel work
> but this implementation does so in an inconsistent way.
>
> - Apart from the same issues as with repurposing of tasks file, why should same
> CPU allocation be used for kernel and user space?
>
Yes, I agree with your concerns. The goal here is to make the interface
less disruptive while still addressing the different use cases.
Background: Customers have identified an issue with the QoS
Bandwidth Control feature: when a CLOS is aggressively throttled
and execution transitions into kernel mode, kernel operations are
also subject to the same aggressive throttling.
Privilege-Level Zero Association (PLZA) allows a user to specify a COS
and/or RMID to be used during execution at Privilege Level Zero. When
PLZA is enabled on a hardware thread, any execution that enters
Privilege Level Zero will have its transactions associated with the PLZA
COS and/or RMID. Otherwise, the thread continues to use the COS and RMID
specified by |PQR_ASSOC|. In other words, the hardware provides a
dedicated COS and/or RMID specifically for kernel-mode execution.
There are multiple ways this feature can be applied. For simplicity, the
discussion below focuses only on CLOSID.
1. Global PLZA enablement
PLZA can be configured as a global feature by setting
|PQR_PLZA_ASSOC.closid = CLOSID| and |PQR_PLZA_ASSOC.plza_en = 1| on all
threads in the system. A dedicated CLOSID is reserved for this purpose,
and all CPU threads use its allocations whenever they enter Privilege
Level Zero. This CLOSID does not need to be associated with any resctrl
group. The user can explicitly enable or disable this feature. There is
no context switch overhead but there is no flexibility with this approach.
2. Group based PLZA allocation : PLZA is managed via dedicated
restctrl group. A separate resctrl group can be created
specifically for PLZA, with a dedicated CLOSID used exclusively
for kernel mode execution. This approach can be further divided
into two association models:
i) CPU based association
CPUs are assigned to the PLZA group, and PLZA is enabled only on those
CPUs. This effectively creates a dedicated PLZA group. MSRs
(|PQR_PLZA_ASSOC)| are programmed only when the user changes CPU
assignments. This approach requires no changes to the context switch
code and introduces no additional context switch overhead.
ii) Task based association
Tasks are explicitly assigned by the user to the PLZA group. Tasks need
to be updated when user adds a new task. Also, this requires updates
during task scheduling so that the MSRs (|PQR_PLZA_ASSOC)| are
programmed on each context switch, which introduces additional context
switch overhead.
I tried to fit these requirements into the interface files in
/sys/fs/resctrl/info/. I may have missed few things while trying to
achieve it. As usual, I am open for the discussion and recommendations.
Thanks,
Babu
^ 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-30 15:47 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: Williams, Dan J, linux-pci@vger.kernel.org,
linux-coco@lists.linux.dev, x86@kernel.org, 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: <1dd0b8f4d1ad5cd07b5139ed8c944e1e1d004661.camel@intel.com>
> > Typical usages of the tdx_page_array:
> >
> > 1. Add control pages:
> > - struct tdx_page_array *array = tdx_page_array_create(nr_pages);
> > - seamcall(TDH_XXX_CREATE, array, ...);
> >
> > 2. Release control pages:
> > - seamcall(TDX_XXX_DELETE, array, &nr_released, &released_hpa);
> > - tdx_page_array_ctrl_release(array, nr_released, released_hpa);
>
> So release is mostly needed because of the need to do the wbvind seamcall? And
> unlike tdx_page_array_free() it returns an error in case that fails. Or other
> sanity checking. But all the callers do the same thing on error, call
> tdx_page_array_ctrl_leak().
>
> Just wondering if we could simplify it somehow. There are two helpers and the
> caller has to know which one to call based on SEAMCALL specifics. What if the
> seamcall wrapper set a bit in the page array while passing it out. The bit would
> specify to the helper if it needs to do wbinvd or not. Then the wrappers could
> encapsulate the type of free needed and not rely on the caller to know. And we
> only need to have one function for it instead of two.
I like the idea, I can have a try.
But we may need more than a bit to finish the release. On release some
SEAMCALLs return the released HPA list and host checks they are sane,
otherwise leak. We need to also record these info in the struct.
>
>
> BTW, do we expect errors from the tdh_phymem_page_wbinvd_hkid() calls here? How
No, it can't happen in normal runtime.
> could the BUSY happen? If we don't think it can happen in normal runtime, we
> could just warn and skip the special leak logic. In KVM side there is a place
> where we can't really handle it for the wbinvd calls. And one where we can. If
But we do leak control pages when wbinvd fails, or even when
tdh_phymem_page_reclaim() fails. So anyway we need leak logic, is it?
Is it a little insane if we failed to reclaim and return the private
page to kernel?
> we need a ton of code to handle a bug somewhere (on kernel side or TDX module),
> it seems too defensive to me. At least it's not in sync with the rest of TDX.
>
> Especially the quite large tdx_page_array_validate_release() logic should need a
> justification that there is something very tricky that needs all this checking.
>
> But maybe you can explain what the special risk is.
I don't see the special risk, actually I don't even see the releasing
failed once.
But we do check the return value of tdh_phymem_page_reclaim() for a
single ctrl page releasing. It is just we also check a list of ctrl
pages releasing. The check becomes naturally complex, e.g., if the
released number matches, if every HPA in the list matches ...
...
> > +struct tdx_page_array {
> > + /* public: */
> > + unsigned int nr_pages;
> > + struct page **pages;
> > +
> > + /* private: */
> > + unsigned int offset;
> > + unsigned int nents;
> > + u64 *root;
>
> 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.
Besides, host may need to vmap and access the (shared) pages.
Thanks,
Yilun
^ permalink raw reply
* Re: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Pratik R. Sampat @ 2026-03-30 15:10 UTC (permalink / raw)
To: Edgecombe, Rick P, pbonzini@redhat.com
Cc: x86@kernel.org, dave.hansen@linux.intel.com,
marcandre.lureau@redhat.com, kas@kernel.org, hpa@zytor.com,
linux-kernel@vger.kernel.org, mingo@redhat.com, bp@alien8.de,
Qiang, Chenyi, tglx@kernel.org, linux-coco@lists.linux.dev,
kvm@vger.kernel.org
In-Reply-To: <424048885a01dcb6a7ef0256f0dc8a9adb546f22.camel@intel.com>
On 3/26/26 4:40 PM, Edgecombe, Rick P wrote:
> Hi Paolo!
>
> On Thu, 2026-03-26 at 19:25 +0100, Paolo Bonzini wrote:
>>> Another option could be to perform a TDG.MEM.PAGE.RELEASE TDCALL from
>>> the guest when it unplugs the memory, to put it in an unaccepted state.
>>> This would be more robust to buggy VMM behavior. But working around
>>> buggy VM behavior would need a high bar.
>>
>> Wouldn't it actually be a very low bar? Just from these two paragraphs
>> of yours, it's clear that the line between buggy and malicious is
>> fine, in fact I think userspace should not care at all about removing
>> the memory. Only the guest cares about acceptance state.
>>
>> Doing a RELEASE TDCALL seems more robust and not hard.
>
> I mean I guess the contract is a bit fuzzy. The reason why I was thinking it was
> a host userspace bug is because the conventional bare metal behavior of
> unplugging memory should be that it is no longer accessible, right? If the guest
> could still use the unplugged memory, it could be surprising for userspace and
> the guest. Also, ideally I'd think the behavior wouldn't cover up guest bugs
> where it tried to keep using the memory. So forgetting about TDX, isn't it
> better behavior in general for unplugging memory, to actually pull it from the
> guest? Did I look at that wrong?
>
> As for the bar to change the guest, I was first imagining it would be the size
> of the accept memory plumbing. Which was not a small effort and has had a steady
> stream of bugs to squash where the accept was missed.
>
> But I didn't actually POC anything to check the scope so maybe that was a bit
> hasty. Should we do a POC? But considering the scope, I wonder if SNP has the
> same problem.
SNP likely has an analogous issue too.
Failing to switch states on remove will cause that RMP entry to remain
validated. A malicious hypervisor could then remap this GPA to another HPA
which would put this in the Guest-Invalid state. On re-hotplug if we ignore
errors suggested by Patch 1 (in our case that'd be PVALIDATE_FAIL_NOUPDATE
error likely), we could have two RMP entries for the same GPA and both being
validated. This is dangerous because hypervisor could swap these at will.
Would it not be better to have this information in the unaccepted bitmap which
we could explicitly query to accept/unaccept?
For ACPI hardware-style hotplug I was working with the UEFI side on a POC to
reflect SRAT hotplug windows in UEFI_UNACCEPTED_MEMORY using
EFI_MEMORY_HOT_PLUGGABLE attribute and working to modify that spec. I’m less
sure what this description for virtio-mem would look like and if it'd be
possible to do this early-boot.
Thanks,
--Pratik
^ permalink raw reply
* Re: [PATCH v2 06/31] x86/virt/tdx: Read global metadata for TDX Module Extensions/Connect
From: Nikolay Borisov @ 2026-03-30 14:23 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-7-yilun.xu@linux.intel.com>
On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> Add reading of the global metadata for TDX Module Extensions & TDX
> Connect. Add them in a batch as TDX Connect is currently the only user
> of TDX Module Extensions and no way to initialize TDX Module Extensions
> without firstly enabling TDX Connect.
>
> TDX Module Extensions & TDX Connect are optional features enumerated by
> TDX_FEATURES0. Check the TDX_FEATURES0 before reading these metadata to
> avoid failing the whole TDX initialization.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx.h | 2 ++
> arch/x86/include/asm/tdx_global_metadata.h | 14 ++++++++
> arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 36 +++++++++++++++++++++
> 3 files changed, 52 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index d5f1d7b7d1e7..d7605235aa9b 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -129,7 +129,9 @@ int tdx_enable(void);
> const char *tdx_dump_mce_info(struct mce *m);
>
> /* Bit definitions of TDX_FEATURES0 metadata field */
> +#define TDX_FEATURES0_TDXCONNECT BIT_ULL(6)
> #define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
> +#define TDX_FEATURES0_EXT BIT_ULL(39)
>
> const struct tdx_sys_info *tdx_get_sysinfo(void);
>
> diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
> index 060a2ad744bf..e7948bca671a 100644
> --- a/arch/x86/include/asm/tdx_global_metadata.h
> +++ b/arch/x86/include/asm/tdx_global_metadata.h
> @@ -34,11 +34,25 @@ struct tdx_sys_info_td_conf {
> u64 cpuid_config_values[128][2];
> };
>
> +struct tdx_sys_info_ext {
> + u16 memory_pool_required_pages;
> + u8 ext_required;
> +};
> +
> +struct tdx_sys_info_connect {
> + u16 ide_mt_page_count;
> + u16 spdm_mt_page_count;
> + u16 iommu_mt_page_count;
> + u16 spdm_max_dev_info_pages;
> +};
> +
> struct tdx_sys_info {
> struct tdx_sys_info_features features;
> struct tdx_sys_info_tdmr tdmr;
> struct tdx_sys_info_td_ctrl td_ctrl;
> struct tdx_sys_info_td_conf td_conf;
> + struct tdx_sys_info_ext ext;
> + struct tdx_sys_info_connect connect;
> };
>
> #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> index 13ad2663488b..a07f1e7b18e8 100644
> --- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> +++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
> @@ -85,6 +85,36 @@ static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf
> return ret;
> }
>
> +static int get_tdx_sys_info_ext(struct tdx_sys_info_ext *sysinfo_ext)
> +{
> + int ret = 0;
> + u64 val;
> +
> + if (!ret && !(ret = read_sys_metadata_field(0x3100000100000000, &val)))
nit: This is likely generated by a script/llm because I see no other
explanation why !ret is being checked here...
> + sysinfo_ext->memory_pool_required_pages = val;
> + if (!ret && !(ret = read_sys_metadata_field(0x3100000100000001, &val)))
> + sysinfo_ext->ext_required = val;
> +
> + return ret;
> +}
> +
> +static int get_tdx_sys_info_connect(struct tdx_sys_info_connect *sysinfo_connect)
> +{
> + int ret = 0;
> + u64 val;
> +
> + if (!ret && !(ret = read_sys_metadata_field(0x3000000100000001, &val)))
Ditto
<snip>
^ permalink raw reply
* Re: [PATCH v2 04/31] x86/virt/tdx: Support allocating contiguous pages for tdx_page_array
From: Nikolay Borisov @ 2026-03-30 13:48 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-5-yilun.xu@linux.intel.com>
On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
> The current tdx_page_array implementation allocates scattered order-0
> pages. However, some TDX Module operations benefit from contiguous
> physical memory. E.g. Enabling TDX Module Extensions (an optional TDX
> feature) requires ~50MB memory and never returns. Such allocation
> would at worst cause ~25GB permanently fragmented memory if each
> allocated page is from a different 2M region.
>
> Support allocating contiguous pages for tdx_page_array by making the
> allocation method configurable. Change the tdx_page_array_alloc() to
> accept a custom allocation function pointer and a context parameter.
> Wrap the specific allocation into a tdx_page_array_alloc_contig()
> helper.
>
> The foreseeable caller will allocate ~50MB memory with this helper,
> exceeding the maximum HPAs (512) a root page can hold, the typical usage
> will be:
>
> - struct tdx_page_array *array = tdx_page_array_alloc_contig(nr_pages);
> - for each 512-page bulk
> - tdx_page_array_populate(array, offset);
> - seamcall(TDH_XXX_ADD, array, ...);
>
> The configurable allocation method would also benefit more
> tdx_page_array usages. TDX Module may require more specific memory
> layouts encoded in the root page. Will introduce them in following
> patches.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 42 +++++++++++++++++++++++++++++++++----
> 1 file changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index a3021e7e2490..6c4ed80e8e5a 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -289,7 +289,8 @@ static void tdx_free_pages_bulk(unsigned int nr_pages, struct page **pages)
> __free_page(pages[i]);
> }
>
> -static int tdx_alloc_pages_bulk(unsigned int nr_pages, struct page **pages)
> +static int tdx_alloc_pages_bulk(unsigned int nr_pages, struct page **pages,
> + void *data)
> {
> unsigned int filled, done = 0;
>
> @@ -326,7 +327,10 @@ void tdx_page_array_free(struct tdx_page_array *array)
> EXPORT_SYMBOL_GPL(tdx_page_array_free);
>
> static struct tdx_page_array *
> -tdx_page_array_alloc(unsigned int nr_pages)
> +tdx_page_array_alloc(unsigned int nr_pages,
> + int (*alloc_fn)(unsigned int nr_pages,
> + struct page **pages, void *data),
> + void *data)
This interface seems cumbersome, since you will always have separate
allocation paths:
Contig, Bulk and Iommu mt pages let's just keep them separate. I.e the
flow should be:
1. Do common allocation by calling tdx_page_array_alloc (you aren't
passing the alloc function) you just get a bare-bones tdx_page_array struct
2. Do the specific allocation in either :
tdx_page_array_create - for the bulk case
tdx_page_array_alloc_contig - for the contig case
tdx_page_array_create_iommu_mt - for the iommu case. Here you can open
code tdx_alloc_pages_iommu_mt.
And keep the specific clearly separate in each function.
> {
> struct tdx_page_array *array = NULL;
> struct page **pages = NULL;
> @@ -348,7 +352,7 @@ tdx_page_array_alloc(unsigned int nr_pages)
> if (!pages)
> goto out_free;
>
> - ret = tdx_alloc_pages_bulk(nr_pages, pages);
> + ret = alloc_fn(nr_pages, pages, data);
> if (ret)
> goto out_free;
>
> @@ -388,7 +392,7 @@ struct tdx_page_array *tdx_page_array_create(unsigned int nr_pages)
> if (nr_pages > TDX_PAGE_ARRAY_MAX_NENTS)
> return NULL;
>
> - array = tdx_page_array_alloc(nr_pages);
> + array = tdx_page_array_alloc(nr_pages, tdx_alloc_pages_bulk, NULL);
> if (!array)
> return NULL;
>
> @@ -521,6 +525,36 @@ int tdx_page_array_ctrl_release(struct tdx_page_array *array,
> }
> EXPORT_SYMBOL_GPL(tdx_page_array_ctrl_release);
>
> +static int tdx_alloc_pages_contig(unsigned int nr_pages, struct page **pages,
> + void *data)
> +{
> + struct page *page;
> + int i;
> +
> + page = alloc_contig_pages(nr_pages, GFP_KERNEL, numa_mem_id(),
> + &node_online_map);
> + if (!page)
> + return -ENOMEM;
> +
> + for (i = 0; i < nr_pages; i++)
> + pages[i] = page + i;
> +
> + return 0;
> +}
> +
> +/*
> + * For holding large number of contiguous pages, usually larger than
> + * TDX_PAGE_ARRAY_MAX_NENTS (512).
> + *
> + * 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 *
> +tdx_page_array_alloc_contig(unsigned int nr_pages)
> +{
> + return tdx_page_array_alloc(nr_pages, tdx_alloc_pages_contig, NULL);
> +}
> +
> #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 v2 03/31] x86/virt/tdx: Add tdx_page_array helpers for new TDX Module objects
From: Nikolay Borisov @ 2026-03-30 13:31 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-4-yilun.xu@linux.intel.com>
On 27.03.26 г. 18:01 ч., Xu Yilun wrote:
<snip>
> +/**
> + * tdx_page_array_ctrl_leak() - Leak data pages and free the container
> + * @array: The tdx_page_array to be leaked.
> + *
> + * Call this function when failed to reclaim the control pages. Free the root
> + * page and the holding structures, but orphan the data pages, to prevent the
> + * host from re-allocating and accessing memory that the hardware may still
> + * consider private.
> + */
> +void tdx_page_array_ctrl_leak(struct tdx_page_array *array)
> +{
> + if (!array)
> + return;
> +
> + kfree(array->pages);
> + kfree(array->root);
> + kfree(array);
> +}
> +EXPORT_SYMBOL_GPL(tdx_page_array_ctrl_leak);
This instantly raises a red flag if by design an API has the ability to
simply leak memory. Under what conditions this might be required, can't
we do something to gracefully handle the case when pages cannot be freed
instantly, i.e queued freeing or some such ? Simply leaking them is a
big NO.
> +
> +static bool tdx_page_array_validate_release(struct tdx_page_array *array,
> + unsigned int offset,
This function is only ever called with offset of 0, if it's intended to
be used later then I'd rather see this argument added in an explicit
patch with rationale why it's needed.
> + unsigned int nr_released,
> + u64 released_hpa)
> +{
> + unsigned int nents;
> +
> + if (offset >= array->nr_pages)
> + return false;
> +
> + nents = umin(array->nr_pages - offset, TDX_PAGE_ARRAY_MAX_NENTS);
> +
> + if (nents != nr_released) {
> + pr_err("%s nr_released [%d] doesn't match page array nents [%d]\n",
> + __func__, nr_released, nents);
> + return false;
> + }
> +
> + /*
> + * Unfortunately TDX has multiple page allocation protocols, check the
> + * "singleton" case required for HPA_ARRAY_T.
> + */
> + if (page_to_phys(array->pages[0]) == released_hpa &&
> + array->nr_pages == 1)
> + return true;
> +
> + /* Then check the "non-singleton" case */
> + if (virt_to_phys(array->root) == released_hpa) {
> + u64 *entries = array->root;
> + int i;
> +
> + for (i = 0; i < nents; i++) {
> + struct page *page = array->pages[offset + i];
> + u64 val = page_to_phys(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);
> + return false;
> + }
> + }
> +
> + return true;
> + }
> +
> + pr_err("%s failed to validate, released_hpa [0x%llx], root page hpa [0x%llx], page0 hpa [%#llx], number pages %u\n",
> + __func__, released_hpa, virt_to_phys(array->root),
> + page_to_phys(array->pages[0]), array->nr_pages);
> +
> + return false;
> +}
> +<snip>
^ permalink raw reply
* Re: [PATCH v6 17/22] x86/virt/tdx: Avoid updates during update-sensitive operations
From: Kiryl Shutsemau @ 2026-03-30 13:07 UTC (permalink / raw)
To: Chao Gao
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: <20260326084448.29947-18-chao.gao@intel.com>
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.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v6 13/22] x86/virt/seamldr: Install a new TDX module
From: Kiryl Shutsemau @ 2026-03-30 12:59 UTC (permalink / raw)
To: Chao Gao
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: <20260326084448.29947-14-chao.gao@intel.com>
On Thu, Mar 26, 2026 at 01:44:04AM -0700, Chao Gao wrote:
> Following the shutdown of the existing TDX module, the update process
> continues with installing the new module. P-SEAMLDR provides the
> SEAMLDR.INSTALL SEAMCALL to perform this installation, which must be
> executed on all CPUs.
>
> Implement SEAMLDR.INSTALL and execute it on every CPU.
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v6 10/22] x86/virt/seamldr: Abort updates if errors occurred midway
From: Kiryl Shutsemau @ 2026-03-30 12:52 UTC (permalink / raw)
To: Chao Gao
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: <20260326084448.29947-11-chao.gao@intel.com>
On Thu, Mar 26, 2026 at 01:44:01AM -0700, Chao Gao wrote:
> The TDX module update process has multiple steps, each of which may
> encounter failures.
>
> The current state machine of updates proceeds to the next step regardless
> of errors. But continuing updates when errors occur midway is pointless.
>
> Abort the update by setting a flag to indicate that a CPU has encountered
> an error, forcing all CPUs to exit the execution loop. Note that failing
> CPUs do not acknowledge the current step. This keeps all other CPUs waiting
> in the current step (since advancing to the next step requires all CPUs to
> acknowledge the current step) until they detect the fault flag and exit the
> loop.
>
> 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>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v6 08/22] x86/virt/seamldr: Allocate and populate a module update request
From: Kiryl Shutsemau @ 2026-03-30 12:44 UTC (permalink / raw)
To: Chao Gao
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: <20260326084448.29947-9-chao.gao@intel.com>
On Thu, Mar 26, 2026 at 01:43:59AM -0700, 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 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.
>
> 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. P-SEAMLDR compatibility
> validation (such as 4KB vs 16KB sigstruct support) is left to userspace,
> which must verify the P-SEAMLDR version meets the TDX module's minimum
> requirements.
>
> Signed-off-by: Chao Gao <chao.gao@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>
See nit below.
> +static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
> +{
> + const struct tdx_blob *blob = (const void *)data;
> + int module_size, sig_size;
> + const void *sig, *module;
> +
> + /* Ensure the size is valid otherwise reading any field from the blob may overflow. */
Comment is too long for single line. Make it multi line.
> + if (size <= sizeof(struct tdx_blob) || size <= blob->offset_of_module)
> + return ERR_PTR(-EINVAL);
> +
> + if (blob->version != TDX_BLOB_VERSION_1) {
> + pr_err("unsupported blob version: %x\n", blob->version);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + /* Split the blob into a sigstruct and a module. */
> + sig = blob->data;
> + sig_size = blob->offset_of_module - sizeof(struct tdx_blob);
> + module = data + blob->offset_of_module;
> + module_size = size - blob->offset_of_module;
> +
> + if (sig_size <= 0 || module_size <= 0 || blob->length != size)
> + return ERR_PTR(-EINVAL);
> +
> + return alloc_seamldr_params(module, module_size, sig, sig_size);
> +}
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v6 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Kiryl Shutsemau @ 2026-03-30 12:41 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, kvm, linux-kernel, 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: <20260326084448.29947-7-chao.gao@intel.com>
On Thu, Mar 26, 2026 at 01:43:57AM -0700, Chao Gao wrote:
> TDX module updates require userspace to select the appropriate module
> to load. Expose necessary information to facilitate this decision. Two
> values are needed:
>
> - P-SEAMLDR version: for compatibility checks between TDX module and
> P-SEAMLDR
> - num_remaining_updates: indicates how many updates can be performed
>
> Expose them as tdx-host device attributes. Make seamldr attributes
> visible only when the update feature is supported, as that's their sole
> purpose. Unconditional exposure is also problematic because reading them
> triggers P-SEAMLDR calls that break KVM on CPUs with a specific erratum
> (to be enumerated and handled in a later patch).
>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
One nit is below.
> @@ -46,7 +47,80 @@ static struct attribute *tdx_host_attrs[] = {
> &dev_attr_version.attr,
> NULL,
> };
> -ATTRIBUTE_GROUPS(tdx_host);
> +
> +static const struct attribute_group tdx_host_group = {
> + .attrs = tdx_host_attrs,
> +};
> +
> +static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct seamldr_info info;
> + int ret;
> +
> + ret = seamldr_get_info(&info);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, TDX_VERSION_FMT"\n", info.major_version,
Space after _FMT, please.
> + info.minor_version,
> + info.update_version);
> +}
--
Kiryl Shutsemau / Kirill A. Shutemov
^ 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