* [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM
@ 2026-06-12 11:34 Fuad Tabba
2026-06-15 10:05 ` Will Deacon
0 siblings, 1 reply; 6+ 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] 6+ 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-15 10:05 ` Will Deacon 2026-06-15 10:52 ` Fuad Tabba 0 siblings, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2026-06-15 15:10 UTC | newest] Thread overview: 6+ 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-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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox