[PATCH v4 05/13] drivers: create binary sysfs for class

[Greg Kroah-Hartman]

On Mon, Apr 15, 2019 at 10:44:12PM +0530, Ramalingam C wrote:
> On 2019-04-15 at 16:47:16 +0200, Greg Kroah-Hartman wrote:
> > On Mon, Apr 15, 2019 at 06:11:13PM +0530, Ramalingam C wrote:
> > > On 2019-04-05 at 14:32:00 +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Apr 05, 2019 at 04:06:22PM +0530, Ramalingam C wrote:
> > > > > On 2019-04-05 at 11:23:00 +0200, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Apr 05, 2019 at 02:12:54PM +0530, Ramalingam C wrote:
> > > > > > > Functions to create and remove the binary sysfs for class are added.
> > > > > > > 
> > > > > > > These are getting introduced as DRM wants to create the common binary
> > > > > > > sysfs across the drm subsystem to handle hdcp srm.
> > > > > > 
> > > > > > Why do you need individual files?  That's almost always a sign that you
> > > > > > are going to race with userspace in a bad way.  Why not just use an
> > > > > > attribute group which provides automatic support for this?
> > > > > Greg,
> > > > > 
> > > > > Reason behind this move is to have a common srm entry path across all drm
> > > > > drivers. And the data fed into this is binary blob. So I am creating a
> > > > > binary sysfs "hdcp_srm" at /sys/class/drm/
> > > > 
> > > > Ah, you want to have a file in your class directory, not your class
> > > > device directory.
> > > > 
> > > > No, please do not do that.  There's a reason I got rid of those same
> > > > types of apis in the past.
> > > > 
> > > > And "binary blobs" are horrid anyway, they are only to be used as a
> > > > pass-through to the device itself, from the kernel, no touching the data
> > > > at all.  If you really need/want this, then put it in the device's
> > > > directory as that is where the data is going to, not the kernel "class"
> > > > code as it sure as heck better not be doing anything with it.
> > > Greg,
> > > 
> > > But here the parsing of the received binary blob is done outside the drm
> > > device/cards. This will be generic code for all drm cardx(drivers). And
> > > this will provide the service helper functions to the drm drivers for HDCP SRM checking.
> > 
> > Again, the kernel is NOT to be parsing any binary data that comes
> > through a sysfs file.  If you need such a crazy thing, do it through
> > your normal drm ioctl.
> > 
> > > So we prefer to have the binary sysfs at /sys/class/drm/ than inside the
> > > cardx folders, so that it will be generic. Could you please suggest a way to achieve that?
> > 
> > What is a "cardx" driver? 
> Meant card0 card1 etc.. Drm drivers.
> > Why can you not do it in a device-specific
> > location?  Are suddenly _ALL_ DRM drivers going to need this
> > information?  What is the use case?  Who is going to be providing this
> > blob and where is it going?  What in the kernel uses it?  What on the
> > hardware uses it?  What is it actually?
> 
> This is for HDCP usecase. List of compromised receivers' ID  called system
> renewability Message (SRM) will be available at userspace which needs to
> be passed to be kernel. And this data can be parsed at kernel used
> across all DRM drivers to implement the HDCP authentication. Hence we
> want generic code to parse the SRM data and provide a helper function to
> all DRM drivers to validate their HDCP sink's ID.
> 
> To achieve this we want to keep the sysfs and parsing logic outside the
> drm drivers at the class level.
> 
> At present I915 will be using these implementation. in future other DRM
> drivers can use the same.

Again, binary sysfs files are not for any data that the kernel has to
parse at all.  So do not use sysfs for this.

thanks,

greg k-h