> -----Original Message----- > From: Greg KH [mailto:greg@xxxxxxxxx] > Sent: Saturday, October 7, 2017 2:35 AM > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx> > Cc: dvhart@xxxxxxxxxxxxx; Andy Shevchenko <andy.shevchenko@xxxxxxxxx>; > LKML <linux-kernel@xxxxxxxxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx; > Andy Lutomirski <luto@xxxxxxxxxx>; quasisec@xxxxxxxxxx; > pali.rohar@xxxxxxxxx; rjw@xxxxxxxxxxxxx; mjg59@xxxxxxxxxx; hch@xxxxxx > Subject: Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when > requested by drivers > > On Fri, Oct 06, 2017 at 11:59:57PM -0500, Mario Limonciello wrote: > > For WMI operations that are only Set or Query read or write sysfs > > attributes created by WMI vendor drivers make sense. > > > > For other WMI operations that are run on Method, there needs to be a > > way to guarantee to userspace that the results from the method call > > belong to the data request to the method call. Sysfs attributes don't > > work well in this scenario because two userspace processes may be > > competing at reading/writing an attribute and step on each other's > > data. > > > > When a WMI vendor driver declares an ioctl callback in the wmi_driver > > the WMI bus driver will create a character device that maps to that > > function. > > > > That character device will correspond to this path: > > /dev/wmi/$driver > > > > The WMI bus driver will interpret the IOCTL calls, test them for > > a valid instance and pass them on to the vendor driver to run. > > > > This creates an implicit policy that only driver per character > > device. If a module matches multiple GUID's, the wmi_devices > > will need to be all handled by the same wmi_driver if the same > > character device is used. > > > > The WMI vendor drivers will be responsible for managing access to > > this character device and proper locking on it. > > > > When a WMI vendor driver is unloaded the WMI bus driver will clean > > up the character device. > > What prevents the vendor driver from being unloaded while the ioctl is > being called in it? I don't see any protection here from that at all :( > In my driver I take a mutex that blocks unloading while running ioctl, but you mean you want one in the bus driver more generally, OK. > > +static long wmi_unlocked_ioctl(struct file *filp, unsigned int cmd, > > + unsigned long arg) > > +{ > > + return match_ioctl(filp, cmd, arg, 0); > > +} > > + > > +static long wmi_compat_ioctl(struct file *filp, unsigned int cmd, > > + unsigned long arg) > > +{ > > + return match_ioctl(filp, cmd, arg, 1); > > +} > > Why a compat ioctl at all? That's for older interfaces, not for brand > new ones where you design the ioctl structures "correctly" to work on > both 32 and 64 bits at the same time. That should not be needed at all. > > thanks, > > greg k-h I was trying to make sure that any other future vendor drivers had it for an option. I wasn't aware that new code shouldn't use it. OK, I'll remove it.
