* [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
@ 2020-06-02 2:56 Alexey Kardashevskiy
2020-06-03 16:34 ` Madhavan Srinivasan
2020-08-27 7:46 ` Michael Ellerman
0 siblings, 2 replies; 6+ messages in thread
From: Alexey Kardashevskiy @ 2020-06-02 2:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Madhavan Srinivasan
The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
generic_compat_pmu which does not have this callback and crashed occur.
This add a NULL pointer check for bhrb_filter_map() which behaves as if
the callback returned an error.
This does not add the same check for config_bhrb() as the only caller
checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3dcfecf858f3..36870569bf9c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
ret = 0;
out:
if (has_branch_stack(event)) {
- power_pmu_bhrb_enable(event);
- cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
- event->attr.branch_sample_type);
+ u64 bhrb_filter = -1;
+
+ if (ppmu->bhrb_filter_map)
+ bhrb_filter = ppmu->bhrb_filter_map(
+ event->attr.branch_sample_type);
+
+ if (bhrb_filter != -1) {
+ cpuhw->bhrb_filter = bhrb_filter;
+ power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
+ }
}
perf_pmu_enable(event->pmu);
@@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
int n;
int err;
struct cpu_hw_events *cpuhw;
- u64 bhrb_filter;
if (!ppmu)
return -ENOENT;
@@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
err = power_check_constraints(cpuhw, events, cflags, n + 1);
if (has_branch_stack(event)) {
- bhrb_filter = ppmu->bhrb_filter_map(
+ u64 bhrb_filter = -1;
+
+ if (ppmu->bhrb_filter_map)
+ bhrb_filter = ppmu->bhrb_filter_map(
event->attr.branch_sample_type);
if (bhrb_filter == -1) {
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
2020-06-02 2:56 [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu Alexey Kardashevskiy
@ 2020-06-03 16:34 ` Madhavan Srinivasan
2020-12-02 3:01 ` Alexey Kardashevskiy
2020-08-27 7:46 ` Michael Ellerman
1 sibling, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2020-06-03 16:34 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Madhavan Srinivasan
On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
> The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
> generic_compat_pmu which does not have this callback and crashed occur.
>
> This add a NULL pointer check for bhrb_filter_map() which behaves as if
> the callback returned an error.
>
> This does not add the same check for config_bhrb() as the only caller
> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
Changes looks fine.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu
driver')
which introduced generic_compat_pmu was merged in v5.2. So we need to
CC stable starting from 5.2 :( . My bad, sorry.
Maddy
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3dcfecf858f3..36870569bf9c 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
> ret = 0;
> out:
> if (has_branch_stack(event)) {
> - power_pmu_bhrb_enable(event);
> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> - event->attr.branch_sample_type);
> + u64 bhrb_filter = -1;
> +
> + if (ppmu->bhrb_filter_map)
> + bhrb_filter = ppmu->bhrb_filter_map(
> + event->attr.branch_sample_type);
> +
> + if (bhrb_filter != -1) {
> + cpuhw->bhrb_filter = bhrb_filter;
> + power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
> + }
> }
>
> perf_pmu_enable(event->pmu);
> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
> int n;
> int err;
> struct cpu_hw_events *cpuhw;
> - u64 bhrb_filter;
>
> if (!ppmu)
> return -ENOENT;
> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
> if (has_branch_stack(event)) {
> - bhrb_filter = ppmu->bhrb_filter_map(
> + u64 bhrb_filter = -1;
> +
> + if (ppmu->bhrb_filter_map)
> + bhrb_filter = ppmu->bhrb_filter_map(
> event->attr.branch_sample_type);
>
> if (bhrb_filter == -1) {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
2020-06-03 16:34 ` Madhavan Srinivasan
@ 2020-12-02 3:01 ` Alexey Kardashevskiy
2020-12-03 5:27 ` Madhavan Srinivasan
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Kardashevskiy @ 2020-12-02 3:01 UTC (permalink / raw)
To: Madhavan Srinivasan, linuxppc-dev; +Cc: Madhavan Srinivasan
Hi Maddy,
I just noticed that I still have "powerpc/perf: Add checks for reserved
values" in my pile (pushed here
https://github.com/aik/linux/commit/61e1bc3f2e19d450e2e2d39174d422160b21957b
), do we still need it? The lockups I saw were fixed by
https://github.com/aik/linux/commit/17899eaf88d689 but it is hardly a
replacement. Thanks,
On 04/06/2020 02:34, Madhavan Srinivasan wrote:
>
>
> On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
>> The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
>> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
>> generic_compat_pmu which does not have this callback and crashed occur.
>>
>> This add a NULL pointer check for bhrb_filter_map() which behaves as if
>> the callback returned an error.
>>
>> This does not add the same check for config_bhrb() as the only caller
>> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
>
> Changes looks fine.
> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>
> The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu
> driver')
> which introduced generic_compat_pmu was merged in v5.2. So we need to
> CC stable starting from 5.2 :( . My bad, sorry.
>
> Maddy
>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c
>> b/arch/powerpc/perf/core-book3s.c
>> index 3dcfecf858f3..36870569bf9c 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event
>> *event, int ef_flags)
>> ret = 0;
>> out:
>> if (has_branch_stack(event)) {
>> - power_pmu_bhrb_enable(event);
>> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
>> - event->attr.branch_sample_type);
>> + u64 bhrb_filter = -1;
>> +
>> + if (ppmu->bhrb_filter_map)
>> + bhrb_filter = ppmu->bhrb_filter_map(
>> + event->attr.branch_sample_type);
>> +
>> + if (bhrb_filter != -1) {
>> + cpuhw->bhrb_filter = bhrb_filter;
>> + power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
>> + }
>> }
>>
>> perf_pmu_enable(event->pmu);
>> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct
>> perf_event *event)
>> int n;
>> int err;
>> struct cpu_hw_events *cpuhw;
>> - u64 bhrb_filter;
>>
>> if (!ppmu)
>> return -ENOENT;
>> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct
>> perf_event *event)
>> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>>
>> if (has_branch_stack(event)) {
>> - bhrb_filter = ppmu->bhrb_filter_map(
>> + u64 bhrb_filter = -1;
>> +
>> + if (ppmu->bhrb_filter_map)
>> + bhrb_filter = ppmu->bhrb_filter_map(
>> event->attr.branch_sample_type);
>>
>> if (bhrb_filter == -1) {
>
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
2020-12-02 3:01 ` Alexey Kardashevskiy
@ 2020-12-03 5:27 ` Madhavan Srinivasan
2021-02-16 1:06 ` Alexey Kardashevskiy
0 siblings, 1 reply; 6+ messages in thread
From: Madhavan Srinivasan @ 2020-12-03 5:27 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Madhavan Srinivasan
On 12/2/20 8:31 AM, Alexey Kardashevskiy wrote:
> Hi Maddy,
>
> I just noticed that I still have "powerpc/perf: Add checks for
> reserved values" in my pile (pushed here
> https://github.com/aik/linux/commit/61e1bc3f2e19d450e2e2d39174d422160b21957b
> ), do we still need it? The lockups I saw were fixed by
> https://github.com/aik/linux/commit/17899eaf88d689 but it is hardly a
> replacement. Thanks,
sorry missed this. Will look at this again. Since we will need
generation specific checks for the reserve field.
Maddy
>
>
> On 04/06/2020 02:34, Madhavan Srinivasan wrote:
>>
>>
>> On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
>>> The bhrb_filter_map ("The Branch History Rolling Buffer")
>>> callback is
>>> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
>>> generic_compat_pmu which does not have this callback and crashed occur.
>>>
>>> This add a NULL pointer check for bhrb_filter_map() which behaves as if
>>> the callback returned an error.
>>>
>>> This does not add the same check for config_bhrb() as the only caller
>>> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
>>
>> Changes looks fine.
>> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>
>> The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu
>> driver')
>> which introduced generic_compat_pmu was merged in v5.2. So we need to
>> CC stable starting from 5.2 :( . My bad, sorry.
>>
>> Maddy
>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
>>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/perf/core-book3s.c
>>> b/arch/powerpc/perf/core-book3s.c
>>> index 3dcfecf858f3..36870569bf9c 100644
>>> --- a/arch/powerpc/perf/core-book3s.c
>>> +++ b/arch/powerpc/perf/core-book3s.c
>>> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event
>>> *event, int ef_flags)
>>> ret = 0;
>>> out:
>>> if (has_branch_stack(event)) {
>>> - power_pmu_bhrb_enable(event);
>>> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
>>> - event->attr.branch_sample_type);
>>> + u64 bhrb_filter = -1;
>>> +
>>> + if (ppmu->bhrb_filter_map)
>>> + bhrb_filter = ppmu->bhrb_filter_map(
>>> + event->attr.branch_sample_type);
>>> +
>>> + if (bhrb_filter != -1) {
>>> + cpuhw->bhrb_filter = bhrb_filter;
>>> + power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
>>> + }
>>> }
>>>
>>> perf_pmu_enable(event->pmu);
>>> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct
>>> perf_event *event)
>>> int n;
>>> int err;
>>> struct cpu_hw_events *cpuhw;
>>> - u64 bhrb_filter;
>>>
>>> if (!ppmu)
>>> return -ENOENT;
>>> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct
>>> perf_event *event)
>>> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>>>
>>> if (has_branch_stack(event)) {
>>> - bhrb_filter = ppmu->bhrb_filter_map(
>>> + u64 bhrb_filter = -1;
>>> +
>>> + if (ppmu->bhrb_filter_map)
>>> + bhrb_filter = ppmu->bhrb_filter_map(
>>> event->attr.branch_sample_type);
>>>
>>> if (bhrb_filter == -1) {
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
2020-12-03 5:27 ` Madhavan Srinivasan
@ 2021-02-16 1:06 ` Alexey Kardashevskiy
0 siblings, 0 replies; 6+ messages in thread
From: Alexey Kardashevskiy @ 2021-02-16 1:06 UTC (permalink / raw)
To: Madhavan Srinivasan, linuxppc-dev; +Cc: Madhavan Srinivasan
On 03/12/2020 16:27, Madhavan Srinivasan wrote:
>
> On 12/2/20 8:31 AM, Alexey Kardashevskiy wrote:
>> Hi Maddy,
>>
>> I just noticed that I still have "powerpc/perf: Add checks for
>> reserved values" in my pile (pushed here
>> https://github.com/aik/linux/commit/61e1bc3f2e19d450e2e2d39174d422160b21957b
>> ), do we still need it? The lockups I saw were fixed by
>> https://github.com/aik/linux/commit/17899eaf88d689 but it is hardly a
>> replacement. Thanks,
>
> sorry missed this. Will look at this again. Since we will need
> generation specific checks for the reserve field.
So any luck with this? Cheers,
>
> Maddy
>
>>
>>
>> On 04/06/2020 02:34, Madhavan Srinivasan wrote:
>>>
>>>
>>> On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
>>>> The bhrb_filter_map ("The Branch History Rolling Buffer")
>>>> callback is
>>>> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
>>>> generic_compat_pmu which does not have this callback and crashed occur.
>>>>
>>>> This add a NULL pointer check for bhrb_filter_map() which behaves as if
>>>> the callback returned an error.
>>>>
>>>> This does not add the same check for config_bhrb() as the only caller
>>>> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
>>>
>>> Changes looks fine.
>>> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>
>>> The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu
>>> driver')
>>> which introduced generic_compat_pmu was merged in v5.2. So we need to
>>> CC stable starting from 5.2 :( . My bad, sorry.
>>>
>>> Maddy
>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>> arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
>>>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/perf/core-book3s.c
>>>> b/arch/powerpc/perf/core-book3s.c
>>>> index 3dcfecf858f3..36870569bf9c 100644
>>>> --- a/arch/powerpc/perf/core-book3s.c
>>>> +++ b/arch/powerpc/perf/core-book3s.c
>>>> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event
>>>> *event, int ef_flags)
>>>> ret = 0;
>>>> out:
>>>> if (has_branch_stack(event)) {
>>>> - power_pmu_bhrb_enable(event);
>>>> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
>>>> - event->attr.branch_sample_type);
>>>> + u64 bhrb_filter = -1;
>>>> +
>>>> + if (ppmu->bhrb_filter_map)
>>>> + bhrb_filter = ppmu->bhrb_filter_map(
>>>> + event->attr.branch_sample_type);
>>>> +
>>>> + if (bhrb_filter != -1) {
>>>> + cpuhw->bhrb_filter = bhrb_filter;
>>>> + power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
>>>> + }
>>>> }
>>>>
>>>> perf_pmu_enable(event->pmu);
>>>> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct
>>>> perf_event *event)
>>>> int n;
>>>> int err;
>>>> struct cpu_hw_events *cpuhw;
>>>> - u64 bhrb_filter;
>>>>
>>>> if (!ppmu)
>>>> return -ENOENT;
>>>> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct
>>>> perf_event *event)
>>>> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>>>>
>>>> if (has_branch_stack(event)) {
>>>> - bhrb_filter = ppmu->bhrb_filter_map(
>>>> + u64 bhrb_filter = -1;
>>>> +
>>>> + if (ppmu->bhrb_filter_map)
>>>> + bhrb_filter = ppmu->bhrb_filter_map(
>>>> event->attr.branch_sample_type);
>>>>
>>>> if (bhrb_filter == -1) {
>>>
>>
--
Alexey
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
2020-06-02 2:56 [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu Alexey Kardashevskiy
2020-06-03 16:34 ` Madhavan Srinivasan
@ 2020-08-27 7:46 ` Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2020-08-27 7:46 UTC (permalink / raw)
To: linuxppc-dev, Alexey Kardashevskiy; +Cc: Madhavan Srinivasan
On Tue, 2 Jun 2020 12:56:12 +1000, Alexey Kardashevskiy wrote:
> The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
> generic_compat_pmu which does not have this callback and crashed occur.
>
> This add a NULL pointer check for bhrb_filter_map() which behaves as if
> the callback returned an error.
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/perf: Fix crashes with generic_compat_pmu & BHRB
https://git.kernel.org/powerpc/c/b460b512417ae9c8b51a3bdcc09020cd6c60ff69
cheers
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-16 1:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-02 2:56 [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu Alexey Kardashevskiy
2020-06-03 16:34 ` Madhavan Srinivasan
2020-12-02 3:01 ` Alexey Kardashevskiy
2020-12-03 5:27 ` Madhavan Srinivasan
2021-02-16 1:06 ` Alexey Kardashevskiy
2020-08-27 7:46 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).