* [PATCH 0/1] platform/x86: Add Slim Bootloader firmware update support @ 2020-04-20 19:44 Jithu Joseph 2020-04-20 19:44 ` [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver Jithu Joseph 0 siblings, 1 reply; 5+ messages in thread From: Jithu Joseph @ 2020-04-20 19:44 UTC (permalink / raw) To: dvhart, andy Cc: platform-driver-x86, linux-kernel, maurice.ma, ravi.p.rangarajan, sean.v.kelley, kuo-lang.tseng, Jithu Joseph Slim Bootloader(SBL) [1] is a small open-source boot firmware, designed for running on certain Intel platforms. SBL can be thought-of as fulfilling the role of a minimal BIOS implementation, i.e initializing the hardware and booting Operating System. This driver creates sysfs interfaces which allows user space entities to trigger update of SBL firmware. Acknowledgment: Initial draft of the driver code was authored by Maurice Ma <maurice.ma@intel.com> [1] https://slimbootloader.github.io Jithu Joseph (1): platform/x86: Add Slim Bootloader firmware update signaling driver MAINTAINERS | 7 ++ drivers/platform/x86/Kconfig | 10 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/sbl_fwu_wmi.c | 143 +++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+) create mode 100644 drivers/platform/x86/sbl_fwu_wmi.c base-commit: 8f3d9f354286745c751374f5f1fcafee6b3f3136 -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver 2020-04-20 19:44 [PATCH 0/1] platform/x86: Add Slim Bootloader firmware update support Jithu Joseph @ 2020-04-20 19:44 ` Jithu Joseph 2020-04-22 13:42 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Jithu Joseph @ 2020-04-20 19:44 UTC (permalink / raw) To: dvhart, andy Cc: platform-driver-x86, linux-kernel, maurice.ma, ravi.p.rangarajan, sean.v.kelley, kuo-lang.tseng, Jithu Joseph Slim Bootloader(SBL) [1] is a small open-source boot firmware, designed for running on certain Intel platforms. SBL can be thought-of as fulfilling the role of a minimal BIOS implementation, i.e initializing the hardware and booting Operating System. Since SBL is not UEFI compliant, firmware update cannot be triggered using standard UEFI runtime services. Further considering performance impact, SBL doesn't look for a firmware update image on every reset and does so only when firmware update signal is asserted. SBL exposes an ACPI-WMI device which comes up in sysfs as /sys/bus/wmi/44FADEB1xxx and this driver adds a "firmware_update_request" device attribute. This attribute normally has a value of 0 and userspace can signal SBL to update firmware, on next reboot, by writing a value of 1: echo 1 > /sys/bus/wmi/devices/44FADEB1-B204-40F2-8581-394BBDC1B651/firmware_update_request This driver only implements a signaling mechanism, the actual firmware update process and various details like firmware update image format, firmware image location etc are defined by SBL [2] and are not in the scope of this driver. [1] https://slimbootloader.github.io [2] https://slimbootloader.github.io/security/firmware-update.html Signed-off-by: Jithu Joseph <jithu.joseph@intel.com> --- MAINTAINERS | 7 ++ drivers/platform/x86/Kconfig | 10 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/sbl_fwu_wmi.c | 143 +++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+) create mode 100644 drivers/platform/x86/sbl_fwu_wmi.c diff --git a/MAINTAINERS b/MAINTAINERS index e64e5db31497..fb5542afff39 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15449,6 +15449,13 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git dev F: include/linux/srcu*.h F: kernel/rcu/srcu*.c +SLIM BOOTLOADER (SBL) FIRMWARE UPDATE WMI DRIVER +M: Jithu Joseph <jithu.joseph@intel.com> +R: Maurice Ma <maurice.ma@intel.com> +S: Maintained +W: https://slimbootloader.github.io/security/firmware-update.html +F: drivers/platform/x86/sbl_fwu_wmi.c + SMACK SECURITY MODULE M: Casey Schaufler <casey@schaufler-ca.com> L: linux-security-module@vger.kernel.org diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0ad7ad8cf8e1..aac2bae19a35 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -114,6 +114,16 @@ config XIAOMI_WMI To compile this driver as a module, choose M here: the module will be called xiaomi-wmi. +config SBL_FWU_WMI + tristate "WMI driver for Slim Bootloader firmware update signaling" + depends on ACPI_WMI + help + Say Y here if you want to be able to use the WMI interface to signal + Slim Bootloader to trigger update on next reboot. + + To compile this driver as a module, choose M here: the module will + be called sbl-fwu-wmi. + config ACERHDF tristate "Acer Aspire One temperature and fan driver" depends on ACPI && THERMAL diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 53408d965874..3d53828e62d5 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o obj-$(CONFIG_MXM_WMI) += mxm-wmi.o obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o +obj-$(CONFIG_SBL_FWU_WMI) += sbl_fwu_wmi.o # Acer obj-$(CONFIG_ACERHDF) += acerhdf.o diff --git a/drivers/platform/x86/sbl_fwu_wmi.c b/drivers/platform/x86/sbl_fwu_wmi.c new file mode 100644 index 000000000000..d3a4a3b0ab5d --- /dev/null +++ b/drivers/platform/x86/sbl_fwu_wmi.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Slim Bootloader(SBL) firmware update signaling driver + * + * Slim Bootloader is a small, open-source, non UEFI compliant, boot firmware + * optimized for running on certain Intel platforms. + * + * SBL exposes an ACPI-WMI device via /sys/bus/wmi/<SBL_FWU_WMI_GUID>. + * This driver further adds "firmware_update_request" device attribute. + * This attribute normally has a value of 0 and userspace can signal SBL + * to update firmware, on next reboot, by writing a value of 1. + * + * More details of SBL firmware update process is available at: + * https://slimbootloader.github.io/security/firmware-update.html + */ + +#include <linux/acpi.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/sysfs.h> +#include <linux/wmi.h> + +#define SBL_FWU_WMI_GUID "44FADEB1-B204-40F2-8581-394BBDC1B651" + +static int get_fwu_request(struct device *dev, u32 *out) +{ + struct acpi_buffer result = {ACPI_ALLOCATE_BUFFER, NULL}; + union acpi_object *obj; + acpi_status status; + + status = wmi_query_block(SBL_FWU_WMI_GUID, 0, &result); + if (ACPI_FAILURE(status)) { + dev_err(dev, "wmi_query_block failed\n"); + return -ENODEV; + } + + obj = (union acpi_object *)result.pointer; + if (!obj || obj->type != ACPI_TYPE_INTEGER) { + dev_warn(dev, "wmi_query_block returned invalid value\n"); + kfree(obj); + return -EINVAL; + } + + *out = obj->integer.value; + kfree(obj); + + return 0; +} + +static int set_fwu_request(struct device *dev, u32 in) +{ + struct acpi_buffer input; + acpi_status status; + u32 value; + + value = in; + input.length = sizeof(u32); + input.pointer = &value; + + status = wmi_set_block(SBL_FWU_WMI_GUID, 0, &input); + if (ACPI_FAILURE(status)) { + dev_err(dev, "wmi_set_block failed\n"); + return -ENODEV; + } + + return 0; +} + +static ssize_t firmware_update_request_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + u32 val; + int ret; + + ret = get_fwu_request(dev, &val); + if (ret) + return ret; + + return sprintf(buf, "%d\n", val); +} + +static ssize_t firmware_update_request_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + bool val; + int ret; + + ret = kstrtobool(buf, &val); + if (ret) + return ret; + + ret = set_fwu_request(dev, val ? 1 : 0); + if (ret) + return ret; + + return count; +} + +static DEVICE_ATTR_RW(firmware_update_request); + +static struct attribute *firmware_update_attrs[] = { + &dev_attr_firmware_update_request.attr, + NULL +}; + +ATTRIBUTE_GROUPS(firmware_update); + +static int sbl_fwu_wmi_probe(struct wmi_device *wdev, const void *context) +{ + dev_info(&wdev->dev, "Slim Bootloader signaling driver attached\n"); + return 0; +} + +static int sbl_fwu_wmi_remove(struct wmi_device *wdev) +{ + dev_info(&wdev->dev, "Slim Bootloader signaling driver removed\n"); + return 0; +} + +static const struct wmi_device_id sbl_fwu_wmi_id_table[] = { + { .guid_string = SBL_FWU_WMI_GUID }, + {} +}; + +static struct wmi_driver sbl_fwu_wmi_driver = { + .driver = { + .name = "sbl-fwu-wmi", + .dev_groups = firmware_update_groups, + }, + .probe = sbl_fwu_wmi_probe, + .remove = sbl_fwu_wmi_remove, + .id_table = sbl_fwu_wmi_id_table, +}; + +module_wmi_driver(sbl_fwu_wmi_driver); + +MODULE_DEVICE_TABLE(wmi, sbl_fwu_wmi_id_table); +MODULE_AUTHOR("Jithu Joseph <jithu.joseph@intel.com>"); +MODULE_DESCRIPTION("Slim Bootloader firmware update signaling driver"); +MODULE_LICENSE("GPL v2"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver 2020-04-20 19:44 ` [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver Jithu Joseph @ 2020-04-22 13:42 ` Andy Shevchenko 2020-04-23 2:02 ` Jithu Joseph 0 siblings, 1 reply; 5+ messages in thread From: Andy Shevchenko @ 2020-04-22 13:42 UTC (permalink / raw) To: Jithu Joseph Cc: Darren Hart, Andy Shevchenko, Platform Driver, Linux Kernel Mailing List, maurice.ma, ravi.p.rangarajan, sean.v.kelley, kuo-lang.tseng On Mon, Apr 20, 2020 at 10:50 PM Jithu Joseph <jithu.joseph@intel.com> wrote: > > Slim Bootloader(SBL) [1] is a small open-source boot firmware, > designed for running on certain Intel platforms. SBL can be > thought-of as fulfilling the role of a minimal BIOS > implementation, i.e initializing the hardware and booting > Operating System. > > Since SBL is not UEFI compliant, firmware update cannot be triggered > using standard UEFI runtime services. Further considering performance > impact, SBL doesn't look for a firmware update image on every reset > and does so only when firmware update signal is asserted. > > SBL exposes an ACPI-WMI device which comes up in sysfs as > /sys/bus/wmi/44FADEB1xxx and this driver adds a > "firmware_update_request" device attribute. This attribute normally > has a value of 0 and userspace can signal SBL to update firmware, > on next reboot, by writing a value of 1: > > echo 1 > /sys/bus/wmi/devices/44FADEB1-B204-40F2-8581-394BBDC1B651/firmware_update_request > > This driver only implements a signaling mechanism, the actual firmware > update process and various details like firmware update image format, > firmware image location etc are defined by SBL [2] and are not in the > scope of this driver. I have noticed that it misses ABI documentation. So, please add. Also some comments below. ... > [1] https://slimbootloader.github.io > [2] https://slimbootloader.github.io/security/firmware-update.html Can you add a DocLink: tag below for the reference to the official documentation? ... > +SLIM BOOTLOADER (SBL) FIRMWARE UPDATE WMI DRIVER > +M: Jithu Joseph <jithu.joseph@intel.com> > +R: Maurice Ma <maurice.ma@intel.com> > +S: Maintained > +W: https://slimbootloader.github.io/security/firmware-update.html > +F: drivers/platform/x86/sbl_fwu_wmi.c I hope you run latest and greatest version of checkpatch.pl and it's okay with this section. ... > @@ -114,6 +114,16 @@ config XIAOMI_WMI > To compile this driver as a module, choose M here: the module will > be called xiaomi-wmi. > > +config SBL_FWU_WMI > + tristate "WMI driver for Slim Bootloader firmware update signaling" > + depends on ACPI_WMI > + help > + Say Y here if you want to be able to use the WMI interface to signal > + Slim Bootloader to trigger update on next reboot. > + > + To compile this driver as a module, choose M here: the module will > + be called sbl-fwu-wmi. > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > +obj-$(CONFIG_SBL_FWU_WMI) += sbl_fwu_wmi.o I didn't get an ordering schema in above files. Shouldn't be rather alphasort? ... > +static ssize_t firmware_update_request_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + bool val; > + int ret; > + > + ret = kstrtobool(buf, &val); > + if (ret) > + return ret; > + > + ret = set_fwu_request(dev, val ? 1 : 0); Hmm... If you are going to extend this, why not to pass integer directly? (And thus take one from user) > + if (ret) > + return ret; > + > + return count; > +} > + Extra blank line. > +static DEVICE_ATTR_RW(firmware_update_request); > + > +static struct attribute *firmware_update_attrs[] = { > + &dev_attr_firmware_update_request.attr, > + NULL > +}; > + Extra blank line. > +ATTRIBUTE_GROUPS(firmware_update); ... > +MODULE_DEVICE_TABLE(wmi, sbl_fwu_wmi_id_table); Move it closer to the table structure. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver 2020-04-22 13:42 ` Andy Shevchenko @ 2020-04-23 2:02 ` Jithu Joseph 2020-04-23 10:21 ` Andy Shevchenko 0 siblings, 1 reply; 5+ messages in thread From: Jithu Joseph @ 2020-04-23 2:02 UTC (permalink / raw) To: Andy Shevchenko Cc: Darren Hart, Andy Shevchenko, Platform Driver, Linux Kernel Mailing List, maurice.ma, ravi.p.rangarajan, sean.v.kelley, kuo-lang.tseng, jithu.joseph Hi Andy, Appreciate your valuable feedback. I think I understood most of your comments but I need clarification regarding the last comment in this reply. On Wed, 2020-04-22 at 16:42 +0300, Andy Shevchenko wrote: > On Mon, Apr 20, 2020 at 10:50 PM Jithu Joseph <jithu.joseph@intel.com > > wrote: > > > > > > This driver only implements a signaling mechanism, the actual > > firmware > > update process and various details like firmware update image > > format, > > firmware image location etc are defined by SBL [2] and are not in > > the > > scope of this driver. > > I have noticed that it misses ABI documentation. So, please add. Also > some comments below. I will add one via a new Documentation/ABI/testing/sysfs-platform-sbl- fwu-wmi file > > ... > > > [1] https://slimbootloader.github.io > > [2] https://slimbootloader.github.io/security/firmware-update.html > > Can you add a DocLink: tag below for the reference to the official > documentation? I wasnt aware of this tag. Will add this. > > ... > > > +SLIM BOOTLOADER (SBL) FIRMWARE UPDATE WMI DRIVER > > +M: Jithu Joseph <jithu.joseph@intel.com> > > +R: Maurice Ma <maurice.ma@intel.com> > > +S: Maintained > > +W: > > https://slimbootloader.github.io/security/firmware-update.html > > +F: drivers/platform/x86/sbl_fwu_wmi.c > > I hope you run latest and greatest version of checkpatch.pl and it's > okay with this section. Yes it was fine, chekpatch.pl was merely asking to update the MAINTAINERS file. And the ordering of the section matches with that of parse-maintainers.pl > > ... > > > @@ -114,6 +114,16 @@ config XIAOMI_WMI > > To compile this driver as a module, choose M here: the > > module will > > be called xiaomi-wmi. > > > > +config SBL_FWU_WMI > > + tristate "WMI driver for Slim Bootloader firmware update > > signaling" > > + depends on ACPI_WMI > > + help > > + Say Y here if you want to be able to use the WMI > > interface to signal > > + Slim Bootloader to trigger update on next reboot. > > + > > + To compile this driver as a module, choose M here: the > > module will > > + be called sbl-fwu-wmi. > > @@ -15,6 +15,7 @@ obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel- > > wmi-thunderbolt.o > > obj-$(CONFIG_MXM_WMI) += mxm-wmi.o > > obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o > > obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o > > +obj-$(CONFIG_SBL_FWU_WMI) += sbl_fwu_wmi.o > > I didn't get an ordering schema in above files. > Shouldn't be rather alphasort? Looks like there is an ordering within the wmi related files, so I will move mine in between PEAQ_WMI and XIAOMI_WMI . > > ... > > > +static ssize_t firmware_update_request_store(struct device *dev, > > + struct > > device_attribute *attr, > > + const char *buf, > > size_t count) > > +{ > > + bool val; > > + int ret; > > + > > + ret = kstrtobool(buf, &val); > > + if (ret) > > + return ret; > > + > > + ret = set_fwu_request(dev, val ? 1 : 0); > > Hmm... If you are going to extend this, why not to pass integer > directly? (And thus take one from user) We have also been thinking about extensibility … So I will modify the code to allow any u32 input by the user to be passed down via wmi_set_block(), though 0 and 1 will be the only inputs documented in the ABI today.( Or did you still mean to error out if the user input is something other than 0 or 1 ?) Thanks Jithu ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver 2020-04-23 2:02 ` Jithu Joseph @ 2020-04-23 10:21 ` Andy Shevchenko 0 siblings, 0 replies; 5+ messages in thread From: Andy Shevchenko @ 2020-04-23 10:21 UTC (permalink / raw) To: Jithu Joseph Cc: Darren Hart, Andy Shevchenko, Platform Driver, Linux Kernel Mailing List, maurice.ma, ravi.p.rangarajan, sean.v.kelley, kuo-lang.tseng On Thu, Apr 23, 2020 at 5:08 AM Jithu Joseph <jithu.joseph@intel.com> wrote: ... > > > +static ssize_t firmware_update_request_store(struct device *dev, > > > + struct > > > device_attribute *attr, > > > + const char *buf, > > > size_t count) > > > +{ > > > + bool val; > > > + int ret; > > > + > > > + ret = kstrtobool(buf, &val); > > > + if (ret) > > > + return ret; > > > + > > > + ret = set_fwu_request(dev, val ? 1 : 0); > > > > Hmm... If you are going to extend this, why not to pass integer > > directly? (And thus take one from user) > > We have also been thinking about extensibility … > > So I will modify the code to allow any u32 input by the user to be > passed down via wmi_set_block(), though 0 and 1 will be the only > inputs documented in the ABI today.( Or did you still mean to error > out if the user input is something other than 0 or 1 ?) I think the best approach is to allow to parse integer input (kstrtouint() is good enough) and return -ERANGE for values > 1. Also put a comment before that check why is done like this (some justification that interface may be extended in the future or so), and pass integer value directly to set_fwu_request(). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-23 10:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-20 19:44 [PATCH 0/1] platform/x86: Add Slim Bootloader firmware update support Jithu Joseph 2020-04-20 19:44 ` [PATCH 1/1] platform/x86: Add Slim Bootloader firmware update signaling driver Jithu Joseph 2020-04-22 13:42 ` Andy Shevchenko 2020-04-23 2:02 ` Jithu Joseph 2020-04-23 10:21 ` Andy Shevchenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.