Hi Raghavendra,
On 10/10/23 01:08, Raghavendra Rao Ananta wrote:
> From: Reiji Watanabe <reijiw@xxxxxxxxxx>
>
> On vCPU reset, PMCNTEN{SET,CLR}_EL0, PMINTEN{SET,CLR}_EL1, and
> PMOVS{SET,CLR}_EL1 for a vCPU are reset by reset_pmu_reg().
PMOVS{SET,CLR}_EL0?
> This function clears RAZ bits of those registers corresponding
> to unimplemented event counters on the vCPU, and sets bits
> corresponding to implemented event counters to a predefined
> pseudo UNKNOWN value (some bits are set to 1).
>
> The function identifies (un)implemented event counters on the
> vCPU based on the PMCR_EL0.N value on the host. Using the host
> value for this would be problematic when KVM supports letting
> userspace set PMCR_EL0.N to a value different from the host value
> (some of the RAZ bits of those registers could end up being set to 1).
>
> Fix this by clearing the registers so that it can ensure
> that all the RAZ bits are cleared even when the PMCR_EL0.N value
> for the vCPU is different from the host value. Use reset_val() to
> do this instead of fixing reset_pmu_reg(), and remove
> reset_pmu_reg(), as it is no longer used.
do you intend to restore the 'unknown' behavior at some point?
Thanks
Eric
>
> Signed-off-by: Reiji Watanabe <reijiw@xxxxxxxxxx>
> Signed-off-by: Raghavendra Rao Ananta <rananta@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 21 +--------------------
> 1 file changed, 1 insertion(+), 20 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 818a52e257ed..3dbb7d276b0e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -717,25 +717,6 @@ static unsigned int pmu_visibility(const struct kvm_vcpu *vcpu,
> return REG_HIDDEN;
> }
>
> -static u64 reset_pmu_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> -{
> - u64 n, mask = BIT(ARMV8_PMU_CYCLE_IDX);
> -
> - /* No PMU available, any PMU reg may UNDEF... */
> - if (!kvm_arm_support_pmu_v3())
> - return 0;
> -
> - n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
> - n &= ARMV8_PMU_PMCR_N_MASK;
> - if (n)
> - mask |= GENMASK(n - 1, 0);
> -
> - reset_unknown(vcpu, r);
> - __vcpu_sys_reg(vcpu, r->reg) &= mask;
> -
> - return __vcpu_sys_reg(vcpu, r->reg);
> -}
> -
> static u64 reset_pmevcntr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
> {
> reset_unknown(vcpu, r);
> @@ -1115,7 +1096,7 @@ static bool access_pmuserenr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> trap_wcr, reset_wcr, 0, 0, get_wcr, set_wcr }
>
> #define PMU_SYS_REG(name) \
> - SYS_DESC(SYS_##name), .reset = reset_pmu_reg, \
> + SYS_DESC(SYS_##name), .reset = reset_val, \
> .visibility = pmu_visibility
>
> /* Macro to expand the PMEVCNTRn_EL0 register */
