Re: [PATCH v2 2/3] KVM: arm64: PMU: Reload when user modifies registers

[Marc Zyngier <maz@kernel.org>]

[dropping Shannon  from the list, as his Linaro email has been
bouncing for about 10 years, and updating Andrew's email]

On Fri, 07 Mar 2025 10:55:29 +0000,
Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> Commit d0c94c49792c ("KVM: arm64: Restore PMU configuration on first
> run") added the code to reload the PMU configuration on first run.
> Trigger the code when a user modifies a PMU register instead so that
> PMU configuration changes made by users will be applied also after the
> first run.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 3 ---
>  arch/arm64/kvm/sys_regs.c | 5 +++++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 1402cce5625bffa706aabe5e6121d1f3817a0aaf..04eb3856b96576fad5afc8927c8916ff9738f9d7 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -888,9 +888,6 @@ int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
>  		   return -EINVAL;
>  	}
>  
> -	/* One-off reload of the PMU on first run */
> -	kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
> -
>  	return 0;
>  }
>  
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 27418dac791df9a89124f867879e899db175e506..51054b7befc0b4bd822cecf717ee4a4740c4a685 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1162,6 +1162,8 @@ static int set_pmreg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r, u64 va
>  	else
>  		__vcpu_sys_reg(vcpu, r->reg) &= ~val;
>  
> +	kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
> +
>  	return 0;
>  }
>  
> @@ -1322,6 +1324,8 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
>  		val |= ARMV8_PMU_PMCR_LC;
>  
>  	__vcpu_sys_reg(vcpu, r->reg) = val;
> +	kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);
> +
>  	return 0;
>  }
>  
> @@ -4276,6 +4280,7 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu)
>  	}
>  
>  	set_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags);
> +	kvm_make_request(KVM_REQ_RELOAD_PMU, vcpu);

This should be restricted to cases where a PMU is actually available.

But I also wonder what the motivation is to move this from first run
to reset? Nothing explains it in the commit message.

Thanks,

	M.

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