[06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface for SMBIOS tokens

Commit Message

Limonciello, Mario Sept. 21, 2017, 1:57 p.m. UTC

Currently userspace tools can access system tokens via the dcdbas
kernel module and a SMI call that will cause the platform to execute
SMM code.

With a goal in mind of deprecating the dcdbas kernel module a different
method for accessing these tokens from userspace needs to be created.

This is intentionally marked to only be readable as root as it can
contain sensitive information about the platform's configuration.

Signed-off-by: Mario Limonciello <[e-mail]>
---
 .../ABI/testing/sysfs-platform-dell-wmi-smbios     | 16 +++++++++
 drivers/platform/x86/dell-wmi-smbios.c             | 38 ++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios

Comments

Pali Rohár Sept. 25, 2017, 4:23 p.m. UTC | #1

On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote:
> Currently userspace tools can access system tokens via the dcdbas
> kernel module and a SMI call that will cause the platform to execute
> SMM code.
> 
> With a goal in mind of deprecating the dcdbas kernel module a different
> method for accessing these tokens from userspace needs to be created.
> 
> This is intentionally marked to only be readable as root as it can
> contain sensitive information about the platform's configuration.

Darren, Andy, any comments? I'm not quite sure if such API is suitable
for long term in kernel.

Basically tokens are list of tuples <id, location, value> with
possibility to active them, right?

Does not kernel have some better API for it?

Also, keep in mind security aspect of tokens. They represent e.g. boot
order priority or enable/disable some machine peripheral.

> Signed-off-by: Mario Limonciello <[e-mail]>
> ---
>  .../ABI/testing/sysfs-platform-dell-wmi-smbios     | 16 +++++++++
>  drivers/platform/x86/dell-wmi-smbios.c             | 38 ++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios b/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios
> new file mode 100644
> index 000000000000..6d151f2dffba
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios
> @@ -0,0 +1,16 @@
> +What:		/sys/devices/platform/<platform>/tokens
> +Date:		October 2017
> +KernelVersion:	4.15
> +Contact:	"Mario Limonciello" <[e-mail]>
> +Description:
> +		A read-only description of Dell platform tokens
> +		available on the machine.
> +
> +		The tokens will be displayed in the following
> +		machine readable format with each token on a
> +		new line:
> +
> +		ID @ Location : value
> +
> +		For example token:
> +		5 @ 5 : 3
> diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
> index 7f896701fb7b..c3701fdadf7b 100644
> --- a/drivers/platform/x86/dell-wmi-smbios.c
> +++ b/drivers/platform/x86/dell-wmi-smbios.c
> @@ -189,6 +189,34 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
>  	}
>  }
>  
> +static ssize_t tokens_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	size_t off = 0;
> +	int i;
> +
> +	for (i = 0; i < da_num_tokens; i++) {
> +		if (off > PAGE_SIZE)
> +			break;
> +		off += scnprintf(buf+off, PAGE_SIZE-off, "%x @ %x : %x\n",
> +		da_tokens[i].tokenID, da_tokens[i].location,
> +		da_tokens[i].value);
> +	}
> +
> +	return off;
> +}
> +
> +DEVICE_ATTR(tokens, 0400, tokens_show, NULL);
> +
> +static struct attribute *smbios_attrs[] = {
> +	&dev_attr_tokens.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group smbios_attribute_group = {
> +	.attrs = smbios_attrs,
> +};
> +
>  static int dell_wmi_smbios_probe(struct wmi_device *wdev)
>  {
>  	int ret;
> @@ -206,8 +234,16 @@ static int dell_wmi_smbios_probe(struct wmi_device *wdev)
>  		goto fail_buffer;
>  	}
>  
> +	ret = sysfs_create_group(&wdev->dev.kobj, &smbios_attribute_group);
> +	if (ret)
> +		goto fail_create_group;
> +	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
> +
>  	return 0;
>  
> +fail_create_group:
> +	free_page((unsigned long)buffer);
> +
>  fail_buffer:
>  	kfree(da_tokens);
>  	return ret;
> @@ -215,8 +251,10 @@ static int dell_wmi_smbios_probe(struct wmi_device *wdev)
>  
>  static int dell_wmi_smbios_remove(struct wmi_device *wdev)
>  {
> +	sysfs_remove_group(&wdev->dev.kobj, &smbios_attribute_group);
>  	kfree(da_tokens);
>  	free_page((unsigned long)buffer);
> +	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
>  	return 0;
>  }
>

Andy Shevchenko Sept. 25, 2017, 5:04 p.m. UTC | #2

On Mon, Sep 25, 2017 at 7:23 PM, Pali Rohár <[e-mail]> wrote:
> On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote:
>> Currently userspace tools can access system tokens via the dcdbas
>> kernel module and a SMI call that will cause the platform to execute
>> SMM code.
>>
>> With a goal in mind of deprecating the dcdbas kernel module a different
>> method for accessing these tokens from userspace needs to be created.
>>
>> This is intentionally marked to only be readable as root as it can
>> contain sensitive information about the platform's configuration.
>
> Darren, Andy, any comments? I'm not quite sure if such API is suitable
> for long term in kernel.

I would try to avoid sysfs interfaces for some particular devices.
Besides we are creating a character device. Would it be suitable there?

> Basically tokens are list of tuples <id, location, value> with
> possibility to active them, right?
>
> Does not kernel have some better API for it?

I think the best what kernel may provide is a CSV-like format with or
without title line and different delimiter (TAB/space/etc).

>
> Also, keep in mind security aspect of tokens. They represent e.g. boot
> order priority or enable/disable some machine peripheral.

For IOCTLs we may use capabilities.
In sysfs case we may zero output based on capabilities or some other factors.

Limonciello, Mario Sept. 25, 2017, 5:31 p.m. UTC | #3

> -----Original Message-----

> From: Andy Shevchenko [mailto:[e-mail]]

> Sent: Monday, September 25, 2017 1:04 PM

> To: Pali Rohár <[e-mail]>

> Cc: Limonciello, Mario <[e-mail]>; [e-mail];

> LKML <[e-mail]>; Platform Driver <platform-driver-

> [e-mail]>; [e-mail]

> Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface

> for SMBIOS tokens

> 

> On Mon, Sep 25, 2017 at 7:23 PM, Pali Rohár <[e-mail]> wrote:

> > On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote:

> >> Currently userspace tools can access system tokens via the dcdbas

> >> kernel module and a SMI call that will cause the platform to execute

> >> SMM code.

> >>

> >> With a goal in mind of deprecating the dcdbas kernel module a different

> >> method for accessing these tokens from userspace needs to be created.

> >>

> >> This is intentionally marked to only be readable as root as it can

> >> contain sensitive information about the platform's configuration.

> >

> > Darren, Andy, any comments? I'm not quite sure if such API is suitable

> > for long term in kernel.

> 

> I would try to avoid sysfs interfaces for some particular devices.

> Besides we are creating a character device. Would it be suitable there?


If the character device having 2 different ioctls for different needs is
acceptable I'm happy to adjust the series to do this instead.

> 

> > Basically tokens are list of tuples <id, location, value> with

> > possibility to active them, right?

> >


I didn't add a way to activate them through this, it was only for
reading purpose.  Activating them should be possible through the
SMBIOS calling interface though.

> > Does not kernel have some better API for it?

> 

> I think the best what kernel may provide is a CSV-like format with or

> without title line and different delimiter (TAB/space/etc).

> 

> >

> > Also, keep in mind security aspect of tokens. They represent e.g. boot

> > order priority or enable/disable some machine peripheral.

> 

> For IOCTLs we may use capabilities.

> In sysfs case we may zero output based on capabilities or some other factors.

> 


Can you recommend what capabilities you would prefer to see this based upon?

Darren Hart Sept. 27, 2017, 4:50 p.m. UTC | #4

On Mon, Sep 25, 2017 at 05:31:05PM +0000, [e-mail] wrote:
> > -----Original Message-----
> > From: Andy Shevchenko [mailto:[e-mail]]
> > Sent: Monday, September 25, 2017 1:04 PM
> > To: Pali Rohár <[e-mail]>
> > Cc: Limonciello, Mario <[e-mail]>; [e-mail];
> > LKML <[e-mail]>; Platform Driver <platform-driver-
> > [e-mail]>; [e-mail]
> > Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface
> > for SMBIOS tokens
> > 
> > On Mon, Sep 25, 2017 at 7:23 PM, Pali Rohár <[e-mail]> wrote:
> > > On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote:
> > >> Currently userspace tools can access system tokens via the dcdbas
> > >> kernel module and a SMI call that will cause the platform to execute
> > >> SMM code.
> > >>
> > >> With a goal in mind of deprecating the dcdbas kernel module a different
> > >> method for accessing these tokens from userspace needs to be created.
> > >>
> > >> This is intentionally marked to only be readable as root as it can
> > >> contain sensitive information about the platform's configuration.
> > >
> > > Darren, Andy, any comments? I'm not quite sure if such API is suitable
> > > for long term in kernel.
> > 
> > I would try to avoid sysfs interfaces for some particular devices.
> > Besides we are creating a character device. Would it be suitable there?
> 
> If the character device having 2 different ioctls for different needs is
> acceptable I'm happy to adjust the series to do this instead.

One piece of feedback I had re the char device was to see if we could avoid the
need for the IOCTL altogether, I'd like to have that discussion before we add
another.

> 
> > 
> > > Basically tokens are list of tuples <id, location, value> with
> > > possibility to active them, right?
> > >
> 
> I didn't add a way to activate them through this, it was only for
> reading purpose.  Activating them should be possible through the
> SMBIOS calling interface though.
> 

These are read-only as I understood it, and only with the right privileges.
Sysfs seemed appropriate for this to me.

Limonciello, Mario Sept. 27, 2017, 6:27 p.m. UTC | #5

> -----Original Message-----
> From: Darren Hart [mailto:[e-mail]]
> Sent: Wednesday, September 27, 2017 12:51 PM
> To: Limonciello, Mario <[e-mail]>
> Cc: [e-mail]; [e-mail]; linux-
> [e-mail]; [e-mail];
> [e-mail]
> Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface
> for SMBIOS tokens
> 
> On Mon, Sep 25, 2017 at 05:31:05PM +0000, [e-mail] wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko [mailto:[e-mail]]
> > > Sent: Monday, September 25, 2017 1:04 PM
> > > To: Pali Rohár <[e-mail]>
> > > Cc: Limonciello, Mario <[e-mail]>; [e-mail];
> > > LKML <[e-mail]>; Platform Driver <platform-driver-
> > > [e-mail]>; [e-mail]
> > > Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs
> interface
> > > for SMBIOS tokens
> > >
> > > On Mon, Sep 25, 2017 at 7:23 PM, Pali Rohár <[e-mail]> wrote:
> > > > On Thursday 21 September 2017 08:57:11 Mario Limonciello wrote:
> > > >> Currently userspace tools can access system tokens via the dcdbas
> > > >> kernel module and a SMI call that will cause the platform to execute
> > > >> SMM code.
> > > >>
> > > >> With a goal in mind of deprecating the dcdbas kernel module a different
> > > >> method for accessing these tokens from userspace needs to be created.
> > > >>
> > > >> This is intentionally marked to only be readable as root as it can
> > > >> contain sensitive information about the platform's configuration.
> > > >
> > > > Darren, Andy, any comments? I'm not quite sure if such API is suitable
> > > > for long term in kernel.
> > >
> > > I would try to avoid sysfs interfaces for some particular devices.
> > > Besides we are creating a character device. Would it be suitable there?
> >
> > If the character device having 2 different ioctls for different needs is
> > acceptable I'm happy to adjust the series to do this instead.
> 
> One piece of feedback I had re the char device was to see if we could avoid the
> need for the IOCTL altogether, I'd like to have that discussion before we add
> another.

My original design was sysfs files for everything but it was raised by several folks
that you run into the potential of two userspace processes stomping on each
other's data when they run the ACPI call.  That's why I need to have a mutex to
protect and make sure that userspace calls get the right results.

> 
> >
> > >
> > > > Basically tokens are list of tuples <id, location, value> with
> > > > possibility to active them, right?
> > > >
> >
> > I didn't add a way to activate them through this, it was only for
> > reading purpose.  Activating them should be possible through the
> > SMBIOS calling interface though.
> >
> 
> These are read-only as I understood it, and only with the right privileges.
> Sysfs seemed appropriate for this to me.

Andy S was against having this data as another sysfs file.   From a userspace
perspective I think it's simpler to just parse a sysfs file with read only static
data as root.  With the current ioctl based solution it requires userspace to run 
an ioctl to determine how many tokens exist, then allocate a chunk of memory 
big enough to hold all the token data and then run another ioctl to get all the tokens.

Andy S, given this change between v1 and v2 what do you feel is better?

Andy Shevchenko Sept. 27, 2017, 6:31 p.m. UTC | #6

On Wed, Sep 27, 2017 at 9:27 PM,  <[e-mail]> wrote:

>> > > > Darren, Andy, any comments? I'm not quite sure if such API is suitable
>> > > > for long term in kernel.
>> > >
>> > > I would try to avoid sysfs interfaces for some particular devices.
>> > > Besides we are creating a character device. Would it be suitable there?
>> >
>> > If the character device having 2 different ioctls for different needs is
>> > acceptable I'm happy to adjust the series to do this instead.
>>
>> One piece of feedback I had re the char device was to see if we could avoid the
>> need for the IOCTL altogether, I'd like to have that discussion before we add
>> another.
>
> My original design was sysfs files for everything but it was raised by several folks
> that you run into the potential of two userspace processes stomping on each
> other's data when they run the ACPI call.  That's why I need to have a mutex to
> protect and make sure that userspace calls get the right results.
>

>> > > > Basically tokens are list of tuples <id, location, value> with
>> > > > possibility to active them, right?
>> > > >
>> >
>> > I didn't add a way to activate them through this, it was only for
>> > reading purpose.  Activating them should be possible through the
>> > SMBIOS calling interface though.
>> >
>>
>> These are read-only as I understood it, and only with the right privileges.
>> Sysfs seemed appropriate for this to me.
>
> Andy S was against having this data as another sysfs file.   From a userspace
> perspective I think it's simpler to just parse a sysfs file with read only static
> data as root.  With the current ioctl based solution it requires userspace to run
> an ioctl to determine how many tokens exist, then allocate a chunk of memory
> big enough to hold all the token data and then run another ioctl to get all the tokens.
>
> Andy S, given this change between v1 and v2 what do you feel is better?

I have no strong opinion on this. That's why I recommended to listen to Andy L.

Darren Hart Sept. 27, 2017, 6:55 p.m. UTC | #7

On Wed, Sep 27, 2017 at 09:31:47PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 27, 2017 at 9:27 PM,  <[e-mail]> wrote:
> 
> >> > > > Darren, Andy, any comments? I'm not quite sure if such API is suitable
> >> > > > for long term in kernel.
> >> > >
> >> > > I would try to avoid sysfs interfaces for some particular devices.
> >> > > Besides we are creating a character device. Would it be suitable there?
> >> >
> >> > If the character device having 2 different ioctls for different needs is
> >> > acceptable I'm happy to adjust the series to do this instead.
> >>
> >> One piece of feedback I had re the char device was to see if we could avoid the
> >> need for the IOCTL altogether, I'd like to have that discussion before we add
> >> another.
> >
> > My original design was sysfs files for everything but it was raised by several folks
> > that you run into the potential of two userspace processes stomping on each
> > other's data when they run the ACPI call.  That's why I need to have a mutex to
> > protect and make sure that userspace calls get the right results.
> >
> 
> >> > > > Basically tokens are list of tuples <id, location, value> with
> >> > > > possibility to active them, right?
> >> > > >
> >> >
> >> > I didn't add a way to activate them through this, it was only for
> >> > reading purpose.  Activating them should be possible through the
> >> > SMBIOS calling interface though.
> >> >
> >>
> >> These are read-only as I understood it, and only with the right privileges.
> >> Sysfs seemed appropriate for this to me.
> >
> > Andy S was against having this data as another sysfs file.   From a userspace
> > perspective I think it's simpler to just parse a sysfs file with read only static
> > data as root.  With the current ioctl based solution it requires userspace to run
> > an ioctl to determine how many tokens exist, then allocate a chunk of memory
> > big enough to hold all the token data and then run another ioctl to get all the tokens.
> >
> > Andy S, given this change between v1 and v2 what do you feel is better?
> 
> I have no strong opinion on this. That's why I recommended to listen to Andy L.

+Andy Lutomirski

Andy L, any preference on your part regarding exporting these tokens via sysfs
or through an additional IOCTL in the chardev?

Andy Lutomirski Sept. 27, 2017, 7:49 p.m. UTC | #8

On 09/27/2017 11:55 AM, Darren Hart wrote:
> On Wed, Sep 27, 2017 at 09:31:47PM +0300, Andy Shevchenko wrote:
>> On Wed, Sep 27, 2017 at 9:27 PM,  <[e-mail]> wrote:
>>
>>>>>>> Darren, Andy, any comments? I'm not quite sure if such API is suitable
>>>>>>> for long term in kernel.
>>>>>>
>>>>>> I would try to avoid sysfs interfaces for some particular devices.
>>>>>> Besides we are creating a character device. Would it be suitable there?
>>>>>
>>>>> If the character device having 2 different ioctls for different needs is
>>>>> acceptable I'm happy to adjust the series to do this instead.
>>>>
>>>> One piece of feedback I had re the char device was to see if we could avoid the
>>>> need for the IOCTL altogether, I'd like to have that discussion before we add
>>>> another.
>>>
>>> My original design was sysfs files for everything but it was raised by several folks
>>> that you run into the potential of two userspace processes stomping on each
>>> other's data when they run the ACPI call.  That's why I need to have a mutex to
>>> protect and make sure that userspace calls get the right results.
>>>
>>
>>>>>>> Basically tokens are list of tuples <id, location, value> with
>>>>>>> possibility to active them, right?
>>>>>>>
>>>>>
>>>>> I didn't add a way to activate them through this, it was only for
>>>>> reading purpose.  Activating them should be possible through the
>>>>> SMBIOS calling interface though.
>>>>>
>>>>
>>>> These are read-only as I understood it, and only with the right privileges.
>>>> Sysfs seemed appropriate for this to me.
>>>
>>> Andy S was against having this data as another sysfs file.   From a userspace
>>> perspective I think it's simpler to just parse a sysfs file with read only static
>>> data as root.  With the current ioctl based solution it requires userspace to run
>>> an ioctl to determine how many tokens exist, then allocate a chunk of memory
>>> big enough to hold all the token data and then run another ioctl to get all the tokens.
>>>
>>> Andy S, given this change between v1 and v2 what do you feel is better?
>>
>> I have no strong opinion on this. That's why I recommended to listen to Andy L.
> 
> +Andy Lutomirski
> 
> Andy L, any preference on your part regarding exporting these tokens via sysfs
> or through an additional IOCTL in the chardev?
> 

Not really.  If this is indeed static data that is potentially useful 
for scripts and such, than sysfs is kind of nice.

Limonciello, Mario Sept. 27, 2017, 7:50 p.m. UTC | #9

> -----Original Message-----

> From: Andy Lutomirski [mailto:[e-mail]]

> Sent: Wednesday, September 27, 2017 3:49 PM

> To: Darren Hart <[e-mail]>; Andy Shevchenko

> <[e-mail]>; Andy Lutomirski <[e-mail]>

> Cc: Limonciello, Mario <[e-mail]>; Pali Rohár

> <[e-mail]>; [e-mail]; Platform Driver <platform-

> [e-mail]>; [e-mail]

> Subject: Re: [PATCH 06/12] platform/x86: dell-wmi-smbios: Add a sysfs interface

> for SMBIOS tokens

> 

> On 09/27/2017 11:55 AM, Darren Hart wrote:

> > On Wed, Sep 27, 2017 at 09:31:47PM +0300, Andy Shevchenko wrote:

> >> On Wed, Sep 27, 2017 at 9:27 PM,  <[e-mail]> wrote:

> >>

> >>>>>>> Darren, Andy, any comments? I'm not quite sure if such API is suitable

> >>>>>>> for long term in kernel.

> >>>>>>

> >>>>>> I would try to avoid sysfs interfaces for some particular devices.

> >>>>>> Besides we are creating a character device. Would it be suitable there?

> >>>>>

> >>>>> If the character device having 2 different ioctls for different needs is

> >>>>> acceptable I'm happy to adjust the series to do this instead.

> >>>>

> >>>> One piece of feedback I had re the char device was to see if we could avoid the

> >>>> need for the IOCTL altogether, I'd like to have that discussion before we add

> >>>> another.

> >>>

> >>> My original design was sysfs files for everything but it was raised by several

> folks

> >>> that you run into the potential of two userspace processes stomping on each

> >>> other's data when they run the ACPI call.  That's why I need to have a mutex to

> >>> protect and make sure that userspace calls get the right results.

> >>>

> >>

> >>>>>>> Basically tokens are list of tuples <id, location, value> with

> >>>>>>> possibility to active them, right?

> >>>>>>>

> >>>>>

> >>>>> I didn't add a way to activate them through this, it was only for

> >>>>> reading purpose.  Activating them should be possible through the

> >>>>> SMBIOS calling interface though.

> >>>>>

> >>>>

> >>>> These are read-only as I understood it, and only with the right privileges.

> >>>> Sysfs seemed appropriate for this to me.

> >>>

> >>> Andy S was against having this data as another sysfs file.   From a userspace

> >>> perspective I think it's simpler to just parse a sysfs file with read only static

> >>> data as root.  With the current ioctl based solution it requires userspace to run

> >>> an ioctl to determine how many tokens exist, then allocate a chunk of memory

> >>> big enough to hold all the token data and then run another ioctl to get all the

> tokens.

> >>>

> >>> Andy S, given this change between v1 and v2 what do you feel is better?

> >>

> >> I have no strong opinion on this. That's why I recommended to listen to Andy L.

> >

> > +Andy Lutomirski

> >

> > Andy L, any preference on your part regarding exporting these tokens via sysfs

> > or through an additional IOCTL in the chardev?

> >

> 

> Not really.  If this is indeed static data that is potentially useful

> for scripts and such, than sysfs is kind of nice.


OK thanks.  I'll switch back to sysfs for v3.

Patch

diff --git a/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios b/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios
new file mode 100644
index 000000000000..6d151f2dffba
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-dell-wmi-smbios
@@ -0,0 +1,16 @@ 
+What:		/sys/devices/platform/<platform>/tokens
+Date:		October 2017
+KernelVersion:	4.15
+Contact:	"Mario Limonciello" <[e-mail]>
+Description:
+		A read-only description of Dell platform tokens
+		available on the machine.
+
+		The tokens will be displayed in the following
+		machine readable format with each token on a
+		new line:
+
+		ID @ Location : value
+
+		For example token:
+		5 @ 5 : 3
diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index 7f896701fb7b..c3701fdadf7b 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -189,6 +189,34 @@  static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 	}
 }
 
+static ssize_t tokens_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
+{
+	size_t off = 0;
+	int i;
+
+	for (i = 0; i < da_num_tokens; i++) {
+		if (off > PAGE_SIZE)
+			break;
+		off += scnprintf(buf+off, PAGE_SIZE-off, "%x @ %x : %x\n",
+		da_tokens[i].tokenID, da_tokens[i].location,
+		da_tokens[i].value);
+	}
+
+	return off;
+}
+
+DEVICE_ATTR(tokens, 0400, tokens_show, NULL);
+
+static struct attribute *smbios_attrs[] = {
+	&dev_attr_tokens.attr,
+	NULL
+};
+
+static const struct attribute_group smbios_attribute_group = {
+	.attrs = smbios_attrs,
+};
+
 static int dell_wmi_smbios_probe(struct wmi_device *wdev)
 {
 	int ret;
@@ -206,8 +234,16 @@  static int dell_wmi_smbios_probe(struct wmi_device *wdev)
 		goto fail_buffer;
 	}
 
+	ret = sysfs_create_group(&wdev->dev.kobj, &smbios_attribute_group);
+	if (ret)
+		goto fail_create_group;
+	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
+
 	return 0;
 
+fail_create_group:
+	free_page((unsigned long)buffer);
+
 fail_buffer:
 	kfree(da_tokens);
 	return ret;
@@ -215,8 +251,10 @@  static int dell_wmi_smbios_probe(struct wmi_device *wdev)
 
 static int dell_wmi_smbios_remove(struct wmi_device *wdev)
 {
+	sysfs_remove_group(&wdev->dev.kobj, &smbios_attribute_group);
 	kfree(da_tokens);
 	free_page((unsigned long)buffer);
+	kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
 	return 0;
 }