* [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM
@ 2026-06-12 11:34 Fuad Tabba
2026-06-12 11:50 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-06-12 11:34 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm,
linux-kernel
Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
Catalin Marinas, Will Deacon, Sascha Bischoff, Andrew Jones,
tabba
When pKVM injects a synchronous exception into a protected guest, it
re-enters without restoring the guest's EL1 sysregs and writes the EL1
exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not
SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE)
only in memory, so the guest's handler reads a stale SPSR_EL1 and
restores the wrong PSTATE on eret.
Write SPSR_EL1 alongside the other exception registers.
Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers")
Reported-by: sashiko <sashiko@sashiko.dev>
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index 8c3fbb413a06..1a7d5cd16d72 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr)
write_sysreg_el1(esr, SYS_ESR);
write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR);
+ write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR);
write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR);
write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
}
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-12 11:34 [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM Fuad Tabba @ 2026-06-12 11:50 ` sashiko-bot 2026-06-12 13:19 ` Fuad Tabba 2026-06-15 10:05 ` Will Deacon 2026-06-17 12:41 ` Marc Zyngier 2 siblings, 1 reply; 11+ messages in thread From: sashiko-bot @ 2026-06-12 11:50 UTC (permalink / raw) To: Fuad Tabba; +Cc: Marc Zyngier, Oliver Upton, kvmarm Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Stale `SCTLR_EL1` used during synchronous exception injection in protected VMs leads to incorrect `PSTATE` computation. - [High] pKVM's `inject_undef64()` injects an undefined instruction exception with the `IL` (Instruction Length) bit cleared in `ESR_EL1`, incorrectly indicating a 16-bit instruction to an AArch64 guest. -- commit 5157ad995c29fa359271604075525691515c2f95 Author: Fuad Tabba <tabba@google.com> KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM When pKVM injects a synchronous exception into a protected guest, it re-enters without restoring the guest's EL1 sysregs. This commit ensures SPSR_EL1 is written alongside the other exception registers to avoid restoring a stale PSTATE on eret. > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > index 8c3fbb413a06d..1a7d5cd16d723 100644 > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > write_sysreg_el1(esr, SYS_ESR); > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); [Severity: High] This isn't a bug introduced by this patch, but since we are fixing sysreg synchronization in inject_sync64(), is there also a missing synchronization for SCTLR_EL1 before pending the exception? Earlier in inject_sync64(), VBAR_EL1 is explicitly synchronized to memory because traps are handled early: arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_sync64() /* * Make sure we have the latest update to VBAR_EL1, as pKVM * handles traps very early, before sysregs are resync'ed */ __vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR)); kvm_pend_exception(vcpu, EXCEPT_AA64_EL1_SYNC); __kvm_adjust_pc(vcpu); However, when __kvm_adjust_pc() eventually invokes enter_exception64(), it reads SCTLR_EL1 from memory to compute the new PSTATE: arch/arm64/kvm/hyp/exception.c:enter_exception64() sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1); Because SCTLR_EL1 wasn't synchronized from hardware like VBAR_EL1 was, could the guest's exception handler end up executing with a stale PSTATE.PAN and PSTATE.SSBS? > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > } [Severity: High] This is a pre-existing issue, but looking at the related inject_undef64() function located just below inject_sync64(): arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_undef64() static void inject_undef64(struct kvm_vcpu *vcpu) { inject_sync64(vcpu, (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT)); } Does this need to set the ESR_ELx_IL bit? According to the ARM Architecture Reference Manual, for an Unknown reason exception (EC = 0x00) originating from AArch64, the Instruction Length (IL) bit must be set to 1 to indicate a 32-bit instruction. Unconditionally leaving it as 0 by only bit-shifting ESR_ELx_EC_UNKNOWN exposes an architecturally invalid exception syndrome register to the guest. Could this cause the guest kernel's exception decoding logic to fail or panic? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260612113414.1022901-1-tabba@google.com?part=1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-12 11:50 ` sashiko-bot @ 2026-06-12 13:19 ` Fuad Tabba 2026-06-14 8:49 ` Marc Zyngier 0 siblings, 1 reply; 11+ messages in thread From: Fuad Tabba @ 2026-06-12 13:19 UTC (permalink / raw) To: sashiko-reviews, Marc Zyngier; +Cc: Oliver Upton, kvmarm, Will Deacon On Fri, 12 Jun 2026 at 12:50, <sashiko-bot@kernel.org> wrote: > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > > Pre-existing issues: > - [High] Stale `SCTLR_EL1` used during synchronous exception injection in protected VMs leads to incorrect `PSTATE` computation. > - [High] pKVM's `inject_undef64()` injects an undefined instruction exception with the `IL` (Instruction Length) bit cleared in `ESR_EL1`, incorrectly indicating a 16-bit instruction to an AArch64 guest. > -- > > commit 5157ad995c29fa359271604075525691515c2f95 > Author: Fuad Tabba <tabba@google.com> > > KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM > > When pKVM injects a synchronous exception into a protected guest, it > re-enters without restoring the guest's EL1 sysregs. This commit ensures > SPSR_EL1 is written alongside the other exception registers to avoid > restoring a stale PSTATE on eret. > > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > index 8c3fbb413a06d..1a7d5cd16d723 100644 > > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > > > write_sysreg_el1(esr, SYS_ESR); > > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > > [Severity: High] > This isn't a bug introduced by this patch, but since we are fixing sysreg > synchronization in inject_sync64(), is there also a missing synchronization > for SCTLR_EL1 before pending the exception? How quickly you forget my AI friend. I submitted a fix here, when you flagged this very issue: https://lore.kernel.org/all/20260612102347.914994-1-tabba@google.com/ > > Earlier in inject_sync64(), VBAR_EL1 is explicitly synchronized to memory > because traps are handled early: > > arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_sync64() > /* > * Make sure we have the latest update to VBAR_EL1, as pKVM > * handles traps very early, before sysregs are resync'ed > */ > __vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR)); > > kvm_pend_exception(vcpu, EXCEPT_AA64_EL1_SYNC); > > __kvm_adjust_pc(vcpu); > > However, when __kvm_adjust_pc() eventually invokes enter_exception64(), it > reads SCTLR_EL1 from memory to compute the new PSTATE: > > arch/arm64/kvm/hyp/exception.c:enter_exception64() > sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1); > > Because SCTLR_EL1 wasn't synchronized from hardware like VBAR_EL1 was, > could the guest's exception handler end up executing with a stale PSTATE.PAN > and PSTATE.SSBS? > > > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > } > > [Severity: High] > This is a pre-existing issue, but looking at the related inject_undef64() > function located just below inject_sync64(): > > arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_undef64() > static void inject_undef64(struct kvm_vcpu *vcpu) > { > inject_sync64(vcpu, (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT)); > } > > Does this need to set the ESR_ELx_IL bit? > > According to the ARM Architecture Reference Manual, for an Unknown reason > exception (EC = 0x00) originating from AArch64, the Instruction Length (IL) > bit must be set to 1 to indicate a 32-bit instruction. > > Unconditionally leaving it as 0 by only bit-shifting ESR_ELx_EC_UNKNOWN > exposes an architecturally invalid exception syndrome register to the guest. > > Could this cause the guest kernel's exception decoding logic to fail or > panic? You're right -- the architecture mandates setting IL here. Like you said, this is pre-existing, and I'll submit a separate patch to fix it. But first, Marc: was there a specific reason for not setting it, or is it just an omission? Cheers, /fuad > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260612113414.1022901-1-tabba@google.com?part=1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-12 13:19 ` Fuad Tabba @ 2026-06-14 8:49 ` Marc Zyngier 2026-06-14 10:36 ` Fuad Tabba 0 siblings, 1 reply; 11+ messages in thread From: Marc Zyngier @ 2026-06-14 8:49 UTC (permalink / raw) To: Fuad Tabba; +Cc: sashiko-reviews, Oliver Upton, kvmarm, Will Deacon On Fri, 12 Jun 2026 14:19:10 +0100, Fuad Tabba <tabba@google.com> wrote: > > On Fri, 12 Jun 2026 at 12:50, <sashiko-bot@kernel.org> wrote: > > > > Could this cause the guest kernel's exception decoding logic to fail or > > panic? > > You're right -- the architecture mandates setting IL here. Like you > said, this is pre-existing, and I'll submit a separate patch to fix > it. But first, Marc: was there a specific reason for not setting it, > or is it just an omission? Just a plain bug. The normal KVM code is usually careful to construct the new ESR based on the source of the exception, meaning that we always convert ESR_EL2 into ESR_EL1, and propagate IL amongst other things. This is obviously not the case here. It'd be worth auditing the rest of the code and make sure this is the only case of IL not being correctly set. Thanks, M. -- Jazz isn't dead. It just smells funny. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-14 8:49 ` Marc Zyngier @ 2026-06-14 10:36 ` Fuad Tabba 0 siblings, 0 replies; 11+ messages in thread From: Fuad Tabba @ 2026-06-14 10:36 UTC (permalink / raw) To: Marc Zyngier; +Cc: sashiko-reviews, Oliver Upton, kvmarm, Will Deacon On Sun, 14 Jun 2026 at 09:46, Marc Zyngier <maz@kernel.org> wrote: > > On Fri, 12 Jun 2026 14:19:10 +0100, > Fuad Tabba <tabba@google.com> wrote: > > > > On Fri, 12 Jun 2026 at 12:50, <sashiko-bot@kernel.org> wrote: > > > > > > Could this cause the guest kernel's exception decoding logic to fail or > > > panic? > > > > You're right -- the architecture mandates setting IL here. Like you > > said, this is pre-existing, and I'll submit a separate patch to fix > > it. But first, Marc: was there a specific reason for not setting it, > > or is it just an omission? > > Just a plain bug. The normal KVM code is usually careful to construct > the new ESR based on the source of the exception, meaning that we > always convert ESR_EL2 into ESR_EL1, and propagate IL amongst other > things. This is obviously not the case here. > > It'd be worth auditing the rest of the code and make sure this is the > only case of IL not being correctly set. Thanks Marc. I'll do that and fix it on the respin. Cheers, /fuad > > Thanks, > > M. > > -- > Jazz isn't dead. It just smells funny. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-12 11:34 [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM Fuad Tabba 2026-06-12 11:50 ` sashiko-bot @ 2026-06-15 10:05 ` Will Deacon 2026-06-15 10:52 ` Fuad Tabba 2026-06-17 12:41 ` Marc Zyngier 2 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2026-06-15 10:05 UTC (permalink / raw) To: Fuad Tabba Cc: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Sascha Bischoff, Andrew Jones On Fri, Jun 12, 2026 at 12:34:14PM +0100, Fuad Tabba wrote: > When pKVM injects a synchronous exception into a protected guest, it > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > only in memory, so the guest's handler reads a stale SPSR_EL1 and > restores the wrong PSTATE on eret. > > Write SPSR_EL1 alongside the other exception registers. > > Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") > Reported-by: sashiko <sashiko@sashiko.dev> > Signed-off-by: Fuad Tabba <tabba@google.com> > --- > arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > index 8c3fbb413a06..1a7d5cd16d72 100644 > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > write_sysreg_el1(esr, SYS_ESR); > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > } Is SPSR_EL1 not set in enter_exception64() using vcpu_cpsr(vcpu), which *is* set here? I'm just a bit wary of the report, as I'd have expected fireworks if we weren't initialising the guest's SPSR on the exception injection path. Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-15 10:05 ` Will Deacon @ 2026-06-15 10:52 ` Fuad Tabba 2026-06-15 14:41 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Fuad Tabba @ 2026-06-15 10:52 UTC (permalink / raw) To: Will Deacon Cc: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Sascha Bischoff, Andrew Jones Hi Will, On Mon, 15 Jun 2026 at 11:05, Will Deacon <will@kernel.org> wrote: > > On Fri, Jun 12, 2026 at 12:34:14PM +0100, Fuad Tabba wrote: > > When pKVM injects a synchronous exception into a protected guest, it > > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > > only in memory, so the guest's handler reads a stale SPSR_EL1 and > > restores the wrong PSTATE on eret. > > > > Write SPSR_EL1 alongside the other exception registers. > > > > Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") > > Reported-by: sashiko <sashiko@sashiko.dev> > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > index 8c3fbb413a06..1a7d5cd16d72 100644 > > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > > > write_sysreg_el1(esr, SYS_ESR); > > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > } > > Is SPSR_EL1 not set in enter_exception64() using vcpu_cpsr(vcpu), which > *is* set here? I'm just a bit wary of the report, as I'd have expected > fireworks if we weren't initialising the guest's SPSR on the exception > injection path. Yes, enter_exception64() sets SPSR_EL1, but only in memory: __vcpu_write_spsr() takes the nVHE path and calls __vcpu_assign_sys_reg(vcpu, SPSR_EL1, val), which writes vcpu->arch.ctxt.sys_regs[SPSR_EL1] without touching the hardware register. In the normal nVHE entry path that memory value reaches hardware via __sysreg_restore_state_nvhe() before __guest_enter(). But inject_sync64() runs inside the fixup_guest_exit() loop, which re-enters the guest directly without a sysreg restore pass. ESR_EL1 and ELR_EL1 are already written to hardware by hand for this reason but SPSR_EL1 was missed. I think we don't see fireworks because inject_sync64() only fires on error paths, e.g., accesses to restricted or undefined sysregs. A well-behaved guest shouldn't trigger these, so the stale SPSR_EL1 isn't observed. Cheers, /fuad > > Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-15 10:52 ` Fuad Tabba @ 2026-06-15 14:41 ` Will Deacon 2026-06-15 15:03 ` Fuad Tabba 0 siblings, 1 reply; 11+ messages in thread From: Will Deacon @ 2026-06-15 14:41 UTC (permalink / raw) To: Fuad Tabba Cc: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Sascha Bischoff, Andrew Jones On Mon, Jun 15, 2026 at 11:52:10AM +0100, Fuad Tabba wrote: > On Mon, 15 Jun 2026 at 11:05, Will Deacon <will@kernel.org> wrote: > > > > On Fri, Jun 12, 2026 at 12:34:14PM +0100, Fuad Tabba wrote: > > > When pKVM injects a synchronous exception into a protected guest, it > > > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > > > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > > > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > > > only in memory, so the guest's handler reads a stale SPSR_EL1 and > > > restores the wrong PSTATE on eret. > > > > > > Write SPSR_EL1 alongside the other exception registers. > > > > > > Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") > > > Reported-by: sashiko <sashiko@sashiko.dev> > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > > --- > > > arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > index 8c3fbb413a06..1a7d5cd16d72 100644 > > > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > > > > > write_sysreg_el1(esr, SYS_ESR); > > > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > > > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > > > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > > } > > > > Is SPSR_EL1 not set in enter_exception64() using vcpu_cpsr(vcpu), which > > *is* set here? I'm just a bit wary of the report, as I'd have expected > > fireworks if we weren't initialising the guest's SPSR on the exception > > injection path. > > Yes, enter_exception64() sets SPSR_EL1, but only in memory: > __vcpu_write_spsr() takes the nVHE path and calls > __vcpu_assign_sys_reg(vcpu, SPSR_EL1, val), which writes > vcpu->arch.ctxt.sys_regs[SPSR_EL1] without touching the hardware > register. > > In the normal nVHE entry path that memory value reaches hardware via > __sysreg_restore_state_nvhe() before __guest_enter(). But > inject_sync64() runs inside the fixup_guest_exit() loop, which > re-enters the guest directly without a sysreg restore pass. ESR_EL1 > and ELR_EL1 are already written to hardware by hand for this reason > but SPSR_EL1 was missed. Ah, I see. Does that mean that all the in-memory updates performed by enter_exception64() (e.g. the construction of the cpsr) are ignored too? Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-15 14:41 ` Will Deacon @ 2026-06-15 15:03 ` Fuad Tabba 2026-06-15 15:10 ` Will Deacon 0 siblings, 1 reply; 11+ messages in thread From: Fuad Tabba @ 2026-06-15 15:03 UTC (permalink / raw) To: Will Deacon Cc: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Sascha Bischoff, Andrew Jones On Mon, 15 Jun 2026 at 15:42, Will Deacon <will@kernel.org> wrote: > > On Mon, Jun 15, 2026 at 11:52:10AM +0100, Fuad Tabba wrote: > > On Mon, 15 Jun 2026 at 11:05, Will Deacon <will@kernel.org> wrote: > > > > > > On Fri, Jun 12, 2026 at 12:34:14PM +0100, Fuad Tabba wrote: > > > > When pKVM injects a synchronous exception into a protected guest, it > > > > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > > > > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > > > > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > > > > only in memory, so the guest's handler reads a stale SPSR_EL1 and > > > > restores the wrong PSTATE on eret. > > > > > > > > Write SPSR_EL1 alongside the other exception registers. > > > > > > > > Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") > > > > Reported-by: sashiko <sashiko@sashiko.dev> > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > > > --- > > > > arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > index 8c3fbb413a06..1a7d5cd16d72 100644 > > > > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > > > > > > > write_sysreg_el1(esr, SYS_ESR); > > > > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > > > > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > > > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > > > > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > > > } > > > > > > Is SPSR_EL1 not set in enter_exception64() using vcpu_cpsr(vcpu), which > > > *is* set here? I'm just a bit wary of the report, as I'd have expected > > > fireworks if we weren't initialising the guest's SPSR on the exception > > > injection path. > > > > Yes, enter_exception64() sets SPSR_EL1, but only in memory: > > __vcpu_write_spsr() takes the nVHE path and calls > > __vcpu_assign_sys_reg(vcpu, SPSR_EL1, val), which writes > > vcpu->arch.ctxt.sys_regs[SPSR_EL1] without touching the hardware > > register. > > > > In the normal nVHE entry path that memory value reaches hardware via > > __sysreg_restore_state_nvhe() before __guest_enter(). But > > inject_sync64() runs inside the fixup_guest_exit() loop, which > > re-enters the guest directly without a sysreg restore pass. ESR_EL1 > > and ELR_EL1 are already written to hardware by hand for this reason > > but SPSR_EL1 was missed. > > Ah, I see. Does that mean that all the in-memory updates performed by > enter_exception64() (e.g. the construction of the cpsr) are ignored too? From looking at the code, no. inject_sync64() reads back the cpsr and PC that enter_exception64() computed in memory and writes them to hardware: write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); So the guest re-enters with the right PSTATE. The only value left stranded in memory is SPSR_EL1. Cheers, /fuad > > Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-15 15:03 ` Fuad Tabba @ 2026-06-15 15:10 ` Will Deacon 0 siblings, 0 replies; 11+ messages in thread From: Will Deacon @ 2026-06-15 15:10 UTC (permalink / raw) To: Fuad Tabba Cc: Marc Zyngier, Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Sascha Bischoff, Andrew Jones On Mon, Jun 15, 2026 at 04:03:17PM +0100, Fuad Tabba wrote: > On Mon, 15 Jun 2026 at 15:42, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Jun 15, 2026 at 11:52:10AM +0100, Fuad Tabba wrote: > > > On Mon, 15 Jun 2026 at 11:05, Will Deacon <will@kernel.org> wrote: > > > > > > > > On Fri, Jun 12, 2026 at 12:34:14PM +0100, Fuad Tabba wrote: > > > > > When pKVM injects a synchronous exception into a protected guest, it > > > > > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > > > > > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > > > > > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > > > > > only in memory, so the guest's handler reads a stale SPSR_EL1 and > > > > > restores the wrong PSTATE on eret. > > > > > > > > > > Write SPSR_EL1 alongside the other exception registers. > > > > > > > > > > Fixes: 6c30bfb18d0b ("KVM: arm64: Add handlers for protected VM System Registers") > > > > > Reported-by: sashiko <sashiko@sashiko.dev> > > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > > > > --- > > > > > arch/arm64/kvm/hyp/nvhe/sys_regs.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > > index 8c3fbb413a06..1a7d5cd16d72 100644 > > > > > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c > > > > > @@ -268,6 +268,7 @@ static void inject_sync64(struct kvm_vcpu *vcpu, u64 esr) > > > > > > > > > > write_sysreg_el1(esr, SYS_ESR); > > > > > write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR); > > > > > + write_sysreg_el1(read_sysreg_el2(SYS_SPSR), SYS_SPSR); > > > > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > > > > > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > > > > } > > > > > > > > Is SPSR_EL1 not set in enter_exception64() using vcpu_cpsr(vcpu), which > > > > *is* set here? I'm just a bit wary of the report, as I'd have expected > > > > fireworks if we weren't initialising the guest's SPSR on the exception > > > > injection path. > > > > > > Yes, enter_exception64() sets SPSR_EL1, but only in memory: > > > __vcpu_write_spsr() takes the nVHE path and calls > > > __vcpu_assign_sys_reg(vcpu, SPSR_EL1, val), which writes > > > vcpu->arch.ctxt.sys_regs[SPSR_EL1] without touching the hardware > > > register. > > > > > > In the normal nVHE entry path that memory value reaches hardware via > > > __sysreg_restore_state_nvhe() before __guest_enter(). But > > > inject_sync64() runs inside the fixup_guest_exit() loop, which > > > re-enters the guest directly without a sysreg restore pass. ESR_EL1 > > > and ELR_EL1 are already written to hardware by hand for this reason > > > but SPSR_EL1 was missed. > > > > Ah, I see. Does that mean that all the in-memory updates performed by > > enter_exception64() (e.g. the construction of the cpsr) are ignored too? > > From looking at the code, no. inject_sync64() reads back the cpsr and > PC that enter_exception64() computed in memory and writes them to > hardware: > > write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR); > write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR); > > So the guest re-enters with the right PSTATE. The only value left > stranded in memory is SPSR_EL1. Ah, thanks. I had pissed that inject_sync64() calls __kvm_adjust_pc() immediately after pending the exception. So you've convinced me: Acked-by: Will Deacon <will@kernel.org> Will ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM 2026-06-12 11:34 [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM Fuad Tabba 2026-06-12 11:50 ` sashiko-bot 2026-06-15 10:05 ` Will Deacon @ 2026-06-17 12:41 ` Marc Zyngier 2 siblings, 0 replies; 11+ messages in thread From: Marc Zyngier @ 2026-06-17 12:41 UTC (permalink / raw) To: Oliver Upton, linux-arm-kernel, kvmarm, linux-kernel, Fuad Tabba Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon, Sascha Bischoff, Andrew Jones On Fri, 12 Jun 2026 12:34:14 +0100, Fuad Tabba wrote: > When pKVM injects a synchronous exception into a protected guest, it > re-enters without restoring the guest's EL1 sysregs and writes the EL1 > exception registers to hardware by hand: ESR_EL1 and ELR_EL1, but not > SPSR_EL1. enter_exception64() sets SPSR_EL1 (the interrupted PSTATE) > only in memory, so the guest's handler reads a stale SPSR_EL1 and > restores the wrong PSTATE on eret. > > [...] Applied to fixes, thanks! [1/1] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM commit: ec40342aaca8162bc8ab2607076535ebab1838b8 Cheers, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-17 12:41 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-12 11:34 [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM Fuad Tabba 2026-06-12 11:50 ` sashiko-bot 2026-06-12 13:19 ` Fuad Tabba 2026-06-14 8:49 ` Marc Zyngier 2026-06-14 10:36 ` Fuad Tabba 2026-06-15 10:05 ` Will Deacon 2026-06-15 10:52 ` Fuad Tabba 2026-06-15 14:41 ` Will Deacon 2026-06-15 15:03 ` Fuad Tabba 2026-06-15 15:10 ` Will Deacon 2026-06-17 12:41 ` Marc Zyngier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox