Hi Marc,
On 7/19/21 4:56 PM, Marc Zyngier wrote:
> On 2021-07-19 16:55, Alexandru Elisei wrote:
>> Hi Marc,
>>
>> On 7/19/21 1:38 PM, Marc Zyngier wrote:
>>> A number of the PMU sysregs expose reset values that are not
>>> compliant with the architecture (set bits in the RES0 ranges,
>>> for example).
>>>
>>> This in turn has the effect that we need to pointlessly mask
>>> some register fields when using them.
>>>
>>> Let's start by making sure we don't have illegal values in the
>>> shadow registers at reset time. This affects all the registers
>>> that dedicate one bit per counter, the counters themselves,
>>> PMEVTYPERn_EL0 and PMSELR_EL0.
>>>
>>> Reported-by: Alexandre Chartre <alexandre.chartre@xxxxxxxxxx>
>>> Reviewed-by: Alexandre Chartre <alexandre.chartre@xxxxxxxxxx>
>>> Acked-by: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
>>> ---
>>> arch/arm64/kvm/sys_regs.c | 43 ++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index f6f126eb6ac1..96bdfa0e68b2 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -603,6 +603,41 @@ static unsigned int pmu_visibility(const struct kvm_vcpu
>>> *vcpu,
>>> return REG_HIDDEN;
>>> }
>>>
>>> +static void 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;
>>> +
>>> + n = read_sysreg(pmcr_el0) >> ARMV8_PMU_PMCR_N_SHIFT;
>>> + n &= ARMV8_PMU_PMCR_N_MASK;
>>> + if (n)
>>> + mask |= GENMASK(n - 1, 0);
>>
>> Hm... seems to be missing the cycle counter.
>
> Check the declaration for 'mask'... :-)
Yeah, sorry for that, I still had in my mind the original function body.
Everything looks alright to me, no changes from the previous version (PMSWINC_EL1
is handled in the last patch) where I had checked that the reset values match the
architecture:
Reviewed-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Thanks,
Alex
