Skip to content

Commit 2de0147

Browse files
mwiniarsjnikula
authored andcommitted
drm/i915/pmu: Avoid using globals for PMU events
Attempting to bind / unbind module from devices where we have both integrated and discreete GPU handled by i915, will cause us to try and double free the global state, hitting null ptr deref in free_event_attributes. Let's move it to i915_pmu. Fixes: 0548867 ("drm/i915/pmu: Support multiple GPUs") Signed-off-by: Michał Winiarski <[email protected]> Cc: Chris Wilson <[email protected]> Cc: Michal Wajdeczko <[email protected]> Cc: Tvrtko Ursulin <[email protected]> Reviewed-by: Chris Wilson <[email protected]> Signed-off-by: Chris Wilson <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit 46129dc) Signed-off-by: Jani Nikula <[email protected]>
1 parent 19ee5e8 commit 2de0147

File tree

2 files changed

+26
-19
lines changed

2 files changed

+26
-19
lines changed

drivers/gpu/drm/i915/i915_pmu.c

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -822,11 +822,6 @@ static ssize_t i915_pmu_event_show(struct device *dev,
822822
return sprintf(buf, "config=0x%lx\n", eattr->val);
823823
}
824824

825-
static struct attribute_group i915_pmu_events_attr_group = {
826-
.name = "events",
827-
/* Patch in attrs at runtime. */
828-
};
829-
830825
static ssize_t
831826
i915_pmu_get_attr_cpumask(struct device *dev,
832827
struct device_attribute *attr,
@@ -846,13 +841,6 @@ static const struct attribute_group i915_pmu_cpumask_attr_group = {
846841
.attrs = i915_cpumask_attrs,
847842
};
848843

849-
static const struct attribute_group *i915_pmu_attr_groups[] = {
850-
&i915_pmu_format_attr_group,
851-
&i915_pmu_events_attr_group,
852-
&i915_pmu_cpumask_attr_group,
853-
NULL
854-
};
855-
856844
#define __event(__config, __name, __unit) \
857845
{ \
858846
.config = (__config), \
@@ -1026,16 +1014,16 @@ err:;
10261014

10271015
static void free_event_attributes(struct i915_pmu *pmu)
10281016
{
1029-
struct attribute **attr_iter = i915_pmu_events_attr_group.attrs;
1017+
struct attribute **attr_iter = pmu->events_attr_group.attrs;
10301018

10311019
for (; *attr_iter; attr_iter++)
10321020
kfree((*attr_iter)->name);
10331021

1034-
kfree(i915_pmu_events_attr_group.attrs);
1022+
kfree(pmu->events_attr_group.attrs);
10351023
kfree(pmu->i915_attr);
10361024
kfree(pmu->pmu_attr);
10371025

1038-
i915_pmu_events_attr_group.attrs = NULL;
1026+
pmu->events_attr_group.attrs = NULL;
10391027
pmu->i915_attr = NULL;
10401028
pmu->pmu_attr = NULL;
10411029
}
@@ -1117,6 +1105,13 @@ static bool is_igp(struct drm_i915_private *i915)
11171105
void i915_pmu_register(struct drm_i915_private *i915)
11181106
{
11191107
struct i915_pmu *pmu = &i915->pmu;
1108+
const struct attribute_group *attr_groups[] = {
1109+
&i915_pmu_format_attr_group,
1110+
&pmu->events_attr_group,
1111+
&i915_pmu_cpumask_attr_group,
1112+
NULL
1113+
};
1114+
11201115
int ret = -ENOMEM;
11211116

11221117
if (INTEL_GEN(i915) <= 2) {
@@ -1143,11 +1138,16 @@ void i915_pmu_register(struct drm_i915_private *i915)
11431138
if (!pmu->name)
11441139
goto err;
11451140

1146-
i915_pmu_events_attr_group.attrs = create_event_attributes(pmu);
1147-
if (!i915_pmu_events_attr_group.attrs)
1141+
pmu->events_attr_group.name = "events";
1142+
pmu->events_attr_group.attrs = create_event_attributes(pmu);
1143+
if (!pmu->events_attr_group.attrs)
11481144
goto err_name;
11491145

1150-
pmu->base.attr_groups = i915_pmu_attr_groups;
1146+
pmu->base.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
1147+
GFP_KERNEL);
1148+
if (!pmu->base.attr_groups)
1149+
goto err_attr;
1150+
11511151
pmu->base.task_ctx_nr = perf_invalid_context;
11521152
pmu->base.event_init = i915_pmu_event_init;
11531153
pmu->base.add = i915_pmu_event_add;
@@ -1159,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
11591159

11601160
ret = perf_pmu_register(&pmu->base, pmu->name, -1);
11611161
if (ret)
1162-
goto err_attr;
1162+
goto err_groups;
11631163

11641164
ret = i915_pmu_register_cpuhp_state(pmu);
11651165
if (ret)
@@ -1169,6 +1169,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
11691169

11701170
err_unreg:
11711171
perf_pmu_unregister(&pmu->base);
1172+
err_groups:
1173+
kfree(pmu->base.attr_groups);
11721174
err_attr:
11731175
pmu->base.event_init = NULL;
11741176
free_event_attributes(pmu);
@@ -1194,6 +1196,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
11941196

11951197
perf_pmu_unregister(&pmu->base);
11961198
pmu->base.event_init = NULL;
1199+
kfree(pmu->base.attr_groups);
11971200
if (!is_igp(i915))
11981201
kfree(pmu->name);
11991202
free_event_attributes(pmu);

drivers/gpu/drm/i915/i915_pmu.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ struct i915_pmu {
107107
* @sleep_last: Last time GT parked for RC6 estimation.
108108
*/
109109
ktime_t sleep_last;
110+
/**
111+
* @events_attr_group: Device events attribute group.
112+
*/
113+
struct attribute_group events_attr_group;
110114
/**
111115
* @i915_attr: Memory block holding device attributes.
112116
*/

0 commit comments

Comments
 (0)