* SVSM Development Call April 29, 2026
From: Jörg Rödel @ 2026-04-28 16:11 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
Currently on the agenda:
- Usual TSC meeting update
- Review of slides for CCC project status update
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH v4 3/3] coco: guest: arm64: Query host IPA-change alignment via RHI
From: Suzuki K Poulose @ 2026-04-28 15:22 UTC (permalink / raw)
To: Marc Zyngier, Aneesh Kumar K.V
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marek Szyprowski, Robin Murphy,
Steven Price, Thomas Gleixner, Will Deacon
In-Reply-To: <86tssvyz2v.wl-maz@kernel.org>
On 28/04/2026 14:49, Marc Zyngier wrote:
> On Tue, 28 Apr 2026 13:49:46 +0100,
> Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote:
>>
>> Marc Zyngier <maz@kernel.org> writes:
>>
>>> On Mon, 27 Apr 2026 07:31:08 +0100,
>>> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
>>>>
>>>> Add the Realm Host Interface support needed to query host configuration
>>>> from a Realm guest. Define the RHI hostconf SMCs, add rsi_host_call(), and
>>>> use them during Realm initialization to retrieve the host IPA-change
>>>> alignment size.
>>>
>>> I don't understand what "IPA-change" means. What you are after is the
>>> host's sharing granule size.
>>>
>>
>> This is part of the RHI specification, and the call is named
>> RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT. The intent is to determine the
>> alignment requirements for changing IPA attributes (protected vs.
>> unprotected IPA
>
> This really is a terrible name. Why the 'change' part? It doesn't
> change, it is a constant.
Agreed, it was supposed to mean IPA_STATE_CHANGE.
>
> Oh well...
>
> [...]
>
...
>>>> +unsigned long rhi_get_ipa_change_alignment(void)
>>>> +{
>>>> + long ret;
>>>> + unsigned long ipa_change_align;
>>>> +
>>>> + hyp_pagesize_rhicall.imm = 0;
>>>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_VERSION;
>>>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>>>> + if (ret != RSI_SUCCESS)
>>>> + goto err_out;
>>>> +
>>>> + if (hyp_pagesize_rhicall.gprs[0] != RHI_HOSTCONF_VER_1_0)
>>>> + goto err_out;
>>>> +
>>>> + hyp_pagesize_rhicall.imm = 0;
>>>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_FEATURES;
>>>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>>>> + if (ret != RSI_SUCCESS)
>>>> + goto err_out;
>>>> +
>>>> + if (!(hyp_pagesize_rhicall.gprs[0] & __RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT))
>>>> + goto err_out;
>>>> +
>>>> + hyp_pagesize_rhicall.imm = 0;
>>>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT;
>>>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>>>> + if (ret != RSI_SUCCESS)
>>>> + goto err_out;
>>>> +
>>>> + ipa_change_align = hyp_pagesize_rhicall.gprs[0];
>>>> + /* This error needs special handling in the caller */
>>>> + if (ipa_change_align & (SZ_4K - 1))
>>>> + return 0;
>>>> +
>>>> + return ipa_change_align;
>>>> +
>>>> +err_out:
>>>> + /*
>>>> + * For failure condition assume host is built with 4K page size
>>>> + * and hence ipa change alignment can be guest PAGE_SIZE.
>>>> + */
>>>> + return PAGE_SIZE;
>>>> +}
>>>
>>> Why can't this be part of rsi.c? This is an RSI call, and it should be
>>> part of the RSI initialisation.
>>>
>>
>> This is an RHI call as per the specification, hence it has been added to
>> rhi.c.
>
> News flash: this is the Linux kernel, not an ARM spec. We organise
> things based on the logical use, not on the TLA associated with it.
>
> And RHI is implemented in terms of RSI. In rsi.c it goes. We don't
> need this pointless proliferation of helper files that only result in
> equally pointless global symbols.
RHI (Realm Host Interface) is not really the same as RSI. The former is
a service mechanism for Realms with the "Non-secure Hypervisor". And
this single call is just one of the services. There are further more
services that will eventually come up (e.g., Device Assignment, Boot
Sync Protocol, Firmware Activity Log etc).
RSI (to be precise, RSI_HOST_CALL) is the transport to talk to the Host,
as that is the only way for the Realm to reach the Host. So, tbh, it
does make sense to keep this in rhic ?
Suzuki
^ permalink raw reply
* Re: [PATCH v4 3/3] coco: guest: arm64: Query host IPA-change alignment via RHI
From: Will Deacon @ 2026-04-28 13:56 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marc Zyngier, Marek Szyprowski,
Robin Murphy, Steven Price, Suzuki K Poulose, Thomas Gleixner,
sebastianene
In-Reply-To: <20260427063108.909019-4-aneesh.kumar@kernel.org>
[+Seb for the ITS]
On Mon, Apr 27, 2026 at 12:01:08PM +0530, Aneesh Kumar K.V (Arm) wrote:
> Add the Realm Host Interface support needed to query host configuration
> from a Realm guest. Define the RHI hostconf SMCs, add rsi_host_call(), and
> use them during Realm initialization to retrieve the host IPA-change
> alignment size.
>
> Expose that alignment through realm_get_hyp_pagesize() and
> mem_decrypt_granule_size() so shared-buffer allocation and
> encryption/decryption paths can honor the ipa change page-size requirement.
>
> If the host reports an invalid alignment (when alginment value is not
> multiple of 4K), do not enable Realm support.
>
> This provides the host alignment information required by the shared buffer
> alignment changes.
>
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> arch/arm64/include/asm/mem_encrypt.h | 3 ++
> arch/arm64/include/asm/rhi.h | 24 +++++++++++++
> arch/arm64/include/asm/rsi.h | 2 ++
> arch/arm64/include/asm/rsi_cmds.h | 10 ++++++
> arch/arm64/include/asm/rsi_smc.h | 7 ++++
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/rhi.c | 54 ++++++++++++++++++++++++++++
> arch/arm64/kernel/rsi.c | 13 +++++++
> arch/arm64/mm/mem_encrypt.c | 8 +++++
> 9 files changed, 122 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/rhi.h
> create mode 100644 arch/arm64/kernel/rhi.c
[...]
> diff --git a/arch/arm64/mm/mem_encrypt.c b/arch/arm64/mm/mem_encrypt.c
> index 38c62c9e4e74..f5d64bc29c20 100644
> --- a/arch/arm64/mm/mem_encrypt.c
> +++ b/arch/arm64/mm/mem_encrypt.c
> @@ -59,3 +59,11 @@ int set_memory_decrypted(unsigned long addr, int numpages)
> return crypt_ops->decrypt(addr, numpages);
> }
> EXPORT_SYMBOL_GPL(set_memory_decrypted);
> +
> +size_t mem_decrypt_granule_size(void)
> +{
> + if (is_realm_world())
> + return max(PAGE_SIZE, realm_get_hyp_pagesize());
> + return PAGE_SIZE;
No, this should be indirected via 'struct arm64_mem_crypt_ops' because
there's nothing particularly unique to realms here. For pKVM protected
guests using a smaller page-size than the host, we'd presumably need
something similar for the ITS (where restricted-dma isn't used).
Will
^ permalink raw reply
* Re: [PATCH v4 3/3] coco: guest: arm64: Query host IPA-change alignment via RHI
From: Marc Zyngier @ 2026-04-28 13:49 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marek Szyprowski, Robin Murphy,
Steven Price, Suzuki K Poulose, Thomas Gleixner, Will Deacon
In-Reply-To: <yq5a4ikv1c7p.fsf@kernel.org>
On Tue, 28 Apr 2026 13:49:46 +0100,
Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote:
>
> Marc Zyngier <maz@kernel.org> writes:
>
> > On Mon, 27 Apr 2026 07:31:08 +0100,
> > "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> >>
> >> Add the Realm Host Interface support needed to query host configuration
> >> from a Realm guest. Define the RHI hostconf SMCs, add rsi_host_call(), and
> >> use them during Realm initialization to retrieve the host IPA-change
> >> alignment size.
> >
> > I don't understand what "IPA-change" means. What you are after is the
> > host's sharing granule size.
> >
>
> This is part of the RHI specification, and the call is named
> RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT. The intent is to determine the
> alignment requirements for changing IPA attributes (protected vs.
> unprotected IPA
This really is a terrible name. Why the 'change' part? It doesn't
change, it is a constant.
Oh well...
[...]
> >> +static inline unsigned long rsi_host_call(struct rsi_host_call *rhi_call)
> >> +{
> >> + phys_addr_t addr = virt_to_phys(rhi_call);
> >> + struct arm_smccc_res res;
> >> +
> >> + arm_smccc_1_1_invoke(SMC_RSI_HOST_CALL, addr, &res);
> >
> > Errr... What guarantees that *rhi_call is *IPA contiguous*? This is
> > incredibly fragile. You should at the very least check that this isn't
> > vmalloc'd.
> >
>
>
> I didn’t quite follow that. We have other RSI calls (even RMI calls)
> that do similar things, and the caller understands that the address
> should be IPA-contiguous.
Does it? Where is it documented? All you get is a pointer, so all
bets are off.
> Are you suggesting that all RSI calls should
> add checks for this?. or are you suggesting to update the API to
>
> unsigned long rsi_host_call(unsigned long rhi_call_phys) ?
I'm suggesting that this API is subtly broken because it makes random
assumption about the physical contiguity of the VA space. It does so
without any check, without any documentation.
Simply changing the parameter to phys_addr_t could at the very least
capture some of the requirements, but I'd like something in big bold
letters.
>
> >> +
> >> + return res.a0;
> >> +}
> >> +
> >> #endif /* __ASM_RSI_CMDS_H */
> >> diff --git a/arch/arm64/include/asm/rsi_smc.h b/arch/arm64/include/asm/rsi_smc.h
> >> index e19253f96c94..9ee8b5c7612e 100644
> >> --- a/arch/arm64/include/asm/rsi_smc.h
> >> +++ b/arch/arm64/include/asm/rsi_smc.h
> >> @@ -182,6 +182,13 @@ struct realm_config {
> >> */
> >> #define SMC_RSI_IPA_STATE_GET SMC_RSI_FID(0x198)
> >>
> >> +struct rsi_host_call {
> >> + union {
> >> + u16 imm;
> >> + u64 padding0;
> >> + };
> >> + u64 gprs[31];
> >> +} __aligned(0x100);
> >> /*
> >> * Make a Host call.
> >> *
> >> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> >> index fe627100d199..3e72dd9584ed 100644
> >> --- a/arch/arm64/kernel/Makefile
> >> +++ b/arch/arm64/kernel/Makefile
> >> @@ -34,7 +34,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
> >> cpufeature.o alternative.o cacheinfo.o \
> >> smp.o smp_spin_table.o topology.o smccc-call.o \
> >> syscall.o proton-pack.o idle.o patching.o pi/ \
> >> - rsi.o jump_label.o
> >> + rsi.o jump_label.o rhi.o
> >>
> >> obj-$(CONFIG_COMPAT) += sys32.o signal32.o \
> >> sys_compat.o
> >> diff --git a/arch/arm64/kernel/rhi.c b/arch/arm64/kernel/rhi.c
> >> new file mode 100644
> >> index 000000000000..7cd6c5102464
> >> --- /dev/null
> >> +++ b/arch/arm64/kernel/rhi.c
> >> @@ -0,0 +1,54 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (C) 2026 ARM Ltd.
> >> + */
> >> +
> >> +#include <linux/mm.h>
> >> +#include <asm/rsi.h>
> >> +#include <asm/rhi.h>
> >> +
> >> +/* we need an aligned rhicall for rsi_host_call. slab is not yet ready */
> >> +static struct rsi_host_call hyp_pagesize_rhicall;
> >
> > Why the "hyp_" prefix? This has absolutely nothing to with the
> > hypervisor.
> >
>
> Sure will update "hyp_" reference to host.
>
>
> >> +unsigned long rhi_get_ipa_change_alignment(void)
> >> +{
> >> + long ret;
> >> + unsigned long ipa_change_align;
> >> +
> >> + hyp_pagesize_rhicall.imm = 0;
> >> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_VERSION;
> >> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
> >> + if (ret != RSI_SUCCESS)
> >> + goto err_out;
> >> +
> >> + if (hyp_pagesize_rhicall.gprs[0] != RHI_HOSTCONF_VER_1_0)
> >> + goto err_out;
> >> +
> >> + hyp_pagesize_rhicall.imm = 0;
> >> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_FEATURES;
> >> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
> >> + if (ret != RSI_SUCCESS)
> >> + goto err_out;
> >> +
> >> + if (!(hyp_pagesize_rhicall.gprs[0] & __RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT))
> >> + goto err_out;
> >> +
> >> + hyp_pagesize_rhicall.imm = 0;
> >> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT;
> >> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
> >> + if (ret != RSI_SUCCESS)
> >> + goto err_out;
> >> +
> >> + ipa_change_align = hyp_pagesize_rhicall.gprs[0];
> >> + /* This error needs special handling in the caller */
> >> + if (ipa_change_align & (SZ_4K - 1))
> >> + return 0;
> >> +
> >> + return ipa_change_align;
> >> +
> >> +err_out:
> >> + /*
> >> + * For failure condition assume host is built with 4K page size
> >> + * and hence ipa change alignment can be guest PAGE_SIZE.
> >> + */
> >> + return PAGE_SIZE;
> >> +}
> >
> > Why can't this be part of rsi.c? This is an RSI call, and it should be
> > part of the RSI initialisation.
> >
>
> This is an RHI call as per the specification, hence it has been added to
> rhi.c.
News flash: this is the Linux kernel, not an ARM spec. We organise
things based on the logical use, not on the TLA associated with it.
And RHI is implemented in terms of RSI. In rsi.c it goes. We don't
need this pointless proliferation of helper files that only result in
equally pointless global symbols.
>
> >> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
> >> index 9e846ce4ef9c..ff735c04e236 100644
> >> --- a/arch/arm64/kernel/rsi.c
> >> +++ b/arch/arm64/kernel/rsi.c
> >> @@ -14,8 +14,10 @@
> >> #include <asm/mem_encrypt.h>
> >> #include <asm/pgtable.h>
> >> #include <asm/rsi.h>
> >> +#include <asm/rhi.h>
> >>
> >> static struct realm_config config;
> >> +static unsigned long ipa_change_alignment = PAGE_SIZE;
> >>
> >> unsigned long prot_ns_shared;
> >> EXPORT_SYMBOL(prot_ns_shared);
> >> @@ -139,6 +141,11 @@ static int realm_ioremap_hook(phys_addr_t phys, size_t size, pgprot_t *prot)
> >> return 0;
> >> }
> >>
> >> +unsigned long realm_get_hyp_pagesize(void)
> >> +{
> >> + return ipa_change_alignment;
> >> +}
> >
> > Again, this has nothing to do with the hypervisor, but the host. And
> > ipa_change_alignment is still a wording I can't wrap my small head
> > around.
> >
> >> +
> >> void __init arm64_rsi_init(void)
> >> {
> >> if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
> >> @@ -147,6 +154,12 @@ void __init arm64_rsi_init(void)
> >> return;
> >> if (WARN_ON(rsi_get_realm_config(&config)))
> >> return;
> >> +
> >> + ipa_change_alignment = rhi_get_ipa_change_alignment();
> >> + /* If we don't get a correct alignment response, don't enable realm */
> >> + if (!ipa_change_alignment)
> >> + return;
> >
> > But at the same time, you override a global value with an error, and
> > then paper over it in mem_decrypt_granule_size()...
> >
>
>
> I believe I received similar feedback on my previous version as well,
> which I didn’t quite follow.
And you didn't think of asking? Sometimes I wonder what these patch
reviews are for... Just to waste some more electrons, I guess :-/.
>
> rhi_get_ipa_change_alignment() only returns an error when the host
> returns a size that is not 4K-aligned. Otherwise, it returns the
> host-determined size, or defaults to guest PAGE_SIZE if the RHI call
> itself is not supported.
You encode the error as 0. You override ipa_change_alignment with 0.
Then...
> >> +size_t mem_decrypt_granule_size(void)
> >> +{
> >> + if (is_realm_world())
> >> + return max(PAGE_SIZE, realm_get_hyp_pagesize());
> >
> > If you didn't mess with ipa_change_alignment above, you shouldn't need
> > this max().
> >
>
> size_t mem_decrypt_granule_size(void)
> {
> if (is_realm_world())
> return max(PAGE_SIZE, realm_get_hyp_pagesize());
> return PAGE_SIZE;
> }
>
> That needs to use max(), because we should align to the guest PAGE_SIZE
> if it is larger than the host-specified alignment value.
... you need to correct that back to PAGE_SIZE because you have stored
something smaller than PAGE_SIZE.
Isn't the problem really obvious? ipa_change_alignment can *NEVER* go
down. It should never be allowed to reduce, because that's exactly
the property you are trying to enforce.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH v4 2/3] swiotlb: dma: its: Enforce host page-size alignment for shared buffers
From: Marc Zyngier @ 2026-04-28 13:31 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marek Szyprowski, Robin Murphy,
Steven Price, Suzuki K Poulose, Thomas Gleixner, Will Deacon
In-Reply-To: <yq5aa4un1dju.fsf@kernel.org>
On Tue, 28 Apr 2026 13:20:53 +0100,
Aneesh Kumar K.V <aneesh.kumar@kernel.org> wrote:
>
> Marc Zyngier <maz@kernel.org> writes:
>
> > On Mon, 27 Apr 2026 07:31:07 +0100,
> > "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> >>
> >> When running private-memory guests, the guest kernel must apply additional
> >> constraints when allocating buffers that are shared with the hypervisor.
> >>
> >> These shared buffers are also accessed by the host kernel and therefore
> >> must be aligned to the host’s page size, and have a size that is a multiple
> >> of the host page size.
> >>
> >> On non-secure hosts, set_guest_memory_attributes() tracks memory at the
> >> host PAGE_SIZE granularity. This creates a mismatch when the guest applies
> >> attributes at 4K boundaries while the host uses 64K pages. In such cases,
> >> set_guest_memory_attributes() call returns -EINVAL, preventing the
> >> conversion of memory regions from private to shared.
> >>
> >> Architectures such as Arm can tolerate realm physical address space
> >> (protected memory) PFNs being mapped as shared memory, as incorrect
> >> accesses are detected and reported as GPC faults. However, relying on this
> >> mechanism is unsafe and can still lead to kernel crashes.
> >>
> >> This is particularly likely when guest_memfd allocations are mmapped and
> >> accessed from userspace. Once exposed to userspace, we cannot guarantee
> >> that applications will only access the intended 4K shared region rather
> >> than the full 64K page mapped into their address space. Such userspace
> >> addresses may also be passed back into the kernel and accessed via the
> >> linear map, resulting in a GPC fault and a kernel crash.
> >>
> >> With CCA, although Stage-2 mappings managed by the RMM still operate at a
> >> 4K granularity, shared pages must nonetheless be aligned to the
> >> host-managed page size and sized as whole host pages to avoid the issues
> >> described above.
> >
> > I thought that was being fixed, and that there was now a strong
> > guarantee that RMM and host are aligned on the page size. Even more,
> > S2 is totally irrelevant here. The only thing that matters is the host
> > page size vs the guest page size. Nothing else.
> >
>
> Yes, the latest RMM update includes the ability to change the granule
> size.
>
> The section above in the commit message was intended to explain that the
> S2 mapping size is irrelevant. I agree it is not clear as written, so I
> will reword it to improve clarity.
Even better, remove it. Nothing CCA-specific should be in this patch.
[...]
> >> static struct gen_pool *itt_pool;
> >> @@ -268,11 +272,13 @@ static void *itt_alloc_pool(int node, int size)
> >> if (addr)
> >> break;
> >>
> >> - page = its_alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
> >> + page = its_alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO,
> >> + get_order(mem_decrypt_granule_size()));
> >
> > You already taught its_alloc_pages_node() about the decrypt granule
> > size stuff. I don't think we need to see more of it (and you don't
> > mess with the call that is just above it).
> >
> >> if (!page)
> >> break;
> >>
> >> - gen_pool_add(itt_pool, (unsigned long)page_address(page), PAGE_SIZE, node);
> >> + gen_pool_add(itt_pool, (unsigned long)page_address(page),
> >> + mem_decrypt_granule_size(), node);
> >
> > I'd rather see something like mem_decrypt_align(PAGE_SIZE), which
> > keeps the intent clear.
> >
>
> The helper was added based on feedback from a previous version. I assume
> you are suggesting that only this caller should switch?
I don't know what you mean by 'this'. What I'd like to see is this
last hunk be changed to:
gen_pool_add(itt_pool, (unsigned long)page_address(page),
mem_decrypt_align(PAGE_SIZE), node);
and the previous hunk simply dropped.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* [PATCH v2 2/2] x86/tdx: Fix zero-extension for 32-bit port I/O
From: Kiryl Shutsemau (Meta) @ 2026-04-28 12:56 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Rick Edgecombe, Kuppuswamy Sathyanarayanan,
Kai Huang, Borys Tsyrulnikov, linux-kernel, linux-coco, kvm,
stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260428125632.129770-1-kas@kernel.org>
According to x86 architecture rules, 32-bit operations zero-extend the
result to 64 bits. The current implementation of handle_in() only masks
the lower 32 bits, which preserves the upper 32 bits of RAX when a
32-bit port IN instruction is emulated.
Update handle_in() to zero out the entire RAX register when the I/O size
is 4 bytes to ensure correct zero-extension. For smaller sizes (1 or 2
bytes), continue to preserve the unaffected upper bits.
Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: stable@vger.kernel.org
---
arch/x86/coco/tdx/tdx.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 65119362f9a2..e09636564237 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -703,8 +703,17 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
*/
success = !__tdx_hypercall(&args);
- /* Update part of the register affected by the emulated instruction */
- regs->ax &= ~mask;
+ /*
+ * Update part of the register affected by the emulated instruction.
+ *
+ * 32-bit operands generate a 32-bit result, zero-extended to a 64-bit
+ * result.
+ */
+ if (size == 4)
+ regs->ax = 0;
+ else
+ regs->ax &= ~mask;
+
if (success)
regs->ax |= args.r11 & mask;
--
2.51.2
^ permalink raw reply related
* [PATCH v2 1/2] x86/tdx: Fix off-by-one in port I/O handling
From: Kiryl Shutsemau (Meta) @ 2026-04-28 12:56 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Rick Edgecombe, Kuppuswamy Sathyanarayanan,
Kai Huang, Borys Tsyrulnikov, linux-kernel, linux-coco, kvm,
stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260428125632.129770-1-kas@kernel.org>
handle_in() and handle_out() in arch/x86/coco/tdx/tdx.c use:
u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
GENMASK(h, l) includes bit h. For size=1 (INB), this produces
GENMASK(8, 0) = 0x1FF (9 bits) instead of GENMASK(7, 0) = 0xFF (8
bits). The mask is one bit too wide for all I/O sizes.
Fix the mask calculation.
Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: stable@vger.kernel.org
---
arch/x86/coco/tdx/tdx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 186915a17c50..65119362f9a2 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -693,7 +693,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
.r13 = PORT_READ,
.r14 = port,
};
- u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+ u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
bool success;
/*
@@ -713,7 +713,7 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
static bool handle_out(struct pt_regs *regs, int size, int port)
{
- u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
+ u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
/*
* Emulate the I/O write via hypercall. More info about ABI can be found
--
2.51.2
^ permalink raw reply related
* [PATCH v2 0/2] x86/tdx: Port I/O emulation fixes
From: Kiryl Shutsemau (Meta) @ 2026-04-28 12:56 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: H . Peter Anvin, Rick Edgecombe, Kuppuswamy Sathyanarayanan,
Kai Huang, Borys Tsyrulnikov, linux-kernel, linux-coco, kvm,
stable, Kiryl Shutsemau (Meta)
This series addresses two technical inaccuracies in the TDX guest port
I/O emulation code reported by Borys Tsyrulnikov.
The first patch fixes an off-by-one error in the GENMASK() macro usage
where the mask was being calculated as one bit too wide (e.g. 9 bits for
an 8-bit operation).
The second patch ensures that 32-bit port I/O operations (INL) correctly
zero-extend the result to the full 64-bit RAX register, as required by
the x86 architecture. Currently, the emulation preserves the upper 32
bits of RAX during such operations.
Both issues were introduced in the initial implementation of the runtime
hypercalls for port I/O.
v1: https://lore.kernel.org/all/20260331112430.71425-1-kas@kernel.org/
Changes in v2:
- Rephrase the size check in handle_in() as "if (size == 4)" for
readability (Kuppuswamy)
- Add Link: to the bug report on both patches (Kuppuswamy)
- Collect Reviewed-by tags (Kai Huang, Kuppuswamy Sathyanarayanan)
- Rebase onto v7.1-rc1
Kiryl Shutsemau (Meta) (2):
x86/tdx: Fix off-by-one in port I/O handling
x86/tdx: Fix zero-extension for 32-bit port I/O
arch/x86/coco/tdx/tdx.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.51.2
^ permalink raw reply
* Re: [PATCH v4 3/3] coco: guest: arm64: Query host IPA-change alignment via RHI
From: Aneesh Kumar K.V @ 2026-04-28 12:49 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marek Szyprowski, Robin Murphy,
Steven Price, Suzuki K Poulose, Thomas Gleixner, Will Deacon
In-Reply-To: <86y0i8zo9f.wl-maz@kernel.org>
Marc Zyngier <maz@kernel.org> writes:
> On Mon, 27 Apr 2026 07:31:08 +0100,
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
>>
>> Add the Realm Host Interface support needed to query host configuration
>> from a Realm guest. Define the RHI hostconf SMCs, add rsi_host_call(), and
>> use them during Realm initialization to retrieve the host IPA-change
>> alignment size.
>
> I don't understand what "IPA-change" means. What you are after is the
> host's sharing granule size.
>
This is part of the RHI specification, and the call is named
RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT. The intent is to determine the
alignment requirements for changing IPA attributes (protected vs.
unprotected IPA
>
>>
>> Expose that alignment through realm_get_hyp_pagesize() and
>> mem_decrypt_granule_size() so shared-buffer allocation and
>> encryption/decryption paths can honor the ipa change page-size requirement.
>>
>> If the host reports an invalid alignment (when alginment value is not
>> multiple of 4K), do not enable Realm support.
>>
>> This provides the host alignment information required by the shared buffer
>> alignment changes.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> arch/arm64/include/asm/mem_encrypt.h | 3 ++
>> arch/arm64/include/asm/rhi.h | 24 +++++++++++++
>> arch/arm64/include/asm/rsi.h | 2 ++
>> arch/arm64/include/asm/rsi_cmds.h | 10 ++++++
>> arch/arm64/include/asm/rsi_smc.h | 7 ++++
>> arch/arm64/kernel/Makefile | 2 +-
>> arch/arm64/kernel/rhi.c | 54 ++++++++++++++++++++++++++++
>> arch/arm64/kernel/rsi.c | 13 +++++++
>> arch/arm64/mm/mem_encrypt.c | 8 +++++
>> 9 files changed, 122 insertions(+), 1 deletion(-)
>> create mode 100644 arch/arm64/include/asm/rhi.h
>> create mode 100644 arch/arm64/kernel/rhi.c
>>
>> diff --git a/arch/arm64/include/asm/mem_encrypt.h b/arch/arm64/include/asm/mem_encrypt.h
>> index 314b2b52025f..5541911eb028 100644
>> --- a/arch/arm64/include/asm/mem_encrypt.h
>> +++ b/arch/arm64/include/asm/mem_encrypt.h
>> @@ -16,6 +16,9 @@ int arm64_mem_crypt_ops_register(const struct arm64_mem_crypt_ops *ops);
>> int set_memory_encrypted(unsigned long addr, int numpages);
>> int set_memory_decrypted(unsigned long addr, int numpages);
>>
>> +#define mem_decrypt_granule_size mem_decrypt_granule_size
>> +size_t mem_decrypt_granule_size(void);
>> +
>> int realm_register_memory_enc_ops(void);
>>
>> static inline bool force_dma_unencrypted(struct device *dev)
>> diff --git a/arch/arm64/include/asm/rhi.h b/arch/arm64/include/asm/rhi.h
>> new file mode 100644
>> index 000000000000..0895dd92ea1d
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/rhi.h
>> @@ -0,0 +1,24 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2026 ARM Ltd.
>> + */
>> +
>> +#ifndef __ASM_RHI_H_
>> +#define __ASM_RHI_H_
>> +
>> +#include <linux/types.h>
>> +
>> +#define SMC_RHI_CALL(func) \
>> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
>> + ARM_SMCCC_SMC_64, \
>> + ARM_SMCCC_OWNER_STANDARD_HYP,\
>> + (func))
>> +
>> +unsigned long rhi_get_ipa_change_alignment(void);
>> +#define RHI_HOSTCONF_VER_1_0 0x10000
>> +#define RHI_HOSTCONF_VERSION SMC_RHI_CALL(0x004E)
>> +
>> +#define __RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT BIT(0)
>> +#define RHI_HOSTCONF_FEATURES SMC_RHI_CALL(0x004F)
>> +#define RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT SMC_RHI_CALL(0x0050)
>> +#endif
>> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
>> index 88b50d660e85..ae54fb3b1429 100644
>> --- a/arch/arm64/include/asm/rsi.h
>> +++ b/arch/arm64/include/asm/rsi.h
>> @@ -67,4 +67,6 @@ static inline int rsi_set_memory_range_shared(phys_addr_t start,
>> return rsi_set_memory_range(start, end, RSI_RIPAS_EMPTY,
>> RSI_CHANGE_DESTROYED);
>> }
>> +
>> +unsigned long realm_get_hyp_pagesize(void);
>> #endif /* __ASM_RSI_H_ */
>> diff --git a/arch/arm64/include/asm/rsi_cmds.h b/arch/arm64/include/asm/rsi_cmds.h
>> index 2c8763876dfb..a341ce0eeda1 100644
>> --- a/arch/arm64/include/asm/rsi_cmds.h
>> +++ b/arch/arm64/include/asm/rsi_cmds.h
>> @@ -159,4 +159,14 @@ static inline unsigned long rsi_attestation_token_continue(phys_addr_t granule,
>> return res.a0;
>> }
>>
>> +static inline unsigned long rsi_host_call(struct rsi_host_call *rhi_call)
>> +{
>> + phys_addr_t addr = virt_to_phys(rhi_call);
>> + struct arm_smccc_res res;
>> +
>> + arm_smccc_1_1_invoke(SMC_RSI_HOST_CALL, addr, &res);
>
> Errr... What guarantees that *rhi_call is *IPA contiguous*? This is
> incredibly fragile. You should at the very least check that this isn't
> vmalloc'd.
>
I didn’t quite follow that. We have other RSI calls (even RMI calls)
that do similar things, and the caller understands that the address
should be IPA-contiguous. Are you suggesting that all RSI calls should
add checks for this?. or are you suggesting to update the API to
unsigned long rsi_host_call(unsigned long rhi_call_phys) ?
>> +
>> + return res.a0;
>> +}
>> +
>> #endif /* __ASM_RSI_CMDS_H */
>> diff --git a/arch/arm64/include/asm/rsi_smc.h b/arch/arm64/include/asm/rsi_smc.h
>> index e19253f96c94..9ee8b5c7612e 100644
>> --- a/arch/arm64/include/asm/rsi_smc.h
>> +++ b/arch/arm64/include/asm/rsi_smc.h
>> @@ -182,6 +182,13 @@ struct realm_config {
>> */
>> #define SMC_RSI_IPA_STATE_GET SMC_RSI_FID(0x198)
>>
>> +struct rsi_host_call {
>> + union {
>> + u16 imm;
>> + u64 padding0;
>> + };
>> + u64 gprs[31];
>> +} __aligned(0x100);
>> /*
>> * Make a Host call.
>> *
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index fe627100d199..3e72dd9584ed 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -34,7 +34,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
>> cpufeature.o alternative.o cacheinfo.o \
>> smp.o smp_spin_table.o topology.o smccc-call.o \
>> syscall.o proton-pack.o idle.o patching.o pi/ \
>> - rsi.o jump_label.o
>> + rsi.o jump_label.o rhi.o
>>
>> obj-$(CONFIG_COMPAT) += sys32.o signal32.o \
>> sys_compat.o
>> diff --git a/arch/arm64/kernel/rhi.c b/arch/arm64/kernel/rhi.c
>> new file mode 100644
>> index 000000000000..7cd6c5102464
>> --- /dev/null
>> +++ b/arch/arm64/kernel/rhi.c
>> @@ -0,0 +1,54 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2026 ARM Ltd.
>> + */
>> +
>> +#include <linux/mm.h>
>> +#include <asm/rsi.h>
>> +#include <asm/rhi.h>
>> +
>> +/* we need an aligned rhicall for rsi_host_call. slab is not yet ready */
>> +static struct rsi_host_call hyp_pagesize_rhicall;
>
> Why the "hyp_" prefix? This has absolutely nothing to with the
> hypervisor.
>
Sure will update "hyp_" reference to host.
>> +unsigned long rhi_get_ipa_change_alignment(void)
>> +{
>> + long ret;
>> + unsigned long ipa_change_align;
>> +
>> + hyp_pagesize_rhicall.imm = 0;
>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_VERSION;
>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>> + if (ret != RSI_SUCCESS)
>> + goto err_out;
>> +
>> + if (hyp_pagesize_rhicall.gprs[0] != RHI_HOSTCONF_VER_1_0)
>> + goto err_out;
>> +
>> + hyp_pagesize_rhicall.imm = 0;
>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_FEATURES;
>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>> + if (ret != RSI_SUCCESS)
>> + goto err_out;
>> +
>> + if (!(hyp_pagesize_rhicall.gprs[0] & __RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT))
>> + goto err_out;
>> +
>> + hyp_pagesize_rhicall.imm = 0;
>> + hyp_pagesize_rhicall.gprs[0] = RHI_HOSTCONF_GET_IPA_CHANGE_ALIGNMENT;
>> + ret = rsi_host_call(lm_alias(&hyp_pagesize_rhicall));
>> + if (ret != RSI_SUCCESS)
>> + goto err_out;
>> +
>> + ipa_change_align = hyp_pagesize_rhicall.gprs[0];
>> + /* This error needs special handling in the caller */
>> + if (ipa_change_align & (SZ_4K - 1))
>> + return 0;
>> +
>> + return ipa_change_align;
>> +
>> +err_out:
>> + /*
>> + * For failure condition assume host is built with 4K page size
>> + * and hence ipa change alignment can be guest PAGE_SIZE.
>> + */
>> + return PAGE_SIZE;
>> +}
>
> Why can't this be part of rsi.c? This is an RSI call, and it should be
> part of the RSI initialisation.
>
This is an RHI call as per the specification, hence it has been added to
rhi.c.
>> diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
>> index 9e846ce4ef9c..ff735c04e236 100644
>> --- a/arch/arm64/kernel/rsi.c
>> +++ b/arch/arm64/kernel/rsi.c
>> @@ -14,8 +14,10 @@
>> #include <asm/mem_encrypt.h>
>> #include <asm/pgtable.h>
>> #include <asm/rsi.h>
>> +#include <asm/rhi.h>
>>
>> static struct realm_config config;
>> +static unsigned long ipa_change_alignment = PAGE_SIZE;
>>
>> unsigned long prot_ns_shared;
>> EXPORT_SYMBOL(prot_ns_shared);
>> @@ -139,6 +141,11 @@ static int realm_ioremap_hook(phys_addr_t phys, size_t size, pgprot_t *prot)
>> return 0;
>> }
>>
>> +unsigned long realm_get_hyp_pagesize(void)
>> +{
>> + return ipa_change_alignment;
>> +}
>
> Again, this has nothing to do with the hypervisor, but the host. And
> ipa_change_alignment is still a wording I can't wrap my small head
> around.
>
>> +
>> void __init arm64_rsi_init(void)
>> {
>> if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
>> @@ -147,6 +154,12 @@ void __init arm64_rsi_init(void)
>> return;
>> if (WARN_ON(rsi_get_realm_config(&config)))
>> return;
>> +
>> + ipa_change_alignment = rhi_get_ipa_change_alignment();
>> + /* If we don't get a correct alignment response, don't enable realm */
>> + if (!ipa_change_alignment)
>> + return;
>
> But at the same time, you override a global value with an error, and
> then paper over it in mem_decrypt_granule_size()...
>
I believe I received similar feedback on my previous version as well,
which I didn’t quite follow.
rhi_get_ipa_change_alignment() only returns an error when the host
returns a size that is not 4K-aligned. Otherwise, it returns the
host-determined size, or defaults to guest PAGE_SIZE if the RHI call
itself is not supported.
ipa_change_align = hyp_pagesize_rhicall.gprs[0];
/* This error needs special handling in the caller */
if (ipa_change_align & (SZ_4K - 1))
return 0;
return ipa_change_align;
err_out:
/*
* For failure condition assume host is built with 4K page size
* and hence ipa change alignment can be guest PAGE_SIZE.
*/
return PAGE_SIZE;
>
>> +
>> prot_ns_shared = __phys_to_pte_val(BIT(config.ipa_bits - 1));
>>
>> if (arm64_ioremap_prot_hook_register(realm_ioremap_hook))
>> diff --git a/arch/arm64/mm/mem_encrypt.c b/arch/arm64/mm/mem_encrypt.c
>> index 38c62c9e4e74..f5d64bc29c20 100644
>> --- a/arch/arm64/mm/mem_encrypt.c
>> +++ b/arch/arm64/mm/mem_encrypt.c
>> @@ -59,3 +59,11 @@ int set_memory_decrypted(unsigned long addr, int numpages)
>> return crypt_ops->decrypt(addr, numpages);
>> }
>> EXPORT_SYMBOL_GPL(set_memory_decrypted);
>> +
>> +size_t mem_decrypt_granule_size(void)
>> +{
>> + if (is_realm_world())
>> + return max(PAGE_SIZE, realm_get_hyp_pagesize());
>
> If you didn't mess with ipa_change_alignment above, you shouldn't need
> this max().
>
size_t mem_decrypt_granule_size(void)
{
if (is_realm_world())
return max(PAGE_SIZE, realm_get_hyp_pagesize());
return PAGE_SIZE;
}
That needs to use max(), because we should align to the guest PAGE_SIZE
if it is larger than the host-specified alignment value.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 2/3] swiotlb: dma: its: Enforce host page-size alignment for shared buffers
From: Aneesh Kumar K.V @ 2026-04-28 12:22 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Marc Zyngier, Marek Szyprowski, Robin Murphy,
Steven Price, Suzuki K Poulose, Thomas Gleixner, Will Deacon
In-Reply-To: <20260427134903.GA740385@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Mon, Apr 27, 2026 at 12:01:07PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> When running private-memory guests, the guest kernel must apply additional
>> constraints when allocating buffers that are shared with the hypervisor.
>
> This patch has way too much stuff in it.
>
> I think your patch structure should be changed around
>
> 1) Patch to add mem_decrypt_granule_size(), and explain it as
> the alignment & size of what can be passed to
> set_memory_encrypted/decrypted()
>
> 2) Add support for mem_decrypt_granule_size() to ARM
>
> Then patches going caller by caller of set_memory_decrypted() to make
> them follow the new rule:
>
> 3) its
>
> 4) swiotlb
>
> 3) dma_alloc_coherent
>
> etc.
>
> don't forget about the new dma buf heaps too:
>
> drivers/dma-buf/heaps/system_heap.c: ret = set_memory_decrypted(addr, nr_pages);
>
> It is worth calling out in the cover letter that all the ARM CCA
> relevant places are fixed but drivers/hv/ is left for future.
>
>> @@ -33,18 +32,30 @@ int arm64_mem_crypt_ops_register(const struct arm64_mem_crypt_ops *ops)
>>
>> int set_memory_encrypted(unsigned long addr, int numpages)
>> {
>> - if (likely(!crypt_ops) || WARN_ON(!PAGE_ALIGNED(addr)))
>> + if (likely(!crypt_ops))
>> return 0;
>>
>> + if (WARN_ON(!IS_ALIGNED(addr, mem_decrypt_granule_size())))
>> + return -EINVAL;
>> +
>> + if (WARN_ON(!IS_ALIGNED(numpages << PAGE_SHIFT, mem_decrypt_granule_size())))
>> + return -EINVAL;
>> +
>> return crypt_ops->encrypt(addr, numpages);
>> }
>> EXPORT_SYMBOL_GPL(set_memory_encrypted);
>>
>> int set_memory_decrypted(unsigned long addr, int numpages)
>> {
>> - if (likely(!crypt_ops) || WARN_ON(!PAGE_ALIGNED(addr)))
>> + if (likely(!crypt_ops))
>> return 0;
>>
>> + if (WARN_ON(!IS_ALIGNED(addr, mem_decrypt_granule_size())))
>> + return -EINVAL;
>> +
>> + if (WARN_ON(!IS_ALIGNED(numpages << PAGE_SHIFT, mem_decrypt_granule_size())))
>> + return -EINVAL;
>> +
>> return crypt_ops->decrypt(addr, numpages);
>> }
>> EXPORT_SYMBOL_GPL(set_memory_decrypted);
>
> This should go in the ARM patch adding mem_decrypt_granule_size() to CCA
>
>> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
>> index 07584c5e36fb..1e01c9ac697f 100644
>> --- a/include/linux/mem_encrypt.h
>> +++ b/include/linux/mem_encrypt.h
>> @@ -11,6 +11,8 @@
>> #define __MEM_ENCRYPT_H__
>>
>> #ifndef __ASSEMBLY__
>> +#include <linux/align.h>
>> +#include <vdso/page.h>
>>
>> #ifdef CONFIG_ARCH_HAS_MEM_ENCRYPT
>>
>> @@ -54,6 +56,18 @@
>> #define dma_addr_canonical(x) (x)
>> #endif
>>
>> +#ifndef mem_decrypt_granule_size
>> +static inline size_t mem_decrypt_granule_size(void)
>> +{
>> + return PAGE_SIZE;
>> +}
>> +#endif
>> +
>> +static inline size_t mem_decrypt_align(size_t size)
>> +{
>> + return ALIGN(size, mem_decrypt_granule_size());
>> +}
>> +
>> #endif /* __ASSEMBLY__ */
>>
>> #endif /* __MEM_ENCRYPT_H__ */
>
> I know it seems a bit small, but put this in its own patch and explain
> how it works. I'd also like to see a kdoc here, and add a kdoc to
> set_memory_decrypted() that links back so people have a better chance
> to know about this.
>
Okay, I’ll update all the above in the next revision.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 2/3] swiotlb: dma: its: Enforce host page-size alignment for shared buffers
From: Aneesh Kumar K.V @ 2026-04-28 12:20 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, iommu, linux-coco, linux-arm-kernel, kvmarm,
Catalin Marinas, Jason Gunthorpe, Marek Szyprowski, Robin Murphy,
Steven Price, Suzuki K Poulose, Thomas Gleixner, Will Deacon
In-Reply-To: <86zf2ozrb8.wl-maz@kernel.org>
Marc Zyngier <maz@kernel.org> writes:
> On Mon, 27 Apr 2026 07:31:07 +0100,
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
>>
>> When running private-memory guests, the guest kernel must apply additional
>> constraints when allocating buffers that are shared with the hypervisor.
>>
>> These shared buffers are also accessed by the host kernel and therefore
>> must be aligned to the host’s page size, and have a size that is a multiple
>> of the host page size.
>>
>> On non-secure hosts, set_guest_memory_attributes() tracks memory at the
>> host PAGE_SIZE granularity. This creates a mismatch when the guest applies
>> attributes at 4K boundaries while the host uses 64K pages. In such cases,
>> set_guest_memory_attributes() call returns -EINVAL, preventing the
>> conversion of memory regions from private to shared.
>>
>> Architectures such as Arm can tolerate realm physical address space
>> (protected memory) PFNs being mapped as shared memory, as incorrect
>> accesses are detected and reported as GPC faults. However, relying on this
>> mechanism is unsafe and can still lead to kernel crashes.
>>
>> This is particularly likely when guest_memfd allocations are mmapped and
>> accessed from userspace. Once exposed to userspace, we cannot guarantee
>> that applications will only access the intended 4K shared region rather
>> than the full 64K page mapped into their address space. Such userspace
>> addresses may also be passed back into the kernel and accessed via the
>> linear map, resulting in a GPC fault and a kernel crash.
>>
>> With CCA, although Stage-2 mappings managed by the RMM still operate at a
>> 4K granularity, shared pages must nonetheless be aligned to the
>> host-managed page size and sized as whole host pages to avoid the issues
>> described above.
>
> I thought that was being fixed, and that there was now a strong
> guarantee that RMM and host are aligned on the page size. Even more,
> S2 is totally irrelevant here. The only thing that matters is the host
> page size vs the guest page size. Nothing else.
>
Yes, the latest RMM update includes the ability to change the granule
size.
The section above in the commit message was intended to explain that the
S2 mapping size is irrelevant. I agree it is not clear as written, so I
will reword it to improve clarity.
>
>>
>> Introduce a new helper, mem_decrypt_align(), to allow callers to enforce
>> the required alignment and size constraints for shared buffers.
>>
>> The architecture-specific implementation of mem_decrypt_align() will be
>> provided in a follow-up patch.
>>
>> Note on restricted-dma-pool:
>> rmem_swiotlb_device_init() uses reserved-memory regions described by
>> firmware. Those regions are not changed in-kernel to satisfy host granule
>> alignment. This is intentional: we do not expect restricted-dma-pool
>> allocations to be used with CCA. If restricted-dma-pool is intended for CCA
>> shared use, firmware must provide base/size aligned to the host IPA-change
>> granule.
>>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> arch/arm64/mm/mem_encrypt.c | 19 +++++++++++++++----
>> drivers/irqchip/irq-gic-v3-its.c | 20 +++++++++++++-------
>> include/linux/mem_encrypt.h | 14 ++++++++++++++
>> kernel/dma/contiguous.c | 10 ++++++++++
>> kernel/dma/direct.c | 16 ++++++++++++++--
>> kernel/dma/pool.c | 4 +++-
>> kernel/dma/swiotlb.c | 21 +++++++++++++--------
>> 7 files changed, 82 insertions(+), 22 deletions(-)
>>
>
> [...]
>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
>> index 291d7668cc8d..239d7e3bc16f 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -213,16 +213,17 @@ static gfp_t gfp_flags_quirk;
>> static struct page *its_alloc_pages_node(int node, gfp_t gfp,
>> unsigned int order)
>> {
>> + unsigned int new_order;
>> struct page *page;
>> int ret = 0;
>>
>> - page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
>> -
>> + new_order = get_order(mem_decrypt_align((PAGE_SIZE << order)));
>> + page = alloc_pages_node(node, gfp | gfp_flags_quirk, new_order);
>> if (!page)
>> return NULL;
>>
>> ret = set_memory_decrypted((unsigned long)page_address(page),
>> - 1 << order);
>> + 1 << new_order);
>> /*
>> * If set_memory_decrypted() fails then we don't know what state the
>> * page is in, so we can't free it. Instead we leak it.
>> @@ -241,13 +242,16 @@ static struct page *its_alloc_pages(gfp_t gfp, unsigned int order)
>>
>> static void its_free_pages(void *addr, unsigned int order)
>> {
>> + int new_order;
>> +
>> + new_order = get_order(mem_decrypt_align((PAGE_SIZE << order)));
>> /*
>> * If the memory cannot be encrypted again then we must leak the pages.
>> * set_memory_encrypted() will already have WARNed.
>> */
>> - if (set_memory_encrypted((unsigned long)addr, 1 << order))
>> + if (set_memory_encrypted((unsigned long)addr, 1 << new_order))
>> return;
>> - free_pages((unsigned long)addr, order);
>> + free_pages((unsigned long)addr, new_order);
>> }
>>
>
> Here's the non-obfuscated version of the two hunks above (and let it
> be on the record that New Order is a terrible, overrated band):
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 291d7668cc8da..a4d555aaee241 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -216,6 +216,7 @@ static struct page *its_alloc_pages_node(int node, gfp_t gfp,
> struct page *page;
> int ret = 0;
>
> + order = get_order(mem_decrypt_align(PAGE_SIZE << order));
> page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
>
> if (!page)
> @@ -245,6 +246,7 @@ static void its_free_pages(void *addr, unsigned int order)
> * If the memory cannot be encrypted again then we must leak the pages.
> * set_memory_encrypted() will already have WARNed.
> */
> + order = get_order(mem_decrypt_align(PAGE_SIZE << order));
> if (set_memory_encrypted((unsigned long)addr, 1 << order))
> return;
> free_pages((unsigned long)addr, order);
>
I will include this in the next revision.
>> static struct gen_pool *itt_pool;
>> @@ -268,11 +272,13 @@ static void *itt_alloc_pool(int node, int size)
>> if (addr)
>> break;
>>
>> - page = its_alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
>> + page = its_alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO,
>> + get_order(mem_decrypt_granule_size()));
>
> You already taught its_alloc_pages_node() about the decrypt granule
> size stuff. I don't think we need to see more of it (and you don't
> mess with the call that is just above it).
>
>> if (!page)
>> break;
>>
>> - gen_pool_add(itt_pool, (unsigned long)page_address(page), PAGE_SIZE, node);
>> + gen_pool_add(itt_pool, (unsigned long)page_address(page),
>> + mem_decrypt_granule_size(), node);
>
> I'd rather see something like mem_decrypt_align(PAGE_SIZE), which
> keeps the intent clear.
>
The helper was added based on feedback from a previous version. I assume
you are suggesting that only this caller should switch?
-aneesh
^ permalink raw reply
* Re: [PATCH v8 01/21] x86/virt/tdx: Move low level SEAMCALL helpers out of <asm/tdx.h>
From: Vishal Annapurve @ 2026-04-27 18:12 UTC (permalink / raw)
To: Chao Gao
Cc: kvm, linux-coco, linux-kernel, x86, binbin.wu, dave.hansen, djbw,
ira.weiny, kai.huang, kas, nik.borisov, paulmck, pbonzini,
reinette.chatre, rick.p.edgecombe, sagis, seanjc, tony.lindgren,
vishal.l.verma, yilun.xu, xiaoyao.li, yan.y.zhao, Zhenzhong Duan,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-2-chao.gao@intel.com>
On Mon, Apr 27, 2026 at 8:30 AM Chao Gao <chao.gao@intel.com> wrote:
>
> From: Kai Huang <kai.huang@intel.com>
>
> TDX host core code implements three seamcall*() helpers to make SEAMCALLs
> to the TDX module. Currently, they are implemented in <asm/tdx.h> and
> are exposed to other kernel code which includes <asm/tdx.h>.
>
> However, other than the TDX host core, seamcall*() are not expected to
> be used by other kernel code directly. For instance, for all SEAMCALLs
> that are used by KVM, the TDX host core exports a wrapper function for
> each of them.
>
> Move seamcall*() and related code out of <asm/tdx.h> and make them only
> visible to TDX host core.
>
> Since TDX host core tdx.c is already very heavy, don't put low level
> seamcall*() code there but to a new dedicated "seamcall_internal.h". Also,
> currently tdx.c has seamcall_prerr*() helpers which additionally print
> error message when calling seamcall*() fails. Move them to
> "seamcall_internal.h" as well. In such way all low level SEAMCALL helpers
> are in a dedicated place, which is much more readable.
>
> Copy the copyright notice from the original files and consolidate the
> date ranges to:
>
> Copyright (C) 2021-2023 Intel Corporation
>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Vishal Annapurve <vannapurve@google.com>
^ permalink raw reply
* [PATCH v8 21/21] x86/virt/tdx: Document TDX module update
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86, linux-doc
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, Jonathan Corbet,
Shuah Khan
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
Document TDX module update as a subsection of "TDX Host Kernel Support" to
provide background information and cover key points that developers and
users may need to know, for example:
- update is done in stop_machine() context
- update instructions and results
- update policy and tooling
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
Documentation/arch/x86/tdx.rst | 36 ++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
index ff6b110291bc..de6b99d9afa7 100644
--- a/Documentation/arch/x86/tdx.rst
+++ b/Documentation/arch/x86/tdx.rst
@@ -73,6 +73,42 @@ initialize::
[..] virt/tdx: TDX-Module initialization failed ...
+TDX module Runtime Update
+-------------------------
+
+The TDX architecture includes a persistent SEAM loader (P-SEAMLDR) that
+runs in SEAM mode separately from the TDX module. The kernel can
+communicate with P-SEAMLDR to perform runtime updates of the TDX module.
+
+During update, the TDX module becomes unresponsive to other TDX operations.
+To prevent components using TDX (such as KVM) from experiencing unexpected
+errors during updates, updates are performed in stop_machine() context.
+
+TDX module update has complex compatibility requirements; the new module
+must be compatible with the current CPU, P-SEAMLDR, and running TDX module.
+Rather than implementing complex module selection and policy enforcement
+logic in the kernel, userspace is responsible for auditing and selecting
+appropriate updates.
+
+Updates use the standard firmware upload interface. See
+Documentation/driver-api/firmware/fw_upload.rst for detailed instructions.
+
+Successful updates are logged in dmesg:
+ [..] virt/tdx: version 1.5.20 -> 1.5.24
+
+If updates failed, running TDs may be killed and further TDX operations may
+not be possible until reboot. For detailed error information, see
+Documentation/ABI/testing/sysfs-devices-faux-tdx-host.
+
+Given the risk of losing existing TDs, userspace should verify that the
+update is compatible with the current system and properly validated before
+applying it.
+
+A reference userspace tool that implements necessary checks is available
+at:
+
+ https://github.com/intel/tdx-module-binaries
+
TDX Interaction to Other Kernel Components
------------------------------------------
--
2.47.1
^ permalink raw reply related
* [PATCH v8 20/21] coco/tdx-host: Document TDX module update compatibility criteria
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Dan Williams
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
The TDX module update protocol facilitates compatible runtime updates.
Document the compatibility criteria and indicators of update failures.
Note that runtime TDX module updates are an "update at your own risk"
operation; userspace is responsible for ensureing that the update meets
the compatibility criteria.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
v8:
- Do not map -EIO and -ENOMEM to separate fw_upload errors. There is no
current need to distinguish them in the userspace ABI, and fw_upload
has no matching error code for -ENOMEM.
- some wording changes.
---
.../ABI/testing/sysfs-devices-faux-tdx-host | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index 65897fe6abc0..ff585c79aa6e 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -26,3 +26,42 @@ Description: (RO) Report the number of remaining updates. TDX maintains a
See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
Interface Specification, Chapter "SEAMLDR_INFO" and Chapter
"SEAMLDR.INSTALL" for more information.
+
+What: /sys/devices/faux/tdx_host/firmware/tdx_module
+Contact: linux-coco@lists.linux.dev
+Description: (Directory) The tdx_module directory implements the fw_upload
+ sysfs ABI, see Documentation/ABI/testing/sysfs-class-firmware
+ for the general description of the attributes @data, @cancel,
+ @error, @loading, @remaining_size, and @status. This ABI
+ facilitates "Compatible TDX module Updates". A compatible update
+ is one that meets the following criteria:
+
+ Does not interrupt or interfere with any current TDX
+ operation or TD VM.
+
+ Does not invalidate any previously consumed module metadata
+ values outside of the TEE_TCB_SVN_2 field (updated Security
+ Version Number) in TD Quotes.
+
+ Does not require validation of new module metadata fields. By
+ implication, new module features and capabilities are only
+ available by installing the module at reboot (BIOS or EFI
+ helper loaded).
+
+ See tdx_host/firmware/tdx_module/error for information on
+ update failure indicators.
+
+What: /sys/devices/faux/tdx_host/firmware/tdx_module/error
+Contact: linux-coco@lists.linux.dev
+Description: (RO) See Documentation/ABI/testing/sysfs-class-firmware for
+ baseline expectations for this file. The <ERROR> part in the
+ <STATUS>:<ERROR> format can be:
+
+ "device-busy": The update conflicts with an in-progress TDX
+ operation.
+
+ "firmware-invalid": The update failed for any other reason.
+
+ A "firmware-invalid" result may be fatal. If the TDX module is
+ lost, further TDX operation is not possible, and reading
+ /sys/devices/faux/tdx_host/version returns -ENXIO.
--
2.47.1
^ permalink raw reply related
* [PATCH v8 19/21] x86/virt/tdx: Enable TDX module runtime updates
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
All pieces of TDX module runtime updates are in place. Enable it if it
is supported.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/tdx.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index b063aabe2554..b9eb1da4f36c 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -36,6 +36,7 @@
#define TDX_UPDATE_COMPAT_SENSITIVE 0x8000051200000000ULL
/* Bit definitions of TDX_FEATURES0 metadata field */
+#define TDX_FEATURES0_TD_PRESERVING BIT_ULL(1)
#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
#define TDX_FEATURES0_UPDATE_COMPAT BIT_ULL(47)
@@ -117,8 +118,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
{
- /* To be enabled when kernel is ready. */
- return false;
+ return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
}
int tdx_guest_keyid_alloc(void);
--
2.47.1
^ permalink raw reply related
* [PATCH v8 18/21] coco/tdx-host: Don't expose P-SEAMLDR features on CPUs with erratum
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
Some TDX-capable CPUs have an erratum, as documented in Intel® Trust
Domain CPU Architectural Extensions (May 2021 edition) Chapter 2.3:
SEAMRET from the P-SEAMLDR clears the current VMCS structure pointed
to by the current-VMCS pointer. A VMM that invokes the P-SEAMLDR using
SEAMCALL must reload the current-VMCS, if required, using the VMPTRLD
instruction.
Clearing the current VMCS behind KVM's back will break KVM.
This erratum is not present when IA32_VMX_BASIC[60] is set. Add a CPU
bug bit for this erratum and refuse to expose P-SEAMLDR features (e.g.,
TDX module updates) on affected CPUs.
== Alternatives ==
Two workarounds were considered but both were rejected:
1. Save/restore the current VMCS around P-SEAMLDR calls. This produces ugly
assembly code [1] and doesn't play well with #MCE or #NMI if they
need to use the current VMCS.
2. Move KVM's VMCS tracking logic to the TDX core code, which would break
the boundary between KVM and the TDX core code [2].
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Link: https://lore.kernel.org/kvm/fedb3192-e68c-423c-93b2-a4dc2f964148@intel.com/ # [1]
Link: https://lore.kernel.org/kvm/aYIXFmT-676oN6j0@google.com/ # [2]
---
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/vmx.h | 1 +
arch/x86/virt/vmx/tdx/tdx.c | 11 +++++++++++
drivers/virt/coco/tdx-host/tdx-host.c | 8 ++++++++
4 files changed, 21 insertions(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 1d506e5d6f46..7b572bc24265 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -573,4 +573,5 @@
#define X86_BUG_ITS_NATIVE_ONLY X86_BUG( 1*32+ 8) /* "its_native_only" CPU is affected by ITS, VMX is not affected */
#define X86_BUG_TSA X86_BUG( 1*32+ 9) /* "tsa" CPU is affected by Transient Scheduler Attacks */
#define X86_BUG_VMSCAPE X86_BUG( 1*32+10) /* "vmscape" CPU is affected by VMSCAPE attacks from guests */
+#define X86_BUG_SEAMRET_INVD_VMCS X86_BUG( 1*32+11) /* "seamret_invd_vmcs" SEAMRET from P-SEAMLDR clears the current VMCS */
#endif /* _ASM_X86_CPUFEATURES_H */
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 37080382df54..49d8551d285d 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -147,6 +147,7 @@ struct vmcs {
#define VMX_BASIC_INOUT BIT_ULL(54)
#define VMX_BASIC_TRUE_CTLS BIT_ULL(55)
#define VMX_BASIC_NO_HW_ERROR_CODE_CC BIT_ULL(56)
+#define VMX_BASIC_NO_SEAMRET_INVD_VMCS BIT_ULL(60)
static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
{
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 7864ab68f4e3..23c5403b3691 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -41,6 +41,7 @@
#include <asm/processor.h>
#include <asm/mce.h>
#include <asm/virt.h>
+#include <asm/vmx.h>
#include "seamcall_internal.h"
#include "tdx.h"
@@ -1495,6 +1496,8 @@ static struct notifier_block tdx_memory_nb = {
static void __init check_tdx_erratum(void)
{
+ u64 basic_msr;
+
/*
* These CPUs have an erratum. A partial write from non-TD
* software (e.g. via MOVNTI variants or UC/WC mapping) to TDX
@@ -1506,6 +1509,14 @@ static void __init check_tdx_erratum(void)
case INTEL_EMERALDRAPIDS_X:
setup_force_cpu_bug(X86_BUG_TDX_PW_MCE);
}
+
+ /*
+ * Some TDX-capable CPUs have an erratum where the current VMCS is
+ * cleared after calling into P-SEAMLDR.
+ */
+ rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
+ if (!(basic_msr & VMX_BASIC_NO_SEAMRET_INVD_VMCS))
+ setup_force_cpu_bug(X86_BUG_SEAMRET_INVD_VMCS);
}
void __init tdx_init(void)
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 14f861c03be4..24bd660ca2cd 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -92,6 +92,14 @@ static bool can_expose_seamldr(void)
if (!sysinfo)
return 0;
+ /*
+ * Calling P-SEAMLDR on CPUs with the seamret_invd_vmcs bug clears
+ * the current VMCS, which breaks KVM. Verify the erratum is not
+ * present before exposing P-SEAMLDR features.
+ */
+ if (boot_cpu_has_bug(X86_BUG_SEAMRET_INVD_VMCS))
+ return false;
+
return tdx_supports_runtime_update(sysinfo);
}
--
2.47.1
^ permalink raw reply related
* [PATCH v8 17/21] x86/virt/seamldr: Abort updates on failure
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
TDX module update is a multi-step process, and each step can fail.
The current update flow continues to later steps after an error.
Continuing after failure leaves the TDX module in an unrecoverable
state.
One failure case must remain recoverable: update contention with an ongoing
TD build. The agreed kernel behavior for this case is to fail the update
with -EBUSY so userspace can retry later.
Abort the update on any failure. For the contention case, this provides
a recoverable failure mode because the failure occurs before any TDX
module state is changed. Use the same rule for all errors to avoid
special-casing -EBUSY.
Introduce a shared "failed" flag. When a CPU fails, set the flag and force
all CPUs to exit the update loop. A failing CPU does not acknowledge the
current step, so other CPUs remain at that step until they observe the
"failed" flag and exit.
Use READ_ONCE()/WRITE_ONCE() for the flag because it is used for lockless
communication between stop_machine workers. Also use WRITE_ONCE() for the
initial clear to keep accesses to the flag uniform and explicit.
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>
---
v8:
- Explain why aborting updates is necessary. [Rick]
- always use READ_ONCE()/WRITE_ONCE() for the "failed" flag.
---
arch/x86/virt/vmx/tdx/seamldr.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index c81b26c4bac1..9b8f571eb03f 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -220,6 +220,7 @@ enum module_update_state {
static struct {
enum module_update_state state;
int thread_ack;
+ bool failed;
/*
* Protect update_data. Raw spinlock as it will be acquired from
* interrupt-disabled contexts.
@@ -284,12 +285,15 @@ static int do_seamldr_install_module(void *seamldr_params)
break;
}
- ack_state();
+ if (ret)
+ WRITE_ONCE(update_data.failed, true);
+ else
+ ack_state();
} else {
touch_nmi_watchdog();
rcu_momentary_eqs();
}
- } while (curstate != MODULE_UPDATE_DONE);
+ } while (curstate != MODULE_UPDATE_DONE && !READ_ONCE(update_data.failed));
return ret;
}
@@ -315,6 +319,7 @@ int seamldr_install_module(const u8 *data, u32 size)
/* Ensure a stable set of online CPUs for the update process. */
guard(cpus_read_lock)();
+ WRITE_ONCE(update_data.failed, false);
set_target_state(MODULE_UPDATE_START + 1);
ret = stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
if (ret)
--
2.47.1
^ permalink raw reply related
* [PATCH v8 16/21] x86/virt/tdx: Reject updates during concurrent TD build
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
tl;dr: A TDX module erratum can silently corrupt TD measurement state if a
module update races with TD build. Handle that by rejecting the update,
instead of introducing new TD-build ioctl failure paths.
Long Version:
Updates must not break unrelated operations. For TDX module updates,
this means an update must not interfere with other TDX flows.
A TDX module erratum violates that expectation: if an update races with
TD build, TD build output can be corrupted, e.g. the measurement hash,
which later causes attestation failure.
The TDX module provides two independent, opt-in mitigations for this
erratum:
1. Reject updates while TD build is in progress. This mitigation can be
requested via TDH.SYS.SHUTDOWN.
2. Do not reject the update for this race, but instead fail later
SEAMCALLs in the overlapping TD build flow. This mitigation can be
requested via TDH.SYS.UPDATE.
The kernel can choose option 1, option 2, or neither.
Choose option 1 to confine failures to the update path and preserve
existing TD build and KVM ioctl behavior. Userspace already controls
update timing, and retrying a rejected update is straightforward.
Option 2 would make TD build failures explicit, but it would also
introduce new error paths in existing KVM ioctls. That complicates KVM
error handling and risks ABI instability. Sean previously rejected that
approach [1].
Choosing neither option was also considered and rejected. Leaving this
erratum unhandled would allow an update racing with TD build to silently
corrupt TD build output. That violates the requirement that TDX module
updates must not interfere with unrelated TDX flows.
Request race detection during TDH.SYS.SHUTDOWN and map a detected race
to -EBUSY, and report it to userspace as FW_UPLOAD_ERR_BUSY. This lets
userspace distinguish the race from other failures and retry the update.
Do not pre-check support for this race-detection capability. If it is
unsupported, rely on the TDX module to reject module shutdown.
This implementation is based on a reference patch by Vishal [2].
Note: moving NO_RBP_MOD definition is to centralize the bit definitions.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Acked-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/linux-coco/aQIbM5m09G0FYTzE@google.com/ # [1]
Link: https://lore.kernel.org/linux-coco/CAGtprH_oR44Vx9Z0cfxvq5-QbyLmy_+Gn3tWm3wzHPmC1nC0eg@mail.gmail.com/ # [2]
---
v8:
- rewrite the changelog [Rick]
- alway pass the compat flag to the TDX module [Rick]
---
arch/x86/include/asm/tdx.h | 11 +++++++++--
arch/x86/kvm/vmx/tdx_errno.h | 2 --
arch/x86/virt/vmx/tdx/tdx.c | 26 +++++++++++++++++++++++---
arch/x86/virt/vmx/tdx/tdx.h | 3 ---
drivers/virt/coco/tdx-host/tdx-host.c | 2 ++
5 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index de822ed9ef0b..b063aabe2554 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -26,11 +26,18 @@
#define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP)
#define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)
+#define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
+
/*
* TDX module SEAMCALL leaf function error codes
*/
-#define TDX_SUCCESS 0ULL
-#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
+#define TDX_SUCCESS 0ULL
+#define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
+#define TDX_UPDATE_COMPAT_SENSITIVE 0x8000051200000000ULL
+
+/* Bit definitions of TDX_FEATURES0 metadata field */
+#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
+#define TDX_FEATURES0_UPDATE_COMPAT BIT_ULL(47)
#ifndef __ASSEMBLER__
diff --git a/arch/x86/kvm/vmx/tdx_errno.h b/arch/x86/kvm/vmx/tdx_errno.h
index 6ff4672c4181..215c00d76a94 100644
--- a/arch/x86/kvm/vmx/tdx_errno.h
+++ b/arch/x86/kvm/vmx/tdx_errno.h
@@ -4,8 +4,6 @@
#ifndef __KVM_X86_TDX_ERRNO_H
#define __KVM_X86_TDX_ERRNO_H
-#define TDX_SEAMCALL_STATUS_MASK 0xFFFFFFFF00000000ULL
-
/*
* TDX SEAMCALL Status Codes (returned in RAX)
*/
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index a7dfa4ee8813..7864ab68f4e3 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1234,10 +1234,13 @@ static __init int tdx_enable(void)
}
subsys_initcall(tdx_enable);
+#define TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE BIT(16)
+
int tdx_module_shutdown(void)
{
struct tdx_sys_info_handoff handoff = {};
struct tdx_module_args args = {};
+ u64 err;
int ret, cpu;
ret = get_tdx_sys_info_handoff(&handoff);
@@ -1248,9 +1251,26 @@ int tdx_module_shutdown(void)
* module can produce and most likely supported by newer modules.
*/
args.rcx = handoff.module_hv;
- ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
- if (ret)
- return ret;
+
+ /*
+ * Mitigate the erratum where updates can break concurrent TD
+ * build. Do not pre-check support for this flag. If unsupported,
+ * rely on the TDX module to reject shutdown requests.
+ */
+ args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
+
+ err = seamcall(TDH_SYS_SHUTDOWN, &args);
+
+ /*
+ * Return -EBUSY to signal that some ongoing flows are incompatible
+ * with updates so that userspace can retry.
+ */
+ if ((err & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
+ return -EBUSY;
+ if (err) {
+ seamcall_err(TDH_SYS_SHUTDOWN, err, &args);
+ return -EIO;
+ }
/*
* Clear global and per-CPU initialization flags so the new module
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index d0e8cac9c1d5..2c8b64eeea8e 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -86,9 +86,6 @@ struct tdmr_info {
DECLARE_FLEX_ARRAY(struct tdmr_reserved_area, reserved_areas);
} __packed __aligned(TDMR_INFO_ALIGNMENT);
-/* Bit definitions of TDX_FEATURES0 metadata field */
-#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
-
/*
* Do not put any hardware-defined TDX structure representations below
* this comment!
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index d9bb1e7ef795..14f861c03be4 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -127,6 +127,8 @@ static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
case 0:
*written = size;
return FW_UPLOAD_ERR_NONE;
+ case -EBUSY:
+ return FW_UPLOAD_ERR_BUSY;
default:
return FW_UPLOAD_ERR_FW_INVALID;
}
--
2.47.1
^ permalink raw reply related
* [PATCH v8 14/21] x86/virt/tdx: Restore TDX module state
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
TDX module state was packed as handoff data during module shutdown. After
per-CPU initialization, the new module can restore TDX module state from
handoff data to preserve running TDs.
Once the restoration is done, the TDX module update is complete, which
means the new module is ready to handle requests from the host and guests.
Implement the new TDH.SYS.UPDATE SEAMCALL to restore TDX module state
and invoke it on one CPU since it only needs to be called once.
For error handling, Intel® Trust Domain Extensions (Intel® TDX)
Module Base Architecture Specification, Chapter "Restore TDX Module
State after a TD-Preserving Update" states
If TDH.SYS.UPDATE returns an error, then the host VMM can continue
with the non-update sequence (TDH.SYS.CONFIG, TDH.SYS.KEY.CONFIG
etc.). In this case all existing TDs are lost. Alternatively, the host
VMM can request the P-SEAMLDR to update to another TDX module. If that
update is successful, existing TDs are preserved.
Given the complexity and uncertain value of above recovery paths, simply
propagate errors.
Also note that the location and the format of handoff data is defined by
the TDX module. The new module knows where to get handoff data and how
to parse it. The kernel doesn't need to provide its location, format etc.
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: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v8:
- don't add a duplicate error code as seamcal_prerr() will do that
- don't reset tdx module status to ERORR on error
---
arch/x86/virt/vmx/tdx/seamldr.c | 5 +++++
arch/x86/virt/vmx/tdx/tdx.c | 13 +++++++++++++
arch/x86/virt/vmx/tdx/tdx.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 04c7a87ac7df..98a8d9d3ae25 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -213,6 +213,7 @@ enum module_update_state {
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_CPU_INIT,
+ MODULE_UPDATE_RUN_UPDATE,
MODULE_UPDATE_DONE,
};
@@ -275,6 +276,10 @@ static int do_seamldr_install_module(void *seamldr_params)
case MODULE_UPDATE_CPU_INIT:
ret = tdx_cpu_enable();
break;
+ case MODULE_UPDATE_RUN_UPDATE:
+ if (primary)
+ ret = tdx_module_run_update();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 3bbb12aefb4b..9e4085a1e683 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1266,6 +1266,19 @@ int tdx_module_shutdown(void)
return 0;
}
+int tdx_module_run_update(void)
+{
+ struct tdx_module_args args = {};
+ int ret;
+
+ ret = seamcall_prerr(TDH_SYS_UPDATE, &args);
+ if (ret)
+ return ret;
+
+ tdx_module_initialized = true;
+ return 0;
+}
+
static bool is_pamt_page(unsigned long phys)
{
struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 36afebf0e04b..5fef813002c2 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -47,6 +47,7 @@
#define TDH_VP_WR 43
#define TDH_SYS_CONFIG 45
#define TDH_SYS_SHUTDOWN 52
+#define TDH_SYS_UPDATE 53
/*
* SEAMCALL leaf:
@@ -112,5 +113,6 @@ struct tdmr_info_list {
};
int tdx_module_shutdown(void);
+int tdx_module_run_update(void);
#endif
--
2.47.1
^ permalink raw reply related
* [PATCH v8 15/21] x86/virt/tdx: Refresh TDX module version after update
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
The kernel exposes the TDX module version through sysfs so userspace can
check update compatibility. That information needs to remain accurate
across runtime updates.
A runtime update may change the module's update_version, so refresh the
cached version after a successful update and emit a log message to show
the version change.
Drop __ro_after_init from tdx_sysinfo because it is now updated at runtime.
Perform the refresh outside of stop_machine() since printk() within
stop_machine() would add significant latency.
Do not refresh the rest of tdx_sysinfo. Refreshing them at runtime could
disrupt running software that relies on the previously reported values.
Note that major and minor versions are not refreshed because runtime updates
are supported only between releases with identical major and minor versions.
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
Sashiko flagged a potential torn-read concern: update_version is read
via sysfs while it is refreshed post-update. But, update_version is a
naturally-aligned u16, and on x86, the compiler won't split aligned u16
accesses. So READ_ONCE/WRITE_ONCE() aren't needed for update_version.
v8:
- drop the unnecessary old/new metadata comparison
- do not refresh the handoff version, since it is no longer cached
- rename the helper to reflect its purpose instead of using the generic
tdx_module_post_update()
---
arch/x86/virt/vmx/tdx/seamldr.c | 8 +++++++-
arch/x86/virt/vmx/tdx/tdx.c | 21 ++++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 1 +
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 2 +-
4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 98a8d9d3ae25..c81b26c4bac1 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -306,6 +306,8 @@ DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
*/
int seamldr_install_module(const u8 *data, u32 size)
{
+ int ret;
+
struct seamldr_params *params __free(free_seamldr_params) =
init_seamldr_params(data, size);
if (IS_ERR(params))
@@ -314,6 +316,10 @@ int seamldr_install_module(const u8 *data, u32 size)
/* Ensure a stable set of online CPUs for the update process. */
guard(cpus_read_lock)();
set_target_state(MODULE_UPDATE_START + 1);
- return stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
+ ret = stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
+ if (ret)
+ return ret;
+
+ return tdx_module_refresh_version();
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 9e4085a1e683..a7dfa4ee8813 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -61,7 +61,7 @@ static int sysinit_ret;
/* All TDX-usable memory regions. Protected by mem_hotplug_lock. */
static LIST_HEAD(tdx_memlist);
-static struct tdx_sys_info tdx_sysinfo __ro_after_init;
+static struct tdx_sys_info tdx_sysinfo;
static bool tdx_module_initialized;
/*
@@ -1279,6 +1279,25 @@ int tdx_module_run_update(void)
return 0;
}
+int tdx_module_refresh_version(void)
+{
+ struct tdx_sys_info_version *old, new;
+ int ret;
+
+ /* Shouldn't fail as the update has succeeded. */
+ ret = get_tdx_sys_info_version(&new);
+ WARN_ON_ONCE(ret);
+
+ old = &tdx_sysinfo.version;
+ pr_info("version " TDX_VERSION_FMT " -> " TDX_VERSION_FMT "\n",
+ old->major_version, old->minor_version, old->update_version,
+ new.major_version, new.minor_version, new.update_version);
+
+ /* Major/minor versions should not change across updates. */
+ tdx_sysinfo.version.update_version = new.update_version;
+ return 0;
+}
+
static bool is_pamt_page(unsigned long phys)
{
struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index 5fef813002c2..d0e8cac9c1d5 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -114,5 +114,6 @@ struct tdmr_info_list {
int tdx_module_shutdown(void);
int tdx_module_run_update(void);
+int tdx_module_refresh_version(void);
#endif
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index e793dec688ab..e49c300f23d4 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -7,7 +7,7 @@
* Include this file to other C file instead.
*/
-static __init int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
+static int get_tdx_sys_info_version(struct tdx_sys_info_version *sysinfo_version)
{
int ret = 0;
u64 val;
--
2.47.1
^ permalink raw reply related
* [PATCH v8 13/21] x86/virt/seamldr: Do TDX per-CPU initialization after module installation
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
After installing the new TDX module, each CPU needs to be initialized
again to make the CPU ready to run any other SEAMCALLs. So, export and
call tdx_cpu_enable() on all CPUs.
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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v8:
- export tdx_cpu_enable(). it is unexported by VMXON series.
---
arch/x86/include/asm/tdx.h | 1 +
arch/x86/virt/vmx/tdx/seamldr.c | 4 ++++
arch/x86/virt/vmx/tdx/tdx.c | 2 +-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 1c5981453ff8..de822ed9ef0b 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -104,6 +104,7 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
#ifdef CONFIG_INTEL_TDX_HOST
void tdx_init(void);
+int tdx_cpu_enable(void);
const char *tdx_dump_mce_info(struct mce *m);
const struct tdx_sys_info *tdx_get_sysinfo(void);
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 317b38c4aa19..04c7a87ac7df 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -212,6 +212,7 @@ enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
+ MODULE_UPDATE_CPU_INIT,
MODULE_UPDATE_DONE,
};
@@ -271,6 +272,9 @@ static int do_seamldr_install_module(void *seamldr_params)
case MODULE_UPDATE_CPU_INSTALL:
ret = seamldr_install(seamldr_params);
break;
+ case MODULE_UPDATE_CPU_INIT:
+ ret = tdx_cpu_enable();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index ff5644f5daa4..3bbb12aefb4b 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -101,7 +101,7 @@ static int try_init_module_global(void)
* (and TDX module global initialization SEAMCALL if not done) on local cpu to
* make this cpu be ready to run any other SEAMCALLs.
*/
-static int tdx_cpu_enable(void)
+int tdx_cpu_enable(void)
{
struct tdx_module_args args = {};
int ret;
--
2.47.1
^ permalink raw reply related
* [PATCH v8 12/21] x86/virt/seamldr: Install a new TDX module
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v8:
- Standardize tdx_module_args initialization. For consistency, initialize
each field on separate lines after declaration, instead of initializing
them in the declaration.
---
arch/x86/virt/vmx/tdx/seamldr.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index f995153f24b9..317b38c4aa19 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -19,6 +19,7 @@
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
+#define P_SEAMLDR_INSTALL 0x8000000000000001
#define SEAMLDR_MAX_NR_MODULE_PAGES 496
#define SEAMLDR_MAX_NR_SIG_PAGES 4
@@ -74,6 +75,14 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+static int seamldr_install(const struct seamldr_params *params)
+{
+ struct tdx_module_args args = {};
+
+ args.rcx = __pa(params);
+ return seamldr_call(P_SEAMLDR_INSTALL, &args);
+}
+
/*
* Intel TDX module blob. Its format is defined at:
* https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
@@ -202,6 +211,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
+ MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_DONE,
};
@@ -258,6 +268,9 @@ static int do_seamldr_install_module(void *seamldr_params)
if (primary)
ret = tdx_module_shutdown();
break;
+ case MODULE_UPDATE_CPU_INSTALL:
+ ret = seamldr_install(seamldr_params);
+ break;
default:
break;
}
--
2.47.1
^ permalink raw reply related
* [PATCH v8 11/21] x86/virt/tdx: Reset software states during TDX module shutdown
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
The TDX module requires a one-time global initialization (TDH.SYS.INIT) and
per-CPU initialization (TDH.SYS.LP.INIT) before use. These initializations
are guarded by software flags to prevent repetition.
After TDX module updates, the new TDX module requires the same global and
per-CPU initializations, but the existing software flags prevent
re-initialization.
Reset all software flags guarding the initialization flows to allow the
global and per-CPU initializations to be triggered again after updates.
As tdx_module_initialized is changed at runtime, drop its "__ro_after_init"
tag.
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: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v8:
- drop "__ro_after_init" from tdx_module_initialized.
---
arch/x86/virt/vmx/tdx/tdx.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index d28421ac4180..ff5644f5daa4 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -55,11 +55,14 @@ static DEFINE_PER_CPU(bool, tdx_lp_initialized);
static struct tdmr_info_list tdx_tdmr_list;
+static bool sysinit_done;
+static int sysinit_ret;
+
/* All TDX-usable memory regions. Protected by mem_hotplug_lock. */
static LIST_HEAD(tdx_memlist);
static struct tdx_sys_info tdx_sysinfo __ro_after_init;
-static bool tdx_module_initialized __ro_after_init;
+static bool tdx_module_initialized;
/*
* Do the module global initialization once and return its result.
@@ -69,8 +72,6 @@ static int try_init_module_global(void)
{
struct tdx_module_args args = {};
static DEFINE_RAW_SPINLOCK(sysinit_lock);
- static bool sysinit_done;
- static int sysinit_ret;
raw_spin_lock(&sysinit_lock);
@@ -1237,7 +1238,7 @@ int tdx_module_shutdown(void)
{
struct tdx_sys_info_handoff handoff = {};
struct tdx_module_args args = {};
- int ret;
+ int ret, cpu;
ret = get_tdx_sys_info_handoff(&handoff);
WARN_ON_ONCE(ret);
@@ -1247,7 +1248,22 @@ int tdx_module_shutdown(void)
* module can produce and most likely supported by newer modules.
*/
args.rcx = handoff.module_hv;
- return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+ ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+ if (ret)
+ return ret;
+
+ /*
+ * Clear global and per-CPU initialization flags so the new module
+ * can be fully re-initialized after a successful update.
+ *
+ * No locks needed as no concurrent accesses can occur here.
+ */
+ tdx_module_initialized = false;
+ sysinit_done = false;
+ sysinit_ret = 0;
+ for_each_possible_cpu(cpu)
+ per_cpu(tdx_lp_initialized, cpu) = false;
+ return 0;
}
static bool is_pamt_page(unsigned long phys)
--
2.47.1
^ permalink raw reply related
* [PATCH v8 10/21] x86/virt/seamldr: Shut down the current TDX module
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
The first step of TDX module updates is shutting down the current TDX
Module. This step also packs state information that needs to be
preserved across updates as handoff data, which will be consumed by the
updated module. The handoff data is stored internally in the SEAM range
and is hidden from the kernel.
To ensure a successful update, the new module must be able to consume
the handoff data generated by the old module. Since handoff data layout
may change between modules, the handoff data is versioned. Each module
has a native handoff version and provides backward support for several
older versions.
The complete handoff versioning protocol is complex as it supports both
module upgrades and downgrades. See details in Intel® Trust Domain
Extensions (Intel® TDX) Module Base Architecture Specification, Chapter
"Handoff Versioning".
Ideally, the kernel needs to retrieve the handoff versions supported by
the current module and the new module and select a version supported by
both. But, since this implementation chooses to only support module
upgrades, simply request the current module to generate handoff data
using its highest supported version, expecting that the new module will
likely support it.
Retrieve the module's handoff version from TDX global metadata and add an
update step to shut down the module. Module shutdown has global effect, so
it only needs to run on one CPU.
Note that the handoff information isn't cached in tdx_sysinfo. It is used
only for module shutdown, and is present only when the TDX module supports
updates. Caching it in get_tdx_sys_info() would require extra update-support
guards and refreshing the cached value across module updates.
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>
---
v8:
- Enhance the changelog to also talk about what the patch does instead
of just "why". [Rick]
- For simplicity, don't cache handoff version in tdx_sysinfo
---
arch/x86/include/asm/tdx_global_metadata.h | 4 ++++
arch/x86/virt/vmx/tdx/seamldr.c | 11 ++++++++++-
arch/x86/virt/vmx/tdx/tdx.c | 19 ++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 3 +++
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 13 +++++++++++++
5 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/tdx_global_metadata.h b/arch/x86/include/asm/tdx_global_metadata.h
index 40689c8dc67e..41150d546589 100644
--- a/arch/x86/include/asm/tdx_global_metadata.h
+++ b/arch/x86/include/asm/tdx_global_metadata.h
@@ -40,6 +40,10 @@ struct tdx_sys_info_td_conf {
u64 cpuid_config_values[128][2];
};
+struct tdx_sys_info_handoff {
+ u16 module_hv;
+};
+
struct tdx_sys_info {
struct tdx_sys_info_version version;
struct tdx_sys_info_features features;
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index aa839aaeb79d..f995153f24b9 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -15,6 +15,7 @@
#include <asm/seamldr.h>
#include "seamcall_internal.h"
+#include "tdx.h"
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
@@ -200,6 +201,7 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
*/
enum module_update_state {
MODULE_UPDATE_START,
+ MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_DONE,
};
@@ -238,8 +240,12 @@ static void ack_state(void)
static int do_seamldr_install_module(void *seamldr_params)
{
enum module_update_state newstate, curstate = MODULE_UPDATE_START;
+ int cpu = smp_processor_id();
+ bool primary;
int ret = 0;
+ primary = cpumask_first(cpu_online_mask) == cpu;
+
do {
/* Chill out and re-read update_data. */
cpu_relax();
@@ -248,7 +254,10 @@ static int do_seamldr_install_module(void *seamldr_params)
if (newstate != curstate) {
curstate = newstate;
switch (curstate) {
- /* TODO: add the update steps. */
+ case MODULE_UPDATE_SHUTDOWN:
+ if (primary)
+ ret = tdx_module_shutdown();
+ break;
default:
break;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 05d241626e48..d28421ac4180 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -287,7 +287,7 @@ static __init int build_tdx_memlist(struct list_head *tmb_list)
return ret;
}
-static __init int read_sys_metadata_field(u64 field_id, u64 *data)
+static int read_sys_metadata_field(u64 field_id, u64 *data)
{
struct tdx_module_args args = {};
int ret;
@@ -1233,6 +1233,23 @@ static __init int tdx_enable(void)
}
subsys_initcall(tdx_enable);
+int tdx_module_shutdown(void)
+{
+ struct tdx_sys_info_handoff handoff = {};
+ struct tdx_module_args args = {};
+ int ret;
+
+ ret = get_tdx_sys_info_handoff(&handoff);
+ WARN_ON_ONCE(ret);
+
+ /*
+ * Use the module's handoff version as it is the highest the
+ * module can produce and most likely supported by newer modules.
+ */
+ args.rcx = handoff.module_hv;
+ return seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
+}
+
static bool is_pamt_page(unsigned long phys)
{
struct tdmr_info_list *tdmr_list = &tdx_tdmr_list;
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index dde219c823b4..36afebf0e04b 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_SHUTDOWN 52
/*
* SEAMCALL leaf:
@@ -110,4 +111,6 @@ struct tdmr_info_list {
int max_tdmrs; /* How many 'tdmr_info's are allocated */
};
+int tdx_module_shutdown(void);
+
#endif
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index d54d4227990c..e793dec688ab 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -100,6 +100,19 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_
return ret;
}
+static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
+{
+ int ret;
+ u64 val;
+
+ ret = read_sys_metadata_field(0x8900000100000000, &val);
+ if (ret)
+ return ret;
+
+ sysinfo_handoff->module_hv = val;
+ return 0;
+}
+
static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
{
int ret = 0;
--
2.47.1
^ permalink raw reply related
* [PATCH v8 09/21] x86/virt/seamldr: Introduce skeleton for TDX module updates
From: Chao Gao @ 2026-04-27 15:28 UTC (permalink / raw)
To: kvm, linux-coco, linux-kernel, x86
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Chao Gao, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20260427152854.101171-1-chao.gao@intel.com>
TDX module updates require careful synchronization with other TDX
operations. The requirements are (#1/#2 reflect current behavior that
must be preserved):
1. SEAMCALLs need to be callable from both process and IRQ contexts.
2. SEAMCALLs need to be able to run concurrently across CPUs
3. During updates, only update-related SEAMCALLs are permitted; all
other SEAMCALLs shouldn't be called.
4. During updates, all online CPUs must participate in the update work.
No single lock primitive satisfies all requirements. For instance,
rwlock_t handles #1/#2 but fails #4: CPUs spinning with IRQs disabled
cannot be directed to perform update work.
Use stop_machine() as it is the only well-understood mechanism that can
meet all requirements.
And TDX module updates consist of several steps (See Intel® Trust Domain
Extensions (Intel® TDX) Module Base Architecture Specification, Chapter
"TD-Preserving TDX module Update"). Ordering requirements between steps
mandate lockstep synchronization across all CPUs.
multi_cpu_stop() is a good example of performing a multi-step task in
lockstep. But it doesn't synchronize steps within the callback function
it takes. So, implement one based on its pattern to establish the
skeleton for TDX module updates. Specifically, add a global state
machine where each state represents a step in the update flow. The state
advances only after all CPUs acknowledge completing their work in the
current state. This acknowledgment mechanism is what ensures lockstep
execution.
Potential alternative to stop_machine()
=======================================
An alternative approach is to lock all KVM entry points and kick all
vCPUs. Here, KVM entry points refer to KVM VM/vCPU ioctl entry points,
implemented in KVM common code (virt/kvm). Adding a locking mechanism
there would affect all architectures KVM supports. And to lock only TDX
vCPUs, new logic would be needed to identify TDX vCPUs, which the KVM
common code currently lacks. This would add significant complexity and
maintenance overhead to KVM for this TDX-specific use case, so don't take
this approach.
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>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
v8:
- Add a "so don't take this approach" after alternative solution
discussion in the changelog [Rick]
- Use imperative mood for a comment [Dave]
---
arch/x86/virt/vmx/tdx/seamldr.c | 79 ++++++++++++++++++++++++++++++++-
1 file changed, 77 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index f70be8e2a07b..aa839aaeb79d 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -7,8 +7,10 @@
#define pr_fmt(fmt) "seamldr: " fmt
#include <linux/mm.h>
+#include <linux/nmi.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/stop_machine.h>
#include <asm/seamldr.h>
@@ -190,6 +192,77 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
return alloc_seamldr_params(blob, size);
}
+/*
+ * During a TDX module update, all CPUs start from MODULE_UPDATE_START and
+ * progress to MODULE_UPDATE_DONE. Each state is associated with certain
+ * work. For some states, just one CPU needs to perform the work, while
+ * other CPUs just wait during those states.
+ */
+enum module_update_state {
+ MODULE_UPDATE_START,
+ MODULE_UPDATE_DONE,
+};
+
+static struct {
+ enum module_update_state state;
+ int thread_ack;
+ /*
+ * Protect update_data. Raw spinlock as it will be acquired from
+ * interrupt-disabled contexts.
+ */
+ raw_spinlock_t lock;
+} update_data = {
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(update_data.lock)
+};
+
+static void set_target_state(enum module_update_state state)
+{
+ /* Reset ack counter. */
+ update_data.thread_ack = num_online_cpus();
+ update_data.state = state;
+}
+
+/* Last one to ack a state moves to the next state. */
+static void ack_state(void)
+{
+ guard(raw_spinlock)(&update_data.lock);
+ update_data.thread_ack--;
+ if (!update_data.thread_ack)
+ set_target_state(update_data.state + 1);
+}
+
+/*
+ * See multi_cpu_stop() from where this multi-cpu state-machine was
+ * adopted, and the rationale for touch_nmi_watchdog().
+ */
+static int do_seamldr_install_module(void *seamldr_params)
+{
+ enum module_update_state newstate, curstate = MODULE_UPDATE_START;
+ int ret = 0;
+
+ do {
+ /* Chill out and re-read update_data. */
+ cpu_relax();
+ newstate = READ_ONCE(update_data.state);
+
+ if (newstate != curstate) {
+ curstate = newstate;
+ switch (curstate) {
+ /* TODO: add the update steps. */
+ default:
+ break;
+ }
+
+ ack_state();
+ } else {
+ touch_nmi_watchdog();
+ rcu_momentary_eqs();
+ }
+ } while (curstate != MODULE_UPDATE_DONE);
+
+ return ret;
+}
+
DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
if (!IS_ERR_OR_NULL(_T)) free_page((unsigned long)_T))
@@ -207,7 +280,9 @@ int seamldr_install_module(const u8 *data, u32 size)
if (IS_ERR(params))
return PTR_ERR(params);
- /* TODO: Update TDX module here */
- return 0;
+ /* Ensure a stable set of online CPUs for the update process. */
+ guard(cpus_read_lock)();
+ set_target_state(MODULE_UPDATE_START + 1);
+ return stop_machine_cpuslocked(do_seamldr_install_module, params, cpu_online_mask);
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
--
2.47.1
^ permalink raw reply related
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