* [PATCH] drm/i915: Fix CFI violations in gt_sysfs
@ 2022-09-22 19:51 Nathan Chancellor
2022-09-23 7:15 ` Tvrtko Ursulin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Nathan Chancellor @ 2022-09-22 19:51 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin
Cc: Kees Cook, Tom Rix, intel-gfx, llvm, Nick Desaulniers, patches,
dri-devel, Nathan Chancellor, Sami Tolvanen
When booting with clang's kernel control flow integrity series [1],
there are numerous violations when accessing the files under
/sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
$ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
$ grep . *
id:0
punit_req_freq_mhz:350
rc6_enable:1
rc6_residency_ms:214934
rps_act_freq_mhz:1300
rps_boost_freq_mhz:1300
rps_cur_freq_mhz:350
rps_max_freq_mhz:1300
rps_min_freq_mhz:350
rps_RP0_freq_mhz:1300
rps_RP1_freq_mhz:350
rps_RPn_freq_mhz:350
throttle_reason_pl1:0
throttle_reason_pl2:0
throttle_reason_pl4:0
throttle_reason_prochot:0
throttle_reason_ratl:0
throttle_reason_status:0
throttle_reason_thermal:0
throttle_reason_vr_tdc:0
throttle_reason_vr_thermalert:0
$ sudo dmesg &| grep "CFI failure at"
[ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
[ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
[ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
[ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
[ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
[ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
[ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
With kCFI, indirect calls are validated against their expected type
versus actual type and failures occur when the two types do not match.
The ultimate issue is that these sysfs functions are expecting to be
called via dev_attr_show() but they may also be called via
kobj_attr_show(), as certain files are created under two different
kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
hence the warnings above. When accessing the gt_ files under
/sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
sysfs functions, there are no violations, meaning the functions are
being called with the proper type.
To make everything work properly, adjust certain functions to match the
type of the ->show() and ->store() members in 'struct kobj_attribute'.
Add a macro to generate functions for that can be called via both
dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
called through both kobject locations without violating kCFI and adjust
the attribute groups to account for this.
[1]: https://lore.kernel.org/20220908215504.3686827-1-samitolvanen@google.com/
Link: https://github.com/ClangBuiltLinux/linux/issues/1716
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 15 +-
drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 2 +-
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 462 +++++++++-----------
3 files changed, 221 insertions(+), 258 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
index d651ccd0ab20..9486dd3bed99 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
@@ -22,11 +22,9 @@ bool is_object_gt(struct kobject *kobj)
return !strncmp(kobj->name, "gt", 2);
}
-struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
+struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
const char *name)
{
- struct kobject *kobj = &dev->kobj;
-
/*
* We are interested at knowing from where the interface
* has been called, whether it's called from gt/ or from
@@ -38,6 +36,7 @@ struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
* "struct drm_i915_private *" type.
*/
if (!is_object_gt(kobj)) {
+ struct device *dev = kobj_to_dev(kobj);
struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
return to_gt(i915);
@@ -51,18 +50,18 @@ static struct kobject *gt_get_parent_obj(struct intel_gt *gt)
return >->i915->drm.primary->kdev->kobj;
}
-static ssize_t id_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t id_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buf)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
return sysfs_emit(buf, "%u\n", gt->info.id);
}
-static DEVICE_ATTR_RO(id);
+static struct kobj_attribute attr_id = __ATTR_RO(id);
static struct attribute *id_attrs[] = {
- &dev_attr_id.attr,
+ &attr_id.attr,
NULL,
};
ATTRIBUTE_GROUPS(id);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
index 6232923a420d..c3a123faee98 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
@@ -30,7 +30,7 @@ static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
void intel_gt_sysfs_register(struct intel_gt *gt);
void intel_gt_sysfs_unregister(struct intel_gt *gt);
-struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
+struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
const char *name);
#endif /* SYSFS_GT_H */
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 904160952369..308d54008983 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -24,14 +24,15 @@ enum intel_gt_sysfs_op {
};
static int
-sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
+sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
int (func)(struct intel_gt *gt, u32 val), u32 val)
{
struct intel_gt *gt;
int ret;
- if (!is_object_gt(&dev->kobj)) {
+ if (!is_object_gt(kobj)) {
int i;
+ struct device *dev = kobj_to_dev(kobj);
struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
for_each_gt(gt, i915, i) {
@@ -40,7 +41,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
break;
}
} else {
- gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
ret = func(gt, val);
}
@@ -48,7 +49,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
}
static u32
-sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
+sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
u32 (func)(struct intel_gt *gt),
enum intel_gt_sysfs_op op)
{
@@ -57,8 +58,9 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
ret = (op == INTEL_GT_SYSFS_MAX) ? 0 : (u32) -1;
- if (!is_object_gt(&dev->kobj)) {
+ if (!is_object_gt(kobj)) {
int i;
+ struct device *dev = kobj_to_dev(kobj);
struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
for_each_gt(gt, i915, i) {
@@ -77,7 +79,7 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
}
}
} else {
- gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
ret = func(gt);
}
@@ -92,6 +94,77 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
#define sysfs_gt_attribute_r_max_func(d, a, f) \
sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
+#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
+ static ssize_t _name##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, char *buff) \
+ { \
+ u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
+ __##_name##_show); \
+ \
+ return sysfs_emit(buff, "%u\n", val); \
+ } \
+ static ssize_t _name##_dev_show(struct device *dev, \
+ struct device_attribute *attr, char *buff) \
+ { \
+ u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
+ __##_name##_show); \
+ \
+ return sysfs_emit(buff, "%u\n", val); \
+ }
+
+#define INTEL_GT_SYSFS_STORE(_name, _func) \
+ static ssize_t _name##_store(struct kobject *kobj, \
+ struct kobj_attribute *attr, const char *buff, \
+ size_t count) \
+ { \
+ int ret; \
+ u32 val; \
+ \
+ ret = kstrtou32(buff, 0, &val); \
+ if (ret) \
+ return ret; \
+ \
+ ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
+ \
+ return ret ?: count; \
+ } \
+ static ssize_t _name##_dev_store(struct device *dev, \
+ struct device_attribute *attr, \
+ const char *buff, size_t count) \
+ { \
+ int ret; \
+ u32 val; \
+ \
+ ret = kstrtou32(buff, 0, &val); \
+ if (ret) \
+ return ret; \
+ \
+ ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
+ \
+ return ret ?: count; \
+ }
+
+#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
+#define INTEL_GT_SYSFS_SHOW_MIN(_name) INTEL_GT_SYSFS_SHOW(_name, min)
+
+#define INTEL_GT_ATTR_RW(_name) \
+ static struct kobj_attribute attr_##_name = __ATTR_RW(_name)
+
+#define INTEL_GT_ATTR_RO(_name) \
+ static struct kobj_attribute attr_##_name = __ATTR_RO(_name)
+
+#define INTEL_GT_DUAL_ATTR_RW(_name) \
+ static struct device_attribute dev_attr_##_name = __ATTR(_name, 0644, \
+ _name##_dev_show, \
+ _name##_dev_store); \
+ INTEL_GT_ATTR_RW(_name)
+
+#define INTEL_GT_DUAL_ATTR_RO(_name) \
+ static struct device_attribute dev_attr_##_name = __ATTR(_name, 0444, \
+ _name##_dev_show, \
+ NULL); \
+ INTEL_GT_ATTR_RO(_name)
+
#ifdef CONFIG_PM
static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
{
@@ -104,11 +177,8 @@ static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
return DIV_ROUND_CLOSEST_ULL(res, 1000);
}
-static ssize_t rc6_enable_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
+static u8 get_rc6_mask(struct intel_gt *gt)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
u8 mask = 0;
if (HAS_RC6(gt->i915))
@@ -118,37 +188,35 @@ static ssize_t rc6_enable_show(struct device *dev,
if (HAS_RC6pp(gt->i915))
mask |= BIT(2);
- return sysfs_emit(buff, "%x\n", mask);
+ return mask;
}
-static u32 __rc6_residency_ms_show(struct intel_gt *gt)
+static ssize_t rc6_enable_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buff)
{
- return get_residency(gt, GEN6_GT_GFX_RC6);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
+
+ return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
}
-static ssize_t rc6_residency_ms_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
+static ssize_t rc6_enable_dev_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buff)
{
- u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
- __rc6_residency_ms_show);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, attr->attr.name);
- return sysfs_emit(buff, "%u\n", rc6_residency);
+ return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
}
-static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
+static u32 __rc6_residency_ms_show(struct intel_gt *gt)
{
- return get_residency(gt, GEN6_GT_GFX_RC6p);
+ return get_residency(gt, GEN6_GT_GFX_RC6);
}
-static ssize_t rc6p_residency_ms_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
+static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
{
- u32 rc6p_residency = sysfs_gt_attribute_r_min_func(dev, attr,
- __rc6p_residency_ms_show);
-
- return sysfs_emit(buff, "%u\n", rc6p_residency);
+ return get_residency(gt, GEN6_GT_GFX_RC6p);
}
static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
@@ -156,67 +224,69 @@ static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
return get_residency(gt, GEN6_GT_GFX_RC6pp);
}
-static ssize_t rc6pp_residency_ms_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
-{
- u32 rc6pp_residency = sysfs_gt_attribute_r_min_func(dev, attr,
- __rc6pp_residency_ms_show);
-
- return sysfs_emit(buff, "%u\n", rc6pp_residency);
-}
-
static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
{
return get_residency(gt, VLV_GT_MEDIA_RC6);
}
-static ssize_t media_rc6_residency_ms_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
-{
- u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
- __media_rc6_residency_ms_show);
+INTEL_GT_SYSFS_SHOW_MIN(rc6_residency_ms);
+INTEL_GT_SYSFS_SHOW_MIN(rc6p_residency_ms);
+INTEL_GT_SYSFS_SHOW_MIN(rc6pp_residency_ms);
+INTEL_GT_SYSFS_SHOW_MIN(media_rc6_residency_ms);
- return sysfs_emit(buff, "%u\n", rc6_residency);
-}
-
-static DEVICE_ATTR_RO(rc6_enable);
-static DEVICE_ATTR_RO(rc6_residency_ms);
-static DEVICE_ATTR_RO(rc6p_residency_ms);
-static DEVICE_ATTR_RO(rc6pp_residency_ms);
-static DEVICE_ATTR_RO(media_rc6_residency_ms);
+INTEL_GT_DUAL_ATTR_RO(rc6_enable);
+INTEL_GT_DUAL_ATTR_RO(rc6_residency_ms);
+INTEL_GT_DUAL_ATTR_RO(rc6p_residency_ms);
+INTEL_GT_DUAL_ATTR_RO(rc6pp_residency_ms);
+INTEL_GT_DUAL_ATTR_RO(media_rc6_residency_ms);
static struct attribute *rc6_attrs[] = {
+ &attr_rc6_enable.attr,
+ &attr_rc6_residency_ms.attr,
+ NULL
+};
+
+static struct attribute *rc6p_attrs[] = {
+ &attr_rc6p_residency_ms.attr,
+ &attr_rc6pp_residency_ms.attr,
+ NULL
+};
+
+static struct attribute *media_rc6_attrs[] = {
+ &attr_media_rc6_residency_ms.attr,
+ NULL
+};
+
+static struct attribute *rc6_dev_attrs[] = {
&dev_attr_rc6_enable.attr,
&dev_attr_rc6_residency_ms.attr,
NULL
};
-static struct attribute *rc6p_attrs[] = {
+static struct attribute *rc6p_dev_attrs[] = {
&dev_attr_rc6p_residency_ms.attr,
&dev_attr_rc6pp_residency_ms.attr,
NULL
};
-static struct attribute *media_rc6_attrs[] = {
+static struct attribute *media_rc6_dev_attrs[] = {
&dev_attr_media_rc6_residency_ms.attr,
NULL
};
static const struct attribute_group rc6_attr_group[] = {
{ .attrs = rc6_attrs, },
- { .name = power_group_name, .attrs = rc6_attrs, },
+ { .name = power_group_name, .attrs = rc6_dev_attrs, },
};
static const struct attribute_group rc6p_attr_group[] = {
{ .attrs = rc6p_attrs, },
- { .name = power_group_name, .attrs = rc6p_attrs, },
+ { .name = power_group_name, .attrs = rc6p_dev_attrs, },
};
static const struct attribute_group media_rc6_attr_group[] = {
{ .attrs = media_rc6_attrs, },
- { .name = power_group_name, .attrs = media_rc6_attrs, },
+ { .name = power_group_name, .attrs = media_rc6_dev_attrs, },
};
static int __intel_gt_sysfs_create_group(struct kobject *kobj,
@@ -271,104 +341,34 @@ static u32 __act_freq_mhz_show(struct intel_gt *gt)
return intel_rps_read_actual_frequency(>->rps);
}
-static ssize_t act_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __act_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", actual_freq);
-}
-
static u32 __cur_freq_mhz_show(struct intel_gt *gt)
{
return intel_rps_get_requested_frequency(>->rps);
}
-static ssize_t cur_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 cur_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __cur_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", cur_freq);
-}
-
static u32 __boost_freq_mhz_show(struct intel_gt *gt)
{
return intel_rps_get_boost_frequency(>->rps);
}
-static ssize_t boost_freq_mhz_show(struct device *dev,
- struct device_attribute *attr,
- char *buff)
-{
- u32 boost_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __boost_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", boost_freq);
-}
-
static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
{
return intel_rps_set_boost_frequency(>->rps, val);
}
-static ssize_t boost_freq_mhz_store(struct device *dev,
- struct device_attribute *attr,
- const char *buff, size_t count)
-{
- ssize_t ret;
- u32 val;
-
- ret = kstrtou32(buff, 0, &val);
- if (ret)
- return ret;
-
- return sysfs_gt_attribute_w_func(dev, attr,
- __boost_freq_mhz_store, val) ?: count;
-}
-
-static u32 __rp0_freq_mhz_show(struct intel_gt *gt)
+static u32 __RP0_freq_mhz_show(struct intel_gt *gt)
{
return intel_rps_get_rp0_frequency(>->rps);
}
-static ssize_t RP0_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 rp0_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __rp0_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", rp0_freq);
-}
-
-static u32 __rp1_freq_mhz_show(struct intel_gt *gt)
-{
- return intel_rps_get_rp1_frequency(>->rps);
-}
-
-static ssize_t RP1_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 rp1_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __rp1_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", rp1_freq);
-}
-
-static u32 __rpn_freq_mhz_show(struct intel_gt *gt)
+static u32 __RPn_freq_mhz_show(struct intel_gt *gt)
{
return intel_rps_get_rpn_frequency(>->rps);
}
-static ssize_t RPn_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
+static u32 __RP1_freq_mhz_show(struct intel_gt *gt)
{
- u32 rpn_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __rpn_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", rpn_freq);
+ return intel_rps_get_rp1_frequency(>->rps);
}
static u32 __max_freq_mhz_show(struct intel_gt *gt)
@@ -376,71 +376,21 @@ static u32 __max_freq_mhz_show(struct intel_gt *gt)
return intel_rps_get_max_frequency(>->rps);
}
-static ssize_t max_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 max_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __max_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", max_freq);
-}
-
static int __set_max_freq(struct intel_gt *gt, u32 val)
{
return intel_rps_set_max_frequency(>->rps, val);
}
-static ssize_t max_freq_mhz_store(struct device *dev,
- struct device_attribute *attr,
- const char *buff, size_t count)
-{
- int ret;
- u32 val;
-
- ret = kstrtou32(buff, 0, &val);
- if (ret)
- return ret;
-
- ret = sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
-
- return ret ?: count;
-}
-
static u32 __min_freq_mhz_show(struct intel_gt *gt)
{
return intel_rps_get_min_frequency(>->rps);
}
-static ssize_t min_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
- __min_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", min_freq);
-}
-
static int __set_min_freq(struct intel_gt *gt, u32 val)
{
return intel_rps_set_min_frequency(>->rps, val);
}
-static ssize_t min_freq_mhz_store(struct device *dev,
- struct device_attribute *attr,
- const char *buff, size_t count)
-{
- int ret;
- u32 val;
-
- ret = kstrtou32(buff, 0, &val);
- if (ret)
- return ret;
-
- ret = sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val);
-
- return ret ?: count;
-}
-
static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
{
struct intel_rps *rps = >->rps;
@@ -448,23 +398,31 @@ static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
return intel_gpu_freq(rps, rps->efficient_freq);
}
-static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
- struct device_attribute *attr, char *buff)
-{
- u32 rpe_freq = sysfs_gt_attribute_r_max_func(dev, attr,
- __vlv_rpe_freq_mhz_show);
-
- return sysfs_emit(buff, "%u\n", rpe_freq);
-}
-
-#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
- static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
- static struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
-
-#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
- INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
-#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
- INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
+INTEL_GT_SYSFS_SHOW_MAX(act_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(boost_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(cur_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(RP0_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(RP1_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(RPn_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(max_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MIN(min_freq_mhz);
+INTEL_GT_SYSFS_SHOW_MAX(vlv_rpe_freq_mhz);
+INTEL_GT_SYSFS_STORE(boost_freq_mhz, __boost_freq_mhz_store);
+INTEL_GT_SYSFS_STORE(max_freq_mhz, __set_max_freq);
+INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
+
+#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store, _show_dev, _store_dev) \
+ static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, \
+ _show_dev, _store_dev); \
+ static struct kobj_attribute attr_rps_##_name = __ATTR(rps_##_name, _mode, \
+ _show, _store)
+
+#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
+ INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL, \
+ _name##_dev_show, NULL)
+#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
+ INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store, \
+ _name##_dev_show, _name##_dev_store)
/* The below macros generate static structures */
INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
@@ -475,32 +433,31 @@ INTEL_GT_RPS_SYSFS_ATTR_RO(RP1_freq_mhz);
INTEL_GT_RPS_SYSFS_ATTR_RO(RPn_freq_mhz);
INTEL_GT_RPS_SYSFS_ATTR_RW(max_freq_mhz);
INTEL_GT_RPS_SYSFS_ATTR_RW(min_freq_mhz);
-
-static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
-
-#define GEN6_ATTR(s) { \
- &dev_attr_##s##_act_freq_mhz.attr, \
- &dev_attr_##s##_cur_freq_mhz.attr, \
- &dev_attr_##s##_boost_freq_mhz.attr, \
- &dev_attr_##s##_max_freq_mhz.attr, \
- &dev_attr_##s##_min_freq_mhz.attr, \
- &dev_attr_##s##_RP0_freq_mhz.attr, \
- &dev_attr_##s##_RP1_freq_mhz.attr, \
- &dev_attr_##s##_RPn_freq_mhz.attr, \
+INTEL_GT_RPS_SYSFS_ATTR_RO(vlv_rpe_freq_mhz);
+
+#define GEN6_ATTR(p, s) { \
+ &p##attr_##s##_act_freq_mhz.attr, \
+ &p##attr_##s##_cur_freq_mhz.attr, \
+ &p##attr_##s##_boost_freq_mhz.attr, \
+ &p##attr_##s##_max_freq_mhz.attr, \
+ &p##attr_##s##_min_freq_mhz.attr, \
+ &p##attr_##s##_RP0_freq_mhz.attr, \
+ &p##attr_##s##_RP1_freq_mhz.attr, \
+ &p##attr_##s##_RPn_freq_mhz.attr, \
NULL, \
}
-#define GEN6_RPS_ATTR GEN6_ATTR(rps)
-#define GEN6_GT_ATTR GEN6_ATTR(gt)
+#define GEN6_RPS_ATTR GEN6_ATTR(, rps)
+#define GEN6_GT_ATTR GEN6_ATTR(dev_, gt)
static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
-static ssize_t punit_req_freq_mhz_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t punit_req_freq_mhz_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
u32 preq = intel_rps_read_punit_req_frequency(>->rps);
return sysfs_emit(buff, "%u\n", preq);
@@ -508,17 +465,17 @@ static ssize_t punit_req_freq_mhz_show(struct device *dev,
struct intel_gt_bool_throttle_attr {
struct attribute attr;
- ssize_t (*show)(struct device *dev, struct device_attribute *attr,
+ ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
char *buf);
i915_reg_t (*reg32)(struct intel_gt *gt);
u32 mask;
};
-static ssize_t throttle_reason_bool_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t throttle_reason_bool_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
struct intel_gt_bool_throttle_attr *t_attr =
(struct intel_gt_bool_throttle_attr *) attr;
bool val = rps_read_mask_mmio(>->rps, t_attr->reg32(gt), t_attr->mask);
@@ -534,7 +491,7 @@ struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
.mask = mask__, \
}
-static DEVICE_ATTR_RO(punit_req_freq_mhz);
+INTEL_GT_ATTR_RO(punit_req_freq_mhz);
static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_status, GT0_PERF_LIMIT_REASONS_MASK);
static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl1, POWER_LIMIT_1_MASK);
static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl2, POWER_LIMIT_2_MASK);
@@ -597,8 +554,8 @@ static const struct attribute *throttle_reason_attrs[] = {
#define U8_8_VAL_MASK 0xffff
#define U8_8_SCALE_TO_VALUE "0.00390625"
-static ssize_t freq_factor_scale_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t freq_factor_scale_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
@@ -610,11 +567,11 @@ static u32 media_ratio_mode_to_factor(u32 mode)
return !mode ? mode : 256 / mode;
}
-static ssize_t media_freq_factor_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t media_freq_factor_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
struct intel_guc_slpc *slpc = >->uc.guc.slpc;
intel_wakeref_t wakeref;
u32 mode;
@@ -641,11 +598,11 @@ static ssize_t media_freq_factor_show(struct device *dev,
return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
}
-static ssize_t media_freq_factor_store(struct device *dev,
- struct device_attribute *attr,
+static ssize_t media_freq_factor_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
const char *buff, size_t count)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
struct intel_guc_slpc *slpc = >->uc.guc.slpc;
u32 factor, mode;
int err;
@@ -670,11 +627,11 @@ static ssize_t media_freq_factor_store(struct device *dev,
return err ?: count;
}
-static ssize_t media_RP0_freq_mhz_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t media_RP0_freq_mhz_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
u32 val;
int err;
@@ -691,11 +648,11 @@ static ssize_t media_RP0_freq_mhz_show(struct device *dev,
return sysfs_emit(buff, "%u\n", val);
}
-static ssize_t media_RPn_freq_mhz_show(struct device *dev,
- struct device_attribute *attr,
+static ssize_t media_RPn_freq_mhz_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
char *buff)
{
- struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+ struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
u32 val;
int err;
@@ -712,17 +669,17 @@ static ssize_t media_RPn_freq_mhz_show(struct device *dev,
return sysfs_emit(buff, "%u\n", val);
}
-static DEVICE_ATTR_RW(media_freq_factor);
-static struct device_attribute dev_attr_media_freq_factor_scale =
+INTEL_GT_ATTR_RW(media_freq_factor);
+static struct kobj_attribute attr_media_freq_factor_scale =
__ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
-static DEVICE_ATTR_RO(media_RP0_freq_mhz);
-static DEVICE_ATTR_RO(media_RPn_freq_mhz);
+INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
+INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
static const struct attribute *media_perf_power_attrs[] = {
- &dev_attr_media_freq_factor.attr,
- &dev_attr_media_freq_factor_scale.attr,
- &dev_attr_media_RP0_freq_mhz.attr,
- &dev_attr_media_RPn_freq_mhz.attr,
+ &attr_media_freq_factor.attr,
+ &attr_media_freq_factor_scale.attr,
+ &attr_media_RP0_freq_mhz.attr,
+ &attr_media_RPn_freq_mhz.attr,
NULL
};
@@ -754,20 +711,29 @@ static const struct attribute * const rps_defaults_attrs[] = {
NULL
};
-static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
- const struct attribute * const *attrs)
+static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
{
+ const struct attribute * const *attrs;
+ struct attribute *vlv_attr;
int ret;
if (GRAPHICS_VER(gt->i915) < 6)
return 0;
+ if (is_object_gt(kobj)) {
+ attrs = gen6_rps_attrs;
+ vlv_attr = &attr_rps_vlv_rpe_freq_mhz.attr;
+ } else {
+ attrs = gen6_gt_attrs;
+ vlv_attr = &dev_attr_gt_vlv_rpe_freq_mhz.attr;
+ }
+
ret = sysfs_create_files(kobj, attrs);
if (ret)
return ret;
if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
- ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
+ ret = sysfs_create_file(kobj, vlv_attr);
return ret;
}
@@ -778,9 +744,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
intel_sysfs_rc6_init(gt, kobj);
- ret = is_object_gt(kobj) ?
- intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
- intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
+ ret = intel_sysfs_rps_init(gt, kobj);
if (ret)
drm_warn(>->i915->drm,
"failed to create gt%u RPS sysfs files (%pe)",
@@ -790,7 +754,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
if (!is_object_gt(kobj))
return;
- ret = sysfs_create_file(kobj, &dev_attr_punit_req_freq_mhz.attr);
+ ret = sysfs_create_file(kobj, &attr_punit_req_freq_mhz.attr);
if (ret)
drm_warn(>->i915->drm,
"failed to create gt%u punit_req_freq_mhz sysfs (%pe)",
base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
--
2.37.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-22 19:51 [PATCH] drm/i915: Fix CFI violations in gt_sysfs Nathan Chancellor
@ 2022-09-23 7:15 ` Tvrtko Ursulin
2022-09-24 4:57 ` Kees Cook
2022-09-29 22:34 ` Andrzej Hajda
2 siblings, 0 replies; 9+ messages in thread
From: Tvrtko Ursulin @ 2022-09-23 7:15 UTC (permalink / raw)
To: Nathan Chancellor, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Andi Shyti, Dixit, Ashutosh
Cc: Kees Cook, Tom Rix, intel-gfx, llvm, Nick Desaulniers, patches,
dri-devel, Sami Tolvanen
Hi Nathan,
On 22/09/2022 20:51, Nathan Chancellor wrote:
> When booting with clang's kernel control flow integrity series [1],
> there are numerous violations when accessing the files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
>
> $ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
>
> $ grep . *
> id:0
> punit_req_freq_mhz:350
> rc6_enable:1
> rc6_residency_ms:214934
> rps_act_freq_mhz:1300
> rps_boost_freq_mhz:1300
> rps_cur_freq_mhz:350
> rps_max_freq_mhz:1300
> rps_min_freq_mhz:350
> rps_RP0_freq_mhz:1300
> rps_RP1_freq_mhz:350
> rps_RPn_freq_mhz:350
> throttle_reason_pl1:0
> throttle_reason_pl2:0
> throttle_reason_pl4:0
> throttle_reason_prochot:0
> throttle_reason_ratl:0
> throttle_reason_status:0
> throttle_reason_thermal:0
> throttle_reason_vr_tdc:0
> throttle_reason_vr_thermalert:0
>
> $ sudo dmesg &| grep "CFI failure at"
CFI = control flow integrity - adding Andi and Ashutosh as primary
authors of the code in question on our side to take a look please.
Regards,
Tvrtko
> [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
> [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
> [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
>
> With kCFI, indirect calls are validated against their expected type
> versus actual type and failures occur when the two types do not match.
> The ultimate issue is that these sysfs functions are expecting to be
> called via dev_attr_show() but they may also be called via
> kobj_attr_show(), as certain files are created under two different
> kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
> hence the warnings above. When accessing the gt_ files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
> sysfs functions, there are no violations, meaning the functions are
> being called with the proper type.
>
> To make everything work properly, adjust certain functions to match the
> type of the ->show() and ->store() members in 'struct kobj_attribute'.
> Add a macro to generate functions for that can be called via both
> dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> called through both kobject locations without violating kCFI and adjust
> the attribute groups to account for this.
>
> [1]: https://lore.kernel.org/20220908215504.3686827-1-samitolvanen@google.com/
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 15 +-
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 462 +++++++++-----------
> 3 files changed, 221 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> index d651ccd0ab20..9486dd3bed99 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> @@ -22,11 +22,9 @@ bool is_object_gt(struct kobject *kobj)
> return !strncmp(kobj->name, "gt", 2);
> }
>
> -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> const char *name)
> {
> - struct kobject *kobj = &dev->kobj;
> -
> /*
> * We are interested at knowing from where the interface
> * has been called, whether it's called from gt/ or from
> @@ -38,6 +36,7 @@ struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> * "struct drm_i915_private *" type.
> */
> if (!is_object_gt(kobj)) {
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> return to_gt(i915);
> @@ -51,18 +50,18 @@ static struct kobject *gt_get_parent_obj(struct intel_gt *gt)
> return >->i915->drm.primary->kdev->kobj;
> }
>
> -static ssize_t id_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t id_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buf)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
>
> return sysfs_emit(buf, "%u\n", gt->info.id);
> }
> -static DEVICE_ATTR_RO(id);
> +static struct kobj_attribute attr_id = __ATTR_RO(id);
>
> static struct attribute *id_attrs[] = {
> - &dev_attr_id.attr,
> + &attr_id.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(id);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> index 6232923a420d..c3a123faee98 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> @@ -30,7 +30,7 @@ static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
>
> void intel_gt_sysfs_register(struct intel_gt *gt);
> void intel_gt_sysfs_unregister(struct intel_gt *gt);
> -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> const char *name);
>
> #endif /* SYSFS_GT_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 904160952369..308d54008983 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -24,14 +24,15 @@ enum intel_gt_sysfs_op {
> };
>
> static int
> -sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> +sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
> int (func)(struct intel_gt *gt, u32 val), u32 val)
> {
> struct intel_gt *gt;
> int ret;
>
> - if (!is_object_gt(&dev->kobj)) {
> + if (!is_object_gt(kobj)) {
> int i;
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> for_each_gt(gt, i915, i) {
> @@ -40,7 +41,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> break;
> }
> } else {
> - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> ret = func(gt, val);
> }
>
> @@ -48,7 +49,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> }
>
> static u32
> -sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> +sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
> u32 (func)(struct intel_gt *gt),
> enum intel_gt_sysfs_op op)
> {
> @@ -57,8 +58,9 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
>
> ret = (op == INTEL_GT_SYSFS_MAX) ? 0 : (u32) -1;
>
> - if (!is_object_gt(&dev->kobj)) {
> + if (!is_object_gt(kobj)) {
> int i;
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> for_each_gt(gt, i915, i) {
> @@ -77,7 +79,7 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> }
> }
> } else {
> - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> ret = func(gt);
> }
>
> @@ -92,6 +94,77 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> #define sysfs_gt_attribute_r_max_func(d, a, f) \
> sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
>
> +#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
> + static ssize_t _name##_show(struct kobject *kobj, \
> + struct kobj_attribute *attr, char *buff) \
> + { \
> + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
> + __##_name##_show); \
> + \
> + return sysfs_emit(buff, "%u\n", val); \
> + } \
> + static ssize_t _name##_dev_show(struct device *dev, \
> + struct device_attribute *attr, char *buff) \
> + { \
> + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
> + __##_name##_show); \
> + \
> + return sysfs_emit(buff, "%u\n", val); \
> + }
> +
> +#define INTEL_GT_SYSFS_STORE(_name, _func) \
> + static ssize_t _name##_store(struct kobject *kobj, \
> + struct kobj_attribute *attr, const char *buff, \
> + size_t count) \
> + { \
> + int ret; \
> + u32 val; \
> + \
> + ret = kstrtou32(buff, 0, &val); \
> + if (ret) \
> + return ret; \
> + \
> + ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
> + \
> + return ret ?: count; \
> + } \
> + static ssize_t _name##_dev_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buff, size_t count) \
> + { \
> + int ret; \
> + u32 val; \
> + \
> + ret = kstrtou32(buff, 0, &val); \
> + if (ret) \
> + return ret; \
> + \
> + ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
> + \
> + return ret ?: count; \
> + }
> +
> +#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
> +#define INTEL_GT_SYSFS_SHOW_MIN(_name) INTEL_GT_SYSFS_SHOW(_name, min)
> +
> +#define INTEL_GT_ATTR_RW(_name) \
> + static struct kobj_attribute attr_##_name = __ATTR_RW(_name)
> +
> +#define INTEL_GT_ATTR_RO(_name) \
> + static struct kobj_attribute attr_##_name = __ATTR_RO(_name)
> +
> +#define INTEL_GT_DUAL_ATTR_RW(_name) \
> + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0644, \
> + _name##_dev_show, \
> + _name##_dev_store); \
> + INTEL_GT_ATTR_RW(_name)
> +
> +#define INTEL_GT_DUAL_ATTR_RO(_name) \
> + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0444, \
> + _name##_dev_show, \
> + NULL); \
> + INTEL_GT_ATTR_RO(_name)
> +
> #ifdef CONFIG_PM
> static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> {
> @@ -104,11 +177,8 @@ static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> return DIV_ROUND_CLOSEST_ULL(res, 1000);
> }
>
> -static ssize_t rc6_enable_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static u8 get_rc6_mask(struct intel_gt *gt)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> u8 mask = 0;
>
> if (HAS_RC6(gt->i915))
> @@ -118,37 +188,35 @@ static ssize_t rc6_enable_show(struct device *dev,
> if (HAS_RC6pp(gt->i915))
> mask |= BIT(2);
>
> - return sysfs_emit(buff, "%x\n", mask);
> + return mask;
> }
>
> -static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> +static ssize_t rc6_enable_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + char *buff)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> +
> + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> }
>
> -static ssize_t rc6_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static ssize_t rc6_enable_dev_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff)
> {
> - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6_residency_ms_show);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, attr->attr.name);
>
> - return sysfs_emit(buff, "%u\n", rc6_residency);
> + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> }
>
> -static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> +static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6p);
> + return get_residency(gt, GEN6_GT_GFX_RC6);
> }
>
> -static ssize_t rc6p_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> {
> - u32 rc6p_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6p_residency_ms_show);
> -
> - return sysfs_emit(buff, "%u\n", rc6p_residency);
> + return get_residency(gt, GEN6_GT_GFX_RC6p);
> }
>
> static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> @@ -156,67 +224,69 @@ static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> return get_residency(gt, GEN6_GT_GFX_RC6pp);
> }
>
> -static ssize_t rc6pp_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 rc6pp_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6pp_residency_ms_show);
> -
> - return sysfs_emit(buff, "%u\n", rc6pp_residency);
> -}
> -
> static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
> {
> return get_residency(gt, VLV_GT_MEDIA_RC6);
> }
>
> -static ssize_t media_rc6_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __media_rc6_residency_ms_show);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6p_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6pp_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(media_rc6_residency_ms);
>
> - return sysfs_emit(buff, "%u\n", rc6_residency);
> -}
> -
> -static DEVICE_ATTR_RO(rc6_enable);
> -static DEVICE_ATTR_RO(rc6_residency_ms);
> -static DEVICE_ATTR_RO(rc6p_residency_ms);
> -static DEVICE_ATTR_RO(rc6pp_residency_ms);
> -static DEVICE_ATTR_RO(media_rc6_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6_enable);
> +INTEL_GT_DUAL_ATTR_RO(rc6_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6p_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6pp_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(media_rc6_residency_ms);
>
> static struct attribute *rc6_attrs[] = {
> + &attr_rc6_enable.attr,
> + &attr_rc6_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *rc6p_attrs[] = {
> + &attr_rc6p_residency_ms.attr,
> + &attr_rc6pp_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *media_rc6_attrs[] = {
> + &attr_media_rc6_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *rc6_dev_attrs[] = {
> &dev_attr_rc6_enable.attr,
> &dev_attr_rc6_residency_ms.attr,
> NULL
> };
>
> -static struct attribute *rc6p_attrs[] = {
> +static struct attribute *rc6p_dev_attrs[] = {
> &dev_attr_rc6p_residency_ms.attr,
> &dev_attr_rc6pp_residency_ms.attr,
> NULL
> };
>
> -static struct attribute *media_rc6_attrs[] = {
> +static struct attribute *media_rc6_dev_attrs[] = {
> &dev_attr_media_rc6_residency_ms.attr,
> NULL
> };
>
> static const struct attribute_group rc6_attr_group[] = {
> { .attrs = rc6_attrs, },
> - { .name = power_group_name, .attrs = rc6_attrs, },
> + { .name = power_group_name, .attrs = rc6_dev_attrs, },
> };
>
> static const struct attribute_group rc6p_attr_group[] = {
> { .attrs = rc6p_attrs, },
> - { .name = power_group_name, .attrs = rc6p_attrs, },
> + { .name = power_group_name, .attrs = rc6p_dev_attrs, },
> };
>
> static const struct attribute_group media_rc6_attr_group[] = {
> { .attrs = media_rc6_attrs, },
> - { .name = power_group_name, .attrs = media_rc6_attrs, },
> + { .name = power_group_name, .attrs = media_rc6_dev_attrs, },
> };
>
> static int __intel_gt_sysfs_create_group(struct kobject *kobj,
> @@ -271,104 +341,34 @@ static u32 __act_freq_mhz_show(struct intel_gt *gt)
> return intel_rps_read_actual_frequency(>->rps);
> }
>
> -static ssize_t act_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __act_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", actual_freq);
> -}
> -
> static u32 __cur_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_requested_frequency(>->rps);
> }
>
> -static ssize_t cur_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 cur_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __cur_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", cur_freq);
> -}
> -
> static u32 __boost_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_boost_frequency(>->rps);
> }
>
> -static ssize_t boost_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 boost_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __boost_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", boost_freq);
> -}
> -
> static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_boost_frequency(>->rps, val);
> }
>
> -static ssize_t boost_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - ssize_t ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - return sysfs_gt_attribute_w_func(dev, attr,
> - __boost_freq_mhz_store, val) ?: count;
> -}
> -
> -static u32 __rp0_freq_mhz_show(struct intel_gt *gt)
> +static u32 __RP0_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_rp0_frequency(>->rps);
> }
>
> -static ssize_t RP0_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rp0_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rp0_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rp0_freq);
> -}
> -
> -static u32 __rp1_freq_mhz_show(struct intel_gt *gt)
> -{
> - return intel_rps_get_rp1_frequency(>->rps);
> -}
> -
> -static ssize_t RP1_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rp1_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rp1_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rp1_freq);
> -}
> -
> -static u32 __rpn_freq_mhz_show(struct intel_gt *gt)
> +static u32 __RPn_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_rpn_frequency(>->rps);
> }
>
> -static ssize_t RPn_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> +static u32 __RP1_freq_mhz_show(struct intel_gt *gt)
> {
> - u32 rpn_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rpn_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rpn_freq);
> + return intel_rps_get_rp1_frequency(>->rps);
> }
>
> static u32 __max_freq_mhz_show(struct intel_gt *gt)
> @@ -376,71 +376,21 @@ static u32 __max_freq_mhz_show(struct intel_gt *gt)
> return intel_rps_get_max_frequency(>->rps);
> }
>
> -static ssize_t max_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 max_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __max_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", max_freq);
> -}
> -
> static int __set_max_freq(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_max_frequency(>->rps, val);
> }
>
> -static ssize_t max_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - int ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
> -
> - return ret ?: count;
> -}
> -
> static u32 __min_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_min_frequency(>->rps);
> }
>
> -static ssize_t min_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
> - __min_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", min_freq);
> -}
> -
> static int __set_min_freq(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_min_frequency(>->rps, val);
> }
>
> -static ssize_t min_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - int ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val);
> -
> - return ret ?: count;
> -}
> -
> static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> {
> struct intel_rps *rps = >->rps;
> @@ -448,23 +398,31 @@ static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> return intel_gpu_freq(rps, rps->efficient_freq);
> }
>
> -static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rpe_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __vlv_rpe_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rpe_freq);
> -}
> -
> -#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
> - static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
> - static struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
> -
> -#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> - INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
> -#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> - INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
> +INTEL_GT_SYSFS_SHOW_MAX(act_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(boost_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(cur_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RP0_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RP1_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RPn_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(max_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MIN(min_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(vlv_rpe_freq_mhz);
> +INTEL_GT_SYSFS_STORE(boost_freq_mhz, __boost_freq_mhz_store);
> +INTEL_GT_SYSFS_STORE(max_freq_mhz, __set_max_freq);
> +INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store, _show_dev, _store_dev) \
> + static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, \
> + _show_dev, _store_dev); \
> + static struct kobj_attribute attr_rps_##_name = __ATTR(rps_##_name, _mode, \
> + _show, _store)
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL, \
> + _name##_dev_show, NULL)
> +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store, \
> + _name##_dev_show, _name##_dev_store)
>
> /* The below macros generate static structures */
> INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
> @@ -475,32 +433,31 @@ INTEL_GT_RPS_SYSFS_ATTR_RO(RP1_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RO(RPn_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RW(max_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RW(min_freq_mhz);
> -
> -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> -
> -#define GEN6_ATTR(s) { \
> - &dev_attr_##s##_act_freq_mhz.attr, \
> - &dev_attr_##s##_cur_freq_mhz.attr, \
> - &dev_attr_##s##_boost_freq_mhz.attr, \
> - &dev_attr_##s##_max_freq_mhz.attr, \
> - &dev_attr_##s##_min_freq_mhz.attr, \
> - &dev_attr_##s##_RP0_freq_mhz.attr, \
> - &dev_attr_##s##_RP1_freq_mhz.attr, \
> - &dev_attr_##s##_RPn_freq_mhz.attr, \
> +INTEL_GT_RPS_SYSFS_ATTR_RO(vlv_rpe_freq_mhz);
> +
> +#define GEN6_ATTR(p, s) { \
> + &p##attr_##s##_act_freq_mhz.attr, \
> + &p##attr_##s##_cur_freq_mhz.attr, \
> + &p##attr_##s##_boost_freq_mhz.attr, \
> + &p##attr_##s##_max_freq_mhz.attr, \
> + &p##attr_##s##_min_freq_mhz.attr, \
> + &p##attr_##s##_RP0_freq_mhz.attr, \
> + &p##attr_##s##_RP1_freq_mhz.attr, \
> + &p##attr_##s##_RPn_freq_mhz.attr, \
> NULL, \
> }
>
> -#define GEN6_RPS_ATTR GEN6_ATTR(rps)
> -#define GEN6_GT_ATTR GEN6_ATTR(gt)
> +#define GEN6_RPS_ATTR GEN6_ATTR(, rps)
> +#define GEN6_GT_ATTR GEN6_ATTR(dev_, gt)
>
> static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
> static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
>
> -static ssize_t punit_req_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t punit_req_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 preq = intel_rps_read_punit_req_frequency(>->rps);
>
> return sysfs_emit(buff, "%u\n", preq);
> @@ -508,17 +465,17 @@ static ssize_t punit_req_freq_mhz_show(struct device *dev,
>
> struct intel_gt_bool_throttle_attr {
> struct attribute attr;
> - ssize_t (*show)(struct device *dev, struct device_attribute *attr,
> + ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf);
> i915_reg_t (*reg32)(struct intel_gt *gt);
> u32 mask;
> };
>
> -static ssize_t throttle_reason_bool_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t throttle_reason_bool_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_gt_bool_throttle_attr *t_attr =
> (struct intel_gt_bool_throttle_attr *) attr;
> bool val = rps_read_mask_mmio(>->rps, t_attr->reg32(gt), t_attr->mask);
> @@ -534,7 +491,7 @@ struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
> .mask = mask__, \
> }
>
> -static DEVICE_ATTR_RO(punit_req_freq_mhz);
> +INTEL_GT_ATTR_RO(punit_req_freq_mhz);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_status, GT0_PERF_LIMIT_REASONS_MASK);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl1, POWER_LIMIT_1_MASK);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl2, POWER_LIMIT_2_MASK);
> @@ -597,8 +554,8 @@ static const struct attribute *throttle_reason_attrs[] = {
> #define U8_8_VAL_MASK 0xffff
> #define U8_8_SCALE_TO_VALUE "0.00390625"
>
> -static ssize_t freq_factor_scale_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t freq_factor_scale_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
> @@ -610,11 +567,11 @@ static u32 media_ratio_mode_to_factor(u32 mode)
> return !mode ? mode : 256 / mode;
> }
>
> -static ssize_t media_freq_factor_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_freq_factor_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> intel_wakeref_t wakeref;
> u32 mode;
> @@ -641,11 +598,11 @@ static ssize_t media_freq_factor_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
> }
>
> -static ssize_t media_freq_factor_store(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_freq_factor_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> const char *buff, size_t count)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> u32 factor, mode;
> int err;
> @@ -670,11 +627,11 @@ static ssize_t media_freq_factor_store(struct device *dev,
> return err ?: count;
> }
>
> -static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_RP0_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 val;
> int err;
>
> @@ -691,11 +648,11 @@ static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", val);
> }
>
> -static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_RPn_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 val;
> int err;
>
> @@ -712,17 +669,17 @@ static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", val);
> }
>
> -static DEVICE_ATTR_RW(media_freq_factor);
> -static struct device_attribute dev_attr_media_freq_factor_scale =
> +INTEL_GT_ATTR_RW(media_freq_factor);
> +static struct kobj_attribute attr_media_freq_factor_scale =
> __ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
> -static DEVICE_ATTR_RO(media_RP0_freq_mhz);
> -static DEVICE_ATTR_RO(media_RPn_freq_mhz);
> +INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
> +INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
>
> static const struct attribute *media_perf_power_attrs[] = {
> - &dev_attr_media_freq_factor.attr,
> - &dev_attr_media_freq_factor_scale.attr,
> - &dev_attr_media_RP0_freq_mhz.attr,
> - &dev_attr_media_RPn_freq_mhz.attr,
> + &attr_media_freq_factor.attr,
> + &attr_media_freq_factor_scale.attr,
> + &attr_media_RP0_freq_mhz.attr,
> + &attr_media_RPn_freq_mhz.attr,
> NULL
> };
>
> @@ -754,20 +711,29 @@ static const struct attribute * const rps_defaults_attrs[] = {
> NULL
> };
>
> -static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> - const struct attribute * const *attrs)
> +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
> {
> + const struct attribute * const *attrs;
> + struct attribute *vlv_attr;
> int ret;
>
> if (GRAPHICS_VER(gt->i915) < 6)
> return 0;
>
> + if (is_object_gt(kobj)) {
> + attrs = gen6_rps_attrs;
> + vlv_attr = &attr_rps_vlv_rpe_freq_mhz.attr;
> + } else {
> + attrs = gen6_gt_attrs;
> + vlv_attr = &dev_attr_gt_vlv_rpe_freq_mhz.attr;
> + }
> +
> ret = sysfs_create_files(kobj, attrs);
> if (ret)
> return ret;
>
> if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
> - ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
> + ret = sysfs_create_file(kobj, vlv_attr);
>
> return ret;
> }
> @@ -778,9 +744,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
>
> intel_sysfs_rc6_init(gt, kobj);
>
> - ret = is_object_gt(kobj) ?
> - intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
> - intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
> + ret = intel_sysfs_rps_init(gt, kobj);
> if (ret)
> drm_warn(>->i915->drm,
> "failed to create gt%u RPS sysfs files (%pe)",
> @@ -790,7 +754,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> if (!is_object_gt(kobj))
> return;
>
> - ret = sysfs_create_file(kobj, &dev_attr_punit_req_freq_mhz.attr);
> + ret = sysfs_create_file(kobj, &attr_punit_req_freq_mhz.attr);
> if (ret)
> drm_warn(>->i915->drm,
> "failed to create gt%u punit_req_freq_mhz sysfs (%pe)",
>
> base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-22 19:51 [PATCH] drm/i915: Fix CFI violations in gt_sysfs Nathan Chancellor
2022-09-23 7:15 ` Tvrtko Ursulin
@ 2022-09-24 4:57 ` Kees Cook
2022-09-25 4:39 ` Nathan Chancellor
2022-09-29 22:34 ` Andrzej Hajda
2 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2022-09-24 4:57 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Tvrtko Ursulin, llvm, Tom Rix, intel-gfx, Nick Desaulniers,
patches, dri-devel, Sami Tolvanen, Rodrigo Vivi
On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote:
> [...]
> To make everything work properly, adjust certain functions to match the
> type of the ->show() and ->store() members in 'struct kobj_attribute'.
> Add a macro to generate functions for that can be called via both
> dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> called through both kobject locations without violating kCFI and adjust
> the attribute groups to account for this.
This was quite a roller coaster! I think the solution looks good, even
if I'm suspicious of the original design that has the same stuff
available twice in different places. (I have a dim memory of rdma
needing a refactoring like this too?)
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-24 4:57 ` Kees Cook
@ 2022-09-25 4:39 ` Nathan Chancellor
2022-09-29 16:46 ` [Intel-gfx] " Andi Shyti
0 siblings, 1 reply; 9+ messages in thread
From: Nathan Chancellor @ 2022-09-25 4:39 UTC (permalink / raw)
To: Kees Cook
Cc: Tvrtko Ursulin, llvm, Tom Rix, intel-gfx, Nick Desaulniers,
patches, dri-devel, Sami Tolvanen, Rodrigo Vivi
On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote:
> On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote:
> > [...]
> > To make everything work properly, adjust certain functions to match the
> > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > Add a macro to generate functions for that can be called via both
> > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > called through both kobject locations without violating kCFI and adjust
> > the attribute groups to account for this.
>
> This was quite a roller coaster! I think the solution looks good, even
> if I'm suspicious of the original design that has the same stuff
> available twice in different places. (I have a dim memory of rdma
> needing a refactoring like this too?)
Right, I noticed this comment in intel_gt_sysfs_register() once I fully
saw what was going on:
/*
* We need to make things right with the
* ABI compatibility. The files were originally
* generated under the parent directory.
*
* We generate the files only for gt 0
* to avoid duplicates.
*/
Makes it seem like there will be userspace breakage if these files do
not exist? I figured this was the cleanest solution within those
parameters.
> Reviewed-by: Kees Cook <keescook@chromium.org>
Thanks for looking it over!
Cheers,
Nathan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-25 4:39 ` Nathan Chancellor
@ 2022-09-29 16:46 ` Andi Shyti
2022-09-29 16:53 ` Nathan Chancellor
0 siblings, 1 reply; 9+ messages in thread
From: Andi Shyti @ 2022-09-29 16:46 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Tvrtko Ursulin, Kees Cook, Tom Rix, intel-gfx, llvm,
Nick Desaulniers, patches, dri-devel, Ashutosh Dixit,
Sami Tolvanen, Rodrigo Vivi
Hi Nathan,
thanks for this refactoring... looks good even though i would
have split it in more patches as this is doing quite many things.
But I will not be stubborn, I understand that it's not trivial to
have things split. I will give my r-b for now but I will check it
again before applying it as it's very easy to get tangled-up in
between all those defines:
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
For now I'm quite surprised to see how easily our CI gives green
lights :-P
On Sat, Sep 24, 2022 at 09:39:30PM -0700, Nathan Chancellor wrote:
> On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote:
> > On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote:
> > > [...]
> > > To make everything work properly, adjust certain functions to match the
> > > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > > Add a macro to generate functions for that can be called via both
> > > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > > called through both kobject locations without violating kCFI and adjust
> > > the attribute groups to account for this.
> >
> > This was quite a roller coaster! I think the solution looks good, even
> > if I'm suspicious of the original design that has the same stuff
> > available twice in different places. (I have a dim memory of rdma
> > needing a refactoring like this too?)
>
> Right, I noticed this comment in intel_gt_sysfs_register() once I fully
> saw what was going on:
>
> /*
> * We need to make things right with the
> * ABI compatibility. The files were originally
> * generated under the parent directory.
> *
> * We generate the files only for gt 0
> * to avoid duplicates.
> */
>
> Makes it seem like there will be userspace breakage if these files do
> not exist? I figured this was the cleanest solution within those
> parameters.
i915 went multi-gt (multitile) so that some interfaces, like the
power management files, have effect only on one of the tiles.
This means that we needed to move some of the files inside the
newly created gt0/gt1 directory.
But, because some of those files are part of an ABI
specification, we can't simply transfer them from the original
position so that we needed to make a "hard" copy (actually the
original files now take the role of affecting all the GTs instead
of only one).
The complexity of this file comes from the necessity of
minimizing code duplication, otherwise we could have had two
perfectly identical files (which looking at the final result it
wouldn't have been a completely bad idea after all).
Thanks again, will let you know when I will get this into our
branch.
Andi
> > Reviewed-by: Kees Cook <keescook@chromium.org>
>
> Thanks for looking it over!
>
> Cheers,
> Nathan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-29 16:46 ` [Intel-gfx] " Andi Shyti
@ 2022-09-29 16:53 ` Nathan Chancellor
0 siblings, 0 replies; 9+ messages in thread
From: Nathan Chancellor @ 2022-09-29 16:53 UTC (permalink / raw)
To: Andi Shyti
Cc: Tvrtko Ursulin, Kees Cook, Tom Rix, intel-gfx, llvm,
Nick Desaulniers, patches, dri-devel, Ashutosh Dixit,
Sami Tolvanen, Rodrigo Vivi
On Thu, Sep 29, 2022 at 06:46:34PM +0200, Andi Shyti wrote:
> Hi Nathan,
>
> thanks for this refactoring... looks good even though i would
> have split it in more patches as this is doing quite many things.
Right, sorry about that :/ I initially thought the problem was much
simpler and the diff was more reasonable before I truly saw what was
happening and by that point, trying to break things apart felt like
navigating a mine field. I will definitely keep that in mind for the
future though.
> But I will not be stubborn, I understand that it's not trivial to
> have things split. I will give my r-b for now but I will check it
> again before applying it as it's very easy to get tangled-up in
> between all those defines:
>
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Appreciate it! I don't have access to some of the hardware that is
special cased in this code so I definitely would not mind some
additional eyes and testing for this change.
> For now I'm quite surprised to see how easily our CI gives green
> lights :-P
>
> On Sat, Sep 24, 2022 at 09:39:30PM -0700, Nathan Chancellor wrote:
> > On Fri, Sep 23, 2022 at 09:57:47PM -0700, Kees Cook wrote:
> > > On Thu, Sep 22, 2022 at 12:51:27PM -0700, Nathan Chancellor wrote:
> > > > [...]
> > > > To make everything work properly, adjust certain functions to match the
> > > > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > > > Add a macro to generate functions for that can be called via both
> > > > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > > > called through both kobject locations without violating kCFI and adjust
> > > > the attribute groups to account for this.
> > >
> > > This was quite a roller coaster! I think the solution looks good, even
> > > if I'm suspicious of the original design that has the same stuff
> > > available twice in different places. (I have a dim memory of rdma
> > > needing a refactoring like this too?)
> >
> > Right, I noticed this comment in intel_gt_sysfs_register() once I fully
> > saw what was going on:
> >
> > /*
> > * We need to make things right with the
> > * ABI compatibility. The files were originally
> > * generated under the parent directory.
> > *
> > * We generate the files only for gt 0
> > * to avoid duplicates.
> > */
> >
> > Makes it seem like there will be userspace breakage if these files do
> > not exist? I figured this was the cleanest solution within those
> > parameters.
>
> i915 went multi-gt (multitile) so that some interfaces, like the
> power management files, have effect only on one of the tiles.
> This means that we needed to move some of the files inside the
> newly created gt0/gt1 directory.
>
> But, because some of those files are part of an ABI
> specification, we can't simply transfer them from the original
> position so that we needed to make a "hard" copy (actually the
> original files now take the role of affecting all the GTs instead
> of only one).
>
> The complexity of this file comes from the necessity of
> minimizing code duplication, otherwise we could have had two
> perfectly identical files (which looking at the final result it
> wouldn't have been a completely bad idea after all).
>
> Thanks again, will let you know when I will get this into our
> branch.
Ah, that all makes sense, good to know that this solution will allow all
of that to continue to work.
If there are any issues or further comments, I am happy to follow up in
whatever way I need to. Thanks again for the review and tentative
acceptance!
Cheers,
Nathan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-22 19:51 [PATCH] drm/i915: Fix CFI violations in gt_sysfs Nathan Chancellor
2022-09-23 7:15 ` Tvrtko Ursulin
2022-09-24 4:57 ` Kees Cook
@ 2022-09-29 22:34 ` Andrzej Hajda
2022-09-29 22:44 ` Nathan Chancellor
2 siblings, 1 reply; 9+ messages in thread
From: Andrzej Hajda @ 2022-09-29 22:34 UTC (permalink / raw)
To: Nathan Chancellor, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin
Cc: Kees Cook, Tom Rix, intel-gfx, llvm, Nick Desaulniers, patches,
dri-devel, Sami Tolvanen
On 22.09.2022 21:51, Nathan Chancellor wrote:
> When booting with clang's kernel control flow integrity series [1],
> there are numerous violations when accessing the files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
>
> $ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
>
> $ grep . *
> id:0
> punit_req_freq_mhz:350
> rc6_enable:1
> rc6_residency_ms:214934
> rps_act_freq_mhz:1300
> rps_boost_freq_mhz:1300
> rps_cur_freq_mhz:350
> rps_max_freq_mhz:1300
> rps_min_freq_mhz:350
> rps_RP0_freq_mhz:1300
> rps_RP1_freq_mhz:350
> rps_RPn_freq_mhz:350
> throttle_reason_pl1:0
> throttle_reason_pl2:0
> throttle_reason_pl4:0
> throttle_reason_prochot:0
> throttle_reason_ratl:0
> throttle_reason_status:0
> throttle_reason_thermal:0
> throttle_reason_vr_tdc:0
> throttle_reason_vr_thermalert:0
>
> $ sudo dmesg &| grep "CFI failure at"
> [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
> [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
> [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
>
> With kCFI, indirect calls are validated against their expected type
> versus actual type and failures occur when the two types do not match.
Have you tried this tool with drm subsytem, IIRC there are also similar
cases with callbacks expecting ptr to different struct than actually passed.
> The ultimate issue is that these sysfs functions are expecting to be
> called via dev_attr_show() but they may also be called via
> kobj_attr_show(), as certain files are created under two different
> kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
> hence the warnings above. When accessing the gt_ files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
> sysfs functions, there are no violations, meaning the functions are
> being called with the proper type.
>
> To make everything work properly, adjust certain functions to match the
> type of the ->show() and ->store() members in 'struct kobj_attribute'.
> Add a macro to generate functions for that can be called via both
> dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> called through both kobject locations without violating kCFI and adjust
> the attribute groups to account for this.
>
> [1]: https://lore.kernel.org/20220908215504.3686827-1-samitolvanen@google.com/
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 15 +-
> drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 2 +-
> drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 462 +++++++++-----------
> 3 files changed, 221 insertions(+), 258 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> index d651ccd0ab20..9486dd3bed99 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> @@ -22,11 +22,9 @@ bool is_object_gt(struct kobject *kobj)
> return !strncmp(kobj->name, "gt", 2);
> }
>
> -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> const char *name)
> {
> - struct kobject *kobj = &dev->kobj;
> -
> /*
> * We are interested at knowing from where the interface
> * has been called, whether it's called from gt/ or from
> @@ -38,6 +36,7 @@ struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> * "struct drm_i915_private *" type.
> */
> if (!is_object_gt(kobj)) {
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> return to_gt(i915);
> @@ -51,18 +50,18 @@ static struct kobject *gt_get_parent_obj(struct intel_gt *gt)
> return >->i915->drm.primary->kdev->kobj;
> }
>
> -static ssize_t id_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t id_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buf)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
>
> return sysfs_emit(buf, "%u\n", gt->info.id);
> }
> -static DEVICE_ATTR_RO(id);
> +static struct kobj_attribute attr_id = __ATTR_RO(id);
>
> static struct attribute *id_attrs[] = {
> - &dev_attr_id.attr,
> + &attr_id.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(id);
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> index 6232923a420d..c3a123faee98 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> @@ -30,7 +30,7 @@ static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
>
> void intel_gt_sysfs_register(struct intel_gt *gt);
> void intel_gt_sysfs_unregister(struct intel_gt *gt);
> -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> const char *name);
>
> #endif /* SYSFS_GT_H */
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 904160952369..308d54008983 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -24,14 +24,15 @@ enum intel_gt_sysfs_op {
> };
>
> static int
> -sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> +sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
> int (func)(struct intel_gt *gt, u32 val), u32 val)
> {
> struct intel_gt *gt;
> int ret;
>
> - if (!is_object_gt(&dev->kobj)) {
> + if (!is_object_gt(kobj)) {
> int i;
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> for_each_gt(gt, i915, i) {
> @@ -40,7 +41,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> break;
> }
> } else {
> - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> ret = func(gt, val);
> }
>
> @@ -48,7 +49,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> }
>
> static u32
> -sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> +sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
> u32 (func)(struct intel_gt *gt),
> enum intel_gt_sysfs_op op)
> {
> @@ -57,8 +58,9 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
>
> ret = (op == INTEL_GT_SYSFS_MAX) ? 0 : (u32) -1;
>
> - if (!is_object_gt(&dev->kobj)) {
> + if (!is_object_gt(kobj)) {
> int i;
> + struct device *dev = kobj_to_dev(kobj);
> struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
>
> for_each_gt(gt, i915, i) {
> @@ -77,7 +79,7 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> }
> }
> } else {
> - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> ret = func(gt);
> }
>
> @@ -92,6 +94,77 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> #define sysfs_gt_attribute_r_max_func(d, a, f) \
> sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
>
> +#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
> + static ssize_t _name##_show(struct kobject *kobj, \
> + struct kobj_attribute *attr, char *buff) \
> + { \
> + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
> + __##_name##_show); \
> + \
> + return sysfs_emit(buff, "%u\n", val); \
> + } \
> + static ssize_t _name##_dev_show(struct device *dev, \
> + struct device_attribute *attr, char *buff) \
> + { \
> + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
> + __##_name##_show); \
> + \
> + return sysfs_emit(buff, "%u\n", val); \
> + }
> +
> +#define INTEL_GT_SYSFS_STORE(_name, _func) \
> + static ssize_t _name##_store(struct kobject *kobj, \
> + struct kobj_attribute *attr, const char *buff, \
> + size_t count) \
> + { \
> + int ret; \
> + u32 val; \
> + \
> + ret = kstrtou32(buff, 0, &val); \
> + if (ret) \
> + return ret; \
> + \
> + ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
> + \
> + return ret ?: count; \
> + } \
> + static ssize_t _name##_dev_store(struct device *dev, \
> + struct device_attribute *attr, \
> + const char *buff, size_t count) \
> + { \
> + int ret; \
> + u32 val; \
> + \
> + ret = kstrtou32(buff, 0, &val); \
> + if (ret) \
> + return ret; \
> + \
> + ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
> + \
> + return ret ?: count; \
> + }
In both cases above I guess 2nd function can just call 1st one instead
of copy/paste (small, but still). For example:
static ssize_t _name##_dev_store(...)
{
return _name##_store(&dev->kobj, attr->attr, _func, val);
}
Beside this:
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Nice work.
Regards
Andrzej
> +
> +#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
> +#define INTEL_GT_SYSFS_SHOW_MIN(_name) INTEL_GT_SYSFS_SHOW(_name, min)
> +
> +#define INTEL_GT_ATTR_RW(_name) \
> + static struct kobj_attribute attr_##_name = __ATTR_RW(_name)
> +
> +#define INTEL_GT_ATTR_RO(_name) \
> + static struct kobj_attribute attr_##_name = __ATTR_RO(_name)
> +
> +#define INTEL_GT_DUAL_ATTR_RW(_name) \
> + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0644, \
> + _name##_dev_show, \
> + _name##_dev_store); \
> + INTEL_GT_ATTR_RW(_name)
> +
> +#define INTEL_GT_DUAL_ATTR_RO(_name) \
> + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0444, \
> + _name##_dev_show, \
> + NULL); \
> + INTEL_GT_ATTR_RO(_name)
> +
> #ifdef CONFIG_PM
> static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> {
> @@ -104,11 +177,8 @@ static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> return DIV_ROUND_CLOSEST_ULL(res, 1000);
> }
>
> -static ssize_t rc6_enable_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static u8 get_rc6_mask(struct intel_gt *gt)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> u8 mask = 0;
>
> if (HAS_RC6(gt->i915))
> @@ -118,37 +188,35 @@ static ssize_t rc6_enable_show(struct device *dev,
> if (HAS_RC6pp(gt->i915))
> mask |= BIT(2);
>
> - return sysfs_emit(buff, "%x\n", mask);
> + return mask;
> }
>
> -static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> +static ssize_t rc6_enable_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + char *buff)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> +
> + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> }
>
> -static ssize_t rc6_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static ssize_t rc6_enable_dev_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buff)
> {
> - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6_residency_ms_show);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, attr->attr.name);
>
> - return sysfs_emit(buff, "%u\n", rc6_residency);
> + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> }
>
> -static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> +static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> {
> - return get_residency(gt, GEN6_GT_GFX_RC6p);
> + return get_residency(gt, GEN6_GT_GFX_RC6);
> }
>
> -static ssize_t rc6p_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> +static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> {
> - u32 rc6p_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6p_residency_ms_show);
> -
> - return sysfs_emit(buff, "%u\n", rc6p_residency);
> + return get_residency(gt, GEN6_GT_GFX_RC6p);
> }
>
> static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> @@ -156,67 +224,69 @@ static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> return get_residency(gt, GEN6_GT_GFX_RC6pp);
> }
>
> -static ssize_t rc6pp_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 rc6pp_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __rc6pp_residency_ms_show);
> -
> - return sysfs_emit(buff, "%u\n", rc6pp_residency);
> -}
> -
> static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
> {
> return get_residency(gt, VLV_GT_MEDIA_RC6);
> }
>
> -static ssize_t media_rc6_residency_ms_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> - __media_rc6_residency_ms_show);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6p_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(rc6pp_residency_ms);
> +INTEL_GT_SYSFS_SHOW_MIN(media_rc6_residency_ms);
>
> - return sysfs_emit(buff, "%u\n", rc6_residency);
> -}
> -
> -static DEVICE_ATTR_RO(rc6_enable);
> -static DEVICE_ATTR_RO(rc6_residency_ms);
> -static DEVICE_ATTR_RO(rc6p_residency_ms);
> -static DEVICE_ATTR_RO(rc6pp_residency_ms);
> -static DEVICE_ATTR_RO(media_rc6_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6_enable);
> +INTEL_GT_DUAL_ATTR_RO(rc6_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6p_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(rc6pp_residency_ms);
> +INTEL_GT_DUAL_ATTR_RO(media_rc6_residency_ms);
>
> static struct attribute *rc6_attrs[] = {
> + &attr_rc6_enable.attr,
> + &attr_rc6_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *rc6p_attrs[] = {
> + &attr_rc6p_residency_ms.attr,
> + &attr_rc6pp_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *media_rc6_attrs[] = {
> + &attr_media_rc6_residency_ms.attr,
> + NULL
> +};
> +
> +static struct attribute *rc6_dev_attrs[] = {
> &dev_attr_rc6_enable.attr,
> &dev_attr_rc6_residency_ms.attr,
> NULL
> };
>
> -static struct attribute *rc6p_attrs[] = {
> +static struct attribute *rc6p_dev_attrs[] = {
> &dev_attr_rc6p_residency_ms.attr,
> &dev_attr_rc6pp_residency_ms.attr,
> NULL
> };
>
> -static struct attribute *media_rc6_attrs[] = {
> +static struct attribute *media_rc6_dev_attrs[] = {
> &dev_attr_media_rc6_residency_ms.attr,
> NULL
> };
>
> static const struct attribute_group rc6_attr_group[] = {
> { .attrs = rc6_attrs, },
> - { .name = power_group_name, .attrs = rc6_attrs, },
> + { .name = power_group_name, .attrs = rc6_dev_attrs, },
> };
>
> static const struct attribute_group rc6p_attr_group[] = {
> { .attrs = rc6p_attrs, },
> - { .name = power_group_name, .attrs = rc6p_attrs, },
> + { .name = power_group_name, .attrs = rc6p_dev_attrs, },
> };
>
> static const struct attribute_group media_rc6_attr_group[] = {
> { .attrs = media_rc6_attrs, },
> - { .name = power_group_name, .attrs = media_rc6_attrs, },
> + { .name = power_group_name, .attrs = media_rc6_dev_attrs, },
> };
>
> static int __intel_gt_sysfs_create_group(struct kobject *kobj,
> @@ -271,104 +341,34 @@ static u32 __act_freq_mhz_show(struct intel_gt *gt)
> return intel_rps_read_actual_frequency(>->rps);
> }
>
> -static ssize_t act_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __act_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", actual_freq);
> -}
> -
> static u32 __cur_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_requested_frequency(>->rps);
> }
>
> -static ssize_t cur_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 cur_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __cur_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", cur_freq);
> -}
> -
> static u32 __boost_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_boost_frequency(>->rps);
> }
>
> -static ssize_t boost_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buff)
> -{
> - u32 boost_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __boost_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", boost_freq);
> -}
> -
> static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_boost_frequency(>->rps, val);
> }
>
> -static ssize_t boost_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - ssize_t ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - return sysfs_gt_attribute_w_func(dev, attr,
> - __boost_freq_mhz_store, val) ?: count;
> -}
> -
> -static u32 __rp0_freq_mhz_show(struct intel_gt *gt)
> +static u32 __RP0_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_rp0_frequency(>->rps);
> }
>
> -static ssize_t RP0_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rp0_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rp0_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rp0_freq);
> -}
> -
> -static u32 __rp1_freq_mhz_show(struct intel_gt *gt)
> -{
> - return intel_rps_get_rp1_frequency(>->rps);
> -}
> -
> -static ssize_t RP1_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rp1_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rp1_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rp1_freq);
> -}
> -
> -static u32 __rpn_freq_mhz_show(struct intel_gt *gt)
> +static u32 __RPn_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_rpn_frequency(>->rps);
> }
>
> -static ssize_t RPn_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> +static u32 __RP1_freq_mhz_show(struct intel_gt *gt)
> {
> - u32 rpn_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __rpn_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rpn_freq);
> + return intel_rps_get_rp1_frequency(>->rps);
> }
>
> static u32 __max_freq_mhz_show(struct intel_gt *gt)
> @@ -376,71 +376,21 @@ static u32 __max_freq_mhz_show(struct intel_gt *gt)
> return intel_rps_get_max_frequency(>->rps);
> }
>
> -static ssize_t max_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 max_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __max_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", max_freq);
> -}
> -
> static int __set_max_freq(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_max_frequency(>->rps, val);
> }
>
> -static ssize_t max_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - int ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
> -
> - return ret ?: count;
> -}
> -
> static u32 __min_freq_mhz_show(struct intel_gt *gt)
> {
> return intel_rps_get_min_frequency(>->rps);
> }
>
> -static ssize_t min_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
> - __min_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", min_freq);
> -}
> -
> static int __set_min_freq(struct intel_gt *gt, u32 val)
> {
> return intel_rps_set_min_frequency(>->rps, val);
> }
>
> -static ssize_t min_freq_mhz_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - int ret;
> - u32 val;
> -
> - ret = kstrtou32(buff, 0, &val);
> - if (ret)
> - return ret;
> -
> - ret = sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val);
> -
> - return ret ?: count;
> -}
> -
> static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> {
> struct intel_rps *rps = >->rps;
> @@ -448,23 +398,31 @@ static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> return intel_gpu_freq(rps, rps->efficient_freq);
> }
>
> -static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr, char *buff)
> -{
> - u32 rpe_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> - __vlv_rpe_freq_mhz_show);
> -
> - return sysfs_emit(buff, "%u\n", rpe_freq);
> -}
> -
> -#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
> - static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
> - static struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
> -
> -#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> - INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
> -#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> - INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
> +INTEL_GT_SYSFS_SHOW_MAX(act_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(boost_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(cur_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RP0_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RP1_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(RPn_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(max_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MIN(min_freq_mhz);
> +INTEL_GT_SYSFS_SHOW_MAX(vlv_rpe_freq_mhz);
> +INTEL_GT_SYSFS_STORE(boost_freq_mhz, __boost_freq_mhz_store);
> +INTEL_GT_SYSFS_STORE(max_freq_mhz, __set_max_freq);
> +INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store, _show_dev, _store_dev) \
> + static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, \
> + _show_dev, _store_dev); \
> + static struct kobj_attribute attr_rps_##_name = __ATTR(rps_##_name, _mode, \
> + _show, _store)
> +
> +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL, \
> + _name##_dev_show, NULL)
> +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store, \
> + _name##_dev_show, _name##_dev_store)
>
> /* The below macros generate static structures */
> INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
> @@ -475,32 +433,31 @@ INTEL_GT_RPS_SYSFS_ATTR_RO(RP1_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RO(RPn_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RW(max_freq_mhz);
> INTEL_GT_RPS_SYSFS_ATTR_RW(min_freq_mhz);
> -
> -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> -
> -#define GEN6_ATTR(s) { \
> - &dev_attr_##s##_act_freq_mhz.attr, \
> - &dev_attr_##s##_cur_freq_mhz.attr, \
> - &dev_attr_##s##_boost_freq_mhz.attr, \
> - &dev_attr_##s##_max_freq_mhz.attr, \
> - &dev_attr_##s##_min_freq_mhz.attr, \
> - &dev_attr_##s##_RP0_freq_mhz.attr, \
> - &dev_attr_##s##_RP1_freq_mhz.attr, \
> - &dev_attr_##s##_RPn_freq_mhz.attr, \
> +INTEL_GT_RPS_SYSFS_ATTR_RO(vlv_rpe_freq_mhz);
> +
> +#define GEN6_ATTR(p, s) { \
> + &p##attr_##s##_act_freq_mhz.attr, \
> + &p##attr_##s##_cur_freq_mhz.attr, \
> + &p##attr_##s##_boost_freq_mhz.attr, \
> + &p##attr_##s##_max_freq_mhz.attr, \
> + &p##attr_##s##_min_freq_mhz.attr, \
> + &p##attr_##s##_RP0_freq_mhz.attr, \
> + &p##attr_##s##_RP1_freq_mhz.attr, \
> + &p##attr_##s##_RPn_freq_mhz.attr, \
> NULL, \
> }
>
> -#define GEN6_RPS_ATTR GEN6_ATTR(rps)
> -#define GEN6_GT_ATTR GEN6_ATTR(gt)
> +#define GEN6_RPS_ATTR GEN6_ATTR(, rps)
> +#define GEN6_GT_ATTR GEN6_ATTR(dev_, gt)
>
> static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
> static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
>
> -static ssize_t punit_req_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t punit_req_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 preq = intel_rps_read_punit_req_frequency(>->rps);
>
> return sysfs_emit(buff, "%u\n", preq);
> @@ -508,17 +465,17 @@ static ssize_t punit_req_freq_mhz_show(struct device *dev,
>
> struct intel_gt_bool_throttle_attr {
> struct attribute attr;
> - ssize_t (*show)(struct device *dev, struct device_attribute *attr,
> + ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
> char *buf);
> i915_reg_t (*reg32)(struct intel_gt *gt);
> u32 mask;
> };
>
> -static ssize_t throttle_reason_bool_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t throttle_reason_bool_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_gt_bool_throttle_attr *t_attr =
> (struct intel_gt_bool_throttle_attr *) attr;
> bool val = rps_read_mask_mmio(>->rps, t_attr->reg32(gt), t_attr->mask);
> @@ -534,7 +491,7 @@ struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
> .mask = mask__, \
> }
>
> -static DEVICE_ATTR_RO(punit_req_freq_mhz);
> +INTEL_GT_ATTR_RO(punit_req_freq_mhz);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_status, GT0_PERF_LIMIT_REASONS_MASK);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl1, POWER_LIMIT_1_MASK);
> static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl2, POWER_LIMIT_2_MASK);
> @@ -597,8 +554,8 @@ static const struct attribute *throttle_reason_attrs[] = {
> #define U8_8_VAL_MASK 0xffff
> #define U8_8_SCALE_TO_VALUE "0.00390625"
>
> -static ssize_t freq_factor_scale_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t freq_factor_scale_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
> @@ -610,11 +567,11 @@ static u32 media_ratio_mode_to_factor(u32 mode)
> return !mode ? mode : 256 / mode;
> }
>
> -static ssize_t media_freq_factor_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_freq_factor_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> intel_wakeref_t wakeref;
> u32 mode;
> @@ -641,11 +598,11 @@ static ssize_t media_freq_factor_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
> }
>
> -static ssize_t media_freq_factor_store(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_freq_factor_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> const char *buff, size_t count)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> u32 factor, mode;
> int err;
> @@ -670,11 +627,11 @@ static ssize_t media_freq_factor_store(struct device *dev,
> return err ?: count;
> }
>
> -static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_RP0_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 val;
> int err;
>
> @@ -691,11 +648,11 @@ static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", val);
> }
>
> -static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> - struct device_attribute *attr,
> +static ssize_t media_RPn_freq_mhz_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> char *buff)
> {
> - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> u32 val;
> int err;
>
> @@ -712,17 +669,17 @@ static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> return sysfs_emit(buff, "%u\n", val);
> }
>
> -static DEVICE_ATTR_RW(media_freq_factor);
> -static struct device_attribute dev_attr_media_freq_factor_scale =
> +INTEL_GT_ATTR_RW(media_freq_factor);
> +static struct kobj_attribute attr_media_freq_factor_scale =
> __ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
> -static DEVICE_ATTR_RO(media_RP0_freq_mhz);
> -static DEVICE_ATTR_RO(media_RPn_freq_mhz);
> +INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
> +INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
>
> static const struct attribute *media_perf_power_attrs[] = {
> - &dev_attr_media_freq_factor.attr,
> - &dev_attr_media_freq_factor_scale.attr,
> - &dev_attr_media_RP0_freq_mhz.attr,
> - &dev_attr_media_RPn_freq_mhz.attr,
> + &attr_media_freq_factor.attr,
> + &attr_media_freq_factor_scale.attr,
> + &attr_media_RP0_freq_mhz.attr,
> + &attr_media_RPn_freq_mhz.attr,
> NULL
> };
>
> @@ -754,20 +711,29 @@ static const struct attribute * const rps_defaults_attrs[] = {
> NULL
> };
>
> -static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> - const struct attribute * const *attrs)
> +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
> {
> + const struct attribute * const *attrs;
> + struct attribute *vlv_attr;
> int ret;
>
> if (GRAPHICS_VER(gt->i915) < 6)
> return 0;
>
> + if (is_object_gt(kobj)) {
> + attrs = gen6_rps_attrs;
> + vlv_attr = &attr_rps_vlv_rpe_freq_mhz.attr;
> + } else {
> + attrs = gen6_gt_attrs;
> + vlv_attr = &dev_attr_gt_vlv_rpe_freq_mhz.attr;
> + }
> +
> ret = sysfs_create_files(kobj, attrs);
> if (ret)
> return ret;
>
> if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
> - ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
> + ret = sysfs_create_file(kobj, vlv_attr);
>
> return ret;
> }
> @@ -778,9 +744,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
>
> intel_sysfs_rc6_init(gt, kobj);
>
> - ret = is_object_gt(kobj) ?
> - intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
> - intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
> + ret = intel_sysfs_rps_init(gt, kobj);
> if (ret)
> drm_warn(>->i915->drm,
> "failed to create gt%u RPS sysfs files (%pe)",
> @@ -790,7 +754,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> if (!is_object_gt(kobj))
> return;
>
> - ret = sysfs_create_file(kobj, &dev_attr_punit_req_freq_mhz.attr);
> + ret = sysfs_create_file(kobj, &attr_punit_req_freq_mhz.attr);
> if (ret)
> drm_warn(>->i915->drm,
> "failed to create gt%u punit_req_freq_mhz sysfs (%pe)",
>
> base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-29 22:34 ` Andrzej Hajda
@ 2022-09-29 22:44 ` Nathan Chancellor
2022-10-03 17:46 ` Nathan Chancellor
0 siblings, 1 reply; 9+ messages in thread
From: Nathan Chancellor @ 2022-09-29 22:44 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Tvrtko Ursulin, llvm, Kees Cook, Tom Rix, intel-gfx,
Nick Desaulniers, patches, dri-devel, Sami Tolvanen,
Rodrigo Vivi
On Fri, Sep 30, 2022 at 12:34:41AM +0200, Andrzej Hajda wrote:
> On 22.09.2022 21:51, Nathan Chancellor wrote:
> > When booting with clang's kernel control flow integrity series [1],
> > there are numerous violations when accessing the files under
> > /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
> >
> > $ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
> >
> > $ grep . *
> > id:0
> > punit_req_freq_mhz:350
> > rc6_enable:1
> > rc6_residency_ms:214934
> > rps_act_freq_mhz:1300
> > rps_boost_freq_mhz:1300
> > rps_cur_freq_mhz:350
> > rps_max_freq_mhz:1300
> > rps_min_freq_mhz:350
> > rps_RP0_freq_mhz:1300
> > rps_RP1_freq_mhz:350
> > rps_RPn_freq_mhz:350
> > throttle_reason_pl1:0
> > throttle_reason_pl2:0
> > throttle_reason_pl4:0
> > throttle_reason_prochot:0
> > throttle_reason_ratl:0
> > throttle_reason_status:0
> > throttle_reason_thermal:0
> > throttle_reason_vr_tdc:0
> > throttle_reason_vr_thermalert:0
> >
> > $ sudo dmesg &| grep "CFI failure at"
> > [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
> > [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> > [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> > [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
> > [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> >
> > With kCFI, indirect calls are validated against their expected type
> > versus actual type and failures occur when the two types do not match.
>
> Have you tried this tool with drm subsytem, IIRC there are also similar
> cases with callbacks expecting ptr to different struct than actually passed.
Yes, I have also run a kCFI kernel on an AMD system that I have and I
have not seen any failures from them. I only have AMD and Intel systems
with graphics so there could be other problems lurking in other drivers.
> > The ultimate issue is that these sysfs functions are expecting to be
> > called via dev_attr_show() but they may also be called via
> > kobj_attr_show(), as certain files are created under two different
> > kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
> > hence the warnings above. When accessing the gt_ files under
> > /sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
> > sysfs functions, there are no violations, meaning the functions are
> > being called with the proper type.
> >
> > To make everything work properly, adjust certain functions to match the
> > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > Add a macro to generate functions for that can be called via both
> > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > called through both kobject locations without violating kCFI and adjust
> > the attribute groups to account for this.
> >
> > [1]: https://lore.kernel.org/20220908215504.3686827-1-samitolvanen@google.com/
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 15 +-
> > drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 2 +-
> > drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 462 +++++++++-----------
> > 3 files changed, 221 insertions(+), 258 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > index d651ccd0ab20..9486dd3bed99 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > @@ -22,11 +22,9 @@ bool is_object_gt(struct kobject *kobj)
> > return !strncmp(kobj->name, "gt", 2);
> > }
> > -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> > const char *name)
> > {
> > - struct kobject *kobj = &dev->kobj;
> > -
> > /*
> > * We are interested at knowing from where the interface
> > * has been called, whether it's called from gt/ or from
> > @@ -38,6 +36,7 @@ struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > * "struct drm_i915_private *" type.
> > */
> > if (!is_object_gt(kobj)) {
> > + struct device *dev = kobj_to_dev(kobj);
> > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > return to_gt(i915);
> > @@ -51,18 +50,18 @@ static struct kobject *gt_get_parent_obj(struct intel_gt *gt)
> > return >->i915->drm.primary->kdev->kobj;
> > }
> > -static ssize_t id_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t id_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buf)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > return sysfs_emit(buf, "%u\n", gt->info.id);
> > }
> > -static DEVICE_ATTR_RO(id);
> > +static struct kobj_attribute attr_id = __ATTR_RO(id);
> > static struct attribute *id_attrs[] = {
> > - &dev_attr_id.attr,
> > + &attr_id.attr,
> > NULL,
> > };
> > ATTRIBUTE_GROUPS(id);
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > index 6232923a420d..c3a123faee98 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > @@ -30,7 +30,7 @@ static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
> > void intel_gt_sysfs_register(struct intel_gt *gt);
> > void intel_gt_sysfs_unregister(struct intel_gt *gt);
> > -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> > const char *name);
> > #endif /* SYSFS_GT_H */
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > index 904160952369..308d54008983 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > @@ -24,14 +24,15 @@ enum intel_gt_sysfs_op {
> > };
> > static int
> > -sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > +sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
> > int (func)(struct intel_gt *gt, u32 val), u32 val)
> > {
> > struct intel_gt *gt;
> > int ret;
> > - if (!is_object_gt(&dev->kobj)) {
> > + if (!is_object_gt(kobj)) {
> > int i;
> > + struct device *dev = kobj_to_dev(kobj);
> > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > for_each_gt(gt, i915, i) {
> > @@ -40,7 +41,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > break;
> > }
> > } else {
> > - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> > ret = func(gt, val);
> > }
> > @@ -48,7 +49,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > }
> > static u32
> > -sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > +sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
> > u32 (func)(struct intel_gt *gt),
> > enum intel_gt_sysfs_op op)
> > {
> > @@ -57,8 +58,9 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > ret = (op == INTEL_GT_SYSFS_MAX) ? 0 : (u32) -1;
> > - if (!is_object_gt(&dev->kobj)) {
> > + if (!is_object_gt(kobj)) {
> > int i;
> > + struct device *dev = kobj_to_dev(kobj);
> > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > for_each_gt(gt, i915, i) {
> > @@ -77,7 +79,7 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > }
> > }
> > } else {
> > - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> > ret = func(gt);
> > }
> > @@ -92,6 +94,77 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > #define sysfs_gt_attribute_r_max_func(d, a, f) \
> > sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
> > +#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
> > + static ssize_t _name##_show(struct kobject *kobj, \
> > + struct kobj_attribute *attr, char *buff) \
> > + { \
> > + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
> > + __##_name##_show); \
> > + \
> > + return sysfs_emit(buff, "%u\n", val); \
> > + } \
> > + static ssize_t _name##_dev_show(struct device *dev, \
> > + struct device_attribute *attr, char *buff) \
> > + { \
> > + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
> > + __##_name##_show); \
> > + \
> > + return sysfs_emit(buff, "%u\n", val); \
> > + }
> > +
> > +#define INTEL_GT_SYSFS_STORE(_name, _func) \
> > + static ssize_t _name##_store(struct kobject *kobj, \
> > + struct kobj_attribute *attr, const char *buff, \
> > + size_t count) \
> > + { \
> > + int ret; \
> > + u32 val; \
> > + \
> > + ret = kstrtou32(buff, 0, &val); \
> > + if (ret) \
> > + return ret; \
> > + \
> > + ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
> > + \
> > + return ret ?: count; \
> > + } \
> > + static ssize_t _name##_dev_store(struct device *dev, \
> > + struct device_attribute *attr, \
> > + const char *buff, size_t count) \
> > + { \
> > + int ret; \
> > + u32 val; \
> > + \
> > + ret = kstrtou32(buff, 0, &val); \
> > + if (ret) \
> > + return ret; \
> > + \
> > + ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
> > + \
> > + return ret ?: count; \
> > + }
>
> In both cases above I guess 2nd function can just call 1st one instead of
> copy/paste (small, but still). For example:
> static ssize_t _name##_dev_store(...)
> {
> return _name##_store(&dev->kobj, attr->attr, _func, val);
> }
Ah great point, I had thought about that but never jumped on it for some
reason... I can send a v2 on Monday (I will be offline Friday) to give a
chance for others to comment.
> Beside this:
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Thank you for taking a look!
Cheers,
Nathan
> Nice work.
>
> Regards
> Andrzej
>
> > +
> > +#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
> > +#define INTEL_GT_SYSFS_SHOW_MIN(_name) INTEL_GT_SYSFS_SHOW(_name, min)
> > +
> > +#define INTEL_GT_ATTR_RW(_name) \
> > + static struct kobj_attribute attr_##_name = __ATTR_RW(_name)
> > +
> > +#define INTEL_GT_ATTR_RO(_name) \
> > + static struct kobj_attribute attr_##_name = __ATTR_RO(_name)
> > +
> > +#define INTEL_GT_DUAL_ATTR_RW(_name) \
> > + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0644, \
> > + _name##_dev_show, \
> > + _name##_dev_store); \
> > + INTEL_GT_ATTR_RW(_name)
> > +
> > +#define INTEL_GT_DUAL_ATTR_RO(_name) \
> > + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0444, \
> > + _name##_dev_show, \
> > + NULL); \
> > + INTEL_GT_ATTR_RO(_name)
> > +
> > #ifdef CONFIG_PM
> > static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> > {
> > @@ -104,11 +177,8 @@ static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> > return DIV_ROUND_CLOSEST_ULL(res, 1000);
> > }
> > -static ssize_t rc6_enable_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > +static u8 get_rc6_mask(struct intel_gt *gt)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > u8 mask = 0;
> > if (HAS_RC6(gt->i915))
> > @@ -118,37 +188,35 @@ static ssize_t rc6_enable_show(struct device *dev,
> > if (HAS_RC6pp(gt->i915))
> > mask |= BIT(2);
> > - return sysfs_emit(buff, "%x\n", mask);
> > + return mask;
> > }
> > -static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> > +static ssize_t rc6_enable_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > + char *buff)
> > {
> > - return get_residency(gt, GEN6_GT_GFX_RC6);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > +
> > + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> > }
> > -static ssize_t rc6_residency_ms_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > +static ssize_t rc6_enable_dev_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buff)
> > {
> > - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > - __rc6_residency_ms_show);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, attr->attr.name);
> > - return sysfs_emit(buff, "%u\n", rc6_residency);
> > + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> > }
> > -static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> > +static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> > {
> > - return get_residency(gt, GEN6_GT_GFX_RC6p);
> > + return get_residency(gt, GEN6_GT_GFX_RC6);
> > }
> > -static ssize_t rc6p_residency_ms_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > +static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> > {
> > - u32 rc6p_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > - __rc6p_residency_ms_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rc6p_residency);
> > + return get_residency(gt, GEN6_GT_GFX_RC6p);
> > }
> > static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> > @@ -156,67 +224,69 @@ static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> > return get_residency(gt, GEN6_GT_GFX_RC6pp);
> > }
> > -static ssize_t rc6pp_residency_ms_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > -{
> > - u32 rc6pp_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > - __rc6pp_residency_ms_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rc6pp_residency);
> > -}
> > -
> > static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
> > {
> > return get_residency(gt, VLV_GT_MEDIA_RC6);
> > }
> > -static ssize_t media_rc6_residency_ms_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > -{
> > - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > - __media_rc6_residency_ms_show);
> > +INTEL_GT_SYSFS_SHOW_MIN(rc6_residency_ms);
> > +INTEL_GT_SYSFS_SHOW_MIN(rc6p_residency_ms);
> > +INTEL_GT_SYSFS_SHOW_MIN(rc6pp_residency_ms);
> > +INTEL_GT_SYSFS_SHOW_MIN(media_rc6_residency_ms);
> > - return sysfs_emit(buff, "%u\n", rc6_residency);
> > -}
> > -
> > -static DEVICE_ATTR_RO(rc6_enable);
> > -static DEVICE_ATTR_RO(rc6_residency_ms);
> > -static DEVICE_ATTR_RO(rc6p_residency_ms);
> > -static DEVICE_ATTR_RO(rc6pp_residency_ms);
> > -static DEVICE_ATTR_RO(media_rc6_residency_ms);
> > +INTEL_GT_DUAL_ATTR_RO(rc6_enable);
> > +INTEL_GT_DUAL_ATTR_RO(rc6_residency_ms);
> > +INTEL_GT_DUAL_ATTR_RO(rc6p_residency_ms);
> > +INTEL_GT_DUAL_ATTR_RO(rc6pp_residency_ms);
> > +INTEL_GT_DUAL_ATTR_RO(media_rc6_residency_ms);
> > static struct attribute *rc6_attrs[] = {
> > + &attr_rc6_enable.attr,
> > + &attr_rc6_residency_ms.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute *rc6p_attrs[] = {
> > + &attr_rc6p_residency_ms.attr,
> > + &attr_rc6pp_residency_ms.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute *media_rc6_attrs[] = {
> > + &attr_media_rc6_residency_ms.attr,
> > + NULL
> > +};
> > +
> > +static struct attribute *rc6_dev_attrs[] = {
> > &dev_attr_rc6_enable.attr,
> > &dev_attr_rc6_residency_ms.attr,
> > NULL
> > };
> > -static struct attribute *rc6p_attrs[] = {
> > +static struct attribute *rc6p_dev_attrs[] = {
> > &dev_attr_rc6p_residency_ms.attr,
> > &dev_attr_rc6pp_residency_ms.attr,
> > NULL
> > };
> > -static struct attribute *media_rc6_attrs[] = {
> > +static struct attribute *media_rc6_dev_attrs[] = {
> > &dev_attr_media_rc6_residency_ms.attr,
> > NULL
> > };
> > static const struct attribute_group rc6_attr_group[] = {
> > { .attrs = rc6_attrs, },
> > - { .name = power_group_name, .attrs = rc6_attrs, },
> > + { .name = power_group_name, .attrs = rc6_dev_attrs, },
> > };
> > static const struct attribute_group rc6p_attr_group[] = {
> > { .attrs = rc6p_attrs, },
> > - { .name = power_group_name, .attrs = rc6p_attrs, },
> > + { .name = power_group_name, .attrs = rc6p_dev_attrs, },
> > };
> > static const struct attribute_group media_rc6_attr_group[] = {
> > { .attrs = media_rc6_attrs, },
> > - { .name = power_group_name, .attrs = media_rc6_attrs, },
> > + { .name = power_group_name, .attrs = media_rc6_dev_attrs, },
> > };
> > static int __intel_gt_sysfs_create_group(struct kobject *kobj,
> > @@ -271,104 +341,34 @@ static u32 __act_freq_mhz_show(struct intel_gt *gt)
> > return intel_rps_read_actual_frequency(>->rps);
> > }
> > -static ssize_t act_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __act_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", actual_freq);
> > -}
> > -
> > static u32 __cur_freq_mhz_show(struct intel_gt *gt)
> > {
> > return intel_rps_get_requested_frequency(>->rps);
> > }
> > -static ssize_t cur_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 cur_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __cur_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", cur_freq);
> > -}
> > -
> > static u32 __boost_freq_mhz_show(struct intel_gt *gt)
> > {
> > return intel_rps_get_boost_frequency(>->rps);
> > }
> > -static ssize_t boost_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr,
> > - char *buff)
> > -{
> > - u32 boost_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __boost_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", boost_freq);
> > -}
> > -
> > static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
> > {
> > return intel_rps_set_boost_frequency(>->rps, val);
> > }
> > -static ssize_t boost_freq_mhz_store(struct device *dev,
> > - struct device_attribute *attr,
> > - const char *buff, size_t count)
> > -{
> > - ssize_t ret;
> > - u32 val;
> > -
> > - ret = kstrtou32(buff, 0, &val);
> > - if (ret)
> > - return ret;
> > -
> > - return sysfs_gt_attribute_w_func(dev, attr,
> > - __boost_freq_mhz_store, val) ?: count;
> > -}
> > -
> > -static u32 __rp0_freq_mhz_show(struct intel_gt *gt)
> > +static u32 __RP0_freq_mhz_show(struct intel_gt *gt)
> > {
> > return intel_rps_get_rp0_frequency(>->rps);
> > }
> > -static ssize_t RP0_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 rp0_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __rp0_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rp0_freq);
> > -}
> > -
> > -static u32 __rp1_freq_mhz_show(struct intel_gt *gt)
> > -{
> > - return intel_rps_get_rp1_frequency(>->rps);
> > -}
> > -
> > -static ssize_t RP1_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 rp1_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __rp1_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rp1_freq);
> > -}
> > -
> > -static u32 __rpn_freq_mhz_show(struct intel_gt *gt)
> > +static u32 __RPn_freq_mhz_show(struct intel_gt *gt)
> > {
> > return intel_rps_get_rpn_frequency(>->rps);
> > }
> > -static ssize_t RPn_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > +static u32 __RP1_freq_mhz_show(struct intel_gt *gt)
> > {
> > - u32 rpn_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __rpn_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rpn_freq);
> > + return intel_rps_get_rp1_frequency(>->rps);
> > }
> > static u32 __max_freq_mhz_show(struct intel_gt *gt)
> > @@ -376,71 +376,21 @@ static u32 __max_freq_mhz_show(struct intel_gt *gt)
> > return intel_rps_get_max_frequency(>->rps);
> > }
> > -static ssize_t max_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 max_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __max_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", max_freq);
> > -}
> > -
> > static int __set_max_freq(struct intel_gt *gt, u32 val)
> > {
> > return intel_rps_set_max_frequency(>->rps, val);
> > }
> > -static ssize_t max_freq_mhz_store(struct device *dev,
> > - struct device_attribute *attr,
> > - const char *buff, size_t count)
> > -{
> > - int ret;
> > - u32 val;
> > -
> > - ret = kstrtou32(buff, 0, &val);
> > - if (ret)
> > - return ret;
> > -
> > - ret = sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
> > -
> > - return ret ?: count;
> > -}
> > -
> > static u32 __min_freq_mhz_show(struct intel_gt *gt)
> > {
> > return intel_rps_get_min_frequency(>->rps);
> > }
> > -static ssize_t min_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
> > - __min_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", min_freq);
> > -}
> > -
> > static int __set_min_freq(struct intel_gt *gt, u32 val)
> > {
> > return intel_rps_set_min_frequency(>->rps, val);
> > }
> > -static ssize_t min_freq_mhz_store(struct device *dev,
> > - struct device_attribute *attr,
> > - const char *buff, size_t count)
> > -{
> > - int ret;
> > - u32 val;
> > -
> > - ret = kstrtou32(buff, 0, &val);
> > - if (ret)
> > - return ret;
> > -
> > - ret = sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val);
> > -
> > - return ret ?: count;
> > -}
> > -
> > static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> > {
> > struct intel_rps *rps = >->rps;
> > @@ -448,23 +398,31 @@ static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> > return intel_gpu_freq(rps, rps->efficient_freq);
> > }
> > -static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr, char *buff)
> > -{
> > - u32 rpe_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > - __vlv_rpe_freq_mhz_show);
> > -
> > - return sysfs_emit(buff, "%u\n", rpe_freq);
> > -}
> > -
> > -#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
> > - static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
> > - static struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
> > -
> > -#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> > - INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
> > -#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> > - INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
> > +INTEL_GT_SYSFS_SHOW_MAX(act_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(boost_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(cur_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(RP0_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(RP1_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(RPn_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(max_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MIN(min_freq_mhz);
> > +INTEL_GT_SYSFS_SHOW_MAX(vlv_rpe_freq_mhz);
> > +INTEL_GT_SYSFS_STORE(boost_freq_mhz, __boost_freq_mhz_store);
> > +INTEL_GT_SYSFS_STORE(max_freq_mhz, __set_max_freq);
> > +INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
> > +
> > +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store, _show_dev, _store_dev) \
> > + static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, \
> > + _show_dev, _store_dev); \
> > + static struct kobj_attribute attr_rps_##_name = __ATTR(rps_##_name, _mode, \
> > + _show, _store)
> > +
> > +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL, \
> > + _name##_dev_show, NULL)
> > +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store, \
> > + _name##_dev_show, _name##_dev_store)
> > /* The below macros generate static structures */
> > INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
> > @@ -475,32 +433,31 @@ INTEL_GT_RPS_SYSFS_ATTR_RO(RP1_freq_mhz);
> > INTEL_GT_RPS_SYSFS_ATTR_RO(RPn_freq_mhz);
> > INTEL_GT_RPS_SYSFS_ATTR_RW(max_freq_mhz);
> > INTEL_GT_RPS_SYSFS_ATTR_RW(min_freq_mhz);
> > -
> > -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> > -
> > -#define GEN6_ATTR(s) { \
> > - &dev_attr_##s##_act_freq_mhz.attr, \
> > - &dev_attr_##s##_cur_freq_mhz.attr, \
> > - &dev_attr_##s##_boost_freq_mhz.attr, \
> > - &dev_attr_##s##_max_freq_mhz.attr, \
> > - &dev_attr_##s##_min_freq_mhz.attr, \
> > - &dev_attr_##s##_RP0_freq_mhz.attr, \
> > - &dev_attr_##s##_RP1_freq_mhz.attr, \
> > - &dev_attr_##s##_RPn_freq_mhz.attr, \
> > +INTEL_GT_RPS_SYSFS_ATTR_RO(vlv_rpe_freq_mhz);
> > +
> > +#define GEN6_ATTR(p, s) { \
> > + &p##attr_##s##_act_freq_mhz.attr, \
> > + &p##attr_##s##_cur_freq_mhz.attr, \
> > + &p##attr_##s##_boost_freq_mhz.attr, \
> > + &p##attr_##s##_max_freq_mhz.attr, \
> > + &p##attr_##s##_min_freq_mhz.attr, \
> > + &p##attr_##s##_RP0_freq_mhz.attr, \
> > + &p##attr_##s##_RP1_freq_mhz.attr, \
> > + &p##attr_##s##_RPn_freq_mhz.attr, \
> > NULL, \
> > }
> > -#define GEN6_RPS_ATTR GEN6_ATTR(rps)
> > -#define GEN6_GT_ATTR GEN6_ATTR(gt)
> > +#define GEN6_RPS_ATTR GEN6_ATTR(, rps)
> > +#define GEN6_GT_ATTR GEN6_ATTR(dev_, gt)
> > static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
> > static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
> > -static ssize_t punit_req_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t punit_req_freq_mhz_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > u32 preq = intel_rps_read_punit_req_frequency(>->rps);
> > return sysfs_emit(buff, "%u\n", preq);
> > @@ -508,17 +465,17 @@ static ssize_t punit_req_freq_mhz_show(struct device *dev,
> > struct intel_gt_bool_throttle_attr {
> > struct attribute attr;
> > - ssize_t (*show)(struct device *dev, struct device_attribute *attr,
> > + ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
> > char *buf);
> > i915_reg_t (*reg32)(struct intel_gt *gt);
> > u32 mask;
> > };
> > -static ssize_t throttle_reason_bool_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t throttle_reason_bool_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > struct intel_gt_bool_throttle_attr *t_attr =
> > (struct intel_gt_bool_throttle_attr *) attr;
> > bool val = rps_read_mask_mmio(>->rps, t_attr->reg32(gt), t_attr->mask);
> > @@ -534,7 +491,7 @@ struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
> > .mask = mask__, \
> > }
> > -static DEVICE_ATTR_RO(punit_req_freq_mhz);
> > +INTEL_GT_ATTR_RO(punit_req_freq_mhz);
> > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_status, GT0_PERF_LIMIT_REASONS_MASK);
> > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl1, POWER_LIMIT_1_MASK);
> > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl2, POWER_LIMIT_2_MASK);
> > @@ -597,8 +554,8 @@ static const struct attribute *throttle_reason_attrs[] = {
> > #define U8_8_VAL_MASK 0xffff
> > #define U8_8_SCALE_TO_VALUE "0.00390625"
> > -static ssize_t freq_factor_scale_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t freq_factor_scale_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
> > @@ -610,11 +567,11 @@ static u32 media_ratio_mode_to_factor(u32 mode)
> > return !mode ? mode : 256 / mode;
> > }
> > -static ssize_t media_freq_factor_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t media_freq_factor_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> > intel_wakeref_t wakeref;
> > u32 mode;
> > @@ -641,11 +598,11 @@ static ssize_t media_freq_factor_show(struct device *dev,
> > return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
> > }
> > -static ssize_t media_freq_factor_store(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t media_freq_factor_store(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > const char *buff, size_t count)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> > u32 factor, mode;
> > int err;
> > @@ -670,11 +627,11 @@ static ssize_t media_freq_factor_store(struct device *dev,
> > return err ?: count;
> > }
> > -static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t media_RP0_freq_mhz_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > u32 val;
> > int err;
> > @@ -691,11 +648,11 @@ static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> > return sysfs_emit(buff, "%u\n", val);
> > }
> > -static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> > - struct device_attribute *attr,
> > +static ssize_t media_RPn_freq_mhz_show(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > char *buff)
> > {
> > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > u32 val;
> > int err;
> > @@ -712,17 +669,17 @@ static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> > return sysfs_emit(buff, "%u\n", val);
> > }
> > -static DEVICE_ATTR_RW(media_freq_factor);
> > -static struct device_attribute dev_attr_media_freq_factor_scale =
> > +INTEL_GT_ATTR_RW(media_freq_factor);
> > +static struct kobj_attribute attr_media_freq_factor_scale =
> > __ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
> > -static DEVICE_ATTR_RO(media_RP0_freq_mhz);
> > -static DEVICE_ATTR_RO(media_RPn_freq_mhz);
> > +INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
> > +INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
> > static const struct attribute *media_perf_power_attrs[] = {
> > - &dev_attr_media_freq_factor.attr,
> > - &dev_attr_media_freq_factor_scale.attr,
> > - &dev_attr_media_RP0_freq_mhz.attr,
> > - &dev_attr_media_RPn_freq_mhz.attr,
> > + &attr_media_freq_factor.attr,
> > + &attr_media_freq_factor_scale.attr,
> > + &attr_media_RP0_freq_mhz.attr,
> > + &attr_media_RPn_freq_mhz.attr,
> > NULL
> > };
> > @@ -754,20 +711,29 @@ static const struct attribute * const rps_defaults_attrs[] = {
> > NULL
> > };
> > -static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> > - const struct attribute * const *attrs)
> > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
> > {
> > + const struct attribute * const *attrs;
> > + struct attribute *vlv_attr;
> > int ret;
> > if (GRAPHICS_VER(gt->i915) < 6)
> > return 0;
> > + if (is_object_gt(kobj)) {
> > + attrs = gen6_rps_attrs;
> > + vlv_attr = &attr_rps_vlv_rpe_freq_mhz.attr;
> > + } else {
> > + attrs = gen6_gt_attrs;
> > + vlv_attr = &dev_attr_gt_vlv_rpe_freq_mhz.attr;
> > + }
> > +
> > ret = sysfs_create_files(kobj, attrs);
> > if (ret)
> > return ret;
> > if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
> > - ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
> > + ret = sysfs_create_file(kobj, vlv_attr);
> > return ret;
> > }
> > @@ -778,9 +744,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> > intel_sysfs_rc6_init(gt, kobj);
> > - ret = is_object_gt(kobj) ?
> > - intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
> > - intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
> > + ret = intel_sysfs_rps_init(gt, kobj);
> > if (ret)
> > drm_warn(>->i915->drm,
> > "failed to create gt%u RPS sysfs files (%pe)",
> > @@ -790,7 +754,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> > if (!is_object_gt(kobj))
> > return;
> > - ret = sysfs_create_file(kobj, &dev_attr_punit_req_freq_mhz.attr);
> > + ret = sysfs_create_file(kobj, &attr_punit_req_freq_mhz.attr);
> > if (ret)
> > drm_warn(>->i915->drm,
> > "failed to create gt%u punit_req_freq_mhz sysfs (%pe)",
> >
> > base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Fix CFI violations in gt_sysfs
2022-09-29 22:44 ` Nathan Chancellor
@ 2022-10-03 17:46 ` Nathan Chancellor
0 siblings, 0 replies; 9+ messages in thread
From: Nathan Chancellor @ 2022-10-03 17:46 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Tvrtko Ursulin, llvm, Kees Cook, Tom Rix, intel-gfx,
Nick Desaulniers, patches, dri-devel, Sami Tolvanen,
Rodrigo Vivi
Hi Andrzej,
On Thu, Sep 29, 2022 at 03:44:40PM -0700, Nathan Chancellor wrote:
> On Fri, Sep 30, 2022 at 12:34:41AM +0200, Andrzej Hajda wrote:
> > On 22.09.2022 21:51, Nathan Chancellor wrote:
> > > When booting with clang's kernel control flow integrity series [1],
> > > there are numerous violations when accessing the files under
> > > /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
> > >
> > > $ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
> > >
> > > $ grep . *
> > > id:0
> > > punit_req_freq_mhz:350
> > > rc6_enable:1
> > > rc6_residency_ms:214934
> > > rps_act_freq_mhz:1300
> > > rps_boost_freq_mhz:1300
> > > rps_cur_freq_mhz:350
> > > rps_max_freq_mhz:1300
> > > rps_min_freq_mhz:350
> > > rps_RP0_freq_mhz:1300
> > > rps_RP1_freq_mhz:350
> > > rps_RPn_freq_mhz:350
> > > throttle_reason_pl1:0
> > > throttle_reason_pl2:0
> > > throttle_reason_pl4:0
> > > throttle_reason_prochot:0
> > > throttle_reason_ratl:0
> > > throttle_reason_status:0
> > > throttle_reason_thermal:0
> > > throttle_reason_vr_tdc:0
> > > throttle_reason_vr_thermalert:0
> > >
> > > $ sudo dmesg &| grep "CFI failure at"
> > > [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
> > > [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> > > [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> > > [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
> > > [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> > > [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > > [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> > >
> > > With kCFI, indirect calls are validated against their expected type
> > > versus actual type and failures occur when the two types do not match.
> >
> > Have you tried this tool with drm subsytem, IIRC there are also similar
> > cases with callbacks expecting ptr to different struct than actually passed.
>
> Yes, I have also run a kCFI kernel on an AMD system that I have and I
> have not seen any failures from them. I only have AMD and Intel systems
> with graphics so there could be other problems lurking in other drivers.
>
> > > The ultimate issue is that these sysfs functions are expecting to be
> > > called via dev_attr_show() but they may also be called via
> > > kobj_attr_show(), as certain files are created under two different
> > > kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
> > > hence the warnings above. When accessing the gt_ files under
> > > /sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
> > > sysfs functions, there are no violations, meaning the functions are
> > > being called with the proper type.
> > >
> > > To make everything work properly, adjust certain functions to match the
> > > type of the ->show() and ->store() members in 'struct kobj_attribute'.
> > > Add a macro to generate functions for that can be called via both
> > > dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> > > called through both kobject locations without violating kCFI and adjust
> > > the attribute groups to account for this.
> > >
> > > [1]: https://lore.kernel.org/20220908215504.3686827-1-samitolvanen@google.com/
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > > ---
> > > drivers/gpu/drm/i915/gt/intel_gt_sysfs.c | 15 +-
> > > drivers/gpu/drm/i915/gt/intel_gt_sysfs.h | 2 +-
> > > drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 462 +++++++++-----------
> > > 3 files changed, 221 insertions(+), 258 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > > index d651ccd0ab20..9486dd3bed99 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.c
> > > @@ -22,11 +22,9 @@ bool is_object_gt(struct kobject *kobj)
> > > return !strncmp(kobj->name, "gt", 2);
> > > }
> > > -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> > > const char *name)
> > > {
> > > - struct kobject *kobj = &dev->kobj;
> > > -
> > > /*
> > > * We are interested at knowing from where the interface
> > > * has been called, whether it's called from gt/ or from
> > > @@ -38,6 +36,7 @@ struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > > * "struct drm_i915_private *" type.
> > > */
> > > if (!is_object_gt(kobj)) {
> > > + struct device *dev = kobj_to_dev(kobj);
> > > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > > return to_gt(i915);
> > > @@ -51,18 +50,18 @@ static struct kobject *gt_get_parent_obj(struct intel_gt *gt)
> > > return >->i915->drm.primary->kdev->kobj;
> > > }
> > > -static ssize_t id_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t id_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buf)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > return sysfs_emit(buf, "%u\n", gt->info.id);
> > > }
> > > -static DEVICE_ATTR_RO(id);
> > > +static struct kobj_attribute attr_id = __ATTR_RO(id);
> > > static struct attribute *id_attrs[] = {
> > > - &dev_attr_id.attr,
> > > + &attr_id.attr,
> > > NULL,
> > > };
> > > ATTRIBUTE_GROUPS(id);
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > > index 6232923a420d..c3a123faee98 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h
> > > @@ -30,7 +30,7 @@ static inline struct intel_gt *kobj_to_gt(struct kobject *kobj)
> > > void intel_gt_sysfs_register(struct intel_gt *gt);
> > > void intel_gt_sysfs_unregister(struct intel_gt *gt);
> > > -struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev,
> > > +struct intel_gt *intel_gt_sysfs_get_drvdata(struct kobject *kobj,
> > > const char *name);
> > > #endif /* SYSFS_GT_H */
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > > index 904160952369..308d54008983 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> > > @@ -24,14 +24,15 @@ enum intel_gt_sysfs_op {
> > > };
> > > static int
> > > -sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > > +sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
> > > int (func)(struct intel_gt *gt, u32 val), u32 val)
> > > {
> > > struct intel_gt *gt;
> > > int ret;
> > > - if (!is_object_gt(&dev->kobj)) {
> > > + if (!is_object_gt(kobj)) {
> > > int i;
> > > + struct device *dev = kobj_to_dev(kobj);
> > > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > > for_each_gt(gt, i915, i) {
> > > @@ -40,7 +41,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > > break;
> > > }
> > > } else {
> > > - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> > > ret = func(gt, val);
> > > }
> > > @@ -48,7 +49,7 @@ sysfs_gt_attribute_w_func(struct device *dev, struct device_attribute *attr,
> > > }
> > > static u32
> > > -sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > > +sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
> > > u32 (func)(struct intel_gt *gt),
> > > enum intel_gt_sysfs_op op)
> > > {
> > > @@ -57,8 +58,9 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > > ret = (op == INTEL_GT_SYSFS_MAX) ? 0 : (u32) -1;
> > > - if (!is_object_gt(&dev->kobj)) {
> > > + if (!is_object_gt(kobj)) {
> > > int i;
> > > + struct device *dev = kobj_to_dev(kobj);
> > > struct drm_i915_private *i915 = kdev_minor_to_i915(dev);
> > > for_each_gt(gt, i915, i) {
> > > @@ -77,7 +79,7 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > > }
> > > }
> > > } else {
> > > - gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
> > > ret = func(gt);
> > > }
> > > @@ -92,6 +94,77 @@ sysfs_gt_attribute_r_func(struct device *dev, struct device_attribute *attr,
> > > #define sysfs_gt_attribute_r_max_func(d, a, f) \
> > > sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
> > > +#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
> > > + static ssize_t _name##_show(struct kobject *kobj, \
> > > + struct kobj_attribute *attr, char *buff) \
> > > + { \
> > > + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
> > > + __##_name##_show); \
> > > + \
> > > + return sysfs_emit(buff, "%u\n", val); \
> > > + } \
> > > + static ssize_t _name##_dev_show(struct device *dev, \
> > > + struct device_attribute *attr, char *buff) \
> > > + { \
> > > + u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
> > > + __##_name##_show); \
> > > + \
> > > + return sysfs_emit(buff, "%u\n", val); \
> > > + }
> > > +
> > > +#define INTEL_GT_SYSFS_STORE(_name, _func) \
> > > + static ssize_t _name##_store(struct kobject *kobj, \
> > > + struct kobj_attribute *attr, const char *buff, \
> > > + size_t count) \
> > > + { \
> > > + int ret; \
> > > + u32 val; \
> > > + \
> > > + ret = kstrtou32(buff, 0, &val); \
> > > + if (ret) \
> > > + return ret; \
> > > + \
> > > + ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
> > > + \
> > > + return ret ?: count; \
> > > + } \
> > > + static ssize_t _name##_dev_store(struct device *dev, \
> > > + struct device_attribute *attr, \
> > > + const char *buff, size_t count) \
> > > + { \
> > > + int ret; \
> > > + u32 val; \
> > > + \
> > > + ret = kstrtou32(buff, 0, &val); \
> > > + if (ret) \
> > > + return ret; \
> > > + \
> > > + ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
> > > + \
> > > + return ret ?: count; \
> > > + }
> >
> > In both cases above I guess 2nd function can just call 1st one instead of
> > copy/paste (small, but still). For example:
> > static ssize_t _name##_dev_store(...)
> > {
> > return _name##_store(&dev->kobj, attr->attr, _func, val);
> > }
>
> Ah great point, I had thought about that but never jumped on it for some
> reason... I can send a v2 on Monday (I will be offline Friday) to give a
> chance for others to comment.
Now that I am back online and looking into a v2, this suggestion will
not quite work. The second parameter to the _name##_store function is of
type 'struct kobj_attribute', not 'struct attribute', so we cannot pass
either 'attr' or 'attr->attr', as neither are 'struct kobj_attribute'.
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:400:1: error: incompatible pointer types passing 'struct device_attribute *' to parameter of type 'struct kobj_attribute *' [-Werror,-Wincompatible-pointer-types]
INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:132:36: note: expanded from macro 'INTEL_GT_SYSFS_STORE'
return _name##_store(&dev->kobj, attr, buff, count); \
^~~~
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:400:1: note: passing argument to parameter 'attr' here
drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c:114:33: note: expanded from macro 'INTEL_GT_SYSFS_STORE'
struct kobj_attribute *attr, const char *buff, \
^
I cannot change the second parameter to 'struct attribute' because the
function prototype has to match the ->show() and ->store() members of
'struct kobj_attribute'.
I could introduce a third function then have the existing functions call
that one to reduce duplication, which might look something like the
following, if that is what you would prefer? I am happy to send a v2
with this included if it works for you.
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 308d54008983..2b5f05b31187 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -24,7 +24,7 @@ enum intel_gt_sysfs_op {
};
static int
-sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
+sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute *attr,
int (func)(struct intel_gt *gt, u32 val), u32 val)
{
struct intel_gt *gt;
@@ -41,7 +41,7 @@ sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
break;
}
} else {
- gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
+ gt = intel_gt_sysfs_get_drvdata(kobj, attr->name);
ret = func(gt, val);
}
@@ -49,7 +49,7 @@ sysfs_gt_attribute_w_func(struct kobject *kobj, struct attribute attr,
}
static u32
-sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
+sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute *attr,
u32 (func)(struct intel_gt *gt),
enum intel_gt_sysfs_op op)
{
@@ -79,7 +79,7 @@ sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
}
}
} else {
- gt = intel_gt_sysfs_get_drvdata(kobj, attr.name);
+ gt = intel_gt_sysfs_get_drvdata(kobj, attr->name);
ret = func(gt);
}
@@ -95,27 +95,29 @@ sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
sysfs_gt_attribute_r_func(d, a, f, INTEL_GT_SYSFS_MAX)
#define INTEL_GT_SYSFS_SHOW(_name, _attr_type) \
- static ssize_t _name##_show(struct kobject *kobj, \
- struct kobj_attribute *attr, char *buff) \
+ static ssize_t _name##_show_common(struct kobject *kobj, \
+ struct attribute *attr, char *buff) \
{ \
- u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr->attr, \
+ u32 val = sysfs_gt_attribute_r_##_attr_type##_func(kobj, attr, \
__##_name##_show); \
\
return sysfs_emit(buff, "%u\n", val); \
} \
+ static ssize_t _name##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, char *buff) \
+ { \
+ return _name ##_show_common(kobj, &attr->attr, buff); \
+ } \
static ssize_t _name##_dev_show(struct device *dev, \
struct device_attribute *attr, char *buff) \
{ \
- u32 val = sysfs_gt_attribute_r_##_attr_type##_func(&dev->kobj, attr->attr, \
- __##_name##_show); \
- \
- return sysfs_emit(buff, "%u\n", val); \
+ return _name##_show_common(&dev->kobj, &attr->attr, buff); \
}
#define INTEL_GT_SYSFS_STORE(_name, _func) \
- static ssize_t _name##_store(struct kobject *kobj, \
- struct kobj_attribute *attr, const char *buff, \
- size_t count) \
+ static ssize_t _name##_store_common(struct kobject *kobj, \
+ struct attribute *attr, \
+ const char *buff, size_t count) \
{ \
int ret; \
u32 val; \
@@ -124,24 +126,21 @@ sysfs_gt_attribute_r_func(struct kobject *kobj, struct attribute attr,
if (ret) \
return ret; \
\
- ret = sysfs_gt_attribute_w_func(kobj, attr->attr, _func, val); \
+ ret = sysfs_gt_attribute_w_func(kobj, attr, _func, val); \
\
return ret ?: count; \
} \
+ static ssize_t _name##_store(struct kobject *kobj, \
+ struct kobj_attribute *attr, const char *buff, \
+ size_t count) \
+ { \
+ return _name##_store_common(kobj, &attr->attr, buff, count); \
+ } \
static ssize_t _name##_dev_store(struct device *dev, \
struct device_attribute *attr, \
const char *buff, size_t count) \
{ \
- int ret; \
- u32 val; \
- \
- ret = kstrtou32(buff, 0, &val); \
- if (ret) \
- return ret; \
- \
- ret = sysfs_gt_attribute_w_func(&dev->kobj, attr->attr, _func, val); \
- \
- return ret ?: count; \
+ return _name##_store_common(&dev->kobj, &attr->attr, buff, count); \
}
#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
> > Beside this:
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> Thank you for taking a look!
>
> Cheers,
> Nathan
>
> > Nice work.
> >
> > Regards
> > Andrzej
> >
> > > +
> > > +#define INTEL_GT_SYSFS_SHOW_MAX(_name) INTEL_GT_SYSFS_SHOW(_name, max)
> > > +#define INTEL_GT_SYSFS_SHOW_MIN(_name) INTEL_GT_SYSFS_SHOW(_name, min)
> > > +
> > > +#define INTEL_GT_ATTR_RW(_name) \
> > > + static struct kobj_attribute attr_##_name = __ATTR_RW(_name)
> > > +
> > > +#define INTEL_GT_ATTR_RO(_name) \
> > > + static struct kobj_attribute attr_##_name = __ATTR_RO(_name)
> > > +
> > > +#define INTEL_GT_DUAL_ATTR_RW(_name) \
> > > + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0644, \
> > > + _name##_dev_show, \
> > > + _name##_dev_store); \
> > > + INTEL_GT_ATTR_RW(_name)
> > > +
> > > +#define INTEL_GT_DUAL_ATTR_RO(_name) \
> > > + static struct device_attribute dev_attr_##_name = __ATTR(_name, 0444, \
> > > + _name##_dev_show, \
> > > + NULL); \
> > > + INTEL_GT_ATTR_RO(_name)
> > > +
> > > #ifdef CONFIG_PM
> > > static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> > > {
> > > @@ -104,11 +177,8 @@ static u32 get_residency(struct intel_gt *gt, i915_reg_t reg)
> > > return DIV_ROUND_CLOSEST_ULL(res, 1000);
> > > }
> > > -static ssize_t rc6_enable_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > +static u8 get_rc6_mask(struct intel_gt *gt)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > u8 mask = 0;
> > > if (HAS_RC6(gt->i915))
> > > @@ -118,37 +188,35 @@ static ssize_t rc6_enable_show(struct device *dev,
> > > if (HAS_RC6pp(gt->i915))
> > > mask |= BIT(2);
> > > - return sysfs_emit(buff, "%x\n", mask);
> > > + return mask;
> > > }
> > > -static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> > > +static ssize_t rc6_enable_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > + char *buff)
> > > {
> > > - return get_residency(gt, GEN6_GT_GFX_RC6);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > +
> > > + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> > > }
> > > -static ssize_t rc6_residency_ms_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > +static ssize_t rc6_enable_dev_show(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buff)
> > > {
> > > - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > > - __rc6_residency_ms_show);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(&dev->kobj, attr->attr.name);
> > > - return sysfs_emit(buff, "%u\n", rc6_residency);
> > > + return sysfs_emit(buff, "%x\n", get_rc6_mask(gt));
> > > }
> > > -static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> > > +static u32 __rc6_residency_ms_show(struct intel_gt *gt)
> > > {
> > > - return get_residency(gt, GEN6_GT_GFX_RC6p);
> > > + return get_residency(gt, GEN6_GT_GFX_RC6);
> > > }
> > > -static ssize_t rc6p_residency_ms_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > +static u32 __rc6p_residency_ms_show(struct intel_gt *gt)
> > > {
> > > - u32 rc6p_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > > - __rc6p_residency_ms_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rc6p_residency);
> > > + return get_residency(gt, GEN6_GT_GFX_RC6p);
> > > }
> > > static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> > > @@ -156,67 +224,69 @@ static u32 __rc6pp_residency_ms_show(struct intel_gt *gt)
> > > return get_residency(gt, GEN6_GT_GFX_RC6pp);
> > > }
> > > -static ssize_t rc6pp_residency_ms_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > -{
> > > - u32 rc6pp_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > > - __rc6pp_residency_ms_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rc6pp_residency);
> > > -}
> > > -
> > > static u32 __media_rc6_residency_ms_show(struct intel_gt *gt)
> > > {
> > > return get_residency(gt, VLV_GT_MEDIA_RC6);
> > > }
> > > -static ssize_t media_rc6_residency_ms_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > -{
> > > - u32 rc6_residency = sysfs_gt_attribute_r_min_func(dev, attr,
> > > - __media_rc6_residency_ms_show);
> > > +INTEL_GT_SYSFS_SHOW_MIN(rc6_residency_ms);
> > > +INTEL_GT_SYSFS_SHOW_MIN(rc6p_residency_ms);
> > > +INTEL_GT_SYSFS_SHOW_MIN(rc6pp_residency_ms);
> > > +INTEL_GT_SYSFS_SHOW_MIN(media_rc6_residency_ms);
> > > - return sysfs_emit(buff, "%u\n", rc6_residency);
> > > -}
> > > -
> > > -static DEVICE_ATTR_RO(rc6_enable);
> > > -static DEVICE_ATTR_RO(rc6_residency_ms);
> > > -static DEVICE_ATTR_RO(rc6p_residency_ms);
> > > -static DEVICE_ATTR_RO(rc6pp_residency_ms);
> > > -static DEVICE_ATTR_RO(media_rc6_residency_ms);
> > > +INTEL_GT_DUAL_ATTR_RO(rc6_enable);
> > > +INTEL_GT_DUAL_ATTR_RO(rc6_residency_ms);
> > > +INTEL_GT_DUAL_ATTR_RO(rc6p_residency_ms);
> > > +INTEL_GT_DUAL_ATTR_RO(rc6pp_residency_ms);
> > > +INTEL_GT_DUAL_ATTR_RO(media_rc6_residency_ms);
> > > static struct attribute *rc6_attrs[] = {
> > > + &attr_rc6_enable.attr,
> > > + &attr_rc6_residency_ms.attr,
> > > + NULL
> > > +};
> > > +
> > > +static struct attribute *rc6p_attrs[] = {
> > > + &attr_rc6p_residency_ms.attr,
> > > + &attr_rc6pp_residency_ms.attr,
> > > + NULL
> > > +};
> > > +
> > > +static struct attribute *media_rc6_attrs[] = {
> > > + &attr_media_rc6_residency_ms.attr,
> > > + NULL
> > > +};
> > > +
> > > +static struct attribute *rc6_dev_attrs[] = {
> > > &dev_attr_rc6_enable.attr,
> > > &dev_attr_rc6_residency_ms.attr,
> > > NULL
> > > };
> > > -static struct attribute *rc6p_attrs[] = {
> > > +static struct attribute *rc6p_dev_attrs[] = {
> > > &dev_attr_rc6p_residency_ms.attr,
> > > &dev_attr_rc6pp_residency_ms.attr,
> > > NULL
> > > };
> > > -static struct attribute *media_rc6_attrs[] = {
> > > +static struct attribute *media_rc6_dev_attrs[] = {
> > > &dev_attr_media_rc6_residency_ms.attr,
> > > NULL
> > > };
> > > static const struct attribute_group rc6_attr_group[] = {
> > > { .attrs = rc6_attrs, },
> > > - { .name = power_group_name, .attrs = rc6_attrs, },
> > > + { .name = power_group_name, .attrs = rc6_dev_attrs, },
> > > };
> > > static const struct attribute_group rc6p_attr_group[] = {
> > > { .attrs = rc6p_attrs, },
> > > - { .name = power_group_name, .attrs = rc6p_attrs, },
> > > + { .name = power_group_name, .attrs = rc6p_dev_attrs, },
> > > };
> > > static const struct attribute_group media_rc6_attr_group[] = {
> > > { .attrs = media_rc6_attrs, },
> > > - { .name = power_group_name, .attrs = media_rc6_attrs, },
> > > + { .name = power_group_name, .attrs = media_rc6_dev_attrs, },
> > > };
> > > static int __intel_gt_sysfs_create_group(struct kobject *kobj,
> > > @@ -271,104 +341,34 @@ static u32 __act_freq_mhz_show(struct intel_gt *gt)
> > > return intel_rps_read_actual_frequency(>->rps);
> > > }
> > > -static ssize_t act_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 actual_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __act_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", actual_freq);
> > > -}
> > > -
> > > static u32 __cur_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > return intel_rps_get_requested_frequency(>->rps);
> > > }
> > > -static ssize_t cur_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 cur_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __cur_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", cur_freq);
> > > -}
> > > -
> > > static u32 __boost_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > return intel_rps_get_boost_frequency(>->rps);
> > > }
> > > -static ssize_t boost_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > - char *buff)
> > > -{
> > > - u32 boost_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __boost_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", boost_freq);
> > > -}
> > > -
> > > static int __boost_freq_mhz_store(struct intel_gt *gt, u32 val)
> > > {
> > > return intel_rps_set_boost_frequency(>->rps, val);
> > > }
> > > -static ssize_t boost_freq_mhz_store(struct device *dev,
> > > - struct device_attribute *attr,
> > > - const char *buff, size_t count)
> > > -{
> > > - ssize_t ret;
> > > - u32 val;
> > > -
> > > - ret = kstrtou32(buff, 0, &val);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - return sysfs_gt_attribute_w_func(dev, attr,
> > > - __boost_freq_mhz_store, val) ?: count;
> > > -}
> > > -
> > > -static u32 __rp0_freq_mhz_show(struct intel_gt *gt)
> > > +static u32 __RP0_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > return intel_rps_get_rp0_frequency(>->rps);
> > > }
> > > -static ssize_t RP0_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 rp0_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __rp0_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rp0_freq);
> > > -}
> > > -
> > > -static u32 __rp1_freq_mhz_show(struct intel_gt *gt)
> > > -{
> > > - return intel_rps_get_rp1_frequency(>->rps);
> > > -}
> > > -
> > > -static ssize_t RP1_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 rp1_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __rp1_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rp1_freq);
> > > -}
> > > -
> > > -static u32 __rpn_freq_mhz_show(struct intel_gt *gt)
> > > +static u32 __RPn_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > return intel_rps_get_rpn_frequency(>->rps);
> > > }
> > > -static ssize_t RPn_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > +static u32 __RP1_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > - u32 rpn_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __rpn_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rpn_freq);
> > > + return intel_rps_get_rp1_frequency(>->rps);
> > > }
> > > static u32 __max_freq_mhz_show(struct intel_gt *gt)
> > > @@ -376,71 +376,21 @@ static u32 __max_freq_mhz_show(struct intel_gt *gt)
> > > return intel_rps_get_max_frequency(>->rps);
> > > }
> > > -static ssize_t max_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 max_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __max_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", max_freq);
> > > -}
> > > -
> > > static int __set_max_freq(struct intel_gt *gt, u32 val)
> > > {
> > > return intel_rps_set_max_frequency(>->rps, val);
> > > }
> > > -static ssize_t max_freq_mhz_store(struct device *dev,
> > > - struct device_attribute *attr,
> > > - const char *buff, size_t count)
> > > -{
> > > - int ret;
> > > - u32 val;
> > > -
> > > - ret = kstrtou32(buff, 0, &val);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = sysfs_gt_attribute_w_func(dev, attr, __set_max_freq, val);
> > > -
> > > - return ret ?: count;
> > > -}
> > > -
> > > static u32 __min_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > return intel_rps_get_min_frequency(>->rps);
> > > }
> > > -static ssize_t min_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 min_freq = sysfs_gt_attribute_r_min_func(dev, attr,
> > > - __min_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", min_freq);
> > > -}
> > > -
> > > static int __set_min_freq(struct intel_gt *gt, u32 val)
> > > {
> > > return intel_rps_set_min_frequency(>->rps, val);
> > > }
> > > -static ssize_t min_freq_mhz_store(struct device *dev,
> > > - struct device_attribute *attr,
> > > - const char *buff, size_t count)
> > > -{
> > > - int ret;
> > > - u32 val;
> > > -
> > > - ret = kstrtou32(buff, 0, &val);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = sysfs_gt_attribute_w_func(dev, attr, __set_min_freq, val);
> > > -
> > > - return ret ?: count;
> > > -}
> > > -
> > > static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> > > {
> > > struct intel_rps *rps = >->rps;
> > > @@ -448,23 +398,31 @@ static u32 __vlv_rpe_freq_mhz_show(struct intel_gt *gt)
> > > return intel_gpu_freq(rps, rps->efficient_freq);
> > > }
> > > -static ssize_t vlv_rpe_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr, char *buff)
> > > -{
> > > - u32 rpe_freq = sysfs_gt_attribute_r_max_func(dev, attr,
> > > - __vlv_rpe_freq_mhz_show);
> > > -
> > > - return sysfs_emit(buff, "%u\n", rpe_freq);
> > > -}
> > > -
> > > -#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store) \
> > > - static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, _show, _store); \
> > > - static struct device_attribute dev_attr_rps_##_name = __ATTR(rps_##_name, _mode, _show, _store)
> > > -
> > > -#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> > > - INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL)
> > > -#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> > > - INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store)
> > > +INTEL_GT_SYSFS_SHOW_MAX(act_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(boost_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(cur_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(RP0_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(RP1_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(RPn_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(max_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MIN(min_freq_mhz);
> > > +INTEL_GT_SYSFS_SHOW_MAX(vlv_rpe_freq_mhz);
> > > +INTEL_GT_SYSFS_STORE(boost_freq_mhz, __boost_freq_mhz_store);
> > > +INTEL_GT_SYSFS_STORE(max_freq_mhz, __set_max_freq);
> > > +INTEL_GT_SYSFS_STORE(min_freq_mhz, __set_min_freq);
> > > +
> > > +#define INTEL_GT_RPS_SYSFS_ATTR(_name, _mode, _show, _store, _show_dev, _store_dev) \
> > > + static struct device_attribute dev_attr_gt_##_name = __ATTR(gt_##_name, _mode, \
> > > + _show_dev, _store_dev); \
> > > + static struct kobj_attribute attr_rps_##_name = __ATTR(rps_##_name, _mode, \
> > > + _show, _store)
> > > +
> > > +#define INTEL_GT_RPS_SYSFS_ATTR_RO(_name) \
> > > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0444, _name##_show, NULL, \
> > > + _name##_dev_show, NULL)
> > > +#define INTEL_GT_RPS_SYSFS_ATTR_RW(_name) \
> > > + INTEL_GT_RPS_SYSFS_ATTR(_name, 0644, _name##_show, _name##_store, \
> > > + _name##_dev_show, _name##_dev_store)
> > > /* The below macros generate static structures */
> > > INTEL_GT_RPS_SYSFS_ATTR_RO(act_freq_mhz);
> > > @@ -475,32 +433,31 @@ INTEL_GT_RPS_SYSFS_ATTR_RO(RP1_freq_mhz);
> > > INTEL_GT_RPS_SYSFS_ATTR_RO(RPn_freq_mhz);
> > > INTEL_GT_RPS_SYSFS_ATTR_RW(max_freq_mhz);
> > > INTEL_GT_RPS_SYSFS_ATTR_RW(min_freq_mhz);
> > > -
> > > -static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> > > -
> > > -#define GEN6_ATTR(s) { \
> > > - &dev_attr_##s##_act_freq_mhz.attr, \
> > > - &dev_attr_##s##_cur_freq_mhz.attr, \
> > > - &dev_attr_##s##_boost_freq_mhz.attr, \
> > > - &dev_attr_##s##_max_freq_mhz.attr, \
> > > - &dev_attr_##s##_min_freq_mhz.attr, \
> > > - &dev_attr_##s##_RP0_freq_mhz.attr, \
> > > - &dev_attr_##s##_RP1_freq_mhz.attr, \
> > > - &dev_attr_##s##_RPn_freq_mhz.attr, \
> > > +INTEL_GT_RPS_SYSFS_ATTR_RO(vlv_rpe_freq_mhz);
> > > +
> > > +#define GEN6_ATTR(p, s) { \
> > > + &p##attr_##s##_act_freq_mhz.attr, \
> > > + &p##attr_##s##_cur_freq_mhz.attr, \
> > > + &p##attr_##s##_boost_freq_mhz.attr, \
> > > + &p##attr_##s##_max_freq_mhz.attr, \
> > > + &p##attr_##s##_min_freq_mhz.attr, \
> > > + &p##attr_##s##_RP0_freq_mhz.attr, \
> > > + &p##attr_##s##_RP1_freq_mhz.attr, \
> > > + &p##attr_##s##_RPn_freq_mhz.attr, \
> > > NULL, \
> > > }
> > > -#define GEN6_RPS_ATTR GEN6_ATTR(rps)
> > > -#define GEN6_GT_ATTR GEN6_ATTR(gt)
> > > +#define GEN6_RPS_ATTR GEN6_ATTR(, rps)
> > > +#define GEN6_GT_ATTR GEN6_ATTR(dev_, gt)
> > > static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
> > > static const struct attribute * const gen6_gt_attrs[] = GEN6_GT_ATTR;
> > > -static ssize_t punit_req_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t punit_req_freq_mhz_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > u32 preq = intel_rps_read_punit_req_frequency(>->rps);
> > > return sysfs_emit(buff, "%u\n", preq);
> > > @@ -508,17 +465,17 @@ static ssize_t punit_req_freq_mhz_show(struct device *dev,
> > > struct intel_gt_bool_throttle_attr {
> > > struct attribute attr;
> > > - ssize_t (*show)(struct device *dev, struct device_attribute *attr,
> > > + ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *attr,
> > > char *buf);
> > > i915_reg_t (*reg32)(struct intel_gt *gt);
> > > u32 mask;
> > > };
> > > -static ssize_t throttle_reason_bool_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t throttle_reason_bool_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > struct intel_gt_bool_throttle_attr *t_attr =
> > > (struct intel_gt_bool_throttle_attr *) attr;
> > > bool val = rps_read_mask_mmio(>->rps, t_attr->reg32(gt), t_attr->mask);
> > > @@ -534,7 +491,7 @@ struct intel_gt_bool_throttle_attr attr_##sysfs_func__ = { \
> > > .mask = mask__, \
> > > }
> > > -static DEVICE_ATTR_RO(punit_req_freq_mhz);
> > > +INTEL_GT_ATTR_RO(punit_req_freq_mhz);
> > > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_status, GT0_PERF_LIMIT_REASONS_MASK);
> > > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl1, POWER_LIMIT_1_MASK);
> > > static INTEL_GT_RPS_BOOL_ATTR_RO(throttle_reason_pl2, POWER_LIMIT_2_MASK);
> > > @@ -597,8 +554,8 @@ static const struct attribute *throttle_reason_attrs[] = {
> > > #define U8_8_VAL_MASK 0xffff
> > > #define U8_8_SCALE_TO_VALUE "0.00390625"
> > > -static ssize_t freq_factor_scale_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t freq_factor_scale_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > return sysfs_emit(buff, "%s\n", U8_8_SCALE_TO_VALUE);
> > > @@ -610,11 +567,11 @@ static u32 media_ratio_mode_to_factor(u32 mode)
> > > return !mode ? mode : 256 / mode;
> > > }
> > > -static ssize_t media_freq_factor_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t media_freq_factor_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> > > intel_wakeref_t wakeref;
> > > u32 mode;
> > > @@ -641,11 +598,11 @@ static ssize_t media_freq_factor_show(struct device *dev,
> > > return sysfs_emit(buff, "%u\n", media_ratio_mode_to_factor(mode));
> > > }
> > > -static ssize_t media_freq_factor_store(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t media_freq_factor_store(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > const char *buff, size_t count)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > struct intel_guc_slpc *slpc = >->uc.guc.slpc;
> > > u32 factor, mode;
> > > int err;
> > > @@ -670,11 +627,11 @@ static ssize_t media_freq_factor_store(struct device *dev,
> > > return err ?: count;
> > > }
> > > -static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t media_RP0_freq_mhz_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > u32 val;
> > > int err;
> > > @@ -691,11 +648,11 @@ static ssize_t media_RP0_freq_mhz_show(struct device *dev,
> > > return sysfs_emit(buff, "%u\n", val);
> > > }
> > > -static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> > > - struct device_attribute *attr,
> > > +static ssize_t media_RPn_freq_mhz_show(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > char *buff)
> > > {
> > > - struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > > + struct intel_gt *gt = intel_gt_sysfs_get_drvdata(kobj, attr->attr.name);
> > > u32 val;
> > > int err;
> > > @@ -712,17 +669,17 @@ static ssize_t media_RPn_freq_mhz_show(struct device *dev,
> > > return sysfs_emit(buff, "%u\n", val);
> > > }
> > > -static DEVICE_ATTR_RW(media_freq_factor);
> > > -static struct device_attribute dev_attr_media_freq_factor_scale =
> > > +INTEL_GT_ATTR_RW(media_freq_factor);
> > > +static struct kobj_attribute attr_media_freq_factor_scale =
> > > __ATTR(media_freq_factor.scale, 0444, freq_factor_scale_show, NULL);
> > > -static DEVICE_ATTR_RO(media_RP0_freq_mhz);
> > > -static DEVICE_ATTR_RO(media_RPn_freq_mhz);
> > > +INTEL_GT_ATTR_RO(media_RP0_freq_mhz);
> > > +INTEL_GT_ATTR_RO(media_RPn_freq_mhz);
> > > static const struct attribute *media_perf_power_attrs[] = {
> > > - &dev_attr_media_freq_factor.attr,
> > > - &dev_attr_media_freq_factor_scale.attr,
> > > - &dev_attr_media_RP0_freq_mhz.attr,
> > > - &dev_attr_media_RPn_freq_mhz.attr,
> > > + &attr_media_freq_factor.attr,
> > > + &attr_media_freq_factor_scale.attr,
> > > + &attr_media_RP0_freq_mhz.attr,
> > > + &attr_media_RPn_freq_mhz.attr,
> > > NULL
> > > };
> > > @@ -754,20 +711,29 @@ static const struct attribute * const rps_defaults_attrs[] = {
> > > NULL
> > > };
> > > -static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> > > - const struct attribute * const *attrs)
> > > +static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj)
> > > {
> > > + const struct attribute * const *attrs;
> > > + struct attribute *vlv_attr;
> > > int ret;
> > > if (GRAPHICS_VER(gt->i915) < 6)
> > > return 0;
> > > + if (is_object_gt(kobj)) {
> > > + attrs = gen6_rps_attrs;
> > > + vlv_attr = &attr_rps_vlv_rpe_freq_mhz.attr;
> > > + } else {
> > > + attrs = gen6_gt_attrs;
> > > + vlv_attr = &dev_attr_gt_vlv_rpe_freq_mhz.attr;
> > > + }
> > > +
> > > ret = sysfs_create_files(kobj, attrs);
> > > if (ret)
> > > return ret;
> > > if (IS_VALLEYVIEW(gt->i915) || IS_CHERRYVIEW(gt->i915))
> > > - ret = sysfs_create_file(kobj, &dev_attr_vlv_rpe_freq_mhz.attr);
> > > + ret = sysfs_create_file(kobj, vlv_attr);
> > > return ret;
> > > }
> > > @@ -778,9 +744,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> > > intel_sysfs_rc6_init(gt, kobj);
> > > - ret = is_object_gt(kobj) ?
> > > - intel_sysfs_rps_init(gt, kobj, gen6_rps_attrs) :
> > > - intel_sysfs_rps_init(gt, kobj, gen6_gt_attrs);
> > > + ret = intel_sysfs_rps_init(gt, kobj);
> > > if (ret)
> > > drm_warn(>->i915->drm,
> > > "failed to create gt%u RPS sysfs files (%pe)",
> > > @@ -790,7 +754,7 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> > > if (!is_object_gt(kobj))
> > > return;
> > > - ret = sysfs_create_file(kobj, &dev_attr_punit_req_freq_mhz.attr);
> > > + ret = sysfs_create_file(kobj, &attr_punit_req_freq_mhz.attr);
> > > if (ret)
> > > drm_warn(>->i915->drm,
> > > "failed to create gt%u punit_req_freq_mhz sysfs (%pe)",
> > >
> > > base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
> >
>
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-10-03 17:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 19:51 [PATCH] drm/i915: Fix CFI violations in gt_sysfs Nathan Chancellor
2022-09-23 7:15 ` Tvrtko Ursulin
2022-09-24 4:57 ` Kees Cook
2022-09-25 4:39 ` Nathan Chancellor
2022-09-29 16:46 ` [Intel-gfx] " Andi Shyti
2022-09-29 16:53 ` Nathan Chancellor
2022-09-29 22:34 ` Andrzej Hajda
2022-09-29 22:44 ` Nathan Chancellor
2022-10-03 17:46 ` Nathan Chancellor
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).