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

[Marc Zyngier <maz@kernel.org>]

On Tue, 05 Aug 2025 14:56:16 +0100,
Fuad Tabba <tabba@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@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?

	M.

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