* [PATCH 04/14] KVM: x86: Use local MMIO fragment variable to clean up emulator_read_write()
From: Sean Christopherson @ 2026-02-25 1:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Yashu Zhang, Rick Edgecombe,
Binbin Wu, Xiaoyao Li, Tom Lendacky, Michael Roth
In-Reply-To: <20260225012049.920665-1-seanjc@google.com>
Grab the MMIO fragment used by emulator_read_write() to initiate an exit
to userspace in a local variable to make the code easier to read.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a74ae3a81076..7cbd6f7d8578 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8231,7 +8231,7 @@ static int emulator_read_write(struct x86_emulate_ctxt *ctxt,
const struct read_write_emulator_ops *ops)
{
struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt);
- gpa_t gpa;
+ struct kvm_mmio_fragment *frag;
int rc;
if (WARN_ON_ONCE((bytes > 8u || !ops->write) && object_is_on_stack(val)))
@@ -8287,17 +8287,16 @@ static int emulator_read_write(struct x86_emulate_ctxt *ctxt,
if (!vcpu->mmio_nr_fragments)
return X86EMUL_CONTINUE;
- gpa = vcpu->mmio_fragments[0].gpa;
-
vcpu->mmio_needed = 1;
vcpu->mmio_cur_fragment = 0;
- vcpu->run->mmio.len = min(8u, vcpu->mmio_fragments[0].len);
+ frag = &vcpu->mmio_fragments[0];
+ vcpu->run->mmio.len = min(8u, frag->len);
vcpu->run->mmio.is_write = vcpu->mmio_is_write = ops->write;
vcpu->run->exit_reason = KVM_EXIT_MMIO;
- vcpu->run->mmio.phys_addr = gpa;
+ vcpu->run->mmio.phys_addr = frag->gpa;
- return ops->read_write_exit_mmio(vcpu, gpa, val, bytes);
+ return ops->read_write_exit_mmio(vcpu, frag->gpa, val, bytes);
}
static int emulator_read_emulated(struct x86_emulate_ctxt *ctxt,
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply related
* [PATCH 03/14] KVM: x86: Trace unsatisfied MMIO reads on a per-page basis
From: Sean Christopherson @ 2026-02-25 1:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Yashu Zhang, Rick Edgecombe,
Binbin Wu, Xiaoyao Li, Tom Lendacky, Michael Roth
In-Reply-To: <20260225012049.920665-1-seanjc@google.com>
Invoke the "unsatisfied MMIO reads" when KVM first detects that a
particular access "chunk" requires an exit to userspace instead of tracing
the entire access at the time KVM initiates the exit to userspace. I.e.
precisely trace the first and/or second fragments of a page split instead
of tracing the entire access, as the GPA could be wrong on a page split
case.
Leave the completion tracepoint alone, at least for now, as fixing the
completion path would incur significantly complexity to track exactly which
fragment(s) of the overall access actually triggered MMIO, but add a
comment that the tracing for completed reads in is technically wrong.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8b1f02cc8196..a74ae3a81076 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7808,6 +7808,9 @@ static int vcpu_mmio_read(struct kvm_vcpu *vcpu, gpa_t addr, int len, void *v)
v += n;
} while (len);
+ if (len)
+ trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, len, addr, NULL);
+
return handled;
}
@@ -8139,7 +8142,6 @@ static int write_mmio(struct kvm_vcpu *vcpu, gpa_t gpa, int bytes, void *val)
static int read_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
void *val, int bytes)
{
- trace_kvm_mmio(KVM_TRACE_MMIO_READ_UNSATISFIED, bytes, gpa, NULL);
return X86EMUL_IO_NEEDED;
}
@@ -8247,6 +8249,11 @@ static int emulator_read_write(struct x86_emulate_ctxt *ctxt,
* is copied to the correct chunk of the correct operand.
*/
if (!ops->write && vcpu->mmio_read_completed) {
+ /*
+ * For simplicity, trace the entire MMIO read in one shot, even
+ * though the GPA might be incorrect if there are two fragments
+ * that aren't contiguous in the GPA space.
+ */
trace_kvm_mmio(KVM_TRACE_MMIO_READ, bytes,
vcpu->mmio_fragments[0].gpa, val);
vcpu->mmio_read_completed = 0;
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply related
* [PATCH 02/14] KVM: x86: Open code handling of completed MMIO reads in emulator_read_write()
From: Sean Christopherson @ 2026-02-25 1:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Yashu Zhang, Rick Edgecombe,
Binbin Wu, Xiaoyao Li, Tom Lendacky, Michael Roth
In-Reply-To: <20260225012049.920665-1-seanjc@google.com>
Open code the handling of completed MMIO reads instead of using an ops
hook, as burying the logic behind a (likely RETPOLINE'd) indirect call,
and with an unintuitive name, makes relatively straightforward code hard
to comprehend.
Opportunistically add comments to explain the dependencies between the
emulator's mem_read cache and the MMIO read completion logic, as it's very
easy to overlook the cache's role in getting the read data into the
correct destination.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/emulate.c | 13 +++++++++++++
arch/x86/kvm/x86.c | 33 ++++++++++++++++-----------------
2 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c8e292e9a24d..70850e591350 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1297,12 +1297,25 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt,
int rc;
struct read_cache *mc = &ctxt->mem_read;
+ /*
+ * If the read gets a cache hit, simply copy the value from the cache.
+ * A "hit" here means that there is unused data in the cache, i.e. when
+ * re-emulating an instruction to complete a userspace exit, KVM relies
+ * on "no decode" to ensure the instruction is re-emulated in the same
+ * sequence, so that multiple reads are fulfilled in the correct order.
+ */
if (mc->pos < mc->end)
goto read_cached;
if (KVM_EMULATOR_BUG_ON((mc->end + size) >= sizeof(mc->data), ctxt))
return X86EMUL_UNHANDLEABLE;
+ /*
+ * Route all reads to the cache. This allows @dest to be an on-stack
+ * variable without triggering use-after-free if KVM needs to exit to
+ * userspace to handle an MMIO read (the MMIO fragment will point at
+ * the current location in the cache).
+ */
rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
&ctxt->exception);
if (rc != X86EMUL_CONTINUE)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff3a6f86973f..8b1f02cc8196 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8109,8 +8109,6 @@ int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
}
struct read_write_emulator_ops {
- int (*read_write_prepare)(struct kvm_vcpu *vcpu, void *val,
- int bytes);
int (*read_write_emulate)(struct kvm_vcpu *vcpu, gpa_t gpa,
void *val, int bytes);
int (*read_write_mmio)(struct kvm_vcpu *vcpu, gpa_t gpa,
@@ -8120,18 +8118,6 @@ struct read_write_emulator_ops {
bool write;
};
-static int read_prepare(struct kvm_vcpu *vcpu, void *val, int bytes)
-{
- if (vcpu->mmio_read_completed) {
- trace_kvm_mmio(KVM_TRACE_MMIO_READ, bytes,
- vcpu->mmio_fragments[0].gpa, val);
- vcpu->mmio_read_completed = 0;
- return 1;
- }
-
- return 0;
-}
-
static int read_emulate(struct kvm_vcpu *vcpu, gpa_t gpa,
void *val, int bytes)
{
@@ -8167,7 +8153,6 @@ static int write_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
}
static const struct read_write_emulator_ops read_emultor = {
- .read_write_prepare = read_prepare,
.read_write_emulate = read_emulate,
.read_write_mmio = vcpu_mmio_read,
.read_write_exit_mmio = read_exit_mmio,
@@ -8250,9 +8235,23 @@ static int emulator_read_write(struct x86_emulate_ctxt *ctxt,
if (WARN_ON_ONCE((bytes > 8u || !ops->write) && object_is_on_stack(val)))
return X86EMUL_UNHANDLEABLE;
- if (ops->read_write_prepare &&
- ops->read_write_prepare(vcpu, val, bytes))
+ /*
+ * If the read was already completed via a userspace MMIO exit, there's
+ * nothing left to do except trace the MMIO read. When completing MMIO
+ * reads, KVM re-emulates the instruction to propagate the value into
+ * the correct destination, e.g. into the correct register, but the
+ * value itself has already been copied to the read cache.
+ *
+ * Note! This is *tightly* coupled to read_emulated() satisfying reads
+ * from the emulator's mem_read cache, so that the MMIO fragment data
+ * is copied to the correct chunk of the correct operand.
+ */
+ if (!ops->write && vcpu->mmio_read_completed) {
+ trace_kvm_mmio(KVM_TRACE_MMIO_READ, bytes,
+ vcpu->mmio_fragments[0].gpa, val);
+ vcpu->mmio_read_completed = 0;
return X86EMUL_CONTINUE;
+ }
vcpu->mmio_nr_fragments = 0;
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply related
* [PATCH 01/14] KVM: x86: Use scratch field in MMIO fragment to hold small write values
From: Sean Christopherson @ 2026-02-25 1:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Yashu Zhang, Rick Edgecombe,
Binbin Wu, Xiaoyao Li, Tom Lendacky, Michael Roth
In-Reply-To: <20260225012049.920665-1-seanjc@google.com>
When exiting to userspace to service an emulated MMIO write, copy the
to-be-written value to a scratch field in the MMIO fragment if the size
of the data payload is 8 bytes or less, i.e. can fit in a single chunk,
instead of pointing the fragment directly at the source value.
This fixes a class of use-after-free bugs that occur when the emulator
initiates a write using an on-stack, local variable as the source, the
write splits a page boundary, *and* both pages are MMIO pages. Because
KVM's ABI only allows for physically contiguous MMIO requests, accesses
that split MMIO pages are separated into two fragments, and are sent to
userspace one at a time. When KVM attempts to complete userspace MMIO in
response to KVM_RUN after the first fragment, KVM will detect the second
fragment and generate a second userspace exit, and reference the on-stack
variable.
The issue is most visible if the second KVM_RUN is performed by a separate
task, in which case the stack of the initiating task can show up as truly
freed data.
==================================================================
BUG: KASAN: use-after-free in complete_emulated_mmio+0x305/0x420
Read of size 1 at addr ffff888009c378d1 by task syz-executor417/984
CPU: 1 PID: 984 Comm: syz-executor417 Not tainted 5.10.0-182.0.0.95.h2627.eulerosv2r13.x86_64 #3
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 Call Trace:
dump_stack+0xbe/0xfd
print_address_description.constprop.0+0x19/0x170
__kasan_report.cold+0x6c/0x84
kasan_report+0x3a/0x50
check_memory_region+0xfd/0x1f0
memcpy+0x20/0x60
complete_emulated_mmio+0x305/0x420
kvm_arch_vcpu_ioctl_run+0x63f/0x6d0
kvm_vcpu_ioctl+0x413/0xb20
__se_sys_ioctl+0x111/0x160
do_syscall_64+0x30/0x40
entry_SYSCALL_64_after_hwframe+0x67/0xd1
RIP: 0033:0x42477d
Code: <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007faa8e6890e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004d7338 RCX: 000000000042477d
RDX: 0000000000000000 RSI: 000000000000ae80 RDI: 0000000000000005
RBP: 00000000004d7330 R08: 00007fff28d546df R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004d733c
R13: 0000000000000000 R14: 000000000040a200 R15: 00007fff28d54720
The buggy address belongs to the page:
page:0000000029f6a428 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9c37
flags: 0xfffffc0000000(node=0|zone=1|lastcpupid=0x1fffff)
raw: 000fffffc0000000 0000000000000000 ffffea0000270dc8 0000000000000000
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888009c37780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888009c37800: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff888009c37880: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff888009c37900: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888009c37980: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
The bug can also be reproduced with a targeted KVM-Unit-Test by hacking
KVM to fill a large on-stack variable in complete_emulated_mmio(), i.e. by
overwrite the data value with garbage.
Limit the use of the scratch fields to 8-byte or smaller accesses, and to
just writes, as larger accesses and reads are not affected thanks to
implementation details in the emulator, but add a sanity check to ensure
those details don't change in the future. Specifically, KVM never uses
on-stack variables for accesses larger that 8 bytes, e.g. uses an operand
in the emulator context, and *all* reads are buffered through the mem_read
cache.
Note! Using the scratch field for reads is not only unnecessary, it's
also extremely difficult to handle correctly. As above, KVM buffers all
reads through the mem_read cache, and heavily relies on that behavior when
re-emulating the instruction after a userspace MMIO read exit. If a read
splits a page, the first page is NOT an MMIO page, and the second page IS
an MMIO page, then the MMIO fragment needs to point at _just_ the second
chunk of the destination, i.e. its position in the mem_read cache. Taking
the "obvious" approach of copying the fragment value into the destination
when re-emulating the instruction would clobber the first chunk of the
destination, i.e. would clobber the data that was read from guest memory.
Fixes: f78146b0f923 ("KVM: Fix page-crossing MMIO")
Suggested-by: Yashu Zhang <zhangjiaji1@huawei.com>
Reported-by: Yashu Zhang <zhangjiaji1@huawei.com>
Closes: https://lore.kernel.org/all/369eaaa2b3c1425c85e8477066391bc7@huawei.com
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 14 +++++++++++++-
include/linux/kvm_host.h | 3 ++-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index db3f393192d9..ff3a6f86973f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8226,7 +8226,13 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
WARN_ON(vcpu->mmio_nr_fragments >= KVM_MAX_MMIO_FRAGMENTS);
frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
frag->gpa = gpa;
- frag->data = val;
+ if (write && bytes <= 8u) {
+ frag->val = 0;
+ frag->data = &frag->val;
+ memcpy(&frag->val, val, bytes);
+ } else {
+ frag->data = val;
+ }
frag->len = bytes;
return X86EMUL_CONTINUE;
}
@@ -8241,6 +8247,9 @@ static int emulator_read_write(struct x86_emulate_ctxt *ctxt,
gpa_t gpa;
int rc;
+ if (WARN_ON_ONCE((bytes > 8u || !ops->write) && object_is_on_stack(val)))
+ return X86EMUL_UNHANDLEABLE;
+
if (ops->read_write_prepare &&
ops->read_write_prepare(vcpu, val, bytes))
return X86EMUL_CONTINUE;
@@ -11847,6 +11856,9 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
frag++;
vcpu->mmio_cur_fragment++;
} else {
+ if (WARN_ON_ONCE(frag->data == &frag->val))
+ return -EIO;
+
/* Go forward to the next mmio piece. */
frag->data += len;
frag->gpa += len;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2c7d76262898..0bb2a34fb93d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -320,7 +320,8 @@ static inline bool kvm_vcpu_can_poll(ktime_t cur, ktime_t stop)
struct kvm_mmio_fragment {
gpa_t gpa;
void *data;
- unsigned len;
+ u64 val;
+ unsigned int len;
};
struct kvm_vcpu {
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply related
* [PATCH 00/14] KVM: x86: Emulator MMIO fix and cleanups
From: Sean Christopherson @ 2026-02-25 1:20 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: kvm, x86, linux-coco, linux-kernel, Yashu Zhang, Rick Edgecombe,
Binbin Wu, Xiaoyao Li, Tom Lendacky, Michael Roth
Fix a UAF stack bug where KVM references a stack pointer around an exit to
userspace, and then clean up the related code to try to make it easier to
maintain (not necessarily "easy", but "easier").
The SEV-ES and TDX changes are compile-tested only.
Sean Christopherson (14):
KVM: x86: Use scratch field in MMIO fragment to hold small write
values
KVM: x86: Open code handling of completed MMIO reads in
emulator_read_write()
KVM: x86: Trace unsatisfied MMIO reads on a per-page basis
KVM: x86: Use local MMIO fragment variable to clean up
emulator_read_write()
KVM: x86: Open code read vs. write userspace MMIO exits in
emulator_read_write()
KVM: x86: Move MMIO write tracing into vcpu_mmio_write()
KVM: x86: Harden SEV-ES MMIO against on-stack use-after-free
KVM: x86: Dedup kvm_sev_es_mmio_{read,write}()
KVM: x86: Consolidate SEV-ES MMIO emulation into a single public API
KVM: x86: Bury emulator read/write ops in
emulator_{read,write}_emulated()
KVM: x86: Fold emulator_write_phys() into write_emulate()
KVM: x86: Rename .read_write_emulate() to .read_write_guest()
KVM: x86: Don't panic the kernel if completing userspace I/O / MMIO
goes sideways
KVM: x86: Add helpers to prepare kvm_run for userspace MMIO exit
arch/x86/include/asm/kvm_host.h | 3 -
arch/x86/kvm/emulate.c | 13 ++
arch/x86/kvm/svm/sev.c | 20 +--
arch/x86/kvm/vmx/tdx.c | 14 +-
arch/x86/kvm/x86.c | 287 ++++++++++++++------------------
arch/x86/kvm/x86.h | 30 +++-
include/linux/kvm_host.h | 3 +-
7 files changed, 178 insertions(+), 192 deletions(-)
base-commit: 183bb0ce8c77b0fd1fb25874112bc8751a461e49
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Binbin Wu @ 2026-02-25 0:18 UTC (permalink / raw)
To: Sean Christopherson
Cc: Rick P Edgecombe, Xiaoyao Li, changyuanl@google.com,
pbonzini@redhat.com, Binbin Wu, Isaku Yamahata, bp@alien8.de,
x86@kernel.org, kas@kernel.org, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
tglx@kernel.org, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <aZ3LxD5XMepnU8jh@google.com>
On 2/25/2026 12:03 AM, Sean Christopherson wrote:
>
>>>> + } else {
>>>> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>>>> + }
>>>>
>>>> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
>>>> + !!(entry->flags &
>>>> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
>>>
>>> It warns on leaf 0x23 for me. Is it intentional?
>>
>> I guess because the list in cpuid_function_is_indexed() is hard-coded
>> and 0x23 is not added into the list yet.
>
> Yeah, I was anticipating that we'd run afoul of leaves that aren't known to
> the kernel. FWIW, it looks like 0x24 is also indexed.
0x24 is there already.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Edgecombe, Rick P @ 2026-02-24 21:44 UTC (permalink / raw)
To: seanjc@google.com
Cc: changyuanl@google.com, linux-coco@lists.linux.dev, Li, Xiaoyao,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, Wu, Binbin,
kas@kernel.org, linux-kernel@vger.kernel.org,
binbin.wu@linux.intel.com, pbonzini@redhat.com, mingo@redhat.com,
Yamahata, Isaku, hpa@zytor.com, tglx@kernel.org, bp@alien8.de,
x86@kernel.org
In-Reply-To: <aZ4NUZdbH6PPUr5K@google.com>
On Tue, 2026-02-24 at 12:42 -0800, Sean Christopherson wrote:
> > I'm still not clear on the impact of this one, but assuming it's not too
> > serious, could we discuss the WIP CPUID bit TDX arch stuff in PUCK before
> > doing the change?
>
> Sure, I don't see a rush on the patch.
Should we try for tomorrow or next week?
>
> > We were initially focusing on the problem of CPUID bits that affect host
> > state, but then recently were discussing how many other categories of
> > potential problems we should worry about at this point. So it would be good
> > to understand the impact here.
> >
> > If this warn is a trend towards doubling back on the initial decision to
> > expose the CPUID interface to userspace,
>
> Maybe I'm missing something, but I think you're reading into the WARN waaaay
> too much. I suggested it purely as a paranoid guard against the TDX Module
> doing something bizarre and/or the kernel fat-fingering a CPUID function.
> I.e. there's no ulterior motive here, unless maybe Changyuan is planning world
> domination or something. :-D
Heh, well we are already seeing new CPUID bits that cause problems. Not
suspecting any secret motives, but more trying to glean something on your
thinking. Will be easier to discuss the topic.
>
> > which I think is still doable and worth considering as an alternative, then
> > this also affects how we would want the TDX module changes to work.
^ permalink raw reply
* Re: [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Borislav Petkov @ 2026-02-24 21:40 UTC (permalink / raw)
To: Dave Hansen, Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Naveen Rao, David Kaplan, Pawan Gupta, stable
In-Reply-To: <6b3b0c86-99eb-406d-b88d-3d71613bef9e@intel.com>
On February 24, 2026 6:22:36 PM UTC, Dave Hansen <dave.hansen@intel.com> wrote:
>On 2/24/26 10:01, Kim Phillips wrote:
>> @@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
>> if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
>> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
>> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
>> - !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
>> + !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
>> + boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
>> pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
>> spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
>> }
>
>Didn't we agree to just use the "Intel feature" name? See this existing
>code:
>
>> /*
>> * AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
>> * flag and protect from vendor-specific bugs via the whitelist.
>> *
>> * Don't use AutoIBRS when SNP is enabled because it degrades host
>> * userspace indirect branch performance.
>> */
>> if ((x86_arch_cap_msr & ARCH_CAP_IBRS_ALL) ||
>> (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
>> !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
>> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
>> if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
>> !(x86_arch_cap_msr & ARCH_CAP_PBRSB_NO))
>> setup_force_cpu_bug(X86_BUG_EIBRS_PBRSB);
>> }
>
>You're probably not seeing X86_FEATURE_IBRS_ENHANCED because it doesn't
>get forced under SNP.
Set the Intel flag somewhere in the SNP init path...?
--
Small device. Typos and formatting crap
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Changyuan Lyu @ 2026-02-24 21:29 UTC (permalink / raw)
To: rick.p.edgecombe
Cc: binbin.wu, bp, changyuanl, dave.hansen, hpa, isaku.yamahata, kas,
kvm, linux-coco, linux-kernel, mingo, pbonzini, seanjc, tglx, x86,
xiaoyao.li
In-Reply-To: <213d614fe73e183a230c8f4e0c8fa1cc3d45df39.camel@intel.com>
Hi Rick!
On Tue, 24 Feb 2026 01:57:46 +0000, Edgecombe, Rick P wrote:
> On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu wrote:
> > [...]
> > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > index 2d7a4d52ccfb4..0c524f9a94a6c 100644
> > --- a/arch/x86/kvm/vmx/tdx.c
> > +++ b/arch/x86/kvm/vmx/tdx.c
> > @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
> > kvm_cpuid_entry2 *entry, unsigned char i
> > entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
> > entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
> >
> > - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
> > + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
> > entry->index = 0;
> > + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>
> There are two callers of this. One is already zeroed, and the other has
> stack garbage in flags. But that second caller doesn't look at the
> flags so it is harmless. Maybe it would be simpler and clearer to just
> zero init the entry struct in that caller. Then you don't need to clear
> it here. Or alternatively set flags to zero above, and then add
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> single bit in a field of garbage, which seems weird.
Thanks for the suggestion. I agree that initializing entry->flags to 0 at
the start of td_init_cpuid_entry2() is much cleaner.
> > + } else {
> > + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> > + }
> >
> > + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
> > + !!(entry->flags &
> > KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
>
> It warns on leaf 0x23 for me. Is it intentional?
Leaf 0x23 is not in the list of cpuid_function_is_indexed.
Thanks Binbin for the explanation!
> This warning kind of begs the question of how how much consistency
> there should be between KVM_TDX_CAPABILITIES and
> KVM_GET_SUPPORTED_CPUID. There was quite a bit of debate on this and in
> the end we moved forward with a solution that did the bare minimum
> consistency checking.
>
> We actually have been looking at some potential TDX module changes to
> fix the deficiencies from not enforcing the consistency. But didn't
> consider this pattern. Can you explain more about the failure mode?
The main purpose of this patch was to make the KVM_TDX_GET_CPUID API
more intuitive from userspace VMM's perspective.
Since both KVM_TDX_CAPABILITIES and KVM_GET_SUPPORTED_CPUID return
struct kvm_cpuid_entry2, I expected the semantic of the flag in both APIs
to be the same, as I didn't find any special notes to the contrary in the
TDX documentation Documentation/virt/kvm/x86/intel-tdx.rst .
> > /*
> > * The TDX module doesn't allow configuring the guest phys
> > addr bits
> > * (EAX[23:16]). However, KVM uses it as an interface to
> > the userspace
> > --
Regarding the WARN_ON_ONCE, I understand it touches on the larger
consistency and compatibility questions that require more discussion
as you and Sean mentioned. Since I am new to TDX and lack the full context
on those prior debates, I removed the WARN_ON_ONCE check and focus only on
the KVM_CPUID_FLAG_SIGNIFCANT_INDEX consistency fix, which was the core of
this patch.
Best,
Changyuan
-----------------------
From 18b967b718911c09872c3717d8ab083fa59c4a70 Mon Sep 17 00:00:00 2001
From: Changyuan Lyu <changyuanl@google.com>
Date: Fri, 20 Feb 2026 09:55:28 -0800
Subject: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
This ensures consistency with the CPUID entries returned by
KVM_GET_SUPPORTED_CPUID.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
---
arch/x86/kvm/vmx/tdx.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2d7a4d52ccfb4..1c039eab2f3d8 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -167,6 +167,7 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
entry->function = (u32)td_conf->cpuid_config_leaves[idx];
entry->index = td_conf->cpuid_config_leaves[idx] >> 32;
+ entry->flags = 0;
entry->eax = (u32)td_conf->cpuid_config_values[idx][0];
entry->ebx = td_conf->cpuid_config_values[idx][0] >> 32;
entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
@@ -174,6 +175,9 @@ static void td_init_cpuid_entry2(struct kvm_cpuid_entry2 *entry, unsigned char i
if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
entry->index = 0;
+ else
+ entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+
/*
* The TDX module doesn't allow configuring the guest phys addr bits
--
2.53.0.414.gf7e9f6c205-goog
^ permalink raw reply related
* SVSM Development Call February 25, 2026
From: Jörg Rödel @ 2026-02-24 21:02 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.
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] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Sean Christopherson @ 2026-02-24 20:42 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: binbin.wu@linux.intel.com, kvm@vger.kernel.org,
changyuanl@google.com, Binbin Wu, x86@kernel.org, kas@kernel.org,
Xiaoyao Li, hpa@zytor.com, mingo@redhat.com, bp@alien8.de,
pbonzini@redhat.com, tglx@kernel.org, Isaku Yamahata,
linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com
In-Reply-To: <d6820308325d5f8fee7918996ef98ab3f7b6ce6d.camel@intel.com>
On Tue, Feb 24, 2026, Rick P Edgecombe wrote:
> On Tue, 2026-02-24 at 08:03 -0800, Sean Christopherson wrote:
> > > But adding the consistency check here would cause compatibility issue.
> > > Generally, if a new CPUID indexed function is added for some new CPU and
> > > the TDX module reports it, KVM versions without the CPUID function in
> > > the list will trigger the warning.
> >
> > IMO, that's a good thing and working as intended. WARNs aren't inherently
> > evil. While the goal is to be WARN-free, in this case triggering the WARN if
> > the TDX Module is updated (or new silicon arrives) is desirable, because it
> > alerts us to that new behavior, so that we can go update KVM.
> >
> > But we should "fix" 0x23 and 0x24 before landing this patch.
>
> Would we backport those changes then? I would usually think that if the TDX
> module updates in such a way that triggers a warning in the kernel then it's a
> TDX module bug.
To stable@? No, I don't think see any reason to do that.
> I'm still not clear on the impact of this one, but assuming it's not too
> serious, could we discuss the WIP CPUID bit TDX arch stuff in PUCK before doing
> the change?
Sure, I don't see a rush on the patch.
> We were initially focusing on the problem of CPUID bits that affect host state,
> but then recently were discussing how many other categories of potential
> problems we should worry about at this point. So it would be good to understand
> the impact here.
>
> If this warn is a trend towards doubling back on the initial decision to expose
> the CPUID interface to userspace,
Maybe I'm missing something, but I think you're reading into the WARN waaaay too
much. I suggested it purely as a paranoid guard against the TDX Module doing
something bizarre and/or the kernel fat-fingering a CPUID function. I.e. there's
no ulterior motive here, unless maybe Changyuan is planning world domination or
something. :-D
> which I think is still doable and worth considering as an alternative, then
> this also affects how we would want the TDX module changes to work.
^ permalink raw reply
* Re: [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: Jason Gunthorpe @ 2026-02-24 19:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <5z6d2etfr24oscoxhk3samf2bbhtcz6hymf65cow76omagsplf@6gdaev2perkk>
On Tue, Feb 24, 2026 at 09:32:01AM +0100, Jiri Pirko wrote:
> >Should there be some global list of leaked decrypted pages such that
> >the mm subsystem could try again later to recover these?
>
> swiotlb does the same non-recovery leakage. I belive is it not worth
> implementing this at this time,
Yeah, I agree
Looking at the callers the purpose of the return code is to trigger
the memory leak because there is no way to recover from this. We have
no idea when in future the hypervisor might permit the operation and
we have no way to keep track of the memory until it does.
It is not a great API design at all, it only makes sense from the
hypervisor perspective where it can run out of memory trying to do
these changes..
Jason
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Edgecombe, Rick P @ 2026-02-24 18:45 UTC (permalink / raw)
To: seanjc@google.com, binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, changyuanl@google.com, Wu, Binbin,
x86@kernel.org, kas@kernel.org, Li, Xiaoyao, hpa@zytor.com,
mingo@redhat.com, bp@alien8.de, pbonzini@redhat.com,
tglx@kernel.org, Yamahata, Isaku, linux-kernel@vger.kernel.org,
linux-coco@lists.linux.dev, dave.hansen@linux.intel.com
In-Reply-To: <aZ3LxD5XMepnU8jh@google.com>
On Tue, 2026-02-24 at 08:03 -0800, Sean Christopherson wrote:
> > But adding the consistency check here would cause compatibility issue.
> > Generally, if a new CPUID indexed function is added for some new CPU and
> > the TDX module reports it, KVM versions without the CPUID function in
> > the list will trigger the warning.
>
> IMO, that's a good thing and working as intended. WARNs aren't inherently
> evil. While the goal is to be WARN-free, in this case triggering the WARN if
> the TDX Module is updated (or new silicon arrives) is desirable, because it
> alerts us to that new behavior, so that we can go update KVM.
>
> But we should "fix" 0x23 and 0x24 before landing this patch.
Would we backport those changes then? I would usually think that if the TDX
module updates in such a way that triggers a warning in the kernel then it's a
TDX module bug.
I'm still not clear on the impact of this one, but assuming it's not too
serious, could we discuss the WIP CPUID bit TDX arch stuff in PUCK before doing
the change?
We were initially focusing on the problem of CPUID bits that affect host state,
but then recently were discussing how many other categories of potential
problems we should worry about at this point. So it would be good to understand
the impact here.
If this warn is a trend towards doubling back on the initial decision to expose
the CPUID interface to userspace, which I think is still doable and worth
considering as an alternative, then this also affects how we would want the TDX
module changes to work.
^ permalink raw reply
* Re: [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Jim Mattson @ 2026-02-24 18:29 UTC (permalink / raw)
To: Dave Hansen
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta, stable
In-Reply-To: <6b3b0c86-99eb-406d-b88d-3d71613bef9e@intel.com>
On Tue, Feb 24, 2026 at 10:23 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/24/26 10:01, Kim Phillips wrote:
> > @@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
> > if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
> > spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
> > spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
> > - !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> > + !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
> > + boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
> > pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
> > spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
> > }
>
> Didn't we agree to just use the "Intel feature" name?
Aren't they quite different? IIRC, IBRS_ENHANCED protects host
userspace from guest indirect branch steering (i.e. VMSCAPE style
attacks), but AUTOIBRS does not.
^ permalink raw reply
* Re: [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Dave Hansen @ 2026-02-24 18:22 UTC (permalink / raw)
To: Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta, stable
In-Reply-To: <20260224180157.725159-2-kim.phillips@amd.com>
On 2/24/26 10:01, Kim Phillips wrote:
> @@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
> if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
> - !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> + !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
> + boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
> pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
> spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
> }
Didn't we agree to just use the "Intel feature" name? See this existing
code:
> /*
> * AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
> * flag and protect from vendor-specific bugs via the whitelist.
> *
> * Don't use AutoIBRS when SNP is enabled because it degrades host
> * userspace indirect branch performance.
> */
> if ((x86_arch_cap_msr & ARCH_CAP_IBRS_ALL) ||
> (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
> !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
> if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
> !(x86_arch_cap_msr & ARCH_CAP_PBRSB_NO))
> setup_force_cpu_bug(X86_BUG_EIBRS_PBRSB);
> }
You're probably not seeing X86_FEATURE_IBRS_ENHANCED because it doesn't
get forced under SNP.
^ permalink raw reply
* [PATCH 3/3] KVM: SEV: Add support for SNP BTB Isolation
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest. CPU hardware tracks each
guest's BTB entries and can flush the BTB if it has been determined
to be contaminated with any prediction information originating outside
the particular guest's context.
To mitigate possible performance penalties incurred by these flushes,
it is recommended that the hypervisor runs with SPEC_CTRL[IBRS] set.
Note that using Automatic IBRS is not an equivalent option here, since
it behaves differently when SEV-SNP is active. See commit acaa4b5c4c85
("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
for more details.
Indicate support for BTB Isolation in sev_supported_vmsa_features,
bit 7.
SNP-active guests can enable (BTB) Isolation through SEV_Status
bit 9 (SNPBTBIsolation).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..2038461c1316 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -305,6 +305,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
+#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ea515cf41168..3c0278871114 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3167,6 +3167,9 @@ void __init sev_hardware_setup(void)
if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (sev_snp_enabled)
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_BTB_ISOLATION;
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
This is to prepare to allow legacy IBRS toggling on AMD systems,
where the BTB Isolation SEV-SNP feature can use it to optimize the
quick VM exit to re-entry path.
There is no reason this wasn't allowed in the first place, therefore
adding the cc: stable and Fixes: tags.
Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/kernel/cpu/bugs.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4eefbff4b19a..67eff5fba629 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2154,11 +2154,6 @@ static void __init spectre_v2_select_mitigation(void)
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
- if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
- pr_err("IBRS selected but not Intel CPU. Switching to AUTO select\n");
- spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
- }
-
if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && !boot_cpu_has(X86_FEATURE_IBRS)) {
pr_err("IBRS selected but CPU doesn't have IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
@@ -2247,7 +2242,7 @@ static void __init spectre_v2_apply_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);
if (spectre_v2_in_ibrs_mode(spectre_v2_enabled)) {
- if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS) && spectre_v2_enabled != SPECTRE_V2_IBRS) {
msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
} else {
x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
The original commit that added support for Automatic IBRS neglected
to amend a condition to include AUTOIBRS in addition to the
X86_FEATURE_IBRS_ENHANCED check. Fix that, and another couple
of minor outliers.
Fixes: e7862eda309e ("x86/cpu: Support AMD Automatic IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/kernel/cpu/bugs.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d0a2847a4bb0..4eefbff4b19a 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
- !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
@@ -2182,7 +2183,8 @@ static void __init spectre_v2_select_mitigation(void)
break;
fallthrough;
case SPECTRE_V2_CMD_FORCE:
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
spectre_v2_enabled = SPECTRE_V2_EIBRS;
break;
}
@@ -2262,7 +2264,8 @@ static void __init spectre_v2_apply_mitigation(void)
case SPECTRE_V2_IBRS:
setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS))
pr_warn(SPECTRE_V2_IBRS_PERF_MSG);
break;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/3] KVM: SEV: Add support for BTB Isolation
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest.
The first patch fixes a longstanding bug where users couldn't select
Automatic IBRS on AMD machines using spectre_v2=eibrs on the kcmdline.
The second patch fixes another longstanding bug where users couldn't
select legacy / toggling SPEC_CTRL[IBRS] on AMD systems, which may
be used by users of the BTB Isolation feature.
The third patch adds support for the feature by adding it to the
supported features bitmask.
Based on git://git.kernel.org/pub/scm/virt/kvm/kvm.git next,
currently b1195183ed42 (tag: tags/kvm-7.0-1, kvm/queue, kvm/next).
This series also available here:
https://github.com/AMDESE/linux/tree/btb-isol-latest
Advance qemu bits (to add btb-isol=on/off switch) available here:
https://github.com/AMDESE/qemu/tree/btb-isol-latest
Qemu bits will be posted upstream once kernel bits are merged.
They depend on Naveen Rao's "target/i386: SEV: Add support for
enabling VMSA SEV features":
https://lore.kernel.org/qemu-devel/cover.1761648149.git.naveen@kernel.org/
Kim Phillips (3):
cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
KVM: SEV: Add support for SNP BTB Isolation
arch/x86/include/asm/svm.h | 1 +
arch/x86/kernel/cpu/bugs.c | 16 +++++++---------
arch/x86/kvm/svm/sev.c | 3 +++
3 files changed, 11 insertions(+), 9 deletions(-)
base-commit: b1195183ed42f1522fae3fe44ebee3af437aa000
--
2.43.0
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Sean Christopherson @ 2026-02-24 16:03 UTC (permalink / raw)
To: Binbin Wu
Cc: Rick P Edgecombe, Xiaoyao Li, changyuanl@google.com,
pbonzini@redhat.com, Binbin Wu, Isaku Yamahata, bp@alien8.de,
x86@kernel.org, kas@kernel.org, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
tglx@kernel.org, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <fd3b58fd-a450-471a-89a3-541c3f88c874@linux.intel.com>
On Tue, Feb 24, 2026, Binbin Wu wrote:
> On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
> >> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >> index 2d7a4d52ccfb4..0c524f9a94a6c 100644
> >> --- a/arch/x86/kvm/vmx/tdx.c
> >> +++ b/arch/x86/kvm/vmx/tdx.c
> >> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
> >> kvm_cpuid_entry2 *entry, unsigned char i
> >> entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
> >> entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
> >>
> >> - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
> >> + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
> >> entry->index = 0;
> >> + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> >
> > There are two callers of this. One is already zeroed, and the other has
> > stack garbage in flags. But that second caller doesn't look at the
> > flags so it is harmless. Maybe it would be simpler and clearer to just
> > zero init the entry struct in that caller. Then you don't need to clear
> > it here. Or alternatively set flags to zero above, and then add
> > KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> > single bit in a field of garbage, which seems weird.
+1, td_init_cpuid_entry2() should initialize flags to '0' and then set
KVM_CPUID_FLAG_SIGNIFCANT_INDEX as appropriate.
> >> + } else {
> >> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> >> + }
> >>
> >> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
> >> + !!(entry->flags &
> >> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
> >
> > It warns on leaf 0x23 for me. Is it intentional?
>
> I guess because the list in cpuid_function_is_indexed() is hard-coded
> and 0x23 is not added into the list yet.
Yeah, I was anticipating that we'd run afoul of leaves that aren't known to
the kernel. FWIW, it looks like 0x24 is also indexed.
> It's fine for existing KVM code because cpuid_function_is_indexed() is
> only used to check that if a CPUID entry is queried without index, it
> shouldn't be included in the indexed list.
>
> But adding the consistency check here would cause compatibility issue.
> Generally, if a new CPUID indexed function is added for some new CPU and
> the TDX module reports it, KVM versions without the CPUID function in
> the list will trigger the warning.
IMO, that's a good thing and working as intended. WARNs aren't inherently evil.
While the goal is to be WARN-free, in this case triggering the WARN if the TDX
Module is updated (or new silicon arrives) is desirable, because it alerts us to
that new behavior, so that we can go update KVM.
But we should "fix" 0x23 and 0x24 before landing this patch.
^ permalink raw reply
* Re: [PATCH 1/2] firmware: smccc: add timeout, touch wdt
From: Andre Przywara @ 2026-02-24 10:58 UTC (permalink / raw)
To: Vedashree Vidwans, salman.nabi, sudeep.holla, lpieralisi,
mark.rutland, trilokkumar.soni
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20260210224023.2341728-2-vvidwans@nvidia.com>
Hi Veda,
On 2/10/26 23:40, Vedashree Vidwans wrote:
> Enhance PRIME/ACTIVATION functions to touch watchdog and implement
> timeout mechanism. This update ensures that any potential hangs are
> detected promptly and that the LFA process is allocated sufficient
> execution time before the watchdog timer expires. These changes improve
> overall system reliability by reducing the risk of undetected process
> stalls and unexpected watchdog resets.
Many thanks for that, I think it's a very good idea to take care of the
watchdog and to avoid an infinite loop in the AGAIN case.
I have some comments about some details below ....
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
> drivers/firmware/smccc/lfa_fw.c | 40 +++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index da6b54fe1685..b0ace6fc8dac 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -17,6 +17,9 @@
> #include <linux/array_size.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
> +#include <linux/nmi.h>
> +#include <linux/ktime.h>
> +#include <linux/delay.h>
>
> #undef pr_fmt
> #define pr_fmt(fmt) "Arm LFA: " fmt
> @@ -37,6 +40,14 @@
> #define LFA_PRIME_CALL_AGAIN BIT(0)
> #define LFA_ACTIVATE_CALL_AGAIN BIT(0)
>
> +/* Prime loop limits, TODO: tune after testing */
> +#define LFA_PRIME_BUDGET_US 30000000 /* 30s cap */
> +#define LFA_PRIME_POLL_DELAY_US 10 /* 10us between polls */
> +
> +/* Activation loop limits, TODO: tune after testing */
> +#define LFA_ACTIVATE_BUDGET_US 20000000 /* 20s cap */
> +#define LFA_ACTIVATE_POLL_DELAY_US 10 /* 10us between polls */
> +
> /* LFA return values */
> #define LFA_SUCCESS 0
> #define LFA_NOT_SUPPORTED 1
> @@ -219,6 +230,7 @@ static int call_lfa_activate(void *data)
> struct image_props *attrs = data;
> struct arm_smccc_1_2_regs args = { 0 };
> struct arm_smccc_1_2_regs res = { 0 };
> + ktime_t end = ktime_add_us(ktime_get(), LFA_ACTIVATE_BUDGET_US);
>
> args.a0 = LFA_1_0_FN_ACTIVATE;
> args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> @@ -232,6 +244,8 @@ static int call_lfa_activate(void *data)
> args.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
>
> for (;;) {
> + /* Touch watchdog, ACTIVATE shouldn't take longer than watchdog_thresh */
> + touch_nmi_watchdog();
> arm_smccc_1_2_invoke(&args, &res);
>
> if ((long)res.a0 < 0) {
> @@ -241,6 +255,15 @@ static int call_lfa_activate(void *data)
> }
> if (!(res.a1 & LFA_ACTIVATE_CALL_AGAIN))
> break; /* ACTIVATE successful */
> +
> + /* SMC returned with call_again flag set */
> + if (ktime_before(ktime_get(), end)) {
> + udelay(LFA_ACTIVATE_POLL_DELAY_US);
I don't think we should wait here at all, and definitely not with
udelay: https://docs.kernel.org/timers/delay_sleep_functions.html
Instead we should move the "call again" (and timeout) mechanism out of
this function, into activate_fw_image(), so that we exit the
stop_machine(). Otherwise we would still block everything. Doing it
there, where we should be preemptible, would give the kernel a chance to
do some housekeeping. If there is nothing for the kernel to do, then I
think it's fine to immediately call lfa_activate() again, after a
cond_resched(), for instance.
> + continue;
> + }
> +
> + pr_err("ACTIVATE for image %s timed out", attrs->image_name);
> + return -ETIMEDOUT;
> }
>
> return res.a0;
> @@ -290,6 +313,7 @@ static int prime_fw_image(struct image_props *attrs)
> {
> struct arm_smccc_1_2_regs args = { 0 };
> struct arm_smccc_1_2_regs res = { 0 };
> + ktime_t end = ktime_add_us(ktime_get(), LFA_PRIME_BUDGET_US);
> int ret;
>
> mutex_lock(&lfa_lock);
> @@ -317,6 +341,8 @@ static int prime_fw_image(struct image_props *attrs)
> args.a0 = LFA_1_0_FN_PRIME;
> args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> for (;;) {
> + /* Touch watchdog, PRIME shouldn't take longer than watchdog_thresh */
> + touch_nmi_watchdog();
> arm_smccc_1_2_invoke(&args, &res);
>
> if ((long)res.a0 < 0) {
> @@ -328,6 +354,20 @@ static int prime_fw_image(struct image_props *attrs)
> }
> if (!(res.a1 & LFA_PRIME_CALL_AGAIN))
> break; /* PRIME successful */
> +
> + /* SMC returned with call_again flag set */
> + if (ktime_before(ktime_get(), end)) {
> + udelay(LFA_PRIME_POLL_DELAY_US);
same comment here, please no udelay().
This should also avoid the discussion about the exact values of the
sleep periods.
I'd just have one generous timeout (a few seconds, basically what your
BUDGET values do above), to avoid looping forever in case of a firmware
bug, for instance.
Cheers,
Andre
> + continue;
> + }
> +
> + pr_err("LFA_PRIME for image %s timed out", attrs->image_name);
> + mutex_unlock(&lfa_lock);
> +
> + ret = lfa_cancel(attrs);
> + if (ret != 0)
> + return ret;
> + return -ETIMEDOUT;
> }
>
> mutex_unlock(&lfa_lock);
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Huang, Kai @ 2026-02-24 10:49 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, bp@alien8.de,
kas@kernel.org, mingo@redhat.com, Chatre, Reinette, Weiny, Ira,
seanjc@google.com, Verma, Vishal L, nik.borisov@suse.com,
binbin.wu@linux.intel.com, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
linux-kernel@vger.kernel.org, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0+j0ohYdJlCACn@intel.com>
On Tue, 2026-02-24 at 14:00 +0800, Chao Gao wrote:
> On Mon, Feb 23, 2026 at 05:25:53PM +0800, Huang, Kai wrote:
> >
> > >
> > > +/*
> > > + * During a TDX Module update, all CPUs start from TDP_START and progress
> >
> > Nit: start from TDP_START or TDP_START + 1 ?
>
> TDP_START. See:
>
> +static int do_seamldr_install_module(void *params)
> +{
> + enum tdp_state newstate, curstate = TDP_START;
> ^^^^^^^^^^^^^^^^^^^^
>
> >
> > The code below says:
> >
> > + set_target_state(TDP_START + 1);
>
> set_target_state() sets a global target (or next) state for all CPUs. Each CPU
> compares its current state to the target. If they don't match, the CPU performs
> the required task and then acks the state.
>
> The global target state must be reset at the start of each update to trigger
> the do-while loop in do_seamldr_install_module().
OK thanks for clarification.
>
> > + ret = stop_machine_cpuslocked(do_seamldr_install_module, params,
> > cpu_online_mask);
> >
> > > + * to TDP_DONE. Each state is associated with certain work. For some
> > > + * states, just one CPU needs to perform the work, while other CPUs just
> > > + * wait during those states.
> > > + */
> > > +enum tdp_state {
> > > + TDP_START,
> > > + TDP_DONE,
> > > +};
> >
> > Nit: just curious, what does "TDP" mean?
> >
> > Maybe something more obvious?
>
> It stands for TD Preserving. Since this term isn't commonly used outside
> Intel, "TDX Module updates" is clearer. I'll change this enum to:
>
> enum module_update_state {
> MODULE_UPDATE_START,
> MODULE_UPDATE_SHUTDOWN,
> MODULE_UPDATE_CPU_INSTALL,
> MODULE_UPDATE_CPU_INIT,
> MODULE_UPDATE_RUN_UPDATE,
> MODULE_UPDATE_DONE,
> };
Thanks.
>
> >
> > > +
> > > +static struct {
> > > + enum tdp_state state;
> > > + atomic_t thread_ack;
> > > +} tdp_data;
> > > +
> > > +static void set_target_state(enum tdp_state state)
> > > +{
> > > + /* Reset ack counter. */
> > > + atomic_set(&tdp_data.thread_ack, num_online_cpus());
> > > + /* Ensure thread_ack is updated before the new state */
> >
> > Nit: perhaps add "so that ..." part to the comment?
>
> how about:
>
> /*
> * Ensure thread_ack is updated before the new state.
> * Otherwise, other CPUs may see the new state and ack
> * it before thread_ack is reset. An ack before reset
> * is effectively lost, causing the system to wait
> * forever for thread_ack to become zero.
> */
>
LGTM.
> >
> > > + smp_wmb();
> > > + WRITE_ONCE(tdp_data.state, state);
> > > +}
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Huang, Kai @ 2026-02-24 10:46 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, bp@alien8.de,
kas@kernel.org, mingo@redhat.com, Chatre, Reinette, Weiny, Ira,
seanjc@google.com, Verma, Vishal L, nik.borisov@suse.com,
binbin.wu@linux.intel.com, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
linux-kernel@vger.kernel.org, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ00DQ2YwcwfgQtP@intel.com>
On Tue, 2026-02-24 at 13:15 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 06:31:24AM +0800, Huang, Kai wrote:
> > On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > > P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
> > > update requests. This structure contains physical addresses pointing to
> > > the module binary and its signature file (or sigstruct), along with an
> > > update scenario field.
> > >
> > > TDX Modules are distributed in the tdx_blob format defined at [1]. A
> > > tdx_blob contains a header, sigstruct, and module binary. This is also
> > > the format supplied by the userspace to the kernel.
> > >
> > > Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
> > > accordingly. This structure will be passed to P-SEAMLDR to initiate the
> > > update.
> > >
> > > Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
> > > a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
> > > Specification" will be published separately. The kernel does not
> > > validate P-SEAMLDR compatibility (for example, whether it supports 4KB
> > > or 16KB sigstruct);
> > >
> >
> > Nit:
> >
> > This sounds like the kernel can validate but chooses not to. But I thought
> > the fact is the kernel cannot validate because there's no P-SEAMLDR ABI to
> > enumerate such compatibility?
>
> Emm, the kernel could validate this by parsing mapping_file.json, but the
> complexity wouldn't be worth it.
Oh making kernel parse JSON file is beyond my imagination, but I see you
have a point here :-)
I think my real comment is the sentence
The kernel does not validate ...
only describes what does the kernel do today, which is not the case here.
Instead, we are making a design choice here, so I think the sentence should
at least be something like:
Don't make the kernel validate ...
>
> >
> > > userspace must ensure the P-SEAMLDR version is
> > > compatible with the selected TDX Module by checking the minimum
> > > P-SEAMLDR version requirements at [2].
> > >
> > > Signed-off-by: Chao Gao <chao.gao@intel.com>
> > > Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> > > Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
> > > Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
> > >
> >
> > Nit:
> >
> > As mentioned in v3, can the link be considered as "stable", e.g., won't
> > disappear couple of years later?
>
> I'm not sure when this link will be outdated, but we'll definitely have a TDX
> Module release repository with a blob_structure.txt file describing the format.
>
> >
> > Not sure we should just have a documentation patch for 'tdx_blob' layout. I
> > suspect the content won't be changed in the future anyway, at least for
> > foreseeable future, given you have already updated the sigstruct part.
> >
> > We can include the links to the actual doc too, and if necessarily, point
> > out the links may get updated in the future. We can actually update the
> > links if they are in some doc.
>
> Regarding the documentation patch, I don't see the value in adding one. It
> would just mirror the code and become outdated when 'tdx_blob' layout is
> updated.
Sure.
>
> If the concern is that tdx_blob layout changes could cause incompatibilities,
> that's not the kernel's responsibility to prevent; the kernel has no control
> over external format changes.
No that's not the main concern.
>
> If the issue is simply that links may become outdated, that's a common problem.
> We can address this by referring to blob_structure.txt in the "Intel TDX Module
> Binaries Repository" and dropping the specific link. For example:
>
> TDX Modules are distributed in the tdx_blob format defined in
> blob_structure.txt from the "Intel TDX Module Binaries Repository". A
> tdx_blob contains a header, sigstruct, and module binary. This is also the
> format supplied by the userspace to the kernel.
I think I prefer this instead of using the Links.
My concern is the links in the changelog won't be stable. If that is
acceptable, then that's fine too.
But in the patch 23, you will update the doc anyway, so I think we can just
provide the link there (you already mentioned the repo link there anyway).
>
> >
> > [...]
> >
> > > +/*
> > > + * Intel TDX Module blob. Its format is defined at:
> > > + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
>
> I will drop this link as well.
>
I am fine keeping it here. We need a link "somewhere in _this_ patch" to
review the code I think.
It's in the comment so we can change in the future if it changes.
^ permalink raw reply
* Re: [PATCH v4 05/24] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Huang, Kai @ 2026-02-24 10:30 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0ULTpWJpGjOKLU@intel.com>
On Tue, 2026-02-24 at 10:59 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 05:36:33PM +0800, Huang, Kai wrote:
> >
> > > +int seamldr_get_info(struct seamldr_info *seamldr_info)
> > > +{
> > > + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
> >
> > Should we have a comment for slow_virt_to_phys()? This patch alone doesn't
> > really tell where is the memory from.
>
> How about:
>
> /*
> * Use slow_virt_to_phys() since @seamldr_info may be allocated on
> * the stack.
> */
>
> I was hesitant to add a comment since most existing slow_virt_to_phys() usage
> lacks comments.
Perhaps this is because in these existing usages "where the memory comes
from" and the "use of slow_virt_to_phys()" are closely together so no
comment is needed?
(disclaimer: I was looking at kvm_register_steal_time().)
So I am fine with either way -- feel free to ignore.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Huang, Kai @ 2026-02-24 10:25 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0Nnay7ygKeXmuC@intel.com>
>
> >
> > But I don't know why do you even need to talk about NP-SEAMLDR.
>
> I included this because Dave had some confusion about NP-SEAMLDR [1], so I
> wanted to clarify it.
>
> [1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/
I thought that was under assumption both NP-SEAMLDR and P-SEAMLDR are SEAM
software (which is why both of them are mentioned). But only P-SEAMLDR is,
so I thought we can skip NP-SEAMLDR.
>
> And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
> helpful to clarify the difference. This follows Dave's earlier suggestion to
> explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].
>
> [2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/
>
Sure. If you feel that helps.
[...]
>
> > > + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> > > + * interact with P-SEAMLDR simultaneously.
> > > + */
> > > +static DEFINE_RAW_SPINLOCK(seamldr_lock);
> > > +
> > > +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> > > +{
> > > + /*
> > > + * Serialize P-SEAMLDR calls and disable interrupts as the calls
> > > + * can be made from IRQ context.
> > > + */
> > > + guard(raw_spinlock_irqsave)(&seamldr_lock);
> >
> > Why do you need to disable IRQ? A plain raw_spinlock should work with both
> > cases where seamldr_call() is called from IRQ disabled context and normal
> > task context?
>
> No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
> fires while a task context already holds the lock, and the interrupt handler
> also tries to acquire the same lock.
I thought that's not possible to happen because during module update we have
a machine state to serialize these P-SEAMLDR SEAMCALLs.
But I agree making it IRQ safe is the simplest way so that we don't need to
worry about the deadlock.
Sorry about the noise.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox