RE: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace

[Mario.Limonciello]

> -----Original Message-----
> From: Pali RohÃr [mailto:pali.rohar@xxxxxxxxx]
> Sent: Tuesday, September 26, 2017 4:10 PM
> To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>
> Cc: dvhart@xxxxxxxxxxxxx; Andy Shevchenko <andy.shevchenko@xxxxxxxxx>;
> LKML <linux-kernel@xxxxxxxxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx;
> quasisec@xxxxxxxxxx
> Subject: Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character
> device for userspace
> 
> On Tuesday 26 September 2017 20:50:10 Mario Limonciello wrote:
> > This userspace character device will be used to perform SMBIOS calls
> > from any applications.
> >
> > It contains 3 operating IOCTL's
> > 1) sending a properly formatted 4k calling interface buffer.
> > 2) Querying number of tokens in the given system
> > 3) Copying the status of all these tokens to a properly formatted
> >    userspace buffer.
> >
> > This character device is intended to deprecate the dcdbas kernel
> > module and the interface that it provides to userspace.
> >
> > It's important for the driver to provide a R/W ioctl to ensure that
> > two competing userspace processes don't race to provide or read each
> > others data.
> >
> > The character device will only be created if the WMI interface was
> > found.
> >
> > The API for interacting with this interface is defined in
> > documentation as well as a uapi header provides the format of the
> > structures.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxxx>
> > ---
> >  Documentation/ABI/testing/dell-wmi-smbios |  10 +++
> >  drivers/platform/x86/dell-wmi-smbios.c    | 110
> > +++++++++++++++++++++++++++---
> > drivers/platform/x86/dell-wmi-smbios.h    |  28 +-------
> >  include/uapi/linux/dell-wmi-smbios.h      |  55 +++++++++++++++
> >  4 files changed, 165 insertions(+), 38 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > diff --git a/Documentation/ABI/testing/dell-wmi-smbios
> > b/Documentation/ABI/testing/dell-wmi-smbios new file mode 100644
> > index 000000000000..8ec98e9b7b34
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/dell-wmi-smbios
> > @@ -0,0 +1,10 @@
> > +What:		/dev/wmi-dell-wmi-smbios
> > +Date:		October 2017
> > +KernelVersion:	4.15
> > +Contact:	"Mario Limonciello" <mario.limonciello@xxxxxxxx>
> > +Description:
> > +		Perform SMBIOS calls on supported Dell machines.
> > +		through the Dell ACPI-WMI interface.
> > +
> > +		IOCTL's and buffer formats are defined in:
> > +		<uapi/linux/dell-wmi-smbios.h>
> 
> Should be "wmi" part of device name, even if whole driver work without
> WMI?

Well actually the driver won't create a character device unless WMI is
used.  Exposing a character device for non-WMI is just duplicating dcdbas
at another endpoint without any security advantage.

> 
> I think that "wmi" should be part of device name only if it is related
> to WMI bus functions, and not in case when it exports some vendor (Dell)
> specific API which may be implemented via WMI.
> 
> E.g. we have /dev/i2c-* for raw access to i2c devices. But we do not
> have /dev/i2c-something for e.g. RTC device connected via i2c...

I see what you mean here, but I think we need Darren and Andy to weigh
in here.  

The character device does use WMI bus methods (wmi_evaluate_method).
The primary reason to create with wmi in the name is to show that the character 
devices are supposed to be created and managed by the WMI bus.  Vendor
drivers should not create their "own".  In my mind this will be especially relevant
once WMI bus can interpret MOF and create a set of mappings for attributes
defined in MOF.