* [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
@ 2024-06-11 15:50 ` Rob Herring (Arm)
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring (Arm) @ 2024-06-11 15:50 UTC (permalink / raw)
To: Will Deacon, Mark Rutland, James Clark; +Cc: linux-arm-kernel, linux-kernel
If the user has requested a counting threshold for the CPU cycles event,
then the fixed cycle counter can't be assigned as it lacks threshold
support. Currently, the thresholds will work or not randomly depending
on which counter the event is assigned.
While using thresholds for CPU cycles doesn't make much sense, it can be
useful for testing purposes.
Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
drivers/perf/arm_pmuv3.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 23fa6c5da82c..2612be29ee23 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
struct hw_perf_event *hwc = &event->hw;
unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
+ bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
/* Always prefer to place a cycle counter into the cycle counter. */
- if (evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
+ if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) && !has_threshold) {
if (!test_and_set_bit(ARMV8_IDX_CYCLE_COUNTER, cpuc->used_mask))
return ARMV8_IDX_CYCLE_COUNTER;
else if (armv8pmu_event_is_64bit(event) &&
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
@ 2024-06-11 15:50 ` Rob Herring (Arm)
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring (Arm) @ 2024-06-11 15:50 UTC (permalink / raw)
To: Will Deacon, Mark Rutland, James Clark; +Cc: linux-arm-kernel, linux-kernel
If the user has requested a counting threshold for the CPU cycles event,
then the fixed cycle counter can't be assigned as it lacks threshold
support. Currently, the thresholds will work or not randomly depending
on which counter the event is assigned.
While using thresholds for CPU cycles doesn't make much sense, it can be
useful for testing purposes.
Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
drivers/perf/arm_pmuv3.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 23fa6c5da82c..2612be29ee23 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
struct hw_perf_event *hwc = &event->hw;
unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
+ bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
/* Always prefer to place a cycle counter into the cycle counter. */
- if (evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
+ if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) && !has_threshold) {
if (!test_and_set_bit(ARMV8_IDX_CYCLE_COUNTER, cpuc->used_mask))
return ARMV8_IDX_CYCLE_COUNTER;
else if (armv8pmu_event_is_64bit(event) &&
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
2024-06-11 15:50 ` Rob Herring (Arm)
@ 2024-06-11 16:13 ` James Clark
-1 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-06-11 16:13 UTC (permalink / raw)
To: Rob Herring (Arm), Will Deacon, Mark Rutland
Cc: linux-arm-kernel, linux-kernel
On 11/06/2024 16:50, Rob Herring (Arm) wrote:
> If the user has requested a counting threshold for the CPU cycles event,
> then the fixed cycle counter can't be assigned as it lacks threshold
> support. Currently, the thresholds will work or not randomly depending
> on which counter the event is assigned.
>
> While using thresholds for CPU cycles doesn't make much sense, it can be
> useful for testing purposes.
>
> Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> drivers/perf/arm_pmuv3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 23fa6c5da82c..2612be29ee23 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> struct hw_perf_event *hwc = &event->hw;
> unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
I was going to say doesn't it need to be (ARMV8_PMU_EVTYPE_TH |
ARMV8_PMU_EVTYPE_TC) for it to give the same results as the hardware.
But then I saw we only enable it if TH != 0, even if TC is set. And now
I'm wondering if I inadvertently disabled a useful combination of options.
The Arm ARM says it's only completely disabled when both TC and TH are 0.
>
> /* Always prefer to place a cycle counter into the cycle counter. */
> - if (evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
> + if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) && !has_threshold) {
> if (!test_and_set_bit(ARMV8_IDX_CYCLE_COUNTER, cpuc->used_mask))
> return ARMV8_IDX_CYCLE_COUNTER;
> else if (armv8pmu_event_is_64bit(event) &&
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
@ 2024-06-11 16:13 ` James Clark
0 siblings, 0 replies; 8+ messages in thread
From: James Clark @ 2024-06-11 16:13 UTC (permalink / raw)
To: Rob Herring (Arm), Will Deacon, Mark Rutland
Cc: linux-arm-kernel, linux-kernel
On 11/06/2024 16:50, Rob Herring (Arm) wrote:
> If the user has requested a counting threshold for the CPU cycles event,
> then the fixed cycle counter can't be assigned as it lacks threshold
> support. Currently, the thresholds will work or not randomly depending
> on which counter the event is assigned.
>
> While using thresholds for CPU cycles doesn't make much sense, it can be
> useful for testing purposes.
>
> Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> drivers/perf/arm_pmuv3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 23fa6c5da82c..2612be29ee23 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> struct hw_perf_event *hwc = &event->hw;
> unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
I was going to say doesn't it need to be (ARMV8_PMU_EVTYPE_TH |
ARMV8_PMU_EVTYPE_TC) for it to give the same results as the hardware.
But then I saw we only enable it if TH != 0, even if TC is set. And now
I'm wondering if I inadvertently disabled a useful combination of options.
The Arm ARM says it's only completely disabled when both TC and TH are 0.
>
> /* Always prefer to place a cycle counter into the cycle counter. */
> - if (evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
> + if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) && !has_threshold) {
> if (!test_and_set_bit(ARMV8_IDX_CYCLE_COUNTER, cpuc->used_mask))
> return ARMV8_IDX_CYCLE_COUNTER;
> else if (armv8pmu_event_is_64bit(event) &&
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
2024-06-11 16:13 ` James Clark
(?)
@ 2024-06-18 16:30 ` James Clark
2024-06-24 16:58 ` Rob Herring
-1 siblings, 1 reply; 8+ messages in thread
From: James Clark @ 2024-06-18 16:30 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: linux-arm-kernel, linux-kernel, Will Deacon, Mark Rutland
On 11/06/2024 17:13, James Clark wrote:
>
>
> On 11/06/2024 16:50, Rob Herring (Arm) wrote:
>> If the user has requested a counting threshold for the CPU cycles event,
>> then the fixed cycle counter can't be assigned as it lacks threshold
>> support. Currently, the thresholds will work or not randomly depending
>> on which counter the event is assigned.
>>
>> While using thresholds for CPU cycles doesn't make much sense, it can be
>> useful for testing purposes.
>>
>> Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
>> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
>> ---
>> drivers/perf/arm_pmuv3.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
>> index 23fa6c5da82c..2612be29ee23 100644
>> --- a/drivers/perf/arm_pmuv3.c
>> +++ b/drivers/perf/arm_pmuv3.c
>> @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
>> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
>> struct hw_perf_event *hwc = &event->hw;
>> unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
>> + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
>
> I was going to say doesn't it need to be (ARMV8_PMU_EVTYPE_TH |
> ARMV8_PMU_EVTYPE_TC) for it to give the same results as the hardware.
> But then I saw we only enable it if TH != 0, even if TC is set. And now
> I'm wondering if I inadvertently disabled a useful combination of options.
>
> The Arm ARM says it's only completely disabled when both TC and TH are 0.
>
If it's easy it might be worth adding a helper function for
has_threshold() that's used in both places. That way if or when this
issue gets fixed up it doesn't break here.
>>
>> /* Always prefer to place a cycle counter into the cycle counter. */
>> - if (evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) {
>> + if ((evtype == ARMV8_PMUV3_PERFCTR_CPU_CYCLES) && !has_threshold) {
>> if (!test_and_set_bit(ARMV8_IDX_CYCLE_COUNTER, cpuc->used_mask))
>> return ARMV8_IDX_CYCLE_COUNTER;
>> else if (armv8pmu_event_is_64bit(event) &&
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
2024-06-18 16:30 ` James Clark
@ 2024-06-24 16:58 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2024-06-24 16:58 UTC (permalink / raw)
To: James Clark; +Cc: linux-arm-kernel, linux-kernel, Will Deacon, Mark Rutland
On Tue, Jun 18, 2024 at 10:30 AM James Clark <james.clark@arm.com> wrote:
>
>
>
> On 11/06/2024 17:13, James Clark wrote:
> >
> >
> > On 11/06/2024 16:50, Rob Herring (Arm) wrote:
> >> If the user has requested a counting threshold for the CPU cycles event,
> >> then the fixed cycle counter can't be assigned as it lacks threshold
> >> support. Currently, the thresholds will work or not randomly depending
> >> on which counter the event is assigned.
> >>
> >> While using thresholds for CPU cycles doesn't make much sense, it can be
> >> useful for testing purposes.
> >>
> >> Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
> >> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> >> ---
> >> drivers/perf/arm_pmuv3.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> >> index 23fa6c5da82c..2612be29ee23 100644
> >> --- a/drivers/perf/arm_pmuv3.c
> >> +++ b/drivers/perf/arm_pmuv3.c
> >> @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
> >> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> >> struct hw_perf_event *hwc = &event->hw;
> >> unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> >> + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
> >
> > I was going to say doesn't it need to be (ARMV8_PMU_EVTYPE_TH |
> > ARMV8_PMU_EVTYPE_TC) for it to give the same results as the hardware.
> > But then I saw we only enable it if TH != 0, even if TC is set. And now
> > I'm wondering if I inadvertently disabled a useful combination of options.
> >
> > The Arm ARM says it's only completely disabled when both TC and TH are 0.
> >
>
> If it's easy it might be worth adding a helper function for
> has_threshold() that's used in both places. That way if or when this
> issue gets fixed up it doesn't break here.
The other place being in armv8pmu_set_event_filter()? A helper doesn't
help there because that looks at the attr value, not config_base.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
2024-06-11 15:50 ` Rob Herring (Arm)
(?)
(?)
@ 2024-06-18 15:41 ` Will Deacon
2024-06-24 20:14 ` Rob Herring
-1 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2024-06-18 15:41 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Mark Rutland, James Clark, linux-arm-kernel, linux-kernel
On Tue, Jun 11, 2024 at 09:50:12AM -0600, Rob Herring (Arm) wrote:
> If the user has requested a counting threshold for the CPU cycles event,
> then the fixed cycle counter can't be assigned as it lacks threshold
> support. Currently, the thresholds will work or not randomly depending
> on which counter the event is assigned.
>
> While using thresholds for CPU cycles doesn't make much sense, it can be
> useful for testing purposes.
>
> Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> drivers/perf/arm_pmuv3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 23fa6c5da82c..2612be29ee23 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
> struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> struct hw_perf_event *hwc = &event->hw;
> unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
Just a nit, but I don't think you need the '!!' here.
Will
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold
2024-06-18 15:41 ` Will Deacon
@ 2024-06-24 20:14 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2024-06-24 20:14 UTC (permalink / raw)
To: Will Deacon; +Cc: Mark Rutland, James Clark, linux-arm-kernel, linux-kernel
On Tue, Jun 18, 2024 at 9:41 AM Will Deacon <will@kernel.org> wrote:
>
> On Tue, Jun 11, 2024 at 09:50:12AM -0600, Rob Herring (Arm) wrote:
> > If the user has requested a counting threshold for the CPU cycles event,
> > then the fixed cycle counter can't be assigned as it lacks threshold
> > support. Currently, the thresholds will work or not randomly depending
> > on which counter the event is assigned.
> >
> > While using thresholds for CPU cycles doesn't make much sense, it can be
> > useful for testing purposes.
> >
> > Fixes: 816c26754447 ("arm64: perf: Add support for event counting threshold")
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> > drivers/perf/arm_pmuv3.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> > index 23fa6c5da82c..2612be29ee23 100644
> > --- a/drivers/perf/arm_pmuv3.c
> > +++ b/drivers/perf/arm_pmuv3.c
> > @@ -939,9 +939,10 @@ static int armv8pmu_get_event_idx(struct pmu_hw_events *cpuc,
> > struct arm_pmu *cpu_pmu = to_arm_pmu(event->pmu);
> > struct hw_perf_event *hwc = &event->hw;
> > unsigned long evtype = hwc->config_base & ARMV8_PMU_EVTYPE_EVENT;
> > + bool has_threshold = !!(hwc->config_base & ARMV8_PMU_EVTYPE_TH);
>
> Just a nit, but I don't think you need the '!!' here.
Right, I guess since bool is a first class type in C9X we don't have
to worry about truncation. Old habits...
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-06-24 20:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-11 15:50 [PATCH] perf: arm_pmuv3: Avoid assigning fixed cycle counter with threshold Rob Herring (Arm)
2024-06-11 15:50 ` Rob Herring (Arm)
2024-06-11 16:13 ` James Clark
2024-06-11 16:13 ` James Clark
2024-06-18 16:30 ` James Clark
2024-06-24 16:58 ` Rob Herring
2024-06-18 15:41 ` Will Deacon
2024-06-24 20:14 ` Rob Herring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.