Re: [PATCH v21 06/15] KVM: arm64: Validate GCS exception lock when emulating ERET

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Sep 30, 2026 at 10:48:16PM +0100, Mark Brown wrote:
> As per DDI0487 R_TYTWB GCS adds an additional case where an illegal
> exception return can be generated. If all of:
>
>  - PSTATE.EXLOCK is 0.
>  - The EL is not being changed by the ERET.
>  - GCSCR_ELx.EXLOCKEN is 1.

Ack can see from [0] in D.1.4.4.2 (M.d):

"If the Effective value of GCSCR_ELx.EXLOCKEN is 1 and PSTATE.EXLOCK is 0, the
execution of an exception return instruction to return to the current Exception
level ELx."

[0]: https://support.arm.com/documentation/ddi0487/md/-Part-D-The-AArch64-System-Level-Architecture/-Chapter-D1-The-AArch64-System-Level-Programmers--Model/-D1-4-Exceptions/-D1-4-4-Exception-return?lang=en

>
> are true then the return is illegal. Emulate this behaviour when
> emulating ERET for nested guests, while we're at it using the symbolic
> definition for EXLOCK in SPSR.
>
> Reviewed-by: Leonardo Bras <leo.bras@arm.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>

Some comments below. In general LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> index 1ed708335809..9e595e8e7642 100644
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h

> +#ifdef CONFIG_ARM64_GCS
> +/* See IllegalExceptionReturn() pseudocode */

Ack I see that at [1].

[1]: https://support.arm.com/documentation/ddi0487/md/-Part-J-Architectural-Pseudocode/-Chapter-J1-A-profile-Architecture-Pseudocode/-J1-4-Shared-pseudocode/-J1-4-647-IllegalExceptionReturn?lang=en

> +static inline bool kvm_check_illegal_exlock_return(struct kvm_vcpu *vcpu,
> +						   u64 spsr)
> +{
> +	u64 pstate, cur_mode, target_mode;
> +
> +	if (!kvm_has_gcs(vcpu->kvm))
> +		return false;
> +
> +	if (vcpu->arch.ctxt.regs.pstate & PSR_EXLOCK_BIT)
> +		return false;
> +
> +	/* Check the EL only, ignore thread mode */
> +	pstate = vcpu->arch.ctxt.regs.pstate;
> +	cur_mode = (pstate & PSR_MODE_MASK) | PSR_MODE_THREAD_BIT;
> +	target_mode = (spsr & PSR_MODE_MASK) | PSR_MODE_THREAD_BIT;
> +
> +	if (cur_mode != target_mode)
> +		return false;
> +
> +	return vcpu_read_sys_reg(vcpu, GCSCR_EL2) & GCSCR_ELx_EXLOCKEN;

This seems identical, logically, to the psuedocode:

    if (IsFeatureImplemented(FEAT_GCS) && PSTATE.EXLOCK == ‘0’ &&
          PSTATE.EL == target && GetCurrentEXLOCKEN()) then
        return TRUE;
    end;
    return FALSE;

So LGTM.

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 625604019fb3..5aa26384720a 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2748,10 +2748,13 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>  	 * - trying to return to an illegal M value
>  	 * - trying to return to a 32bit EL
>  	 * - trying to return to EL1 with HCR_EL2.TGE set
> +	 * - GCSCR_ELx.EXLOCKEN is 1 and PSTATE.EXLOCK is 0 when attempting
> +	 *   to return from ELx the same EL.

This seems a little unclear, 'when attempting to return to the same EL'
perhaps?

> @@ -2778,7 +2781,7 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>
>  		mask = PSR_MODE_MASK | PSR_MODE32_BIT;
>  		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))

Could use kvm_has_gcs()?

> -			mask |= BIT_ULL(34);	/* PSTATE.EXLOCK */
> +			mask |= PSR_EXLOCK_BIT;

Ah nice to not hard code that any more!

> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 7875911c0506..af59d1bf51ad 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -383,6 +383,10 @@ static bool kvm_hyp_handle_eret(struct kvm_vcpu *vcpu, u64 *exit_code)
>  		return false;
>  	}
>
> +	/* Push GCS exception lock failures into the slow path */
> +	if (kvm_check_illegal_exlock_return(vcpu, spsr))
> +		return false;
> +

Ah yeah because false -> slow path and illegal exceptions shouldn't be fast path
:)

--
Cheers, Lorenzo