[PATCH v1 3/4] KVM: arm64: Sync protected guest VBAR_EL1 on injecting an undef exception

[Fuad Tabba]

Hi Marc,

On Tue, 5 Aug 2025 at 19:41, Marc Zyngier <maz at kernel.org> wrote:
>
> On Tue, 05 Aug 2025 14:56:16 +0100,
> Fuad Tabba <tabba at google.com> wrote:
> >
> > In pKVM, a race condition can occur if a guest updates its VBAR_EL1
> > register and, before a vCPU exit synchronizes this change, the
> > hypervisor needs to inject an undefined exception into a protected
> > guest.
> >
> > In this scenario, the vCPU still holds the stale VBAR_EL1 value from
> > before the guest's update. When pKVM injects the exception, it ends up
> > using the stale value.
> >
> > Explicitly read the live value of VBAR_EL1 from the guest and update the
> > vCPU value immediately before pending the exception. This ensures the
> > vCPU's value is the same as the guest's and that the exception will be
> > handled at the correct address upon resuming the guest.
> >
> > Signed-off-by: Fuad Tabba <tabba at 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 bbd60013cf9e..b34b10be1ad7 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> > @@ -253,6 +253,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
> >
> >       *vcpu_pc(vcpu) = read_sysreg_el2(SYS_ELR);
> >       *vcpu_cpsr(vcpu) = read_sysreg_el2(SYS_SPSR);
> > +     vcpu_write_sys_reg(vcpu, read_sysreg_el1(SYS_VBAR), VBAR_EL1);
> >
> >       kvm_pend_exception(vcpu, EXCEPT_AA64_EL1_SYNC);
> >
>
> There is something I don't understand. vcpu_write_sys_reg() is only
> useful if you make use of the SYSREGS_ON_CPU flag. Which is only
> driven by the VHE code (in arch/arm64/kvm/hyp/vhe/sysreg-sr.c).
>
> As a consequence, this only writes to memory, since the flag is always
> false, and we take the following path:
>
> static inline void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
> {
>         u64 (*xlate)(u64) = NULL;
>         unsigned int el1r;
>
>         if (!vcpu_get_flag(vcpu, SYSREGS_ON_CPU))
>                 goto memory_write;
>
> [...]
> memory_write:
>         __vcpu_assign_sys_reg(vcpu, reg, val);
> }
>
> My conclusion so far is that you only ever need to write to the shadow
> view of the register, and that the previous patch serves no purpose.
>
> Am I missing anything?

No, you're not. I realized that when I replied to Will (patch 2). I'll
drop that patch and fix this when I repin.

Cheers,
/fuad

/fuad

>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.