Message ID | 20230526171658.3886-1-mpearson-lenovo@squebb.ca |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp643311vqr; Fri, 26 May 2023 10:32:12 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7spUKaQYhgZBBYuo/l5bNHGUHoMS7AtM5SuDK3BaiShqu2TMW6M6fVTmY8PK0F+cMYl5Py X-Received: by 2002:a17:90b:3695:b0:253:44e7:9454 with SMTP id mj21-20020a17090b369500b0025344e79454mr7195760pjb.17.1685122331844; Fri, 26 May 2023 10:32:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685122331; cv=none; d=google.com; s=arc-20160816; b=svOX9NkCDIVtzz50lhlaCntEUtutCzIfY9aNEBHNm45NRLn3iwwA63BOb8Dvx/+Ho7 if/l1fmMObK/sX4daWnhFpjJKIN42AqJNLR13Hqop4xm6sZMcyLWP6/ZXYhg+bLY0aLX 13auWE70AAXpZ9UkoovfliUwP/03HSIH/zW+YkEmzsppRpl5/a/y984fEYX8mK4VdIER 5/yfzG8bjTOR9NnYxi4IOzzFcGC30BWFc5lOwbw0SnT+fX91Q56c1Z8nkXlDX093rwLH O9+X4LV9eksCu/fkhrxmgg7kREfaKtd8OVGZNX4K9/us1RX8yqi/4yjybf6OUuOqg6I0 3ogw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :feedback-id:dkim-signature:dkim-signature; bh=3H+Xr8+kBb0oLRwshP3ohk2SfeItphOtUJ4BgiDd58o=; b=EH94yESZSibleVDkCW2iE1QH0kEDWvphwo34fdC7rmZQPYY4zXKeW83rup+/mc/q2I eMMmEPpcz1LdMs1fqUET/XlgVM/tB76gCotiugyoraikfrPw89K0TRJLoc7kzTa/qgOD 5QPLwYSCf8YNGn9bfFrYyGaetkzRt7KcC9xAb4p8hOdDE8BXd4OqCB4mxS54aLPvjQ4X IuK3Un4kLwERrVELsLGiXXI0wbvgkWJVn4kCUFgPP5ePmhG4RL3q4MFmnTEk5S9tW0ez l3u2pixzP1DszyAyXV6Ikn7hB0JOCMjaFXjF+XFY1UyPqypCOEomxtbau2dV1pCGKcvF sLJw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@squebb.ca header.s=fm2 header.b=GjduknsK; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=R3BmtsiE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i64-20020a639d43000000b0051b70b1f1f5si3093724pgd.608.2023.05.26.10.31.56; Fri, 26 May 2023 10:32:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@squebb.ca header.s=fm2 header.b=GjduknsK; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=R3BmtsiE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237003AbjEZRRT (ORCPT <rfc822;zhanglyra.2023@gmail.com> + 99 others); Fri, 26 May 2023 13:17:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37976 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230376AbjEZRRR (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 26 May 2023 13:17:17 -0400 Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE258B2; Fri, 26 May 2023 10:17:14 -0700 (PDT) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 0626F5C00C4; Fri, 26 May 2023 13:17:14 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Fri, 26 May 2023 13:17:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=squebb.ca; h=cc :cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm2; t=1685121434; x= 1685207834; bh=3H+Xr8+kBb0oLRwshP3ohk2SfeItphOtUJ4BgiDd58o=; b=G jduknsKae90MfW2LyUQZfGgKk+QNKW0iUGr25IpGpV+e3YmVwOC7/8XrG2j4diBv GPS6Hd4pYABNFAk/DH/0rEfCdyg4kKwD+5Gy3lU0937wZjQlZOGiFpbrn6z9tdDy MrI4omcrIwjA2PZU2DlbS07rgKmRAt4ces9eae5dXxbj7P6lyKkm8YCObEx3UpUb Lkm+LF1pZYCGzmUqUn/RiCWZNzGHAsIOEHwYfkVd4fwBn2qxXYIubSAmD23Cg3Yw zkvbq0I5Ed7I3v8gGG+yG2GKzNsh8FTyJjCG3eLaNa8i61nNbHX0mjdVZQUm/6UA SlYS7yjX2vHNz5GFPYQdQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1685121434; x= 1685207834; bh=3H+Xr8+kBb0oLRwshP3ohk2SfeItphOtUJ4BgiDd58o=; b=R 3BmtsiEzBOBdYJrpouC8kU8xnj7T57Lc4FPIQQxWPkQ6WkXkxS6kEfsyc91R3KBF xss3DywV6Fxh3e4xWez3+oaeRRFpcqOTs3FX9LJD3pCIwWfGeUahheM9ucsDuwra W/Hpg+VDjXUUS6GbvYo6QWglPLBZ4YR2ZotU+GKRCE/9uh9zAd7e0nYR2d2FQCGI VJwBubkH5tENtC0scKvKRdgDoLA3B3xzZMgYQQ/51/rDZYIHoXwqKfSKFfRD+CxX xlm6BiLrrU8XaDKRAQNKcBKfDBo8FulAFkV7P89ivyE50oknUPQhYJ1bwjxgW7R5 y25yvmMAaykx0Nn5mHixw== X-ME-Sender: <xms:melwZD5Ru3K1QlNIYk_oR3QdTc5rs2cN18-TjsC9dWx5lUrmXIyyiQ> <xme:melwZI5n3RV-ZBDnqfdg8UuEP-xBwdPzR58ovcTocReYebpEnms6mgkE2_3OoI6wm FQ8X98m5PkOCc3wwI4> X-ME-Received: <xmr:melwZKcDl8zuH50l6tSc96d7VE_iee6bOYJQYS3BT0vmgXVeuNhU1IPvtE2duJKcnGsXv9BEjoYigO7x36IJzaUrbk7UkXTw6PM2QQls85fZ-RWRrXlxGBkEDQ> X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeejledguddtkecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecuogetfedtuddqtdduucdludehmdenucfjughrpe fhvfevufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpeforghrkhcurfgvrghr shhonhcuoehmphgvrghrshhonhdqlhgvnhhovhhosehsqhhuvggssgdrtggrqeenucggtf frrghtthgvrhhnpeeftddvjeefleffvefhgfejjeehudetteeigeeugfekhffhgeejudeu teehgfdvffenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhroh hmpehmphgvrghrshhonhdqlhgvnhhovhhosehsqhhuvggssgdrtggr X-ME-Proxy: <xmx:melwZEIEbD4WA3XqBE1Ky5LKal4IGAzX_-MdeAvpaSkCFk59U8YWXQ> <xmx:melwZHLg7ARXnqsWXRQ0TaEpwMQQY_0B6LFHyBgJxbXO-dRhzwMUBA> <xmx:melwZNyYgDAUgr-hThgJI_cx_7DNfjs-112MvSqenUBw1Bhtzt9h3g> <xmx:mulwZP29-MCdgYbTTAbX4IX6giu_w7qxNRVOAwaGRHMzsE722x1Eyw> Feedback-ID: ibe194615:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 26 May 2023 13:17:13 -0400 (EDT) From: Mark Pearson <mpearson-lenovo@squebb.ca> To: mpearson-lenovo@squebb.ca Cc: hdegoede@redhat.com, markgross@kernel.org, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3 1/5] platform/x86: think-lmi: Enable opcode support on BIOS settings Date: Fri, 26 May 2023 13:16:54 -0400 Message-Id: <20230526171658.3886-1-mpearson-lenovo@squebb.ca> X-Mailer: git-send-email 2.40.1 In-Reply-To: <mpearson-lenovo@squebb.ca> References: <mpearson-lenovo@squebb.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1766167530633866281?= X-GMAIL-MSGID: =?utf-8?q?1766978834375376608?= |
Series |
[v3,1/5] platform/x86: think-lmi: Enable opcode support on BIOS settings
|
|
Commit Message
Mark Pearson
May 26, 2023, 5:16 p.m. UTC
Whilst reviewing some documentation from the FW team on using WMI on
Lenovo system I noticed that we weren't using Opcode support when
changing BIOS settings in the thinkLMI driver.
We should be doing this to ensure we're future proof as the old
non-opcode mechanism has been deprecated.
Tested on X1 Carbon G10 and G11.
Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
Changes in v2: Update comment for clearer explanation of what the driver
is doing
Changes in v3: None. Version bump with rest of series
drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
Comments
On Fri, 26 May 2023, Mark Pearson wrote: > Whilst reviewing some documentation from the FW team on using WMI on > Lenovo system I noticed that we weren't using Opcode support when > changing BIOS settings in the thinkLMI driver. > > We should be doing this to ensure we're future proof as the old > non-opcode mechanism has been deprecated. > > Tested on X1 Carbon G10 and G11. > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > --- > Changes in v2: Update comment for clearer explanation of what the driver > is doing > Changes in v3: None. Version bump with rest of series > > drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 1138f770149d..2745224f62ab 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, > tlmi_priv.pwd_admin->save_signature); > if (ret) > goto out; > - } else { /* Non certiifcate based authentication */ > + } else if (tlmi_priv.opcode_support) { > + /* > + * If opcode support is present use that interface. > + * Note - this sets the variable and then the password as separate > + * WMI calls. Function tlmi_save_bios_settings will error if the > + * password is incorrect. > + */ > + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, > + new_setting); Alignment. > + if (!set_str) { > + ret = -ENOMEM; > + goto out; > + } > + > + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); > + if (ret) > + goto out; > + > + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", > + tlmi_priv.pwd_admin->password); Align. > + if (ret) > + goto out; > + } > + > + ret = tlmi_save_bios_settings(""); > + } else { /* old non opcode based authentication method (deprecated)*/ non missing hyphen. Missing space at the comment closing. > if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;", > tlmi_priv.pwd_admin->password, > Except for those style issues, it look okay to me: Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
On Mon, May 29, 2023, at 7:51 AM, Ilpo Järvinen wrote: > On Fri, 26 May 2023, Mark Pearson wrote: > >> Whilst reviewing some documentation from the FW team on using WMI on >> Lenovo system I noticed that we weren't using Opcode support when >> changing BIOS settings in the thinkLMI driver. >> >> We should be doing this to ensure we're future proof as the old >> non-opcode mechanism has been deprecated. >> >> Tested on X1 Carbon G10 and G11. >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> --- >> Changes in v2: Update comment for clearer explanation of what the driver >> is doing >> Changes in v3: None. Version bump with rest of series >> >> drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++- >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c >> index 1138f770149d..2745224f62ab 100644 >> --- a/drivers/platform/x86/think-lmi.c >> +++ b/drivers/platform/x86/think-lmi.c >> @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, >> tlmi_priv.pwd_admin->save_signature); >> if (ret) >> goto out; >> - } else { /* Non certiifcate based authentication */ >> + } else if (tlmi_priv.opcode_support) { >> + /* >> + * If opcode support is present use that interface. >> + * Note - this sets the variable and then the password as separate >> + * WMI calls. Function tlmi_save_bios_settings will error if the >> + * password is incorrect. >> + */ >> + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, >> + new_setting); > > Alignment. OK - I assume you want the new_setting lined up under the bracket. I've not seen that called out as a requirement (https://www.kernel.org/doc/html/v4.10/process/coding-style.html) but I don't mind fixing....but if you can point me at the specifics it's appreciated > >> + if (!set_str) { >> + ret = -ENOMEM; >> + goto out; >> + } >> + >> + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); >> + if (ret) >> + goto out; >> + >> + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { >> + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", >> + tlmi_priv.pwd_admin->password); > > Align. Ack. > >> + if (ret) >> + goto out; >> + } >> + >> + ret = tlmi_save_bios_settings(""); >> + } else { /* old non opcode based authentication method (deprecated)*/ > > non missing hyphen. non-opcode I assume? > > Missing space at the comment closing. Will fix. > >> if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { >> auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;", >> tlmi_priv.pwd_admin->password, >> > > Except for those style issues, it look okay to me: > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Thanks for the review! Mark
On Mon, 29 May 2023, Mark Pearson wrote: > On Mon, May 29, 2023, at 7:51 AM, Ilpo Järvinen wrote: > > On Fri, 26 May 2023, Mark Pearson wrote: > > > >> Whilst reviewing some documentation from the FW team on using WMI on > >> Lenovo system I noticed that we weren't using Opcode support when > >> changing BIOS settings in the thinkLMI driver. > >> > >> We should be doing this to ensure we're future proof as the old > >> non-opcode mechanism has been deprecated. > >> > >> Tested on X1 Carbon G10 and G11. > >> > >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > >> --- > >> Changes in v2: Update comment for clearer explanation of what the driver > >> is doing > >> Changes in v3: None. Version bump with rest of series > >> > >> drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++- > >> 1 file changed, 27 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > >> index 1138f770149d..2745224f62ab 100644 > >> --- a/drivers/platform/x86/think-lmi.c > >> +++ b/drivers/platform/x86/think-lmi.c > >> @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, > >> tlmi_priv.pwd_admin->save_signature); > >> if (ret) > >> goto out; > >> - } else { /* Non certiifcate based authentication */ > >> + } else if (tlmi_priv.opcode_support) { > >> + /* > >> + * If opcode support is present use that interface. > >> + * Note - this sets the variable and then the password as separate > >> + * WMI calls. Function tlmi_save_bios_settings will error if the > >> + * password is incorrect. > >> + */ > >> + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, > >> + new_setting); > > > > Alignment. > > OK - I assume you want the new_setting lined up under the bracket. > I've not seen that called out as a requirement (https://www.kernel.org/doc/html/v4.10/process/coding-style.html) but I don't mind fixing....but if you can point me at the specifics it's appreciated Yes, I meant aligning to the column following the opening parenthesis. I guess it's not a hard requirement, however, there's a benefit from certain things aligning because it helps in the brains in the process of converting text into structure with less effort (when not specifically not focusing on that particular line). > >> + if (!set_str) { > >> + ret = -ENOMEM; > >> + goto out; > >> + } > >> + > >> + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); > >> + if (ret) > >> + goto out; > >> + > >> + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > >> + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", > >> + tlmi_priv.pwd_admin->password); > > > > Align. > > Ack. > > > > >> + if (ret) > >> + goto out; > >> + } > >> + > >> + ret = tlmi_save_bios_settings(""); > >> + } else { /* old non opcode based authentication method (deprecated)*/ > > > > non missing hyphen. > > non-opcode I assume? I think the most proper English would be non-opcode-based since "opcode based" belong together (but I'm not a native speaker here).
On Mon, May 29, 2023, at 11:36 AM, Ilpo Järvinen wrote: > On Mon, 29 May 2023, Mark Pearson wrote: >> On Mon, May 29, 2023, at 7:51 AM, Ilpo Järvinen wrote: >> > On Fri, 26 May 2023, Mark Pearson wrote: >> > >> >> Whilst reviewing some documentation from the FW team on using WMI on >> >> Lenovo system I noticed that we weren't using Opcode support when >> >> changing BIOS settings in the thinkLMI driver. >> >> >> >> We should be doing this to ensure we're future proof as the old >> >> non-opcode mechanism has been deprecated. >> >> >> >> Tested on X1 Carbon G10 and G11. >> >> >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> >> --- >> >> Changes in v2: Update comment for clearer explanation of what the driver >> >> is doing >> >> Changes in v3: None. Version bump with rest of series >> >> >> >> drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++- >> >> 1 file changed, 27 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c >> >> index 1138f770149d..2745224f62ab 100644 >> >> --- a/drivers/platform/x86/think-lmi.c >> >> +++ b/drivers/platform/x86/think-lmi.c >> >> @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, >> >> tlmi_priv.pwd_admin->save_signature); >> >> if (ret) >> >> goto out; >> >> - } else { /* Non certiifcate based authentication */ >> >> + } else if (tlmi_priv.opcode_support) { >> >> + /* >> >> + * If opcode support is present use that interface. >> >> + * Note - this sets the variable and then the password as separate >> >> + * WMI calls. Function tlmi_save_bios_settings will error if the >> >> + * password is incorrect. >> >> + */ >> >> + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, >> >> + new_setting); >> > >> > Alignment. >> >> OK - I assume you want the new_setting lined up under the bracket. >> I've not seen that called out as a requirement (https://www.kernel.org/doc/html/v4.10/process/coding-style.html) but I don't mind fixing....but if you can point me at the specifics it's appreciated > > Yes, I meant aligning to the column following the opening parenthesis. > > I guess it's not a hard requirement, however, there's a benefit from > certain things aligning because it helps in the brains in the process of > converting text into structure with less effort (when not specifically not > focusing on that particular line). Not a problem. Happy to make this change along with the others. Was just curious :) > >> >> + if (!set_str) { >> >> + ret = -ENOMEM; >> >> + goto out; >> >> + } >> >> + >> >> + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); >> >> + if (ret) >> >> + goto out; >> >> + >> >> + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { >> >> + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", >> >> + tlmi_priv.pwd_admin->password); >> > >> > Align. >> >> Ack. >> >> > >> >> + if (ret) >> >> + goto out; >> >> + } >> >> + >> >> + ret = tlmi_save_bios_settings(""); >> >> + } else { /* old non opcode based authentication method (deprecated)*/ >> > >> > non missing hyphen. >> >> non-opcode I assume? > > I think the most proper English would be non-opcode-based since "opcode > based" belong together (but I'm not a native speaker here). I am a native speaker....and I don't know :) (English is weird...) Let's go with non-opcode; adding the based on there feels wrong to me (somewhat arbitrarily). > > -- > i.
Hi Mark, On 5/26/23 19:16, Mark Pearson wrote: > Whilst reviewing some documentation from the FW team on using WMI on > Lenovo system I noticed that we weren't using Opcode support when > changing BIOS settings in the thinkLMI driver. > > We should be doing this to ensure we're future proof as the old > non-opcode mechanism has been deprecated. > > Tested on X1 Carbon G10 and G11. > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> Thank you for this new version. Please prepare a v4 addressing Ilpo's review remarks. About the aligning function arguments on the next line to the '(' of the function call start at the previous line, checkpatch also checks for this. It is always a good idea to run checkpatch before submitting patches. E.g.: git format-patch -v3 HEAD~5 scripts/checkpatch.pl v3-00*.patch <check output is clean> git send-email v3-00*.patch Regards, Hans > --- > Changes in v2: Update comment for clearer explanation of what the driver > is doing > Changes in v3: None. Version bump with rest of series > > drivers/platform/x86/think-lmi.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 1138f770149d..2745224f62ab 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, > tlmi_priv.pwd_admin->save_signature); > if (ret) > goto out; > - } else { /* Non certiifcate based authentication */ > + } else if (tlmi_priv.opcode_support) { > + /* > + * If opcode support is present use that interface. > + * Note - this sets the variable and then the password as separate > + * WMI calls. Function tlmi_save_bios_settings will error if the > + * password is incorrect. > + */ > + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, > + new_setting); > + if (!set_str) { > + ret = -ENOMEM; > + goto out; > + } > + > + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); > + if (ret) > + goto out; > + > + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", > + tlmi_priv.pwd_admin->password); > + if (ret) > + goto out; > + } > + > + ret = tlmi_save_bios_settings(""); > + } else { /* old non opcode based authentication method (deprecated)*/ > if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;", > tlmi_priv.pwd_admin->password,
Hi Hans On Tue, May 30, 2023, at 6:54 AM, Hans de Goede wrote: > Hi Mark, > > On 5/26/23 19:16, Mark Pearson wrote: >> Whilst reviewing some documentation from the FW team on using WMI on >> Lenovo system I noticed that we weren't using Opcode support when >> changing BIOS settings in the thinkLMI driver. >> >> We should be doing this to ensure we're future proof as the old >> non-opcode mechanism has been deprecated. >> >> Tested on X1 Carbon G10 and G11. >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > > Thank you for this new version. Please prepare a v4 addressing Ilpo's > review remarks. Will do > > About the aligning function arguments on the next line to the '(' > of the function call start at the previous line, checkpatch also > checks for this. > > It is always a good idea to run checkpatch before submitting patches. I always do - and checkpatch isn't complaining about the alignment here. Mark
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 1138f770149d..2745224f62ab 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -1001,7 +1001,33 @@ static ssize_t current_value_store(struct kobject *kobj, tlmi_priv.pwd_admin->save_signature); if (ret) goto out; - } else { /* Non certiifcate based authentication */ + } else if (tlmi_priv.opcode_support) { + /* + * If opcode support is present use that interface. + * Note - this sets the variable and then the password as separate + * WMI calls. Function tlmi_save_bios_settings will error if the + * password is incorrect. + */ + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, + new_setting); + if (!set_str) { + ret = -ENOMEM; + goto out; + } + + ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, set_str); + if (ret) + goto out; + + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", + tlmi_priv.pwd_admin->password); + if (ret) + goto out; + } + + ret = tlmi_save_bios_settings(""); + } else { /* old non opcode based authentication method (deprecated)*/ if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;", tlmi_priv.pwd_admin->password,