Re: [PATCH v2 05/27] KVM: arm64: nv: Respect exception routing rules for SEAs

[Marc Zyngier <maz@kernel.org>]

On Tue, 17 Jun 2025 00:02:46 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Synchronous external aborts are taken to EL2 if ELIsInHost() or
> HCR_EL2.TEA=1. Rework the SEA injection plumbing to respect the imposed
> routing of the guest hypervisor and opportunistically rephrase things to
> make their function a bit more obvious.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_emulate.h | 14 +++++++--
>  arch/arm64/kvm/emulate-nested.c      |  9 ++++++
>  arch/arm64/kvm/guest.c               |  8 +++--
>  arch/arm64/kvm/inject_fault.c        | 45 +++++++++++-----------------
>  arch/arm64/kvm/mmio.c                |  6 ++--
>  arch/arm64/kvm/mmu.c                 | 15 +++-------
>  6 files changed, 50 insertions(+), 47 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 19ffe9b0d3c1..1a0d51c74b42 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -46,15 +46,25 @@ void kvm_skip_instr32(struct kvm_vcpu *vcpu);
>  
>  void kvm_inject_undefined(struct kvm_vcpu *vcpu);
>  void kvm_inject_vabt(struct kvm_vcpu *vcpu);
> -void kvm_inject_dabt(struct kvm_vcpu *vcpu, unsigned long addr);
> -void kvm_inject_pabt(struct kvm_vcpu *vcpu, unsigned long addr);
> +int kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr);
>  void kvm_inject_size_fault(struct kvm_vcpu *vcpu);
>  
> +static inline int kvm_inject_sea_dabt(struct kvm_vcpu *vcpu, u64 addr)
> +{
> +	return kvm_inject_sea(vcpu, false, addr);
> +}
> +
> +static inline int kvm_inject_sea_iabt(struct kvm_vcpu *vcpu, u64 addr)
> +{
> +	return kvm_inject_sea(vcpu, true, addr);
> +}
> +
>  void kvm_vcpu_wfi(struct kvm_vcpu *vcpu);
>  
>  void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu);
>  int kvm_inject_nested_sync(struct kvm_vcpu *vcpu, u64 esr_el2);
>  int kvm_inject_nested_irq(struct kvm_vcpu *vcpu);
> +int kvm_inject_nested_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr);
>  
>  static inline void kvm_inject_nested_sve_trap(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 1de4a9001d9d..aa5527ddf506 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2811,3 +2811,12 @@ int kvm_inject_nested_irq(struct kvm_vcpu *vcpu)
>  	/* esr_el2 value doesn't matter for exits due to irqs. */
>  	return kvm_inject_nested(vcpu, 0, except_type_irq);
>  }
> +
> +int kvm_inject_nested_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr)
> +{
> +	u64 esr = FIELD_PREP(ESR_ELx_EC_MASK,
> +			     iabt ? ESR_ELx_EC_IABT_LOW : ESR_ELx_EC_DABT_LOW);
> +	esr |= ESR_ELx_FSC_EXTABT | ESR_ELx_IL;
> +
> +	return kvm_inject_s2_fault(vcpu, esr);

I think this may be slightly abusive. R_FKLWR gives a list of all
faults that populate HPFAR_EL2, and SEA isn't one of them. I think
only populating ESR/FAR should be enough, and avoid leaking stale
fault addresses from arch.fault.hpfar_el2.

> +}
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 2196979a24a3..dd5cce0006f3 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -839,6 +839,7 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
>  	bool serror_pending = events->exception.serror_pending;
>  	bool has_esr = events->exception.serror_has_esr;
>  	bool ext_dabt_pending = events->exception.ext_dabt_pending;
> +	int ret;

Initialise ret to 0...

>
>  	if (serror_pending && has_esr) {
>  		if (!cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> @@ -852,8 +853,11 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
>  		kvm_inject_vabt(vcpu);
>  	}
>  
> -	if (ext_dabt_pending)
> -		kvm_inject_dabt(vcpu, kvm_vcpu_get_hfar(vcpu));
> +	if (ext_dabt_pending) {
> +		ret = kvm_inject_sea_dabt(vcpu, kvm_vcpu_get_hfar(vcpu));
> +		if (ret < 0)
> +			return ret;

... drop this test ...

> +	}
>  
>  	return 0;

... and return ret?

Otherwise, LGTM.

	M.

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