Re: [PATCH v20 08/14] KVM: arm64: Enforce EXLOCK for SPSR and ELR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM incorrectly injects spurious EXLOCK exceptions for ELR_EL1 and SPSR_EL1 writes when executed from guest EL2.
--

commit 9a420a46393dc947b55c66f63966668a4f79d12b
Author: Mark Brown <broonie@kernel.org>

KVM: arm64: Enforce EXLOCK for SPSR and ELR

This commit adds enforcement of the EXLOCK rules for attempts to write to
SPSR_ELx and ELR_ELx from a guest. It injects an EXLOCK exception if both
GCSCR_ELx.EXLOCKEN and PSTATE.EXLOCK are set.

[ ... ]

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c5ce18b3f7d8d..1f80c26a98393 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2866,14 +2866,42 @@ static bool access_sp_el1(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> +static inline bool sysregs_exlocked(struct kvm_vcpu *vcpu)
> +{
> +	u64 gcscr;
> +
> +	if (!kvm_has_gcs(vcpu->kvm))
> +		return false;
> +
> +	if (!(vcpu->arch.ctxt.regs.pstate & PSR_EXLOCK_BIT))
> +		return false;
> +
> +	/*
> +	 * Note that the EXLOCKEN for the running EL is checked
> +	 * regardless of the register written to.
> +	 */
> +	if (is_hyp_ctxt(vcpu))
> +		gcscr = vcpu_read_sys_reg(vcpu, GCSCR_EL2);
> +	else
> +		gcscr = vcpu_read_sys_reg(vcpu, GCSCR_EL1);

[Severity: High]
Does this logic correctly follow the architectural requirements for EXLOCK
exceptions?

According to the ARM Architecture Reference Manual, the pseudocode for
AArch64.CheckEXLOCK(target_el) restricts EXLOCK exception generation to cases
where PSTATE.EL == target_el.

When a guest running at vEL2 accesses ELR_EL1 or SPSR_EL1, the target_el is 1,
but the PSTATE.EL is 2. Since 2 != 1, the hardware would not generate an
exception here. However, by checking the running EL's EXLOCKEN (GCSCR_EL2)
and ignoring the target EL of the register written to, KVM will inject a
spurious EXLOCK exception into the nested EL2 guest.

Can we update this to skip the check when target_el != PSTATE.EL to prevent
spurious exceptions during guest context switching?

> +
> +	return gcscr & GCSCR_ELx_EXLOCKEN;
> +}
> +
>  static bool access_elr(struct kvm_vcpu *vcpu,
>  		       struct sys_reg_params *p,
>  		       const struct sys_reg_desc *r)
>  {
> -	if (p->is_write)
> +	if (p->is_write) {
> +		if (sysregs_exlocked(vcpu)) {
> +			kvm_inject_exlock(vcpu);
> +			return false;
> +		}
> +

[Severity: High]
Can this cause spurious exceptions for vEL2 guests?

When a guest hypervisor writes to ELR_EL1 (which is architecturally valid and
necessary for nested virtualization), this check applies the flawed EXLOCK
logic from sysregs_exlocked() and triggers an incorrect exception injection.

>  		vcpu_write_sys_reg(vcpu, p->regval, ELR_EL1);
> -	else
> +	} else {
>  		p->regval = vcpu_read_sys_reg(vcpu, ELR_EL1);
> +	}
>  
>  	return true;
>  }
> @@ -2882,10 +2910,16 @@ static bool access_spsr(struct kvm_vcpu *vcpu,
>  			struct sys_reg_params *p,
>  			const struct sys_reg_desc *r)
>  {
> -	if (p->is_write)
> +	if (p->is_write) {
> +		if (sysregs_exlocked(vcpu)) {
> +			kvm_inject_exlock(vcpu);
> +			return false;
> +		}
> +

[Severity: High]
Similarly, can this lead to an incorrect exception injection when a guest
hypervisor accesses SPSR_EL1 from vEL2?

>  		__vcpu_assign_sys_reg(vcpu, SPSR_EL1, p->regval);
> -	else
> +	} else {
>  		p->regval = __vcpu_sys_reg(vcpu, SPSR_EL1);
> +	}
>  
>  	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-arm64-gcs-v20-0-f31750bdfadb@kernel.org?part=8