Re: [RFC PATCH v7 21/28] KVM: arm64: Handle SPE maintenance interrupts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] SPE system registers are not cleared during VCPU reset, leaking stale state across warm resets.

Pre-existing issues:
- [High] Hardware profiling buffer is enabled before `PMBSR_EL1` is restored, creating a race condition that clobbers hardware events.
--

commit 0111e834cf6f2849f99044fcfd2c2f6ef3480c83
Author: Alexandru Elisei <alexandru.elisei@arm.com>

KVM: arm64: Handle SPE maintenance interrupts

This commit introduces handling for SPE maintenance interrupts in KVM. It
injects the interrupt when the guest sets the PMBSR_EL1.S bit and re-injects
hardware-raised maintenance interrupts.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 4c4bf88c9aa53..ebe1c933f8d55 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1595,6 +1595,11 @@ static int get_user_spe_sysreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc
>  	return 0;
>  }
>  
> +static u64 reset_spe_sysreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> +{
> +	return kvm_spe_reset_sysreg(vcpu, r->reg);
> +}
> +

[Severity: Medium]
Does this change leak stale SPE register state across warm resets?

Previously, SPE registers used reset_val as their reset callback, which
actively updated the vCPU sysreg array via __vcpu_assign_sys_reg().

Here, the new reset_spe_sysreg() callback returns 0 but misses a call to
__vcpu_assign_sys_reg() to clear the VCPU's system register memory.
Because the KVM sysreg reset caller ignores the return value, the values in
vcpu->arch.ctxt.sys_regs remain untouched.

If a VM goes through a warm reset (e.g., via PSCI CPU_OFF / CPU_ON) while
profiling is active, the new OS environment inherits a stale profiling state.
Should kvm_spe_reset_sysreg() explicitly clear the register state?

>  static bool access_spe_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  			   const struct sys_reg_desc *r)
>  {
> @@ -1642,7 +1647,7 @@ static bool access_spe_reg(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  
>  #define SPE_SYS_REG(name)						\
>  	SYS_DESC(SYS_##name), .reg = name, .access = access_spe_reg,	\
> -	.reset = reset_val, .val = 0, .visibility = spe_visibility,	\
> +	.reset = reset_spe_sysreg, .visibility = spe_visibility,	\
>  	.set_user = set_user_spe_sysreg, .get_user = get_user_spe_sysreg

[Severity: High]
This is a pre-existing issue, but does __kvm_spe_restore_guest_state_vhe()
clobber hardware events by enabling the profiling buffer before restoring
PMBSR_EL1?

In arch/arm64/kvm/hyp/vhe/spe-sr.c:__kvm_spe_restore_guest_state_vhe():

	if (__kvm_spe_profiling_buffer_enabled(pmblimitr, pmbsr)) {
		write_sysreg_s(ctxt_sys_reg(guest_ctxt, PMBPTR_EL1), SYS_PMBPTR_EL1);
		isb();
		write_sysreg_s(pmblimitr, SYS_PMBLIMITR_EL1);
		isb();
		/* A buffer management event preserves fields in PMBSR_EL1 */
		write_sysreg_s(pmbsr, SYS_PMBSR_EL1);
	}

Because PMBLIMITR_EL1 is restored (with the E bit set) and an ISB executed
before PMBSR_EL1 is restored, hardware can autonomously trigger a buffer
management event right after the ISB if the buffer is near full.

KVM's subsequent software write of the guest's PMBSR_EL1 (which has S = 0)
overwrites the hardware register, wiping the event and clearing the
interrupt.

This violates the ARM Architecture Reference Manual D10.1.5, which forbids
software from writing to PMBSR_EL1 while PMBLIMITR_EL1.E == 1.
Should PMBSR_EL1 be restored before PMBLIMITR_EL1 to prevent this race
condition?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903160623.315525-1-alexandru.elisei@arm.com?part=21