diff mbox

[v10,11/15] platform/x86: dell-smbios-smm: test for WSMT

Message ID 657d281490de3d0a60f210f8bc57e075c9afc40e.1508434514.git.mario.limonciello@dell.com (mailing list archive)
State Superseded, archived
Delegated to: Darren Hart
Headers show

Commit Message

Limonciello, Mario Oct. 19, 2017, 5:50 p.m. UTC 
WSMT is as an attestation to the OS that the platform won't
modify memory outside of pre-defined areas.

If a platform has WSMT enabled in BIOS setup, SMM calls through
dcdbas will fail.  The only way to access platform data in these
instances is through the WMI SMBIOS calling interface.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Reviewed-by: Edward O'Callaghan <quasisec@google.com>
---
 drivers/platform/x86/dell-smbios-smm.c | 33 +++++++++++++++++++++++++++++++++
 drivers/platform/x86/dell-smbios.h     |  2 ++
 2 files changed, 35 insertions(+)

Comments

Pali Rohár Oct. 19, 2017, 6:09 p.m. UTC | #1 
On Thursday 19 October 2017 12:50:14 Mario Limonciello wrote:
> +/* When enabled this indicates that SMM won't work */
> +static bool test_wsmt_enabled(void)
> +{
> +	struct calling_interface_token *token;
> +
> +	/* if token doesn't exist, SMM will work */
> +	token = dell_smbios_find_token(WSMT_EN_TOKEN);
> +	if (!token)
> +		return false;
> +
> +	/* if token exists, try to access over SMM */
> +	buffer->class = CLASS_TOKEN_READ;
> +	buffer->select = SELECT_TOKEN_STD;
> +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> +	buffer->input[0] = token->location;
> +	dell_smbios_smm_call(buffer);
> +
> +	/* if lookup failed, we know WSMT was enabled */
> +	if (buffer->output[0] != 0)
> +		return true;
> +
> +	/* query token status if it didn't fail */
> +	return (buffer->output[1] == token->value);
> +}
> +
>  static int __init dell_smbios_smm_init(void)
>  {
>  	int ret;
> @@ -115,6 +140,13 @@ static int __init dell_smbios_smm_init(void)
>  
>  	dmi_walk(find_cmd_address, NULL);
>  
> +	ret = test_wsmt_enabled();

ret is int, but test_wsmt_enabled() returns bool.
Limonciello, Mario Oct. 19, 2017, 6:12 p.m. UTC | #2 
> -----Original Message-----

> From: Pali Rohár [mailto:pali.rohar@gmail.com]

> Sent: Thursday, October 19, 2017 1:09 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; Andy

> Lutomirski <luto@kernel.org>; quasisec@google.com; rjw@rjwysocki.net;

> mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>; Alan Cox

> <gnomes@lxorguk.ukuu.org.uk>

> Subject: Re: [PATCH v10 11/15] platform/x86: dell-smbios-smm: test for WSMT

> 

> On Thursday 19 October 2017 12:50:14 Mario Limonciello wrote:

> > +/* When enabled this indicates that SMM won't work */

> > +static bool test_wsmt_enabled(void)

> > +{

> > +	struct calling_interface_token *token;

> > +

> > +	/* if token doesn't exist, SMM will work */

> > +	token = dell_smbios_find_token(WSMT_EN_TOKEN);

> > +	if (!token)

> > +		return false;

> > +

> > +	/* if token exists, try to access over SMM */

> > +	buffer->class = CLASS_TOKEN_READ;

> > +	buffer->select = SELECT_TOKEN_STD;

> > +	memset(buffer, 0, sizeof(struct calling_interface_buffer));

> > +	buffer->input[0] = token->location;

> > +	dell_smbios_smm_call(buffer);

> > +

> > +	/* if lookup failed, we know WSMT was enabled */

> > +	if (buffer->output[0] != 0)

> > +		return true;

> > +

> > +	/* query token status if it didn't fail */

> > +	return (buffer->output[1] == token->value);

> > +}

> > +

> >  static int __init dell_smbios_smm_init(void)

> >  {

> >  	int ret;

> > @@ -115,6 +140,13 @@ static int __init dell_smbios_smm_init(void)

> >

> >  	dmi_walk(find_cmd_address, NULL);

> >

> > +	ret = test_wsmt_enabled();

> 

> ret is int, but test_wsmt_enabled() returns bool.


Yes, ret is re-used within this method.
0, 1 enum is a subset of int, so this seemed like a logical thing to me to do.
Darren Hart Oct. 19, 2017, 10:46 p.m. UTC | #3 
On Thu, Oct 19, 2017 at 06:12:09PM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@gmail.com]
> > Sent: Thursday, October 19, 2017 1:09 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; Andy
> > Lutomirski <luto@kernel.org>; quasisec@google.com; rjw@rjwysocki.net;
> > mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>; Alan Cox
> > <gnomes@lxorguk.ukuu.org.uk>
> > Subject: Re: [PATCH v10 11/15] platform/x86: dell-smbios-smm: test for WSMT
> > 
> > On Thursday 19 October 2017 12:50:14 Mario Limonciello wrote:
> > > +/* When enabled this indicates that SMM won't work */
> > > +static bool test_wsmt_enabled(void)
> > > +{
> > > +	struct calling_interface_token *token;
> > > +
> > > +	/* if token doesn't exist, SMM will work */
> > > +	token = dell_smbios_find_token(WSMT_EN_TOKEN);
> > > +	if (!token)
> > > +		return false;
> > > +
> > > +	/* if token exists, try to access over SMM */
> > > +	buffer->class = CLASS_TOKEN_READ;
> > > +	buffer->select = SELECT_TOKEN_STD;
> > > +	memset(buffer, 0, sizeof(struct calling_interface_buffer));
> > > +	buffer->input[0] = token->location;
> > > +	dell_smbios_smm_call(buffer);
> > > +
> > > +	/* if lookup failed, we know WSMT was enabled */
> > > +	if (buffer->output[0] != 0)
> > > +		return true;
> > > +
> > > +	/* query token status if it didn't fail */
> > > +	return (buffer->output[1] == token->value);
> > > +}
> > > +
> > >  static int __init dell_smbios_smm_init(void)
> > >  {
> > >  	int ret;
> > > @@ -115,6 +140,13 @@ static int __init dell_smbios_smm_init(void)
> > >
> > >  	dmi_walk(find_cmd_address, NULL);
> > >
> > > +	ret = test_wsmt_enabled();
> > 
> > ret is int, but test_wsmt_enabled() returns bool.
> 
> Yes, ret is re-used within this method.
> 0, 1 enum is a subset of int, so this seemed like a logical thing to me to do.

Agreed, the conversion is automatic and he tests for it and returns an
appropriate error code. I don't see a real problem with this.
diff mbox

Patch

diff --git a/drivers/platform/x86/dell-smbios-smm.c b/drivers/platform/x86/dell-smbios-smm.c
index 53eabb14fb48..4175ccbded15 100644
--- a/drivers/platform/x86/dell-smbios-smm.c
+++ b/drivers/platform/x86/dell-smbios-smm.c
@@ -102,6 +102,31 @@  int dell_smbios_smm_call(struct calling_interface_buffer *input)
 	return 0;
 }
 
+/* When enabled this indicates that SMM won't work */
+static bool test_wsmt_enabled(void)
+{
+	struct calling_interface_token *token;
+
+	/* if token doesn't exist, SMM will work */
+	token = dell_smbios_find_token(WSMT_EN_TOKEN);
+	if (!token)
+		return false;
+
+	/* if token exists, try to access over SMM */
+	buffer->class = CLASS_TOKEN_READ;
+	buffer->select = SELECT_TOKEN_STD;
+	memset(buffer, 0, sizeof(struct calling_interface_buffer));
+	buffer->input[0] = token->location;
+	dell_smbios_smm_call(buffer);
+
+	/* if lookup failed, we know WSMT was enabled */
+	if (buffer->output[0] != 0)
+		return true;
+
+	/* query token status if it didn't fail */
+	return (buffer->output[1] == token->value);
+}
+
 static int __init dell_smbios_smm_init(void)
 {
 	int ret;
@@ -115,6 +140,13 @@  static int __init dell_smbios_smm_init(void)
 
 	dmi_walk(find_cmd_address, NULL);
 
+	ret = test_wsmt_enabled();
+	pr_debug("WSMT enable test: %d\n", ret);
+	if (ret) {
+		ret = -ENODEV;
+		goto fail_wsmt;
+	}
+
 	platform_device = platform_device_alloc("dell-smbios", 1);
 	if (!platform_device) {
 		ret = -ENOMEM;
@@ -138,6 +170,7 @@  static int __init dell_smbios_smm_init(void)
 fail_platform_device_add:
 	platform_device_put(platform_device);
 
+fail_wsmt:
 fail_platform_device_alloc:
 	free_page((unsigned long)buffer);
 	return ret;
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 079776421b46..956fe9304ed7 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -44,6 +44,8 @@ 
 #define KBD_LED_AUTO_100_TOKEN	0x02F6
 #define GLOBAL_MIC_MUTE_ENABLE	0x0364
 #define GLOBAL_MIC_MUTE_DISABLE	0x0365
+#define WSMT_EN_TOKEN		0x04EC
+#define WSMT_DIS_TOKEN		0x04ED
 
 struct notifier_block;