On 10/17/18 12:20 PM, Aaron Lindsay wrote:
> On Oct 16 17:09, Richard Henderson wrote:
>> On 10/10/18 1:37 PM, Aaron Lindsay wrote:
>>> This both advertises that we support four counters and enables them
>>> because the pmu_num_counters() reads this value from PMCR.
>>>
>>> Signed-off-by: Aaron Lindsay <alind...@codeaurora.org>
>>> ---
>>> target/arm/helper.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> It looks like this should have been folded into patch 10,
>> or patch 10 split to include the code changes that go with
>> these comment changes.
>
> By setting PMCR.N to 4, this commit is what causes all the previous
> implementation of non-PMCCNTR counters to become usable by the guest -
> both because the OS can now detect their presence in PMCR, and because
> the system registers used to setup/access the counters are gated on
> pmu_num_counters(), which uses PMCR.N to determine if a counter is
> implemented.
But e.g.
> @@ -5412,8 +5412,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> }
> if (arm_feature(env, ARM_FEATURE_V7)) {
> /* v7 performance monitor control register: same implementor
> - * field as main ID register, and we implement only the cycle
> - * count register.
> + * field as main ID register, and we implement four counters in
> + * addition to the cycle count register.
> */
> unsigned int i, pmcrn = 4;
> ARMCPRegInfo pmcr = {
clearly belongs with the previous patch, since pmcrn is already 4.
> @@ -1706,7 +1706,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
> .access = PL1_W, .type = ARM_CP_NOP },
> /* Performance monitors are implementation defined in v7,
> * but with an ARM recommended set of registers, which we
> - * follow (although we don't actually implement any counters)
> + * follow.
> *
> * Performance registers fall into three categories:
> * (a) always UNDEF in PL0, RW in PL1 (PMINTENSET, PMINTENCLR)
And this is out of date as well, and probably belonged elsewhere.
Which leaves only
> @@ -5430,7 +5430,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> .access = PL0_RW, .accessfn = pmreg_access,
> .type = ARM_CP_IO,
> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> - .resetvalue = cpu->midr & 0xff000000,
> + .resetvalue = (cpu->midr & 0xff000000) | (pmcrn << PMCRN_SHIFT),
> .writefn = pmcr_write, .raw_writefn = raw_write,
> };
which suggests that this one line belonged to the patch that set pmcrn in the
first place.
r~
