Re: [PATCH v7 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code

On Wed, 29 Jul 2026 at 14:34, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Pre-existing bug: `enter_exception64` reads stale `SCTLR_EL1` during early trap handling in nVHE/pKVM.

Already fixed:
https://lore.kernel.org/all/[email protected]/.


/fuad

> --
>
> --- Patch [2]: [PATCH v7 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code ---
> commit e62bbea08d3143161297d701776a47e619a76db1
> Author: Fuad Tabba <[email protected]>
>
> KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code
>
> This commit replaces the locally open-coded __vcpu_read_sys_reg and
> __vcpu_write_sys_reg wrappers in exception.c with the canonical
> vcpu_{read,write}_sys_reg accessors. For nVHE builds, it maps these
> accessors directly to the raw in-memory context accessors, allowing HYP
> code to reuse helpers without function duplication.
>
> > diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> > index bef40ddb16dbc..754e2dc1df54a 100644
> > --- a/arch/arm64/kvm/hyp/exception.c
> > +++ b/arch/arm64/kvm/hyp/exception.c
>
> [ ... ]
>
> > @@ -101,14 +85,14 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
> >
> >       switch (target_mode) {
> >       case PSR_MODE_EL1h:
> > -             vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
> > -             sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
> > -             __vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
> > +             vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
> > +             sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>
> [Severity: High]
> This is a pre-existing issue, but does enter_exception64() read a stale
> SCTLR_EL1 value from the in-memory context here?
>
> When a protected VM triggers an early EL2 trap, the hypervisor handles it
> via the kvm_handle_pvm_sysreg() -> inject_undef64() -> inject_sync64() call
> chain. At that point, the guest's system registers aren't fully saved to the
> in-memory vcpu->arch.ctxt.
>
> While inject_sync64() explicitly synchronizes VBAR_EL1 from hardware to memory:
>
> arch/arm64/kvm/hyp/nvhe/sys_regs.c:inject_sync64() {
>     ...
>     __vcpu_assign_sys_reg(vcpu, VBAR_EL1, read_sysreg_el1(SYS_VBAR));
>     ...
> }
>
> It doesn't appear to synchronize SCTLR_EL1 before triggering exception
> injection.
>
> Since nVHE builds map vcpu_read_sys_reg() directly to the memory context,
> enter_exception64() seems to use a stale SCTLR_EL1 value to calculate the
> PSR_PAN_BIT and PSR_SSBS_BIT for the exception's PSTATE.
>
> This corrupted PSTATE is then written directly to the hardware SYS_SPSR_EL2
> by inject_sync64(), potentially poisoning the guest's state upon ERET.
>
> Should SCTLR_EL1 also be synchronized from hardware before injection?
>
> > +             vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL1);
> >               break;
> >       case PSR_MODE_EL2h:
> > -             vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL2);
> > -             sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> > -             __vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
> > +             vbar = vcpu_read_sys_reg(vcpu, VBAR_EL2);
> > +             sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL2);
> > +             vcpu_write_sys_reg(vcpu, *vcpu_pc(vcpu), ELR_EL2);
> >               break;
> >       default:
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2