Re: [PATCH v19 05/14] KVM: arm64: Set PSTATE.EXLOCK when entering an exception

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `compute_exlock()` uses `is_hyp_ctxt(vcpu)` to select between `GCSCR_EL1` and `GCSCR_EL2`, which evaluates the current execution context rather than the explicit target exception level.
--

--- Patch [5]: [PATCH v19 05/14] KVM: arm64: Set PSTATE.EXLOCK when entering an exception ---
commit 0b3b4f11382cdcd92d8ee1a1c35e2013e9df99ba
Author: Mark Brown <[email protected]>

KVM: arm64: Set PSTATE.EXLOCK when entering an exception

This commit implements the management of PSTATE.EXLOCK when entering an
exception in KVM's AArch64 exception injection path. When taking an exception
to the same EL, EXLOCK is set to the value of GCSCR_ELx.EXLOCKEN.

> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index bef40ddb16dbc..15b00e0ee1ec7 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
[ ... ]
> @@ -65,6 +65,38 @@ static void __vcpu_write_spsr_und(struct kvm_vcpu *vcpu, u64 val)
>  		vcpu->arch.ctxt.spsr_und = val;
>  }
>  
> +static unsigned long compute_exlock(struct kvm_vcpu *vcpu,
> +				    unsigned long mode,
> +				    unsigned long target_mode)
> +{
> +	u64 gcscr;
> +
> +	if (!kvm_has_gcs(kern_hyp_va(vcpu->kvm)))
> +		return 0;
> +
> +	/* GCS can't be enabled for 32 bit */
> +	if (mode & PSR_MODE32_BIT)
> +		return 0;
> +
> +	/* When taking an exception to a higher EL EXLOCK is cleared. */
> +	if ((mode | PSR_MODE_THREAD_BIT) != target_mode)
> +		return 0;
> +
> +	/*
> +	 * When taking an exception to the same EL EXLOCK is set to
> +	 * the effective value of GCSR_ELx.EXLOCKEN.
> +	 */
> +	if (is_hyp_ctxt(vcpu))
> +		gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL2);
> +	else
> +		gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL1);

[Severity: Medium]
Does this code read the correct GCSCR_ELx register if the target exception
level differs from the current hypervisor context?

By using is_hyp_ctxt(vcpu), this evaluates the current execution context
(for example, if HCR_EL2.TGE is set) rather than relying on target_mode. In
enter_exception64(), the target mode is explicitly used to select the
correct registers:

	switch (target_mode) {
	case PSR_MODE_EL1h:
		vbar = __vcpu_read_sys_reg(vcpu, VBAR_EL1);
		sctlr = __vcpu_read_sys_reg(vcpu, SCTLR_EL1);
		...

If KVM injects an exception to EL1 (where target_mode == PSR_MODE_EL1h)
while is_hyp_ctxt(vcpu) evaluates to true, compute_exlock() would
incorrectly read GCSCR_EL2 instead of GCSCR_EL1.

Should this logic be updated to use target_mode == PSR_MODE_EL2h instead of
is_hyp_ctxt(vcpu) to ensure consistency with the rest of the exception
injection path?

> +
> +	if (gcscr & GCSCR_ELx_EXLOCKEN)
> +		return PSR_EXLOCK_BIT;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5