Re: [PATCH v2 08/27] KVM: arm64: nv: Use guest hypervisor's vSError state

[Marc Zyngier <maz@kernel.org>]

On Tue, 17 Jun 2025 00:02:49 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> When HCR_EL2.AMO is set, physical SErrors are routed to EL2 and virtual
> SError injection is enabled for EL1. Conceptually treating
> host-initiated SErrors as 'physical', this means we can delegate control
> of the vSError injection context to the guest hypervisor when nesting &&
> AMO is set.
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_emulate.h       |  5 +++
>  arch/arm64/kvm/hyp/include/hyp/switch.h    | 38 +++++++++++++++++++---
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++--
>  3 files changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 45029dd5e9c7..3ac11effed7d 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -257,6 +257,11 @@ static inline bool is_nested_ctxt(struct kvm_vcpu *vcpu)
>  	return vcpu_has_nv(vcpu) && !is_hyp_ctxt(vcpu);
>  }
>  
> +static inline bool vserror_state_is_nested(struct kvm_vcpu *vcpu)
> +{
> +	return is_nested_ctxt(vcpu) && vcpu_el2_amo_is_set(vcpu);
> +}
> +
>  /*
>   * The layout of SPSR for an AArch32 state is different when observed from an
>   * AArch64 SPSR_ELx or an AArch32 SPSR_*. This function generates the AArch32
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 71a66c910ab2..6a6ebc824aba 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -346,21 +346,49 @@ static inline void ___activate_traps(struct kvm_vcpu *vcpu, u64 hcr)
>  
>  	write_sysreg_hcr(hcr);
>  
> -	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN) && (hcr & HCR_VSE))
> -		write_sysreg_s(vcpu->arch.vsesr_el2, SYS_VSESR_EL2);
> +	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN) && (hcr & HCR_VSE)) {
> +		u64 vsesr;
> +
> +		/*
> +		 * When HCR_EL2.AMO is set, physical SErrors are taken to EL2
> +		 * and vSError injection is enabled for EL1. Conveniently, for
> +		 * NV this means that it is never the case where a 'physical'
> +		 * SError (injected by KVM or userspace) and vSError are
> +		 * deliverable to the same context.
> +		 *
> +		 * As such, we can trivially select between the host or guest's
> +		 * VSESR_EL2.
> +		 */
> +		if (vserror_state_is_nested(vcpu))
> +			vsesr = __vcpu_sys_reg(vcpu, VSESR_EL2);
> +		else
> +			vsesr = vcpu->arch.vsesr_el2;
> +
> +		write_sysreg_s(vsesr, SYS_VSESR_EL2);
> +	}
>  }
>  
>  static inline void ___deactivate_traps(struct kvm_vcpu *vcpu)
>  {
> +	u64 *hcr;
> +
> +	if (vserror_state_is_nested(vcpu))
> +		hcr = __ctxt_sys_reg(&vcpu->arch.ctxt, HCR_EL2);
> +	else
> +		hcr = &vcpu->arch.hcr_el2;
> +
>  	/*
>  	 * If we pended a virtual abort, preserve it until it gets
>  	 * cleared. See D1.14.3 (Virtual Interrupts) for details, but
>  	 * the crucial bit is "On taking a vSError interrupt,
>  	 * HCR_EL2.VSE is cleared to 0."
> +	 *
> +	 * Additionally, when in a nested context we need to propagate the
> +	 * updated state to the guest hypervisor's HCR_EL2.
>  	 */
> -	if (vcpu->arch.hcr_el2 & HCR_VSE) {
> -		vcpu->arch.hcr_el2 &= ~HCR_VSE;
> -		vcpu->arch.hcr_el2 |= read_sysreg(hcr_el2) & HCR_VSE;
> +	if (*hcr & HCR_VSE) {
> +		*hcr &= ~HCR_VSE;
> +		*hcr |= read_sysreg(hcr_el2) & HCR_VSE;
>  	}
>  }
>  
> diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> index 4d0dbea4c56f..7fffeaf71dec 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
> @@ -159,7 +159,12 @@ static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt)
>  	if (!has_vhe() && ctxt->__hyp_running_vcpu)
>  		ctxt->regs.pstate	= read_sysreg_el2(SYS_SPSR);
>  
> -	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> +	if (!cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> +		return;
> +
> +	if (vserror_state_is_nested(ctxt_to_vcpu(ctxt)))
> +		ctxt_sys_reg(ctxt, VDISR_EL2) = read_sysreg_s(SYS_VDISR_EL2);
> +	else
>  		ctxt_sys_reg(ctxt, DISR_EL1) = read_sysreg_s(SYS_VDISR_EL2);
>  }
>  
> @@ -293,7 +298,12 @@ static inline void __sysreg_restore_el2_return_state(struct kvm_cpu_context *ctx
>  	write_sysreg_el2(ctxt->regs.pc,			SYS_ELR);
>  	write_sysreg_el2(pstate,			SYS_SPSR);
>  
> -	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> +	if (!cpus_have_final_cap(ARM64_HAS_RAS_EXTN))
> +		return;
> +
> +	if (vserror_state_is_nested(ctxt_to_vcpu(ctxt)))
> +		write_sysreg_s(ctxt_sys_reg(ctxt, VDISR_EL2), SYS_VDISR_EL2);
> +	else
>  		write_sysreg_s(ctxt_sys_reg(ctxt, DISR_EL1), SYS_VDISR_EL2);
>  }
>  

I have the same question as for one of the previous patches: should
this also be gated on RAS being exposed to the guest?

Thanks,

	M.

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