* 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
* Re: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Kiryl Shutsemau @ 2026-03-30 12:29 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: pbonzini@redhat.com, x86@kernel.org, dave.hansen@linux.intel.com,
marcandre.lureau@redhat.com, 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 Thu, Mar 26, 2026 at 08:40:06PM +0000, 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.
Doing RELEASE will be required with TDX Connect in the picture.
Otherwise userspace wouldn't be able to pull the memory out of TD.
So, let's do it and drop the first patch.
We can suggest that userspace actually remove the memory, but I don't
think it should be part of the contract. Userspace might have a reasons
to keep the memory around.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH 2/2] x86/tdx: Accept hotplugged memory before online
From: Marc-André Lureau @ 2026-03-30 12:17 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, Chenyi Qiang,
linux-kernel, linux-coco, kvm
In-Reply-To: <acY/rr7g+gJmkXfg@yzhao56-desk.sh.intel.com>
Hi
On Fri, Mar 27, 2026 at 1:09 PM Yan Zhao <yan.y.zhao@intel.com> wrote:
>
> On Tue, Mar 24, 2026 at 07:21:48PM +0400, Marc-André Lureau wrote:
> > In TDX guests, hotplugged memory (e.g., via virtio-mem) is never
> > accepted before use. The first access triggers a fatal "SEPT entry in
> > PENDING state" EPT violation and KVM terminates the guest.
> >
> > Fix this by registering a MEM_GOING_ONLINE memory hotplug notifier that
> > calls tdx_accept_memory() for the range being onlined.
> >
> > The notifier returns NOTIFY_BAD on acceptance failure, preventing the
> > memory from going online.
> >
> > Assisted-by: Claude:claude-opus-4-6
> > Reported-by: Chenyi Qiang <chenyi.qiang@intel.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > arch/x86/coco/tdx/tdx.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> > index 7b2833705d475..89f90bc303258 100644
> > --- a/arch/x86/coco/tdx/tdx.c
> > +++ b/arch/x86/coco/tdx/tdx.c
> > @@ -8,6 +8,7 @@
> > #include <linux/export.h>
> > #include <linux/io.h>
> > #include <linux/kexec.h>
> > +#include <linux/memory.h>
> > #include <asm/coco.h>
> > #include <asm/tdx.h>
> > #include <asm/vmx.h>
> > @@ -1194,3 +1195,40 @@ void __init tdx_early_init(void)
> >
> > tdx_announce();
> > }
> > +
> > +#ifdef CONFIG_MEMORY_HOTPLUG
> > +static int tdx_guest_memory_notifier(struct notifier_block *nb,
> > + unsigned long action, void *v)
> > +{
> > + struct memory_notify *mn = v;
> > + phys_addr_t start, end;
> > +
> > + if (action != MEM_GOING_ONLINE)
> > + return NOTIFY_OK;
> > +
> > + start = PFN_PHYS(mn->start_pfn);
> > + end = start + PFN_PHYS(mn->nr_pages);
> > +
> > + if (!tdx_accept_memory(start, end)) {
> > + pr_err("Failed to accept memory [0x%llx, 0x%llx)\n",
> > + (unsigned long long)start,
> > + (unsigned long long)end);
> > + return NOTIFY_BAD;
> > + }
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block tdx_guest_memory_nb = {
> > + .notifier_call = tdx_guest_memory_notifier,
> > +};
> > +
> > +static int __init tdx_guest_memory_init(void)
> > +{
> > + if (!cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> > + return 0;
> > +
> > + return register_memory_notifier(&tdx_guest_memory_nb);
> > +}
> If I read the code correctly,
>
> online_pages
> 1. memory_notify(MEM_GOING_ONLINE, &mem_arg);
> 2. online_pages_range(pfn, nr_pages);
> (*online_page_callback)(page, order);
> generic_online_page
> __free_pages_core(page, order, MEMINIT_HOTPLUG);
>
> In __free_pages_core(), there's accept_memory() already:
>
> if (page_contains_unaccepted(page, order)) {
> if (order == MAX_PAGE_ORDER && __free_unaccepted(page))
> return;
>
> accept_memory(page_to_phys(page), PAGE_SIZE << order);
> }
>
> __free_unaccepted() also adds the pages to the unaccepted_pages list, so
> cond_accept_memory() will accept the memory later:
>
> So, is it because the virtio mem sets online_page_callback to
> virtio_mem_online_page_cb, which doesn't invoke __free_pages_core() properly?
>
> Or am I missing something that makes the memory notifier approach necessary?
virtio-mem doesn't modify efi_unaccepted_memory bitmap (populated by
TDVF code when the VM is started)
^ permalink raw reply
* Re: [PATCH v2 5/5] x86/virt/tdx: Remove kexec docs
From: Kiryl Shutsemau @ 2026-03-30 12:04 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Rick Edgecombe, Sean Christopherson,
Paolo Bonzini, linux-kernel, linux-coco, kvm
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-5-87a36409e051@intel.com>
On Mon, Mar 23, 2026 at 02:59:08PM -0600, Vishal Verma wrote:
> From: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
> Recent changes have removed the hard limitations for using kexec and
> TDX together. So remove the section in the TDX docs.
>
> Users on partial write erratums will need an updated TDX module to
> handle the rare edge cases. The docs do not currently provide any
> guidance on recommended TDX module versions, so don't keep a whole
> section around to document this interaction.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v2 4/5] x86/tdx: Disable the TDX module during kexec and kdump
From: Kiryl Shutsemau @ 2026-03-30 12:03 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Rick Edgecombe, Sean Christopherson,
Paolo Bonzini, linux-kernel, linux-coco, kvm
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-4-87a36409e051@intel.com>
On Mon, Mar 23, 2026 at 02:59:07PM -0600, Vishal Verma wrote:
> Use the TDH.SYS.DISABLE SEAMCALL, which disables the TDX module,
> reclaims all memory resources assigned to TDX, and clears any
> partial-write induced poison, to allow kexec and kdump on platforms with
> the partial write errata.
>
> On TDX-capable platforms with the partial write erratum, kexec has been
> disabled because the new kernel could hit a machine check reading a
> previously poisoned memory location.
>
> Later TDX modules support TDH.SYS.DISABLE, which disables the module and
> reclaims all TDX memory resources, allowing the new kernel to re-initialize
> TDX from scratch. This operation also clears the old memory, cleaning up
> any poison.
>
> Add tdx_sys_disable() to tdx_shutdown(), which is called in the
> syscore_shutdown path for kexec. This is done just before tdx_shutdown()
> disables VMX on all CPUs.
>
> For kdump, call tdx_sys_disable() in the crash path before
> x86_virt_emergency_disable_virtualization_cpu() does VMXOFF.
>
> Since this clears any poison on TDX-managed memory, remove the
> X86_BUG_TDX_PW_MCE check in machine_kexec() that blocked kexec on
> partial write errata platforms.
>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v2 3/5] x86/virt/tdx: Add SEAMCALL wrapper for TDH.SYS.DISABLE
From: Kiryl Shutsemau @ 2026-03-30 11:58 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Rick Edgecombe, Sean Christopherson,
Paolo Bonzini, linux-kernel, linux-coco, kvm
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-3-87a36409e051@intel.com>
On Mon, Mar 23, 2026 at 02:59:06PM -0600, Vishal Verma wrote:
> Some early TDX-capable platforms have an erratum where a partial write
> to TDX private memory can cause a machine check on a subsequent read.
> On these platforms, kexec and kdump have been disabled in these cases,
> because the old kernel cannot safely hand off TDX state to the new
> kernel. Later TDX modules support the TDH.SYS.DISABLE SEAMCALL, which
> provides a way to cleanly disable TDX and allow kexec to proceed.
>
> The new SEAMCALL has an enumeration bit, but that is ignored. It is
> expected that users will be using the latest TDX module, and the failure
> mode for running the missing SEAMCALL on an older module is not fatal.
>
> This can be a long running operation, and the time needed largely
> depends on the amount of memory that has been allocated to TDs. If all
> TDs have been destroyed prior to the sys_disable call, then it is fast,
> with only needing to override the TDX module memory.
>
> After the SEAMCALL completes, the TDX module is disabled and all memory
> resources allocated to TDX are freed and reset. The next kernel can then
> re-initialize the TDX module from scratch via the normal TDX bring-up
> sequence.
>
> The SEAMCALL can return two different error codes that expect a retry.
> - TDX_INTERRUPTED_RESUMABLE can be returned in the case of a host
> interrupt. However, it will not return until it makes some forward
> progress, so we can expect to complete even in the case of interrupt
> storms.
> - TDX_SYS_BUSY will be returned on contention with other TDH.SYS.*
> SEAMCALLs, however a side effect of TDH.SYS.DISABLE is that it will
> block other SEAMCALLs once it gets going. So this contention will be
> short lived.
>
> So loop infinitely on either of these error codes, until success or other
> error.
>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
> arch/x86/include/asm/shared/tdx_errno.h | 1 +
> arch/x86/include/asm/tdx.h | 3 +++
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 28 ++++++++++++++++++++++++++++
> 4 files changed, 33 insertions(+)
>
> diff --git a/arch/x86/include/asm/shared/tdx_errno.h b/arch/x86/include/asm/shared/tdx_errno.h
> index 8bf6765cf082..246b4fd54a48 100644
> --- a/arch/x86/include/asm/shared/tdx_errno.h
> +++ b/arch/x86/include/asm/shared/tdx_errno.h
> @@ -15,6 +15,7 @@
> #define TDX_NON_RECOVERABLE_TD_NON_ACCESSIBLE 0x6000000500000000ULL
> #define TDX_NON_RECOVERABLE_TD_WRONG_APIC_MODE 0x6000000700000000ULL
> #define TDX_INTERRUPTED_RESUMABLE 0x8000000300000000ULL
> +#define TDX_SYS_BUSY 0x8000020200000000ULL
> #define TDX_OPERAND_INVALID 0xC000010000000000ULL
> #define TDX_OPERAND_BUSY 0x8000020000000000ULL
> #define TDX_PREVIOUS_TLB_EPOCH_BUSY 0x8000020100000000ULL
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 7674fc530090..a0a4a15142fc 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -172,6 +172,8 @@ static inline int pg_level_to_tdx_sept_level(enum pg_level level)
> return level - 1;
> }
>
> +void tdx_sys_disable(void);
> +
> u64 tdh_vp_enter(struct tdx_vp *vp, struct tdx_module_args *args);
> u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page);
> u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, struct page *page, struct page *source, u64 *ext_err1, u64 *ext_err2);
> @@ -203,6 +205,7 @@ static inline void tdx_init(void) { }
> static inline u32 tdx_get_nr_guest_keyids(void) { return 0; }
> static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; }
> static inline const struct tdx_sys_info *tdx_get_sysinfo(void) { return NULL; }
> +static inline void tdx_sys_disable(void) { }
> #endif /* CONFIG_INTEL_TDX_HOST */
>
> #endif /* !__ASSEMBLER__ */
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index dde219c823b4..e2cf2dd48755 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -46,6 +46,7 @@
> #define TDH_PHYMEM_PAGE_WBINVD 41
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG 45
> +#define TDH_SYS_DISABLE 69
>
> /*
> * SEAMCALL leaf:
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 0802d0fd18a4..3a76000dec7a 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -37,6 +37,7 @@
> #include <asm/msr.h>
> #include <asm/cpufeature.h>
> #include <asm/tdx.h>
> +#include <asm/shared/tdx_errno.h>
> #include <asm/cpu_device_id.h>
> #include <asm/processor.h>
> #include <asm/mce.h>
> @@ -1940,3 +1941,30 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
> return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
> +
> +void tdx_sys_disable(void)
> +{
> + struct tdx_module_args args = {};
> + u64 ret;
> +
> + /*
> + * Don't loop forever.
Nit: Add a new line here.
> + * - TDX_INTERRUPTED_RESUMABLE guarantees forward progress between
> + * calls.
And here.
> + * - 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.
I assumed that if the SEAMCALL fails other SEAMCALLs suppose to be
functional. Hm?
> + *
> + * This is a 'destructive' SEAMCALL, in that no other SEAMCALL can be
> + * run after this until a full reinitialization is done.
> + */
> + do {
> + ret = seamcall(TDH_SYS_DISABLE, &args);
> + } while (ret == TDX_INTERRUPTED_RESUMABLE || ret == TDX_SYS_BUSY);
> +
> + /*
> + * Print SEAMCALL failures, but not SW-defined error codes
> + * (SEAMCALL faulted with #GP/#UD, TDX not supported).
> + */
> + if (ret && (ret & TDX_SW_ERROR) != TDX_SW_ERROR)
> + pr_err("TDH.SYS.DISABLE failed: 0x%016llx\n", ret);
> +}
>
> --
> 2.53.0
>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Jason Gunthorpe @ 2026-03-30 11:49 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Xu Yilun, Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci,
gregkh, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <d5059386-1ed1-4110-8283-16e3a8407760@amd.com>
On Mon, Mar 30, 2026 at 04:47:44PM +1100, Alexey Kardashevskiy wrote:
> What do I miss? Thanks,
You can't tell where things start so there is no way to relate the
offsets to something the kernel can understand.
Jason
^ permalink raw reply
* Re: [PATCH v2 2/5] x86/virt/tdx: Pull kexec cache flush logic into arch/x86
From: Kiryl Shutsemau @ 2026-03-30 11:42 UTC (permalink / raw)
To: Vishal Verma
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Rick Edgecombe, Sean Christopherson,
Paolo Bonzini, linux-kernel, linux-coco, kvm, Kai Huang
In-Reply-To: <20260323-fuller_tdx_kexec_support-v2-2-87a36409e051@intel.com>
On Mon, Mar 23, 2026 at 02:59:05PM -0600, Vishal Verma wrote:
> From: Rick Edgecombe <rick.p.edgecombe@intel.com>
>
> KVM tries to take care of some required cache flushing earlier in the
> kexec path in order to be kind to some long standing races that can occur
> later in the operation. Until recently, VMXOFF was handled within KVM.
> Since VMX being enabled is required to make a SEAMCALL, it had the best
> per-cpu scoped operation to plug the flushing into. So it is kicked off
> from there.
>
> This early kexec cache flushing in KVM happens via a syscore shutdown
> callback. Now that VMX enablement control has moved to arch/x86, which has
> grown its own syscore shutdown callback, it no longer make sense for it to
> live in KVM. It fits better with the TDX enablement managing code.
>
> In addition, future changes will add a SEAMCALL that happens immediately
> before VMXOFF, which means the cache flush in KVM will be too late to
> flush the cache before the last SEAMCALL. So move it to the newly added TDX
> arch/x86 syscore shutdown handler.
>
> Since tdx_cpu_flush_cache_for_kexec() is no longer needed by KVM, make it
> static and remove the export. Since it is also not part of an operation
> spread across disparate components, remove the redundant comments and
> verbose naming.
>
> In the existing KVM based code, CPU offline also funnels through
> tdx_cpu_flush_cache_for_kexec(). So the centralization to the arch/x86
> syscore shutdown callback elides this CPU offline time behavior. However,
> WBINVD is already generally done at CPU offline as matter of course. So
> don't bother adding TDX specific logic for this, and rely on the normal
> WBINVD to handle it.
>
> Acked-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
Acked-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v13 34/48] arm64: RMI: support RSI_HOST_CALL
From: Suzuki K Poulose @ 2026-03-30 10:58 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Joey Gouly, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, 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-35-steven.price@arm.com>
On 18/03/2026 15:53, Steven Price wrote:
> From: Joey Gouly <joey.gouly@arm.com>
>
> Forward RSI_HOST_CALLS to KVM's HVC handler.
Minor nit: Please could we add a line or two, explaining what
RSI_HOST_CALL is ? e.g.:
Realm's can talk to the hypervisor using the RSI_HOST_CALL, which
the RMM forwards to the KVM. Handle them as regular hypercalls.
Suzuki
>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> ---
> Changes since v7:
> * Avoid turning a negative return from kvm_smccc_call_handler() into a
> error response to the guest. Instead propogate the error back to user
> space.
> Changes since v4:
> * Setting GPRS is now done by kvm_rec_enter() rather than
> rec_exit_host_call() (see previous patch - arm64: RME: Handle realm
> enter/exit). This fixes a bug where the registers set by user space
> were being ignored.
> ---
> arch/arm64/kvm/rmi-exit.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/kvm/rmi-exit.c b/arch/arm64/kvm/rmi-exit.c
> index b4843f094615..7eff6967530c 100644
> --- a/arch/arm64/kvm/rmi-exit.c
> +++ b/arch/arm64/kvm/rmi-exit.c
> @@ -116,6 +116,19 @@ static int rec_exit_ripas_change(struct kvm_vcpu *vcpu)
> return -EFAULT;
> }
>
> +static int rec_exit_host_call(struct kvm_vcpu *vcpu)
> +{
> + int i;
> + struct realm_rec *rec = &vcpu->arch.rec;
> +
> + vcpu->stat.hvc_exit_stat++;
> +
> + for (i = 0; i < REC_RUN_GPRS; i++)
> + vcpu_set_reg(vcpu, i, rec->run->exit.gprs[i]);
> +
> + return kvm_smccc_call_handler(vcpu);
> +}
> +
> static void update_arch_timer_irq_lines(struct kvm_vcpu *vcpu)
> {
> struct realm_rec *rec = &vcpu->arch.rec;
> @@ -183,6 +196,8 @@ int handle_rec_exit(struct kvm_vcpu *vcpu, int rec_run_ret)
> return rec_exit_psci(vcpu);
> case RMI_EXIT_RIPAS_CHANGE:
> return rec_exit_ripas_change(vcpu);
> + case RMI_EXIT_HOST_CALL:
> + return rec_exit_host_call(vcpu);
> }
Probably we should move the RMI_EXIT_HOST_CALL case addition in
kvm_rec_pre_enter() to this hunk to keep all in one place ?
Otherwise, looks good to me.
Suzuki
>
> kvm_pr_unimpl("Unsupported exit reason: %u\n",
^ permalink raw reply
* Re: [PATCH v13 32/48] arm64: Don't expose stolen time for realm guests
From: Suzuki K Poulose @ 2026-03-30 10:52 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Zenghui Yu, linux-arm-kernel, linux-kernel,
Joey Gouly, Alexandru Elisei, Christoffer Dall, Fuad Tabba,
linux-coco, Ganapatrao Kulkarni, Gavin Shan, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve
In-Reply-To: <20260318155413.793430-33-steven.price@arm.com>
On 18/03/2026 15:53, Steven Price wrote:
> It doesn't make much sense as a realm guest wouldn't want to trust the
> host. It will also need some extra work to ensure that KVM will only
> attempt to write into a shared memory region. So for now just disable
> it.
>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v7:
> * Update the documentation to add a note about stolen time being
> unavailable in a realm.
> ---
> Documentation/virt/kvm/api.rst | 3 +++
> arch/arm64/kvm/arm.c | 5 ++++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index bc180c853faf..70911fe6d435 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -9240,6 +9240,9 @@ is supported, than the other should as well and vice versa. For arm64
> see Documentation/virt/kvm/devices/vcpu.rst "KVM_ARM_VCPU_PVTIME_CTRL".
> For x86 see Documentation/virt/kvm/x86/msr.rst "MSR_KVM_STEAL_TIME".
>
> +Note that steal time accounting is not available when a guest is running
> +within a Arm CCA realm (machine type KVM_VM_TYPE_ARM_REALM).
> +
> 8.25 KVM_CAP_S390_DIAG318
> -------------------------
>
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 61182eb0cf70..7d92ddb06460 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -469,7 +469,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> r = system_supports_mte();
> break;
> case KVM_CAP_STEAL_TIME:
> - r = kvm_arm_pvtime_supported();
> + if (kvm_is_realm(kvm))
> + r = 0;
> + else
> + r = kvm_arm_pvtime_supported();
Could this be handled in kvm_realm_ext_allowed() ?
Suzuki
> break;
> case KVM_CAP_ARM_EL1_32BIT:
> r = cpus_have_final_cap(ARM64_HAS_32BIT_EL1);
^ permalink raw reply
* Re: [PATCH v13 31/48] KVM: arm64: WARN on injected undef exceptions
From: Suzuki K Poulose @ 2026-03-30 10:50 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Zenghui Yu, linux-arm-kernel, linux-kernel,
Joey Gouly, Alexandru Elisei, Christoffer Dall, Fuad Tabba,
linux-coco, Ganapatrao Kulkarni, Gavin Shan, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve
In-Reply-To: <20260318155413.793430-32-steven.price@arm.com>
On 18/03/2026 15:53, Steven Price wrote:
> The RMM doesn't allow injection of a undefined exception into a realm
> guest. Add a WARN to catch if this ever happens.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> Reviewed-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v6:
> * if (x) WARN(1, ...) makes no sense, just WARN(x, ...)!
> ---
> arch/arm64/kvm/inject_fault.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index 6492397b73d7..613f223bc7a3 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -327,6 +327,7 @@ void kvm_inject_size_fault(struct kvm_vcpu *vcpu)
> */
> void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> {
> + WARN(vcpu_is_rec(vcpu), "Unexpected undefined exception injection to REC");
> if (vcpu_el1_is_32bit(vcpu))
> inject_undef32(vcpu);
> else
^ 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 10:25 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>
> 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/
> 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.
>
> >
> > 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.
>
> >
> > 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.
Please allow me to stop here, will continue later...
Thanks.
^ permalink raw reply
* Re: [PATCH v13 30/48] KVM: arm64: Handle Realm PSCI requests
From: Suzuki K Poulose @ 2026-03-30 10:36 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Zenghui Yu, linux-arm-kernel, linux-kernel,
Joey Gouly, Alexandru Elisei, Christoffer Dall, Fuad Tabba,
linux-coco, Ganapatrao Kulkarni, Gavin Shan, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve
In-Reply-To: <20260318155413.793430-31-steven.price@arm.com>
On 18/03/2026 15:53, Steven Price wrote:
> The RMM needs to be informed of the target REC when a PSCI call is made
> with an MPIDR argument. Expose an ioctl to the userspace in case the PSCI
> is handled by it.
>
> [NOTE: A future version of the RMM specification is likely to remove the
> need for this ioctl.]
This will need to stay for the PSCI_CPU_ON case, where the host has to
acknowledge the onlining of a vCPU.
>
> Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
For the record, we can drop the UAPI following our discussions and
implicitly do the PSCI complete before REC_ENTER, similar to what
we do for the SET_RIPAS request. The VMM/KVM can treat the case
as a normal PSCI_CPU_ON request and return the result as in normal
VMs.
The target REC may ENTER before we complete the reporting, but we can
handle this error case (RMI_ERROR_REC) and return -EAGAIN to the
userspace.
Suzuki
> ---
> Changes since v12:
> * Chance return code for non-realms to -ENXIO to better represent that
> the ioctl is invalid for non-realms (checkpatch is insistent that
> "ENOSYS means 'invalid syscall nr' and nothing else").
> Changes since v11:
> * RMM->RMI renaming.
> Changes since v6:
> * Use vcpu_is_rec() rather than kvm_is_realm(vcpu->kvm).
> * Minor renaming/formatting fixes.
> ---
> arch/arm64/include/asm/kvm_rmi.h | 3 +++
> arch/arm64/kvm/arm.c | 25 +++++++++++++++++++++++++
> arch/arm64/kvm/psci.c | 30 ++++++++++++++++++++++++++++++
> arch/arm64/kvm/rmi.c | 14 ++++++++++++++
> 4 files changed, 72 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_rmi.h b/arch/arm64/include/asm/kvm_rmi.h
> index 38208be3c602..1ee5ed0f5ab2 100644
> --- a/arch/arm64/include/asm/kvm_rmi.h
> +++ b/arch/arm64/include/asm/kvm_rmi.h
> @@ -117,6 +117,9 @@ int realm_map_non_secure(struct realm *realm,
> unsigned long size,
> enum kvm_pgtable_prot prot,
> struct kvm_mmu_memory_cache *memcache);
> +int realm_psci_complete(struct kvm_vcpu *source,
> + struct kvm_vcpu *target,
> + unsigned long status);
>
> static inline bool kvm_realm_is_private_address(struct realm *realm,
> unsigned long addr)
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 304fb1f2b3ff..61182eb0cf70 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1846,6 +1846,22 @@ static int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> return __kvm_arm_vcpu_set_events(vcpu, events);
> }
>
> +static int kvm_arm_vcpu_rmi_psci_complete(struct kvm_vcpu *vcpu,
> + struct kvm_arm_rmi_psci_complete *arg)
> +{
> + struct kvm_vcpu *target = kvm_mpidr_to_vcpu(vcpu->kvm, arg->target_mpidr);
> +
> + if (!target)
> + return -EINVAL;
> +
> + /*
> + * RMM v1.0 only supports PSCI_RET_SUCCESS or PSCI_RET_DENIED
> + * for the status. But, let us leave it to the RMM to filter
> + * for making this future proof.
> + */
> + return realm_psci_complete(vcpu, target, arg->psci_status);
> +}
> +
> long kvm_arch_vcpu_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -1974,6 +1990,15 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>
> return kvm_arm_vcpu_finalize(vcpu, what);
> }
> + case KVM_ARM_VCPU_RMI_PSCI_COMPLETE: {
> + struct kvm_arm_rmi_psci_complete req;
> +
> + if (!vcpu_is_rec(vcpu))
> + return -ENXIO;
> + if (copy_from_user(&req, argp, sizeof(req)))
> + return -EFAULT;
> + return kvm_arm_vcpu_rmi_psci_complete(vcpu, &req);
> + }
> default:
> r = -EINVAL;
> }
> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index 3b5dbe9a0a0e..a68f3c1878a5 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -103,6 +103,12 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>
> reset_state->reset = true;
> kvm_make_request(KVM_REQ_VCPU_RESET, vcpu);
> + /*
> + * Make sure we issue PSCI_COMPLETE before the VCPU can be
> + * scheduled.
> + */
> + if (vcpu_is_rec(vcpu))
> + realm_psci_complete(source_vcpu, vcpu, PSCI_RET_SUCCESS);
>
> /*
> * Make sure the reset request is observed if the RUNNABLE mp_state is
> @@ -115,6 +121,11 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>
> out_unlock:
> spin_unlock(&vcpu->arch.mp_state_lock);
> + if (vcpu_is_rec(vcpu) && ret != PSCI_RET_SUCCESS) {
> + realm_psci_complete(source_vcpu, vcpu,
> + ret == PSCI_RET_ALREADY_ON ?
> + PSCI_RET_SUCCESS : PSCI_RET_DENIED);
> + }
> return ret;
> }
>
> @@ -142,6 +153,25 @@ static unsigned long kvm_psci_vcpu_affinity_info(struct kvm_vcpu *vcpu)
> /* Ignore other bits of target affinity */
> target_affinity &= target_affinity_mask;
>
> + if (vcpu_is_rec(vcpu)) {
> + struct kvm_vcpu *target_vcpu;
> +
> + /* RMM supports only zero affinity level */
> + if (lowest_affinity_level != 0)
> + return PSCI_RET_INVALID_PARAMS;
> +
> + target_vcpu = kvm_mpidr_to_vcpu(kvm, target_affinity);
> + if (!target_vcpu)
> + return PSCI_RET_INVALID_PARAMS;
> +
> + /*
> + * Provide the references of the source and target RECs to the
> + * RMM so that the RMM can complete the PSCI request.
> + */
> + realm_psci_complete(vcpu, target_vcpu, PSCI_RET_SUCCESS);
> + return PSCI_RET_SUCCESS;
> + }
> +
> /*
> * If one or more VCPU matching target affinity are running
> * then ON else OFF
> diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
> index 30292814b1ec..e56c8af2ad61 100644
> --- a/arch/arm64/kvm/rmi.c
> +++ b/arch/arm64/kvm/rmi.c
> @@ -353,6 +353,20 @@ static void free_rtt(phys_addr_t phys)
> kvm_account_pgtable_pages(phys_to_virt(phys), -1);
> }
>
> +int realm_psci_complete(struct kvm_vcpu *source, struct kvm_vcpu *target,
> + unsigned long status)
> +{
> + int ret;
> +
> + ret = rmi_psci_complete(virt_to_phys(source->arch.rec.rec_page),
> + virt_to_phys(target->arch.rec.rec_page),
> + status);
> + if (ret)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int realm_rtt_create(struct realm *realm,
> unsigned long addr,
> int level,
^ permalink raw reply
* Re: [PATCH v2 02/31] x86/virt/tdx: Move bit definitions of TDX_FEATURES0 to public header
From: Xu Yilun @ 2026-03-30 8:07 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: <4228c004ebd59f9737fb39c6c9815bf6e7711acc.camel@intel.com>
On Fri, Mar 27, 2026 at 11:45:39PM +0000, Edgecombe, Rick P wrote:
> On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > Move bit definitions of TDX_FEATURES0 to TDX core public header.
>
> Patch seems reasonable, but not sure about the "public header" language. Maybe
> more widely accessibly header? Or a better name?
>
Yes, I try to make it more explicit, is it better:
x86/virt/tdx: Move TDX_FEATURES0 bit defines to arch x86 header
Move TDX_FEATURES0 bit definitions to arch x86 header.
...
^ permalink raw reply
* Re: [PATCH v2 01/31] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Xu Yilun @ 2026-03-30 7:10 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: <a78f0ac35b137eed6306dd7ad8db37dd9d930aac.camel@intel.com>
> I think it is missing Kai's RB from sys disable v1, and your sign off.
Ah, yes. I picked from the latest post:
https://lore.kernel.org/all/20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com/
But didn't realize the older thread has different RB set and b4 didn't
catch, will add it manually.
I'll also add my sign off.
Thanks
^ permalink raw reply
* Re: [PATCH v2 01/31] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Xu Yilun @ 2026-03-30 7:07 UTC (permalink / raw)
To: Dan Williams
Cc: Edgecombe, Rick P, 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: <69c72bdec3ce8_178904100bf@dwillia2-mobl4.notmuch>
> In this case, for the tsm.git#staging branch, I will replace this with a
> fresh application of this:
>
> https://lore.kernel.org/all/20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com/
This is the exact patch I picked, but Kai's RB is in previous version
https://lore.kernel.org/all/20260307010358.819645-2-rick.p.edgecombe@intel.com/
and b4 didn't catch it. I think should add his RB manually.
>
> Yilun, going forward, if you borrow a patch from another set, be sure to
> both add your own signed-off-by, but also a:
>
> Link: https://patch.msgid.link/20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com
>
> ...to make it extra clear you are including a patch that is already on
> the list in another set.
Yes. I listed the link in cover letter, but yes add it here should be
clearer and necessary.
>
> I suspect that by the time this set is ready to move from
> tsm.git#staging to tsm.git#next a stable commit-id may be available for
> a rebase.
^ permalink raw reply
* Re: [PATCH v2 09/19] PCI/TSM: Support creating encrypted MMIO descriptors via TDISP Report
From: Alexey Kardashevskiy @ 2026-03-30 5:47 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Xu Yilun, Aneesh Kumar K.V, Dan Williams, linux-coco, linux-pci,
gregkh, bhelgaas, alistair23, lukas, Arnd Bergmann
In-Reply-To: <20260327114921.GL67624@nvidia.com>
On 27/3/26 22:49, Jason Gunthorpe wrote:
> On Fri, Mar 27, 2026 at 10:38:15AM +1100, Alexey Kardashevskiy wrote:
>>
>>
>> On 24/3/26 05:20, Jason Gunthorpe wrote:
>>> On Mon, Mar 16, 2026 at 04:19:30PM +1100, Alexey Kardashevskiy wrote:
>>>
>>>> and btw this only works if the entity generating the MMIO reporting
>>>> offset (==TSM) knows about BARs sizes, which is not the case for AMD
>>>> - the FW has no access to the config space (so the HV needs to feed
>>>> this to the FW? may be). Thanks,
>>>
>>> Then your platform just shouldn't use the mmio offset feature. Set it
>>> to 0 always.
>>
>> pcie r7, Table 11-16 TDI Report Structure, MMIO_RANGE:
>>
>> "Each MMIO Range of the TDI is reported with the MMIO reporting offset added."
>>
>> My english struggles here - can the above be interpreted as "Each reported MMIO Range ..."?
>>
>> as if it is each (except msix), then I know where msix is and can
>> amend the report inside the VM if msix is not locked. Thanks,
>
> To do this you must be convert between the offset'd and phys_addr_t
> versions otherwise you have no idea where the translated ones fall
> within the BAR, so you can't figure out if msix is covered or not.
I know if MSIX is covered because I know (from the PSP) if it is locked so it must be reported, with specific MSIX/PBA flags. If it is not locked, then skipped in the report but I still know where it is. For other ranges, if the device is not skipping them randomly, then, with a preserved order (as PCIe mandates), it can be reconstructed.
PCIe:
===
MMIO ranges assigned via BAR(s) must be reported in ascending order starting with the lowest numbered BAR such that
the first range corresponds to the first BAR and so on. The range ID reports the BAR equivalent Indicator (BEI). Values 0-7
of the Range ID are reserved to indicate the BEI. The device must report the BAR equivalent Indicator (BEI) for ranges
associated with a PCIe BAR.
When reporting the MMIO range for a TDI, the MMIO ranges must be reported in the logical order in which the TDI MMIO
range is configured such that the first range reported corresponds to first range of pages in the TDI and so on.
===
What do I miss? Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
From: kernel test robot @ 2026-03-28 19:58 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: oe-kbuild-all, chao.gao, dave.jiang, baolu.lu, yilun.xu, yilun.xu,
zhenzhong.duan, kvm, rick.p.edgecombe, dave.hansen, kas,
xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <20260327160132.2946114-20-yilun.xu@linux.intel.com>
Hi Xu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 11439c4635edd669ae435eec308f4ab8a0804808]
url: https://github.com/intel-lab-lkp/linux/commits/Xu-Yilun/x86-tdx-Move-all-TDX-error-defines-into-asm-shared-tdx_errno-h/20260328-151524
base: 11439c4635edd669ae435eec308f4ab8a0804808
patch link: https://lore.kernel.org/r/20260327160132.2946114-20-yilun.xu%40linux.intel.com
patch subject: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20260329/202603290317.BVIn0aoy-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603290317.BVIn0aoy-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603290317.BVIn0aoy-lkp@intel.com/
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> WARNING: modpost: vmlinux: section mismatch in reference: alloc_iommu.cold+0x49 (section: .text.unlikely) -> acpi_table_parse_keyp (section: .init.text)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
From: kernel test robot @ 2026-03-28 16:57 UTC (permalink / raw)
To: Xu Yilun, linux-coco, linux-pci, dan.j.williams, x86
Cc: oe-kbuild-all, chao.gao, dave.jiang, baolu.lu, yilun.xu, yilun.xu,
zhenzhong.duan, kvm, rick.p.edgecombe, dave.hansen, kas,
xiaoyao.li, vishal.l.verma, linux-kernel
In-Reply-To: <20260327160132.2946114-20-yilun.xu@linux.intel.com>
Hi Xu,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 11439c4635edd669ae435eec308f4ab8a0804808]
url: https://github.com/intel-lab-lkp/linux/commits/Xu-Yilun/x86-tdx-Move-all-TDX-error-defines-into-asm-shared-tdx_errno-h/20260328-151524
base: 11439c4635edd669ae435eec308f4ab8a0804808
patch link: https://lore.kernel.org/r/20260327160132.2946114-20-yilun.xu%40linux.intel.com
patch subject: [PATCH v2 19/31] iommu/vt-d: Reserve the MSB domain ID bit for the TDX module
config: i386-randconfig-141-20260328 (https://download.01.org/0day-ci/archive/20260329/202603290006.za7iiDgF-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
smatch: v0.5.0-9004-gb810ac53
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603290006.za7iiDgF-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603290006.za7iiDgF-lkp@intel.com/
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> WARNING: modpost: vmlinux: section mismatch in reference: iommu_max_domain_id+0x55 (section: .text.iommu_max_domain_id) -> acpi_table_parse_keyp (section: .init.text)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ 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-28 1:35 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-4-yilun.xu@linux.intel.com>
Hi,
In general I'm struggling to understand the design decisions. It seems a very
specific design and quite a bit of code to manage an array of pages. Questions
below.
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.
>
> 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
these types can be input or output params. The TDX module could hold on to the
pages for a long time, or just transiently. For that latter part I think you are
trying to say sometimes they need flushing and sometimes they don't?
>
> 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
probably deserves more explanation. I'm not sure, but the populate loop seems to
drive a lot of the struct design?
>
> 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.
>
> 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.
BTW, do we expect errors from the tdh_phymem_page_wbinvd_hkid() calls here? How
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
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.
>
> 3. Exchange data blobs:
> - struct tdx_page_array *array = tdx_page_array_create(nr_pages);
> - seamcall(TDX_XXX, array, ...);
> - Read data from array.
>
>
> 4. Note the root page contains 512 HPAs at most, if more pages are
> required, re-populate the tdx_page_array is needed.
>
> - struct tdx_page_array *array = tdx_page_array_alloc(nr_pages);
> - for each 512-page bulk
> - tdx_page_array_populate(array, offset);
> - seamcall(TDH_XXX_ADD, array, ...);
>
> In case 2, SEAMCALLs output the released page array in the form of
> HPA_ARRAY_T or PAGE_LIST_INFO. Use tdx_page_array_ctrl_release() to
> check if the output pages match the original input pages. If failed,
> TDX Module is buggy. In this case the safer way is to leak the
> control pages, call tdx_page_array_ctrl_leak().
>
> The usage of tdx_page_array will be in following patches.
>
> 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/include/asm/tdx.h | 37 +++++
> arch/x86/virt/vmx/tdx/tdx.c | 299 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 336 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 65c4da396450..9173a432b312 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -139,6 +139,43 @@ void tdx_guest_keyid_free(unsigned int keyid);
>
> void tdx_quirk_reset_page(struct page *page);
>
> +/**
> + * struct tdx_page_array - Represents a list of pages for TDX Module access
> + * @nr_pages: Total number of data pages in the collection
> + * @pages: Array of data page pointers containing all the data
> + *
> + * @offset: Internal: The starting index in @pages, positions the currently
> + * populated page window in @root.
> + * @nents: Internal: Number of valid HPAs for the page window in @root
> + * @root: Internal: A single 4KB page holding the 8-byte HPAs of the page
> + * window. The page window max size is constrained by the root page,
> + * which is 512 HPAs.
> + *
> + * This structure abstracts several TDX Module defined object types, e.g.,
> + * HPA_ARRAY_T and HPA_LIST_INFO. Typically they all use a "root page" as the
> + * medium to exchange a list of data pages between host and TDX Module. This
> + * structure serves as a unified parameter type for SEAMCALL wrappers, where
> + * these hardware object types are needed.
> + */
> +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?
Pausing for now. Still looking through the callers and it's the end of the day.
^ permalink raw reply
* Re: [PATCH v2 01/31] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Dan Williams @ 2026-03-28 1:16 UTC (permalink / raw)
To: Edgecombe, Rick P, 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: <a78f0ac35b137eed6306dd7ad8db37dd9d930aac.camel@intel.com>
Edgecombe, Rick P wrote:
> On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Today there are two separate locations where TDX error codes are defined:
> > arch/x86/include/asm/tdx.h
> > arch/x86/kvm/vmx/tdx_errno.h
> >
> > They have some overlap that is already defined similarly. Reduce the
> > duplication and prepare to introduce some helpers for these error codes in
> > the central place by unifying them. Join them at:
> > asm/shared/tdx_errno.h
> > ...and update the headers that contained the duplicated definitions to
> > include the new unified header.
> >
> > "asm/shared" is used for sharing TDX code between the early compressed
> > code and the normal kernel code. While the compressed code for the guest
> > doesn't use these error code header definitions today, it does make the
> > types of calls that return the values they define. So place the defines in
> > "shared" location so that it can, but leave such cleanups for future
> > changes.
> >
> > Also, adjust BITUL() -> _BITULL() to address 32 bit build errors after the
> > move.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > [enhance log]
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > Reviewed-by: Chao Gao <chao.gao@intel.com>
>
> I think it is missing Kai's RB from sys disable v1, and your sign off.
>
> This patch is in three series now, hence the long SOB chain. And I think it is
> also the only KVM touch point in the series. If sys disable gets merged ahead of
> time it can be dropped. But if there is any lag there we should see if Dave will
> just take it instead of trying to keep it in sync.
Thanks for the heads up.
I think if we are committed to the idea that some TDX related sets may
go through different upstreams and some of those sets have a small
handful of common infrastructure patches then there are a few options.
Either have someone keep an eye for these and publish stable-commits for
folks to share, accept that duplication collisions will happen and
rebase when they do, or accept that duplication collisions be ok with
that small bit of mess showing up in the history.
In this case, for the tsm.git#staging branch, I will replace this with a
fresh application of this:
https://lore.kernel.org/all/20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com/
Yilun, going forward, if you borrow a patch from another set, be sure to
both add your own signed-off-by, but also a:
Link: https://patch.msgid.link/20260323-fuller_tdx_kexec_support-v2-1-87a36409e051@intel.com
...to make it extra clear you are including a patch that is already on
the list in another set.
I suspect that by the time this set is ready to move from
tsm.git#staging to tsm.git#next a stable commit-id may be available for
a rebase.
^ permalink raw reply
* Re: [PATCH v2 02/31] x86/virt/tdx: Move bit definitions of TDX_FEATURES0 to public header
From: Edgecombe, Rick P @ 2026-03-27 23:45 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-3-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> Move bit definitions of TDX_FEATURES0 to TDX core public header.
Patch seems reasonable, but not sure about the "public header" language. Maybe
more widely accessibly header? Or a better name?
>
> Kernel users get TDX_FEATURES0 bitmap via tdx_get_sysinfo(). It is
> reasonable to also public the definitions of each bit. TDX Connect (a
> new TDX feature to enable Trusted I/O virtualization) will add new bits
> and check them in separate kernel modules.
>
> Take the opportunity to change its type to BIT_ULL since TDX_FEATURES0
> is explicitly defined as 64-bit in both TDX Module Specification and
> TDX core code.
>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
^ permalink raw reply
* Re: [PATCH v2 01/31] x86/tdx: Move all TDX error defines into <asm/shared/tdx_errno.h>
From: Edgecombe, Rick P @ 2026-03-27 23:37 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-2-yilun.xu@linux.intel.com>
On Sat, 2026-03-28 at 00:01 +0800, Xu Yilun wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Today there are two separate locations where TDX error codes are defined:
> arch/x86/include/asm/tdx.h
> arch/x86/kvm/vmx/tdx_errno.h
>
> They have some overlap that is already defined similarly. Reduce the
> duplication and prepare to introduce some helpers for these error codes in
> the central place by unifying them. Join them at:
> asm/shared/tdx_errno.h
> ...and update the headers that contained the duplicated definitions to
> include the new unified header.
>
> "asm/shared" is used for sharing TDX code between the early compressed
> code and the normal kernel code. While the compressed code for the guest
> doesn't use these error code header definitions today, it does make the
> types of calls that return the values they define. So place the defines in
> "shared" location so that it can, but leave such cleanups for future
> changes.
>
> Also, adjust BITUL() -> _BITULL() to address 32 bit build errors after the
> move.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> Reviewed-by: Chao Gao <chao.gao@intel.com>
I think it is missing Kai's RB from sys disable v1, and your sign off.
This patch is in three series now, hence the long SOB chain. And I think it is
also the only KVM touch point in the series. If sys disable gets merged ahead of
time it can be dropped. But if there is any lag there we should see if Dave will
just take it instead of trying to keep it in sync.
^ permalink raw reply
* Re: [GIT PULL] Confidential Computing: Attestation fixes for 7.0-rc6
From: pr-tracker-bot @ 2026-03-27 23:32 UTC (permalink / raw)
To: Dan Williams
Cc: Linus Torvalds, linux-coco, Zubin Mithra, Kiryl Shutsemau,
Kuppuswamy Sathyanarayanan, Dave Hansen, Xiaoyao Li, Mikko Ylinen,
linux-kernel
In-Reply-To: <69c6eea861b3a_1b0cc610021@dwillia2-mobl4.notmuch>
The pull request you sent on Fri, 27 Mar 2026 13:55:04 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm tags/tsm-fixes-7.0-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/dd09eb443372f9390d36051d86ebe06e9919aeec
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Reinette Chatre @ 2026-03-27 22:11 UTC (permalink / raw)
To: Babu Moger, 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: <47c0db32-d0e0-4c53-90bd-b74863d233dc@amd.com>
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.
...
>>>
>>> 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?
>
> 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?
Reinette
^ 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