[PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

[PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

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

Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

[sashiko-bot]

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

Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

[Fuad Tabba]

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

Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

[Marc Zyngier]

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.

Re: [PATCH] KVM: arm64: Sync SPSR_EL1 when injecting an exception into a pVM

[Fuad Tabba]

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.