Commit 2de0147d authored by Michał Winiarski's avatar Michał Winiarski Committed by Jani Nikula

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: 05488673 ("drm/i915/pmu: Support multiple GPUs")
Signed-off-by: default avatarMichał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20200219161822.24592-2-michal.winiarski@intel.com
(cherry picked from commit 46129dc1)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent 19ee5e8d
...@@ -822,11 +822,6 @@ static ssize_t i915_pmu_event_show(struct device *dev, ...@@ -822,11 +822,6 @@ static ssize_t i915_pmu_event_show(struct device *dev,
return sprintf(buf, "config=0x%lx\n", eattr->val); return sprintf(buf, "config=0x%lx\n", eattr->val);
} }
static struct attribute_group i915_pmu_events_attr_group = {
.name = "events",
/* Patch in attrs at runtime. */
};
static ssize_t static ssize_t
i915_pmu_get_attr_cpumask(struct device *dev, i915_pmu_get_attr_cpumask(struct device *dev,
struct device_attribute *attr, struct device_attribute *attr,
...@@ -846,13 +841,6 @@ static const struct attribute_group i915_pmu_cpumask_attr_group = { ...@@ -846,13 +841,6 @@ static const struct attribute_group i915_pmu_cpumask_attr_group = {
.attrs = i915_cpumask_attrs, .attrs = i915_cpumask_attrs,
}; };
static const struct attribute_group *i915_pmu_attr_groups[] = {
&i915_pmu_format_attr_group,
&i915_pmu_events_attr_group,
&i915_pmu_cpumask_attr_group,
NULL
};
#define __event(__config, __name, __unit) \ #define __event(__config, __name, __unit) \
{ \ { \
.config = (__config), \ .config = (__config), \
...@@ -1026,16 +1014,16 @@ err:; ...@@ -1026,16 +1014,16 @@ err:;
static void free_event_attributes(struct i915_pmu *pmu) static void free_event_attributes(struct i915_pmu *pmu)
{ {
struct attribute **attr_iter = i915_pmu_events_attr_group.attrs; struct attribute **attr_iter = pmu->events_attr_group.attrs;
for (; *attr_iter; attr_iter++) for (; *attr_iter; attr_iter++)
kfree((*attr_iter)->name); kfree((*attr_iter)->name);
kfree(i915_pmu_events_attr_group.attrs); kfree(pmu->events_attr_group.attrs);
kfree(pmu->i915_attr); kfree(pmu->i915_attr);
kfree(pmu->pmu_attr); kfree(pmu->pmu_attr);
i915_pmu_events_attr_group.attrs = NULL; pmu->events_attr_group.attrs = NULL;
pmu->i915_attr = NULL; pmu->i915_attr = NULL;
pmu->pmu_attr = NULL; pmu->pmu_attr = NULL;
} }
...@@ -1117,6 +1105,13 @@ static bool is_igp(struct drm_i915_private *i915) ...@@ -1117,6 +1105,13 @@ static bool is_igp(struct drm_i915_private *i915)
void i915_pmu_register(struct drm_i915_private *i915) void i915_pmu_register(struct drm_i915_private *i915)
{ {
struct i915_pmu *pmu = &i915->pmu; struct i915_pmu *pmu = &i915->pmu;
const struct attribute_group *attr_groups[] = {
&i915_pmu_format_attr_group,
&pmu->events_attr_group,
&i915_pmu_cpumask_attr_group,
NULL
};
int ret = -ENOMEM; int ret = -ENOMEM;
if (INTEL_GEN(i915) <= 2) { if (INTEL_GEN(i915) <= 2) {
...@@ -1143,11 +1138,16 @@ void i915_pmu_register(struct drm_i915_private *i915) ...@@ -1143,11 +1138,16 @@ void i915_pmu_register(struct drm_i915_private *i915)
if (!pmu->name) if (!pmu->name)
goto err; goto err;
i915_pmu_events_attr_group.attrs = create_event_attributes(pmu); pmu->events_attr_group.name = "events";
if (!i915_pmu_events_attr_group.attrs) pmu->events_attr_group.attrs = create_event_attributes(pmu);
if (!pmu->events_attr_group.attrs)
goto err_name; goto err_name;
pmu->base.attr_groups = i915_pmu_attr_groups; pmu->base.attr_groups = kmemdup(attr_groups, sizeof(attr_groups),
GFP_KERNEL);
if (!pmu->base.attr_groups)
goto err_attr;
pmu->base.task_ctx_nr = perf_invalid_context; pmu->base.task_ctx_nr = perf_invalid_context;
pmu->base.event_init = i915_pmu_event_init; pmu->base.event_init = i915_pmu_event_init;
pmu->base.add = i915_pmu_event_add; pmu->base.add = i915_pmu_event_add;
...@@ -1159,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915) ...@@ -1159,7 +1159,7 @@ void i915_pmu_register(struct drm_i915_private *i915)
ret = perf_pmu_register(&pmu->base, pmu->name, -1); ret = perf_pmu_register(&pmu->base, pmu->name, -1);
if (ret) if (ret)
goto err_attr; goto err_groups;
ret = i915_pmu_register_cpuhp_state(pmu); ret = i915_pmu_register_cpuhp_state(pmu);
if (ret) if (ret)
...@@ -1169,6 +1169,8 @@ void i915_pmu_register(struct drm_i915_private *i915) ...@@ -1169,6 +1169,8 @@ void i915_pmu_register(struct drm_i915_private *i915)
err_unreg: err_unreg:
perf_pmu_unregister(&pmu->base); perf_pmu_unregister(&pmu->base);
err_groups:
kfree(pmu->base.attr_groups);
err_attr: err_attr:
pmu->base.event_init = NULL; pmu->base.event_init = NULL;
free_event_attributes(pmu); free_event_attributes(pmu);
...@@ -1194,6 +1196,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915) ...@@ -1194,6 +1196,7 @@ void i915_pmu_unregister(struct drm_i915_private *i915)
perf_pmu_unregister(&pmu->base); perf_pmu_unregister(&pmu->base);
pmu->base.event_init = NULL; pmu->base.event_init = NULL;
kfree(pmu->base.attr_groups);
if (!is_igp(i915)) if (!is_igp(i915))
kfree(pmu->name); kfree(pmu->name);
free_event_attributes(pmu); free_event_attributes(pmu);
......
...@@ -107,6 +107,10 @@ struct i915_pmu { ...@@ -107,6 +107,10 @@ struct i915_pmu {
* @sleep_last: Last time GT parked for RC6 estimation. * @sleep_last: Last time GT parked for RC6 estimation.
*/ */
ktime_t sleep_last; ktime_t sleep_last;
/**
* @events_attr_group: Device events attribute group.
*/
struct attribute_group events_attr_group;
/** /**
* @i915_attr: Memory block holding device attributes. * @i915_attr: Memory block holding device attributes.
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment