* [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