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-samitolva...@google.com/
> > > 
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> > > Signed-off-by: Nathan Chancellor <nat...@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 &gt->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.ha...@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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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(&gt->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 = &gt->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(&gt->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(&gt->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 = &gt->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 = &gt->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(&gt->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(&gt->i915->drm,
> > >                            "failed to create gt%u punit_req_freq_mhz 
> > > sysfs (%pe)",
> > > 
> > > base-commit: 783f6f852cc061e59962e53aa9824aa785de0d8c
> > 
> 

Reply via email to