Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1740118 > unrolled thread

[PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 06/14] platform/x86: wmi: Cleanup exit routine in reverse order of init Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to dell-wmi-smbios Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:10 +0200
        RE: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios <Mario.Limonciello@dell.com> - 2017-09-26 22:20 +0200
          Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios Darren Hart <dvhart@infradead.org> - 2017-09-28 00:40 +0200
      Re: [PATCH v2 09/14] platform/x86: dell-smbios: rename to  dell-wmi-smbios kbuild test robot <lkp@intel.com> - 2017-09-29 17:40 +0200
    [PATCH v2 07/14] platform/x86: wmi: destroy on cleanup rather than unregister Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce  character device for userspace Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-26 21:10 +0200
      RE: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce  character device for userspace <Mario.Limonciello@dell.com> - 2017-09-26 22:20 +0200
      Re: [PATCH v2 12/14] platform/x86: dell-wmi-smbios: introduce character device for userspace Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:20 +0200
    [PATCH v2 10/14] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI interface Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 08/14] platform/x86: dell-smbios: Introduce a WMI-ACPI  interface Darren Hart <dvhart@infradead.org> - 2017-09-28 00:20 +0200
    [PATCH v2 11/14] platform/x86: wmi: create character devices when requested by drivers Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 01/14] platform/x86: dell-wmi: label driver as handling notifications Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 13/14] platform/x86: Kconfig: Change the default settings for dell-wmi-smbios Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias Darren Hart <dvhart@infradead.org> - 2017-09-27 19:30 +0200
        RE: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias <Mario.Limonciello@dell.com> - 2017-09-27 19:40 +0200
          Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on  descriptor GUID modalias Darren Hart <dvhart@infradead.org> - 2017-09-27 19:50 +0200
    [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check Pali Rohár <pali.rohar@gmail.com> - 2017-09-26 22:20 +0200
        RE: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi  descriptor check <Mario.Limonciello@dell.com> - 2017-09-26 22:30 +0200
    [PATCH v2 04/14] platform/x86: dell-smbios: Add pr_fmt definition to driver Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 05/14] platform/x86: wmi: sort include list Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
    [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes Mario Limonciello <mario.limonciello@dell.com> - 2017-09-26 21:00 +0200
      Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes Darren Hart <dvhart@infradead.org> - 2017-09-27 19:50 +0200
        RE: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes <Mario.Limonciello@dell.com> - 2017-09-27 19:50 +0200
        Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless  includes Darren Hart <dvhart@infradead.org> - 2017-09-27 20:00 +0200
    Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-09-26 21:10 +0200
      RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-26 21:20 +0200
        Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Darren Hart <dvhart@infradead.org> - 2017-09-27 19:20 +0200
          RE: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI <Mario.Limonciello@dell.com> - 2017-09-27 19:40 +0200
            Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI Darren Hart <dvhart@infradead.org> - 2017-09-27 19:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1740916 — RE: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:22 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor
> GUID modalias
> 
> On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> > The descriptor GUID is not used to indicate that WMI notifications
> > in the dell-wmi driver work properly.  As such a modalias should
> > not be present that causes this driver to load on systems with this
> > GUID.
> >
> > Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> I can do this manually, but for future reference:
> 
> Nit, in general, the author's sign off should be first, followed by
> reviewers and testers. The maintainer will add their sign off at the end
> - this keeps things clear regarding the development and delivery path.
> In general, assume chronological order and you'll have the right idea.
> 
> Informational tags, like Reported, Suggested, Tested, and Fixes can come
> before the Author sign off, which is consistent with the chronological
> order.
> 

Operationally is there is a flag I'm missing in git-format-patch that does
this?  git format-patch -s puts my sign off at the very bottom, so this is a
manual action to re-order if I'm adding in Reviewed-by: in the patch
description.

[toc] | [prev] | [next] | [standalone]

#1740923 — Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor GUID modalias

On Wed, Sep 27, 2017 at 05:30:40PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Wednesday, September 27, 2017 1:22 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> > kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> > quasisec@google.com; pali.rohar@gmail.com
> > Subject: Re: [PATCH v2 03/14] platform/x86: dell-wmi: Don't match on descriptor
> > GUID modalias
> > 
> > On Tue, Sep 26, 2017 at 01:50:01PM -0500, Mario Limonciello wrote:
> > > The descriptor GUID is not used to indicate that WMI notifications
> > > in the dell-wmi driver work properly.  As such a modalias should
> > > not be present that causes this driver to load on systems with this
> > > GUID.
> > >
> > > Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
> > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > 
> > I can do this manually, but for future reference:
> > 
> > Nit, in general, the author's sign off should be first, followed by
> > reviewers and testers. The maintainer will add their sign off at the end
> > - this keeps things clear regarding the development and delivery path.
> > In general, assume chronological order and you'll have the right idea.
> > 
> > Informational tags, like Reported, Suggested, Tested, and Fixes can come
> > before the Author sign off, which is consistent with the chronological
> > order.
> > 
> 
> Operationally is there is a flag I'm missing in git-format-patch that does
> this?  git format-patch -s puts my sign off at the very bottom, so this is a
> manual action to re-order if I'm adding in Reviewed-by: in the patch
> description.

Yes, this is a manual step. Git will add your sign off for you, but it doesn't
do it contextually, always at the end. Works for the first writing, after that
you have to manage the tags manually. (or at least that is what I do)

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1740131 — [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

Some cases the wrong type was used for errors and checks can be
done more cleanly.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-wmi-smbios.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/dell-wmi-smbios.c b/drivers/platform/x86/dell-wmi-smbios.c
index 232a9dd482b1..290e9853818b 100644
--- a/drivers/platform/x86/dell-wmi-smbios.c
+++ b/drivers/platform/x86/dell-wmi-smbios.c
@@ -340,16 +340,16 @@ int dell_wmi_check_descriptor_buffer(struct wmi_device *wdev, u32 *version)
 	}
 	desc_buffer = (u32 *)obj->buffer.pointer;
 
-	if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] != 0x494D5720)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%*ph)\n",
-			8, desc_buffer);
+	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
+			desc_buffer);
 
 	if (desc_buffer[2] != 0 && desc_buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%d)\n",
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
 			desc_buffer[2]);
 
 	if (desc_buffer[3] != 4096)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%d)\n",
+		dev_warn(&wdev->dev, "Dell descriptor buffer has invalid buffer length (%u)\n",
 			desc_buffer[3]);
 
 	*version = desc_buffer[2];
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1740176 — Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

[Multipart message — attachments visible in raw view] — view raw

On Tuesday 26 September 2017 20:50:12 Mario Limonciello wrote:
> +	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)

Should not be this memcmp? IIRC we want to compare that buffer is equal 
to some bytes, right?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1740189 — RE: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi descriptor check

> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, September 26, 2017 4:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com
> Subject: Re: [PATCH v2 14/14] platform/x86: dell-wmi-smbios: clean up wmi
> descriptor check
> 
> On Tuesday 26 September 2017 20:50:12 Mario Limonciello wrote:
> > +	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0)
> 
> Should not be this memcmp? IIRC we want to compare that buffer is equal
> to some bytes, right?
> 

It used to be comparing the bytes, but Andy S. thought this wasn't very clear.
That's why I adjusted it to compare to the strings that the bytes used instead.

[toc] | [prev] | [next] | [standalone]

#1740132 — [PATCH v2 04/14] platform/x86: dell-smbios: Add pr_fmt definition to driver

pr_fmt provides formatting to be used by the driver when
displaying errors and messages.

Reviewed-by: Pali Pohar <pali.rohar@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 6654ad44a04c..579be67de2a3 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -12,6 +12,7 @@
  *  it under the terms of the GNU General Public License version 2 as
  *  published by the Free Software Foundation.
  */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1740134 — [PATCH v2 05/14] platform/x86: wmi: sort include list

The include list is randomly assembled right now.  Sort in alphabetical
order.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/wmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 0765b1797d4c..e19b074df01d 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -33,17 +33,17 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/types.h>
+#include <linux/acpi.h>
 #include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/list.h>
-#include <linux/acpi.h>
-#include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/wmi.h>
+#include <linux/slab.h>
+#include <linux/types.h>
 #include <linux/uuid.h>
+#include <linux/wmi.h>
 
 ACPI_MODULE_NAME("wmi");
 MODULE_AUTHOR("Carlos Corbacho");
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1740135 — [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

These aren't needed to compile the module.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
---
 drivers/platform/x86/dell-smbios.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 0a5723468bff..6654ad44a04c 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -17,10 +17,7 @@
 #include <linux/module.h>
 #include <linux/dmi.h>
 #include <linux/err.h>
-#include <linux/gfp.h>
 #include <linux/mutex.h>
-#include <linux/slab.h>
-#include <linux/io.h>
 #include "../../firmware/dcdbas.h"
 #include "dell-smbios.h"
 
-- 
2.14.1

[toc] | [prev] | [next] | [standalone]

#1740920 — Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> These aren't needed to compile the module.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

Hrm...

CONFIG_DELL_SMBIOS=m

$ make drivers/platform/x86/dell-smbios.ko

  CC [M]  drivers/platform/x86/dell-smbios.o
drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function ‘krealloc’ [-Werror=implicit-function-declaration]
  new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                  ^~~~~~~~
drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
  new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
                ^
drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration]
  kfree(da_tokens);
  ^~~~~

> ---
>  drivers/platform/x86/dell-smbios.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index 0a5723468bff..6654ad44a04c 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -17,10 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/dmi.h>
>  #include <linux/err.h>
> -#include <linux/gfp.h>
>  #include <linux/mutex.h>
> -#include <linux/slab.h>

Appears you do require slab.h...

> -#include <linux/io.h>
>  #include "../../firmware/dcdbas.h"
>  #include "dell-smbios.h"
>  
> -- 
> 2.14.1



> 
> 

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1740924 — RE: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:40 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>; LKML <linux-
> kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> quasisec@google.com; pali.rohar@gmail.com
> Subject: Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes
> 
> On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> > These aren't needed to compile the module.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Hrm...
> 
> CONFIG_DELL_SMBIOS=m
> 
> $ make drivers/platform/x86/dell-smbios.ko
> 
>   CC [M]  drivers/platform/x86/dell-smbios.o
> drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function
> ‘krealloc’ [-Werror=implicit-function-declaration]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                   ^~~~~~~~
> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer
> from integer without a cast [-Wint-conversion]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                 ^
> drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function
> ‘kfree’ [-Werror=implicit-function-declaration]
>   kfree(da_tokens);
>   ^~~~~
> 

Thanks, I've obviously been testing this and didn't notice it.
Not sure why that wasn't exploding on my end.  
Anyway I'll fix for v3.

> > ---
> >  drivers/platform/x86/dell-smbios.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-
> smbios.c
> > index 0a5723468bff..6654ad44a04c 100644
> > --- a/drivers/platform/x86/dell-smbios.c
> > +++ b/drivers/platform/x86/dell-smbios.c
> > @@ -17,10 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/dmi.h>
> >  #include <linux/err.h>
> > -#include <linux/gfp.h>
> >  #include <linux/mutex.h>
> > -#include <linux/slab.h>
> 
> Appears you do require slab.h...
> 
> > -#include <linux/io.h>
> >  #include "../../firmware/dcdbas.h"
> >  #include "dell-smbios.h"
> >
> > --
> > 2.14.1
> 
> 
> 
> >
> >
> 
> --
> Darren Hart
> VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1740930 — Re: [PATCH v2 02/14] platform/x86: dell-smbios: drop needless includes

On Wed, Sep 27, 2017 at 10:40:19AM -0700, Darren Hart wrote:
> On Tue, Sep 26, 2017 at 01:50:00PM -0500, Mario Limonciello wrote:
> > These aren't needed to compile the module.
> > 
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> 
> Hrm...
> 
> CONFIG_DELL_SMBIOS=m
> 
> $ make drivers/platform/x86/dell-smbios.ko
> 
>   CC [M]  drivers/platform/x86/dell-smbios.o
> drivers/platform/x86/dell-smbios.c: In function ‘parse_da_table’:
> drivers/platform/x86/dell-smbios.c:142:18: error: implicit declaration of function ‘krealloc’ [-Werror=implicit-function-declaration]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                   ^~~~~~~~
> drivers/platform/x86/dell-smbios.c:142:16: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
>   new_da_tokens = krealloc(da_tokens, (da_num_tokens + tokens) *
>                 ^
> drivers/platform/x86/dell-smbios.c: In function ‘dell_smbios_init’:
> drivers/platform/x86/dell-smbios.c:193:2: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration]
>   kfree(da_tokens);
>   ^~~~~
> 
> > ---
> >  drivers/platform/x86/dell-smbios.c | 3 ---
...
> > -#include <linux/slab.h>
> 
> Appears you do require slab.h...
> 

I've dropped this one line from this patch, and build is working again. Patches
1-7 look ready to go to me. I'm pushing these to my review branch from some
additional automated testing.

Let me know if there are any objections for 1-7. If we can get these queued now,
we can simplify the review effort for the major functional changes in 8-14.

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1740140

On Tue, Sep 26, 2017 at 9:49 PM, Mario Limonciello
<mario.limonciello@dell.com> wrote:
> The existing way that the dell-smbios helper module and associated
> other drivers (dell-laptop, dell-wmi) communicate with the platform
> really isn't secure.  It requires creating a buffer in physical
> DMA32 memory space and passing that to the platform via SMM.
>
> Since the platform got a physical memory pointer, you've just got
> to trust that the platform has only modified (and accessed) memory
> within that buffer.
>
> Dell Platform designers recognize this security risk and offer a
> safer way to communicate with the platform over ACPI.  This is
> in turn exposed via a WMI interface to the OS.
>
> When communicating over WMI-ACPI the communication doesn't occur
> with physical memory pointers.  When the ASL is invoked, the fixed
> length ACPI buffer is copied to a small operating region.  The ASL
> will invoke the SMI, and SMM will only have access to this operating
> region.  When the ASL returns the buffer is copied back for the OS
> to process.
>
> This method of communication should also deprecate the usage of the
> dcdbas kernel module and software dependent upon it's interface.
> Instead offer a character device interface for communicating with this
> ASL method to allow userspace to use instead.
>
> To faciliate that this patch series introduces a generic way for WMI
> drivers to be able to create discoverable character devices through
> the WMI bus when desired.
> Requiring WMI drivers to explicitly ask for this functionality will
> act as an effective vendor whitelist to character device creation.
>

Thanks for an update, looks better now.
I'm in the middle of going through it and noticed that patches 6 and 7
sound like a fix to me.
Perhaps better to move it to be first part of the series.

Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
interested in changes like these to WMI.

> changes between v1 and v2:
>  * Introduce another patch to sort the includes in wmi.c
>  * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
>    checks.
>  * Add a commit message to the pr_fmt commit
>  * Introduce includes to wmi.c in proper location
>  * Add Reviewed-by to relevant patches from Pali
>  * Make the WMI introduction patch fallback to legacy SMI
>    if compiled with CONFIG_DCDBAS
>  * Separate format of WMI and SMI buffers.  WMI buffer supports more
>    arguments and data.
>  * Adjust the rename patch for changes to fallback
>  * Drop sysfs token creation patch
>  * Adjust WMI descriptor check patch for changes to fallback
>  * introduce another patch to remove needless includes in dell-smbios.c
>  * Add token ioctl interface to character device.
>    - Can query number of tokens
>    - Can query values in all tokens
>  * Expose format of all buffers and IOCTL commands to uapi header
>  * Drop the read interface from character device.  It doesn't make
>    sense with multiple different ioctl methods.
>  * Default WMI interface to 32k (This would normally be queried via
>    MOF, but that's not possible yet)
>  * Create separate buffers for WMI and SMI.  If WMI is available,
>    free the SMI buffer.
>  * Reorder patches so all fixups come first in the series.
>
> Mario Limonciello (14):
>   platform/x86: dell-wmi: label driver as handling notifications
>   platform/x86: dell-smbios: drop needless includes
>   platform/x86: dell-wmi: Don't match on descriptor GUID modalias
>   platform/x86: dell-smbios: Add pr_fmt definition to driver
>   platform/x86: wmi: sort include list
>   platform/x86: wmi: Cleanup exit routine in reverse order of init
>   platform/x86: wmi: destroy on cleanup rather than unregister
>   platform/x86: dell-smbios: Introduce a WMI-ACPI interface
>   platform/x86: dell-smbios: rename to dell-wmi-smbios
>   platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
>   platform/x86: wmi: create character devices when requested by drivers
>   platform/x86: dell-wmi-smbios: introduce character device for
>     userspace
>   platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
>   platform/x86: dell-wmi-smbios: clean up wmi descriptor check
>
>  Documentation/ABI/testing/dell-wmi-smbios          |  10 +
>  MAINTAINERS                                        |   8 +-
>  drivers/platform/x86/Kconfig                       |  13 +-
>  drivers/platform/x86/Makefile                      |   2 +-
>  drivers/platform/x86/dell-laptop.c                 |   2 +-
>  drivers/platform/x86/dell-smbios.c                 | 213 ---------
>  drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
>  .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
>  drivers/platform/x86/dell-wmi.c                    |  78 +---
>  drivers/platform/x86/wmi.c                         | 116 ++++-
>  include/linux/wmi.h                                |   1 +
>  include/uapi/linux/dell-wmi-smbios.h               |  55 +++
>  12 files changed, 674 insertions(+), 333 deletions(-)
>  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
>  delete mode 100644 drivers/platform/x86/dell-smbios.c
>  create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
>  rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
>  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
>
> --
> 2.14.1
>



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1740145

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com]
> Sent: Tuesday, September 26, 2017 3:06 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; LKML <linux-kernel@vger.kernel.org>; Platform Driver
> <platform-driver-x86@vger.kernel.org>; quasisec@google.com; Pali Rohár
> <pali.rohar@gmail.com>
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 9:49 PM, Mario Limonciello
> <mario.limonciello@dell.com> wrote:
> > The existing way that the dell-smbios helper module and associated
> > other drivers (dell-laptop, dell-wmi) communicate with the platform
> > really isn't secure.  It requires creating a buffer in physical
> > DMA32 memory space and passing that to the platform via SMM.
> >
> > Since the platform got a physical memory pointer, you've just got
> > to trust that the platform has only modified (and accessed) memory
> > within that buffer.
> >
> > Dell Platform designers recognize this security risk and offer a
> > safer way to communicate with the platform over ACPI.  This is
> > in turn exposed via a WMI interface to the OS.
> >
> > When communicating over WMI-ACPI the communication doesn't occur
> > with physical memory pointers.  When the ASL is invoked, the fixed
> > length ACPI buffer is copied to a small operating region.  The ASL
> > will invoke the SMI, and SMM will only have access to this operating
> > region.  When the ASL returns the buffer is copied back for the OS
> > to process.
> >
> > This method of communication should also deprecate the usage of the
> > dcdbas kernel module and software dependent upon it's interface.
> > Instead offer a character device interface for communicating with this
> > ASL method to allow userspace to use instead.
> >
> > To faciliate that this patch series introduces a generic way for WMI
> > drivers to be able to create discoverable character devices through
> > the WMI bus when desired.
> > Requiring WMI drivers to explicitly ask for this functionality will
> > act as an effective vendor whitelist to character device creation.
> >
> 
> Thanks for an update, looks better now.
> I'm in the middle of going through it and noticed that patches 6 and 7
> sound like a fix to me.
> Perhaps better to move it to be first part of the series.

In v2 I'd say that all of 1 through 7 are fixes that even if the rest of the
series doesn't get pulled in immediately should land sooner.
I'll reorder 6 and 7 for the beginning though for v3 if v3 is necessary.

8 through 13 are the real "meat", and 14 is a fixup.

> 
> Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> interested in changes like these to WMI.

Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
requires a v3 I'll include you.

> 
> > changes between v1 and v2:
> >  * Introduce another patch to sort the includes in wmi.c
> >  * Introduce another patch to cleanup dell_wmi_check_descriptor_buffer
> >    checks.
> >  * Add a commit message to the pr_fmt commit
> >  * Introduce includes to wmi.c in proper location
> >  * Add Reviewed-by to relevant patches from Pali
> >  * Make the WMI introduction patch fallback to legacy SMI
> >    if compiled with CONFIG_DCDBAS
> >  * Separate format of WMI and SMI buffers.  WMI buffer supports more
> >    arguments and data.
> >  * Adjust the rename patch for changes to fallback
> >  * Drop sysfs token creation patch
> >  * Adjust WMI descriptor check patch for changes to fallback
> >  * introduce another patch to remove needless includes in dell-smbios.c
> >  * Add token ioctl interface to character device.
> >    - Can query number of tokens
> >    - Can query values in all tokens
> >  * Expose format of all buffers and IOCTL commands to uapi header
> >  * Drop the read interface from character device.  It doesn't make
> >    sense with multiple different ioctl methods.
> >  * Default WMI interface to 32k (This would normally be queried via
> >    MOF, but that's not possible yet)
> >  * Create separate buffers for WMI and SMI.  If WMI is available,
> >    free the SMI buffer.
> >  * Reorder patches so all fixups come first in the series.
> >
> > Mario Limonciello (14):
> >   platform/x86: dell-wmi: label driver as handling notifications
> >   platform/x86: dell-smbios: drop needless includes
> >   platform/x86: dell-wmi: Don't match on descriptor GUID modalias
> >   platform/x86: dell-smbios: Add pr_fmt definition to driver
> >   platform/x86: wmi: sort include list
> >   platform/x86: wmi: Cleanup exit routine in reverse order of init
> >   platform/x86: wmi: destroy on cleanup rather than unregister
> >   platform/x86: dell-smbios: Introduce a WMI-ACPI interface
> >   platform/x86: dell-smbios: rename to dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
> >   platform/x86: wmi: create character devices when requested by drivers
> >   platform/x86: dell-wmi-smbios: introduce character device for
> >     userspace
> >   platform/x86: Kconfig: Change the default settings for dell-wmi-smbios
> >   platform/x86: dell-wmi-smbios: clean up wmi descriptor check
> >
> >  Documentation/ABI/testing/dell-wmi-smbios          |  10 +
> >  MAINTAINERS                                        |   8 +-
> >  drivers/platform/x86/Kconfig                       |  13 +-
> >  drivers/platform/x86/Makefile                      |   2 +-
> >  drivers/platform/x86/dell-laptop.c                 |   2 +-
> >  drivers/platform/x86/dell-smbios.c                 | 213 ---------
> >  drivers/platform/x86/dell-wmi-smbios.c             | 481 +++++++++++++++++++++
> >  .../x86/{dell-smbios.h => dell-wmi-smbios.h}       |  28 +-
> >  drivers/platform/x86/dell-wmi.c                    |  78 +---
> >  drivers/platform/x86/wmi.c                         | 116 ++++-
> >  include/linux/wmi.h                                |   1 +
> >  include/uapi/linux/dell-wmi-smbios.h               |  55 +++
> >  12 files changed, 674 insertions(+), 333 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/dell-wmi-smbios
> >  delete mode 100644 drivers/platform/x86/dell-smbios.c
> >  create mode 100644 drivers/platform/x86/dell-wmi-smbios.c
> >  rename drivers/platform/x86/{dell-smbios.h => dell-wmi-smbios.h} (74%)
> >  create mode 100644 include/uapi/linux/dell-wmi-smbios.h
> >
> > --
> > 2.14.1
> >
> 
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1740897

On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > 
> > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > interested in changes like these to WMI.
> 
> Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> requires a v3 I'll include you.

Andy should be Cc'd on all patches to  platform drivers x86 as we
co-maintain the subsystem together.

The get_maintainer.pl script is helpful when determining the Cc list.

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1740917

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, September 27, 2017 1:12 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: andy.shevchenko@gmail.com; luto@kernel.org; linux-kernel@vger.kernel.org;
> platform-driver-x86@vger.kernel.org; quasisec@google.com;
> pali.rohar@gmail.com
> Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> 
> On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > >
> > > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > > interested in changes like these to WMI.
> >
> > Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> > requires a v3 I'll include you.
> 
> Andy should be Cc'd on all patches to  platform drivers x86 as we
> co-maintain the subsystem together.

Andy S. was CC'ed on all of these, Andy S was recommending that I add Andy L.

[toc] | [prev] | [next] | [standalone]

#1740918

On Wed, Sep 27, 2017 at 05:31:14PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Darren Hart [mailto:dvhart@infradead.org]
> > Sent: Wednesday, September 27, 2017 1:12 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: andy.shevchenko@gmail.com; luto@kernel.org; linux-kernel@vger.kernel.org;
> > platform-driver-x86@vger.kernel.org; quasisec@google.com;
> > pali.rohar@gmail.com
> > Subject: Re: [PATCH v2 00/14] Introduce support for Dell SMBIOS over WMI
> > 
> > On Tue, Sep 26, 2017 at 07:17:17PM +0000, Mario.Limonciello@dell.com wrote:
> > > >
> > > > Please, include Andy Lutomirski to Cc list, I'm pretty sure he is
> > > > interested in changes like these to WMI.
> > >
> > > Added.  Andy you weren't CC'ed on the original or v2 submission, but if this
> > > requires a v3 I'll include you.
> > 
> > Andy should be Cc'd on all patches to  platform drivers x86 as we
> > co-maintain the subsystem together.
> 
> Andy S. was CC'ed on all of these, Andy S was recommending that I add Andy L.

Ah, of course. Sorry, too many Andys :-)

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web