Re: [PATCH v3 1/3] arm64: perf: Add support caps in sysfs

From: Shaokun Zhang <zhangshaokun@hisilicon.com>
To: Will Deacon <will@kernel.org>, Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 1/3] arm64: perf: Add support caps in sysfs
Date: Tue, 21 Jul 2020 16:05:45 +0800	[thread overview]
Message-ID: <d96e1068-8525-e636-f29c-86bbd9992f8f@hisilicon.com> (raw)
In-Reply-To: <20200720105410.GD11516@willie-the-truck>

Hi Will/Mark,

在 2020/7/20 18:54, Will Deacon 写道:
> On Mon, Jul 20, 2020 at 11:50:19AM +0100, Mark Rutland wrote:
>> On Mon, Jul 20, 2020 at 11:15:19AM +0100, Will Deacon wrote:
>>> On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:
>>
>>>> +static umode_t
>>>> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
>>>> +			      int unused)
>>>> +{
>>>> +	int pmuver = armv8pmu_get_pmu_version();
>>>> +
>>>> +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
>>>> +		return attr->mode;
>>>
>>> Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:
>>>
>>>   | If ARMv8.4-PMU is implemented:
>>>   | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
>>>   |   the PMMIR System registers are implemented.
>>>   | * If STALL_SLOT is implemented, then the PMMIR System registers are
>>>   |   implemented.
>>>
>>> whereas the register description for PMMIR_EL1 says:
>>>
>>>   | This register is present only when ARMv8.4-PMU is implemented.
>>
>> I think this is trying to say that when ARMv8.4-PMU is not implemented,
>> PMMIR definitely isn't implemented (i.e. the the presence of PMMIR_EL1
>> implies the presence of ARMv8.4-PMU).
>>
>>> Mark -- please could you clarify whether or not we need to check STALL_SLOT
>>> as well as the PMUVer?
>>
>> Given the explciit wording that it's IMP DEF, I suspect that we need to
>> check both.
>>
>> I'll go chase this up.
> 
> Thanks. In the meantime, Shaokun, can you send updated versions of the first
> two patches, please? We'll play it safe and check STALL_SLOT as well pending

In Arm ARM for PMMIR_EL1:

SLOTS, bits [7:0]
	Operation width. The largest value by which the STALL_SLOT event might increment by in a
	single cycle. If the STALL_SLOT event is not implemented, this field might read as zero.

And it's IMP DEF, even if the STALL_SLOT event is not implemented, and it can take
the SLOTS value and we can display it in sysfs if I follow it correctly.

Thanks,
Shaokun

> Mark's findings.
> 
> Will
> 
> .
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel