| Message ID | a318e4edc13e5a3ff95b901871b8929746535715.1635317102.git.yu.c.chen@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Introduce Platform Firmware Runtime Update and Telemetry drivers | expand |
On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > Introduce the pfru_update driver which can be used for Platform Firmware > Runtime code injection and driver update [1]. The user is expected to > provide the update firmware in the form of capsule file, and pass it to > the driver via ioctl. Then the driver would hand this capsule file to the > Platform Firmware Runtime Update via the ACPI device _DSM method. At last > the low level Management Mode would do the firmware update. > > The corresponding userspace tool and man page will be introduced at > tools/power/acpi/pfru. > > Link: https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf # [1] > Signed-off-by: Chen Yu <yu.c.chen@intel.com> > --- > v7: Use ida_alloc() to allocate a ID, and release the ID when > device is removed. (Greg Kroah-Hartman) > Check the _DSM method at early stage, before allocate or parse > anything in acpi_pfru_probe(). (Greg Kroah-Hartman) > Set the parent of the misc device. (Greg Kroah-Hartman) > Use module_platform_driver() instead of platform_driver_register() > in module_init(). Separate pfru driver and pfru_telemetry driver > to two files. (Greg Kroah-Hartman) > v6: Use Link: tag to add the specification download address. > (Andy Shevchenko) > Remove linux/uuid.h and use raw buffers to contain uuid. > (Andy Shevchenko) > Drop comma for each terminator entry in the enum structure. > (Andy Shevchenko) > Remove redundant 'else' in get_image_type(). > (Andy Shevchenko) > Directly return results from the switch cases in adjust_efi_size() > and pfru_ioctl().(Andy Shevchenko) > Keep comment style consistent by removing the period for > one line comment. > (Andy Shevchenko) > Remove devm_kfree() if .probe() failed. > (Andy Shevchenko) > v5: Remove Documentation/ABI/pfru, and move the content to kernel doc > in include/uapi/linux/pfru.h (Greg Kroah-Hartman) > Shrink the range of ioctl numbers declared in > Documentation/userspace-api/ioctl/ioctl-number.rst > from 16 to 8. (Greg Kroah-Hartman) > Change global variable struct pfru_device *pfru_dev to > per ACPI device. (Greg Kroah-Hartman) > Unregister the misc device in acpi_pfru_remove(). > (Greg Kroah-Hartman) > Convert the kzalloc() to devm_kzalloc() in the driver so > as to avoid freeing the memory. (Greg Kroah-Hartman) > Fix the compile error by declaring the pfru_log_ioctl() as > static. (kernel test robot LKP) > Change to global variable misc_device to per ACPI device. > (Greg Kroah-Hartman) > v4: Replace all pr_err() with dev_dbg() (Greg Kroah-Hartman, > Rafael J. Wysocki) > Returns ENOTTY rather than ENOIOCTLCMD if invalid ioctl command > is provided. (Greg Kroah-Hartman) > Remove compat ioctl. (Greg Kroah-Hartman) > Rename /dev/pfru/pfru_update to /dev/acpi_pfru (Greg Kroah-Hartman) > Simplify the check for element of the package in query_capability() > (Rafael J. Wysocki) > Remove the loop in query_capability(), query_buffer() and query > the package elemenet directly. (Rafael J. Wysocki) > Check the the number of elements in case the number of package > elements is too small. (Rafael J. Wysocki) > Doing the assignment as initialization in get_image_type(). > Meanwhile, returns the type or a negative error code in > get_image_type(). (Rafael J. Wysocki) > Put the comments inside the function. (Rafael J. Wysocki) > Returns the size or a negative error code in adjust_efi_size() > (Rafael J. Wysocki) > Fix the return value from EFAULT to EINVAL if pfru_valid_revid() > does not pass. (Rafael J. Wysocki) > Change the write() to be the code injection/update, the read() to > be telemetry retrieval and all of the rest to be ioctl()s under > one special device file.(Rafael J. Wysocki) > Putting empty code lines after an if () statement that is not > followed by a block. (Rafael J. Wysocki) > Remove "goto" tags to make the code more readable. (Rafael J. Wysocki) > v3: Use __u32 instead of int and __64 instead of unsigned long > in include/uapi/linux/pfru.h (Greg Kroah-Hartman) > Rename the structure in uapi to start with a prefix pfru so as > to avoid confusing in the global namespace. (Greg Kroah-Hartman) > v2: Add sanity check for duplicated instance of ACPI device. > Update the driver to work with allocated pfru_device objects. > (Mike Rapoport) > For each switch case pair, get rid of the magic case numbers > and add a default clause with the error handling. > (Mike Rapoport) > Move the obj->type checks outside the switch to reduce redundancy. > (Mike Rapoport) > Parse the code_inj_id and drv_update_id at driver initialization time > to reduce the re-parsing at runtime.(Mike Rapoport) > Explain in detail how the size needs to be adjusted when doing > version check.(Mike Rapoport) > Rename parse_update_result() to dump_update_result()(Mike Rapoport) > Remove redundant return.(Mike Rapoport) > Do not expose struct capsulate_buf_info to uapi, since it is > not needed in userspace.(Mike Rapoport) > --- > .../userspace-api/ioctl/ioctl-number.rst | 1 + > drivers/acpi/Kconfig | 1 + > drivers/acpi/Makefile | 1 + > drivers/acpi/pfru/Kconfig | 13 + > drivers/acpi/pfru/Makefile | 2 + > drivers/acpi/pfru/pfru_update.c | 572 ++++++++++++++++++ > include/uapi/linux/pfru.h | 186 ++++++ > 7 files changed, 776 insertions(+) > create mode 100644 drivers/acpi/pfru/Kconfig > create mode 100644 drivers/acpi/pfru/Makefile > create mode 100644 drivers/acpi/pfru/pfru_update.c > create mode 100644 include/uapi/linux/pfru.h > > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst > index 2e8134059c87..6f7c86b6deb7 100644 > --- a/Documentation/userspace-api/ioctl/ioctl-number.rst > +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst > @@ -365,6 +365,7 @@ Code Seq# Include File Comments > <mailto:aherrman@de.ibm.com> > 0xE5 00-3F linux/fuse.h > 0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver > +0xEE 00-08 uapi/linux/pfru.h Platform Firmware Runtime Update and Telemetry > 0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development) > <mailto:thomas@winischhofer.net> > 0xF6 all LTTng Linux Trace Toolkit Next Generation > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig > index 1da360c51d66..1d8d2e2cefac 100644 > --- a/drivers/acpi/Kconfig > +++ b/drivers/acpi/Kconfig > @@ -482,6 +482,7 @@ source "drivers/acpi/nfit/Kconfig" > source "drivers/acpi/numa/Kconfig" > source "drivers/acpi/apei/Kconfig" > source "drivers/acpi/dptf/Kconfig" > +source "drivers/acpi/pfru/Kconfig" > > config ACPI_WATCHDOG > bool > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile > index 3018714e87d9..9c2c5ddff6ec 100644 > --- a/drivers/acpi/Makefile > +++ b/drivers/acpi/Makefile > @@ -102,6 +102,7 @@ obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o > obj-$(CONFIG_ACPI_SPCR_TABLE) += spcr.o > obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o > obj-$(CONFIG_ACPI_PPTT) += pptt.o > +obj-$(CONFIG_ACPI_PFRU) += pfru/ > > # processor has its own "processor." module_param namespace > processor-y := processor_driver.o > diff --git a/drivers/acpi/pfru/Kconfig b/drivers/acpi/pfru/Kconfig > new file mode 100644 > index 000000000000..5b31675b173a > --- /dev/null > +++ b/drivers/acpi/pfru/Kconfig > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0 > +config ACPI_PFRU > + tristate "ACPI Platform Firmware Runtime Update (PFRU)" > + depends on 64BIT > + help > + In order to reduce the system reboot times and update the platform firmware > + in time, Platform Firmware Runtime Update is leveraged to patch the system > + without reboot. This driver supports Platform Firmware Runtime Update, > + which is composed of two parts: code injection and driver update. It also > + allows telemetry data to be retrieved from the platform firmware. > + > + To compile this driver as a module, choose M here: > + the module will be called pfru_update. > diff --git a/drivers/acpi/pfru/Makefile b/drivers/acpi/pfru/Makefile > new file mode 100644 > index 000000000000..098cbe80cf3d > --- /dev/null > +++ b/drivers/acpi/pfru/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_ACPI_PFRU) += pfru_update.o > diff --git a/drivers/acpi/pfru/pfru_update.c b/drivers/acpi/pfru/pfru_update.c > new file mode 100644 > index 000000000000..a9953eb6c2d5 > --- /dev/null > +++ b/drivers/acpi/pfru/pfru_update.c > @@ -0,0 +1,572 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * ACPI Platform Firmware Runtime Update Device Driver > + * > + * Copyright (C) 2021 Intel Corporation > + * Author: Chen Yu <yu.c.chen@intel.com> > + */ > +#include <linux/acpi.h> > +#include <linux/device.h> > +#include <linux/efi.h> > +#include <linux/err.h> > +#include <linux/errno.h> > +#include <linux/file.h> > +#include <linux/fs.h> > +#include <linux/idr.h> > +#include <linux/miscdevice.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/string.h> > +#include <linux/uaccess.h> > +#include <linux/uio.h> > +#include <linux/uuid.h> > +#include <uapi/linux/pfru.h> > + > +enum cap_index { > + CAP_STATUS_IDX, > + CAP_UPDATE_IDX, > + CAP_CODE_TYPE_IDX, > + CAP_FW_VER_IDX, > + CAP_CODE_RT_VER_IDX, > + CAP_DRV_TYPE_IDX, > + CAP_DRV_RT_VER_IDX, > + CAP_DRV_SVN_IDX, > + CAP_PLAT_ID_IDX, > + CAP_OEM_ID_IDX, > + CAP_OEM_INFO_IDX, > + CAP_NR_IDX > +}; > + > +enum buf_index { > + BUF_STATUS_IDX, > + BUF_EXT_STATUS_IDX, > + BUF_ADDR_LOW_IDX, > + BUF_ADDR_HI_IDX, > + BUF_SIZE_IDX, > + BUF_NR_IDX > +}; > + > +enum update_index { > + UPDATE_STATUS_IDX, > + UPDATE_EXT_STATUS_IDX, > + UPDATE_AUTH_TIME_LOW_IDX, > + UPDATE_AUTH_TIME_HI_IDX, > + UPDATE_EXEC_TIME_LOW_IDX, > + UPDATE_EXEC_TIME_HI_IDX, > + UPDATE_NR_IDX > +}; > + > +struct pfru_device { > + guid_t uuid, code_uuid, drv_uuid; > + int rev_id, index; shouldn't rev_id be a u32 as you are copying this to/from userspace? > + struct device *dev; What device is this, the parent? Child? Sibling? need a hint... > + struct miscdevice miscdev; > +}; > + > +static DEFINE_IDA(pfru_ida); > + > +static inline struct pfru_device *to_pfru_dev(struct file *file) > +{ > + return container_of(file->private_data, struct pfru_device, miscdev); > +} > + > +static int query_capability(struct pfru_update_cap_info *cap, > + struct pfru_device *pfru_dev) > +{ > + union acpi_object *out_obj; > + acpi_handle handle; > + int ret = -EINVAL; > + > + handle = ACPI_HANDLE(pfru_dev->dev); > + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, > + pfru_dev->rev_id, > + FUNC_QUERY_UPDATE_CAP, > + NULL, ACPI_TYPE_PACKAGE); > + if (!out_obj) > + return ret; > + > + if (out_obj->package.count < CAP_NR_IDX) > + goto free_acpi_buffer; > + > + if (out_obj->package.elements[CAP_STATUS_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->status = out_obj->package.elements[CAP_STATUS_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_UPDATE_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->update_cap = out_obj->package.elements[CAP_UPDATE_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_CODE_TYPE_IDX].type != ACPI_TYPE_BUFFER) > + goto free_acpi_buffer; > + > + memcpy(&cap->code_type, > + out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.pointer, > + out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length); > + > + if (out_obj->package.elements[CAP_FW_VER_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->fw_version = > + out_obj->package.elements[CAP_FW_VER_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_CODE_RT_VER_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->code_rt_version = > + out_obj->package.elements[CAP_CODE_RT_VER_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_DRV_TYPE_IDX].type != ACPI_TYPE_BUFFER) > + goto free_acpi_buffer; > + > + memcpy(&cap->drv_type, > + out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.pointer, > + out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length); > + > + if (out_obj->package.elements[CAP_DRV_RT_VER_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->drv_rt_version = > + out_obj->package.elements[CAP_DRV_RT_VER_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_DRV_SVN_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + cap->drv_svn = > + out_obj->package.elements[CAP_DRV_SVN_IDX].integer.value; > + > + if (out_obj->package.elements[CAP_PLAT_ID_IDX].type != ACPI_TYPE_BUFFER) > + goto free_acpi_buffer; > + > + memcpy(&cap->platform_id, > + out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.pointer, > + out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length); > + > + if (out_obj->package.elements[CAP_OEM_ID_IDX].type != ACPI_TYPE_BUFFER) > + goto free_acpi_buffer; > + > + memcpy(&cap->oem_id, > + out_obj->package.elements[CAP_OEM_ID_IDX].buffer.pointer, > + out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length); > + ret = 0; > +free_acpi_buffer: > + ACPI_FREE(out_obj); > + > + return ret; > +} > + > +static int query_buffer(struct pfru_com_buf_info *info, > + struct pfru_device *pfru_dev) > +{ > + union acpi_object *out_obj; > + acpi_handle handle; > + int ret = -EINVAL; > + > + handle = ACPI_HANDLE(pfru_dev->dev); > + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, > + pfru_dev->rev_id, FUNC_QUERY_BUF, > + NULL, ACPI_TYPE_PACKAGE); > + if (!out_obj) > + return ret; > + > + if (out_obj->package.count < BUF_NR_IDX) > + goto free_acpi_buffer; > + > + if (out_obj->package.elements[BUF_STATUS_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + info->status = out_obj->package.elements[BUF_STATUS_IDX].integer.value; > + > + if (out_obj->package.elements[BUF_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + info->ext_status = > + out_obj->package.elements[BUF_EXT_STATUS_IDX].integer.value; > + > + if (out_obj->package.elements[BUF_ADDR_LOW_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + info->addr_lo = > + out_obj->package.elements[BUF_ADDR_LOW_IDX].integer.value; > + > + if (out_obj->package.elements[BUF_ADDR_HI_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + info->addr_hi = > + out_obj->package.elements[BUF_ADDR_HI_IDX].integer.value; > + > + if (out_obj->package.elements[BUF_SIZE_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + info->buf_size = out_obj->package.elements[BUF_SIZE_IDX].integer.value; > + > + ret = 0; > +free_acpi_buffer: > + ACPI_FREE(out_obj); > + > + return ret; > +} > + > +static int get_image_type(struct efi_manage_capsule_image_header *img_hdr, > + struct pfru_device *pfru_dev) > +{ > + guid_t *image_type_id = &img_hdr->image_type_id; > + > + /* check whether this is a code injection or driver update */ > + if (guid_equal(image_type_id, &pfru_dev->code_uuid)) > + return CODE_INJECT_TYPE; > + > + if (guid_equal(image_type_id, &pfru_dev->drv_uuid)) > + return DRIVER_UPDATE_TYPE; > + > + return -EINVAL; > +} > + > +static int adjust_efi_size(struct efi_manage_capsule_image_header *img_hdr, > + int size) > +{ > + /* > + * The (u64 hw_ins) was introduced in UEFI spec version 2, > + * and (u64 capsule_support) was introduced in version 3. > + * The size needs to be adjusted accordingly. That is to > + * say, version 1 should subtract the size of hw_ins+capsule_support, > + * and version 2 should sbstract the size of capsule_support. > + */ > + size += sizeof(struct efi_manage_capsule_image_header); > + switch (img_hdr->ver) { > + case 1: > + return size - 2 * sizeof(u64); > + case 2: > + return size - sizeof(u64); > + default: > + /* only support version 1 and 2 */ > + return -EINVAL; > + } > +} > + > +static bool valid_version(const void *data, struct pfru_update_cap_info *cap, > + struct pfru_device *pfru_dev) > +{ > + struct pfru_payload_hdr *payload_hdr; > + efi_capsule_header_t *cap_hdr; > + struct efi_manage_capsule_header *m_hdr; > + struct efi_manage_capsule_image_header *m_img_hdr; > + struct efi_image_auth *auth; > + int type, size; > + > + /* > + * Sanity check if the capsule image has a newer version > + * than current one. > + */ > + cap_hdr = (efi_capsule_header_t *)data; > + size = cap_hdr->headersize; > + m_hdr = (struct efi_manage_capsule_header *)(data + size); > + /* > + * Current data structure size plus variable array indicated > + * by number of (emb_drv_cnt + payload_cnt) > + */ > + size += sizeof(struct efi_manage_capsule_header) + > + (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64); > + m_img_hdr = (struct efi_manage_capsule_image_header *)(data + size); > + > + type = get_image_type(m_img_hdr, pfru_dev); > + if (type < 0) > + return false; > + > + size = adjust_efi_size(m_img_hdr, size); > + if (size < 0) > + return false; > + > + auth = (struct efi_image_auth *)(data + size); > + size += sizeof(u64) + auth->auth_info.hdr.len; > + payload_hdr = (struct pfru_payload_hdr *)(data + size); > + > + /* finally compare the version */ > + if (type == CODE_INJECT_TYPE) > + return payload_hdr->rt_ver >= cap->code_rt_version; > + else > + return payload_hdr->rt_ver >= cap->drv_rt_version; > +} > + > +static void dump_update_result(struct pfru_updated_result *result, > + struct pfru_device *pfru_dev) > +{ > + dev_dbg(pfru_dev->dev, "Update result:\n"); > + dev_dbg(pfru_dev->dev, "Status:%d\n", result->status); > + dev_dbg(pfru_dev->dev, "Extended Status:%d\n", result->ext_status); > + dev_dbg(pfru_dev->dev, "Authentication Time Low:%lld\n", > + result->low_auth_time); > + dev_dbg(pfru_dev->dev, "Authentication Time High:%lld\n", > + result->high_auth_time); > + dev_dbg(pfru_dev->dev, "Execution Time Low:%lld\n", > + result->low_exec_time); > + dev_dbg(pfru_dev->dev, "Execution Time High:%lld\n", > + result->high_exec_time); > +} > + > +static int start_acpi_update(int action, struct pfru_device *pfru_dev) > +{ > + union acpi_object *out_obj, in_obj, in_buf; > + struct pfru_updated_result update_result; > + acpi_handle handle; > + int ret = -EINVAL; > + > + memset(&in_obj, 0, sizeof(in_obj)); > + memset(&in_buf, 0, sizeof(in_buf)); > + in_obj.type = ACPI_TYPE_PACKAGE; > + in_obj.package.count = 1; > + in_obj.package.elements = &in_buf; > + in_buf.type = ACPI_TYPE_INTEGER; > + in_buf.integer.value = action; > + > + handle = ACPI_HANDLE(pfru_dev->dev); > + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, > + pfru_dev->rev_id, FUNC_START, > + &in_obj, ACPI_TYPE_PACKAGE); > + if (!out_obj) > + return ret; > + > + if (out_obj->package.count < UPDATE_NR_IDX) > + goto free_acpi_buffer; > + > + if (out_obj->package.elements[UPDATE_STATUS_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.status = > + out_obj->package.elements[UPDATE_STATUS_IDX].integer.value; > + > + if (out_obj->package.elements[UPDATE_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.ext_status = > + out_obj->package.elements[UPDATE_EXT_STATUS_IDX].integer.value; > + > + if (out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.low_auth_time = > + out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].integer.value; > + > + if (out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.high_auth_time = > + out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].integer.value; > + > + if (out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.low_exec_time = > + out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].integer.value; > + > + if (out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].type != ACPI_TYPE_INTEGER) > + goto free_acpi_buffer; > + > + update_result.high_exec_time = > + out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].integer.value; > + > + dump_update_result(&update_result, pfru_dev); > + ret = 0; > + > +free_acpi_buffer: > + ACPI_FREE(out_obj); > + > + return ret; > +} > + > +static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > +{ > + struct pfru_update_cap_info cap; You have a variable-length structure on the stack. What ever could go wrong with that? {sigh} How was this tested??? > + struct pfru_device *pfru_dev; > + void __user *p; > + int ret, rev; > + > + pfru_dev = to_pfru_dev(file); > + p = (void __user *)arg; > + > + switch (cmd) { > + case PFRU_IOC_QUERY_CAP: > + ret = query_capability(&cap, pfru_dev); > + if (ret) > + return ret; > + > + if (copy_to_user(p, &cap, sizeof(cap))) But "sizeof" is not the real size of this structure, it's a variable length structure, right? Again, how was this tested? > + return -EFAULT; > + > + return 0; > + case PFRU_IOC_SET_REV: > + if (copy_from_user(&rev, p, sizeof(unsigned int))) But this isn't an unsigned int, it's an "int" here. So again, make this explicit, probably u32, right? > + return -EFAULT; > + > + if (!pfru_valid_revid(rev)) > + return -EINVAL; > + > + pfru_dev->rev_id = rev; Why can't this just be a sysfs file? Heck, why can't all of these be sysfs files, why do you need a char device at all? Who is going to use this thing? > + > + return 0; > + case PFRU_IOC_STAGE: > + return start_acpi_update(START_STAGE, pfru_dev); > + case PFRU_IOC_ACTIVATE: > + return start_acpi_update(START_ACTIVATE, pfru_dev); > + case PFRU_IOC_STAGE_ACTIVATE: > + return start_acpi_update(START_STAGE_ACTIVATE, pfru_dev); > + default: > + return -ENOTTY; > + } > +} > + > +static ssize_t pfru_write(struct file *file, const char __user *buf, > + size_t len, loff_t *ppos) > +{ > + struct pfru_update_cap_info cap; > + struct pfru_com_buf_info info; > + struct pfru_device *pfru_dev; > + phys_addr_t phy_addr; > + struct iov_iter iter; > + struct iovec iov; > + char *buf_ptr; > + int ret; > + > + pfru_dev = to_pfru_dev(file); > + > + ret = query_buffer(&info, pfru_dev); > + if (ret) > + return ret; > + > + if (len > info.buf_size) > + return -EINVAL; > + > + iov.iov_base = (void __user *)buf; > + iov.iov_len = len; > + iov_iter_init(&iter, WRITE, &iov, 1, len); > + > + /* map the communication buffer */ > + phy_addr = (phys_addr_t)(info.addr_lo | (info.addr_hi << 32)); > + buf_ptr = memremap(phy_addr, info.buf_size, MEMREMAP_WB); > + if (IS_ERR(buf_ptr)) > + return PTR_ERR(buf_ptr); > + > + if (!copy_from_iter_full(buf_ptr, len, &iter)) { > + ret = -EINVAL; > + goto unmap; > + } > + > + /* check if the capsule header has a valid version number */ > + ret = query_capability(&cap, pfru_dev); > + if (ret) > + goto unmap; > + > + if (cap.status != DSM_SUCCEED) > + ret = -EBUSY; > + else if (!valid_version(buf_ptr, &cap, pfru_dev)) > + ret = -EINVAL; > +unmap: > + memunmap(buf_ptr); > + > + return ret ?: len; > +} > + > +static const struct file_operations acpi_pfru_fops = { > + .owner = THIS_MODULE, > + .write = pfru_write, > + .unlocked_ioctl = pfru_ioctl, > + .llseek = noop_llseek, > +}; > + > +static int acpi_pfru_remove(struct platform_device *pdev) > +{ > + struct pfru_device *pfru_dev = platform_get_drvdata(pdev); > + > + misc_deregister(&pfru_dev->miscdev); > + ida_free(&pfru_ida, pfru_dev->index); > + > + return 0; > +} > + > +static int acpi_pfru_probe(struct platform_device *pdev) > +{ > + struct pfru_device *pfru_dev; > + acpi_handle handle; > + int ret; > + > + handle = ACPI_HANDLE(&pdev->dev); > + if (!acpi_has_method(handle, "_DSM")) { > + dev_dbg(&pdev->dev, "Missing _DSM\n"); > + return -ENODEV; > + } > + > + pfru_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_dev), GFP_KERNEL); > + if (!pfru_dev) > + return -ENOMEM; > + > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > + if (ret) > + return ret; > + > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > + if (ret) > + return ret; > + > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > + if (ret) > + return ret; > + > + ret = ida_alloc(&pfru_ida, GFP_KERNEL); > + if (ret < 0) > + return ret; > + > + pfru_dev->index = ret; > + /* default rev id is 1 */ > + pfru_dev->rev_id = 1; > + pfru_dev->dev = &pdev->dev; > + > + pfru_dev->miscdev.minor = MISC_DYNAMIC_MINOR; > + pfru_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "pfru%d", pfru_dev->index); > + if (!pfru_dev->miscdev.name) { > + ret = -ENOMEM; > + goto failed; > + } > + > + pfru_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL, > + "acpi_pfru%d", pfru_dev->index); > + if (!pfru_dev->miscdev.nodename) { > + ret = -ENOMEM; > + goto failed; > + } > + > + pfru_dev->miscdev.fops = &acpi_pfru_fops; > + pfru_dev->miscdev.parent = &pdev->dev; > + > + ret = misc_register(&pfru_dev->miscdev); > + if (ret) > + goto failed; > + > + platform_set_drvdata(pdev, pfru_dev); > + > + return 0; > +failed: > + ida_free(&pfru_ida, pfru_dev->index); > + > + return ret; > +} > + > +static const struct acpi_device_id acpi_pfru_ids[] = { > + {"INTC1080", 0}, > + {} > +}; > +MODULE_DEVICE_TABLE(acpi, acpi_pfru_ids); > + > +static struct platform_driver acpi_pfru_driver = { > + .driver = { > + .name = "pfru_update", > + .acpi_match_table = acpi_pfru_ids, > + }, > + .probe = acpi_pfru_probe, > + .remove = acpi_pfru_remove, > +}; > +module_platform_driver(acpi_pfru_driver); Are you cleaning up your ida structure when the module is unloaded? > + > +MODULE_DESCRIPTION("Platform Firmware Runtime Update device driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h > new file mode 100644 > index 000000000000..d6f57a75afd2 > --- /dev/null > +++ b/include/uapi/linux/pfru.h > @@ -0,0 +1,186 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* > + * Platform Firmware Runtime Update header > + * > + * Copyright(c) 2021 Intel Corporation. All rights reserved. > + */ > +#ifndef __PFRU_H__ > +#define __PFRU_H__ > + > +#include <linux/types.h> > +#include <linux/ioctl.h> > + > +#define PFRU_UUID "ECF9533B-4A3C-4E89-939E-C77112601C6D" > +#define PFRU_CODE_INJ_UUID "B2F84B79-7B6E-4E45-885F-3FB9BB185402" > +#define PFRU_DRV_UPDATE_UUID "4569DD8C-75F1-429A-A3D6-24DE8097A0DF" > + > +#define FUNC_STANDARD_QUERY 0 > +#define FUNC_QUERY_UPDATE_CAP 1 > +#define FUNC_QUERY_BUF 2 > +#define FUNC_START 3 > + > +#define CODE_INJECT_TYPE 1 > +#define DRIVER_UPDATE_TYPE 2 > + > +#define REVID_1 1 > +#define REVID_2 2 > + > +#define PFRU_MAGIC 0xEE > + > +#define UUID_SIZE 16 > + > +/** > + * PFRU_IOC_SET_REV - _IOW(PFRU_MAGIC, 0x01, unsigned int) > + * > + * Return: 0 on success, -errno on failure > + * > + * Set the Revision ID for PFRU Runtime Update. It could be either 1 or 2. > + */ > +#define PFRU_IOC_SET_REV _IOW(PFRU_MAGIC, 0x01, unsigned int) > +/** Please use blank lines between these, it's hard to read. > + * PFRU_IOC_STAGE - _IOW(PFRU_MAGIC, 0x02, unsigned int) > + * > + * Return: 0 on success, -errno on failure What -errno? Same for all of these, be explicit please. > + * > + * Stage a capsule image from communication buffer and perform authentication. > + */ > +#define PFRU_IOC_STAGE _IOW(PFRU_MAGIC, 0x02, unsigned int) > +/** > + * PFRU_IOC_ACTIVATE - _IOW(PFRU_MAGIC, 0x03, unsigned int) > + * > + * Return: 0 on success, -errno on failure > + * > + * Activate a previous staged capsule image. > + */ > +#define PFRU_IOC_ACTIVATE _IOW(PFRU_MAGIC, 0x03, unsigned int) > +/** > + * PFRU_IOC_STAGE_ACTIVATE - _IOW(PFRU_MAGIC, 0x04, unsigned int) > + * > + * Return: 0 on success, -errno on failure > + * > + * Perform both stage and activation actions. > + */ > +#define PFRU_IOC_STAGE_ACTIVATE _IOW(PFRU_MAGIC, 0x04, unsigned int) > +/** > + * PFRU_IOC_QUERY_CAP - _IOR(PFRU_MAGIC, 0x05, > + * struct pfru_update_cap_info) > + * > + * Return: 0 on success, -errno on failure. > + * > + * Retrieve information about the PFRU Runtime Update capability. > + * The information is a struct pfru_update_cap_info. > + */ > +#define PFRU_IOC_QUERY_CAP _IOR(PFRU_MAGIC, 0x05, struct pfru_update_cap_info) > + > +static inline int pfru_valid_revid(int id) > +{ > + return id == REVID_1 || id == REVID_2; Why is this in a uapi file? You just poluted the global namespace of all userspace programs with this function :( > +} > + > +/** > + * struct pfru_payload_hdr - Capsule file payload header. > + * > + * @sig: Signature of this capsule file. > + * @hdr_version: Revision of this header structure. > + * @hdr_size: Size of this header, including the OemHeader bytes. > + * @hw_ver: The supported firmware version. > + * @rt_ver: Version of the code injection image. > + * @platform_id: A platform specific GUID to specify the platform what > + * this capsule image support. > + */ > +struct pfru_payload_hdr { > + __u32 sig; > + __u32 hdr_version; > + __u32 hdr_size; > + __u32 hw_ver; > + __u32 rt_ver; > + __u8 platform_id[UUID_SIZE]; > +}; > + > +enum pfru_start_action { > + START_STAGE, > + START_ACTIVATE, > + START_STAGE_ACTIVATE, > +}; > + > +enum pfru_dsm_status { > + DSM_SUCCEED, > + DSM_FUNC_NOT_SUPPORT, > + DSM_INVAL_INPUT, > + DSM_HARDWARE_ERR, > + DSM_RETRY_SUGGESTED, > + DSM_UNKNOWN, > + DSM_FUNC_SPEC_ERR, Do you need to be explicit about the numbers here? I never remember if this is guaranteed by all C compilers or not... > +}; > + > +/** > + * struct pfru_update_cap_info - Runtime update capability information. > + * > + * @status: Indicator of whether this query succeed. > + * @update_cap: Bitmap to indicate whether the feature is supported. > + * @code_type: A buffer containing an image type GUID. > + * @fw_version: Platform firmware version. > + * @code_rt_version: Code injection runtime version for anti-rollback. > + * @drv_type: A buffer containing an image type GUID. > + * @drv_rt_version: The version of the driver update runtime code. > + * @drv_svn: The secure version number(SVN) of the driver update runtime code. > + * @platform_id: A buffer containing a platform ID GUID. > + * @oem_id: A buffer containing an OEM ID GUID. > + * @oem_info: A buffer containing the vendor specific information. > + */ > +struct pfru_update_cap_info { > + __u32 status; > + __u32 update_cap; > + > + __u8 code_type[UUID_SIZE]; Don't we have a uuid structure that crosses the user/kernel boundry? > + __u32 fw_version; > + __u32 code_rt_version; > + > + __u8 drv_type[UUID_SIZE]; > + __u32 drv_rt_version; > + __u32 drv_svn; > + > + __u8 platform_id[UUID_SIZE]; > + __u8 oem_id[UUID_SIZE]; > + > + char oem_info[]; As mentioned above, this is variable length, right? Where are you copying this extra data and how big do you know it is going to be? thanks, greg k-h
On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > Introduce the pfru_update driver which can be used for Platform Firmware > Runtime code injection and driver update [1]. The user is expected to > provide the update firmware in the form of capsule file, and pass it to > the driver via ioctl. Then the driver would hand this capsule file to the > Platform Firmware Runtime Update via the ACPI device _DSM method. At last > the low level Management Mode would do the firmware update. > > The corresponding userspace tool and man page will be introduced at > tools/power/acpi/pfru. ... > +static int get_image_type(struct efi_manage_capsule_image_header *img_hdr, > + struct pfru_device *pfru_dev) > +{ > + guid_t *image_type_id = &img_hdr->image_type_id; efi_guid_t ? > + /* check whether this is a code injection or driver update */ > + if (guid_equal(image_type_id, &pfru_dev->code_uuid)) > + return CODE_INJECT_TYPE; > + > + if (guid_equal(image_type_id, &pfru_dev->drv_uuid)) > + return DRIVER_UPDATE_TYPE; > + > + return -EINVAL; > +} ... > +static bool valid_version(const void *data, struct pfru_update_cap_info *cap, > + struct pfru_device *pfru_dev) > +{ > + struct pfru_payload_hdr *payload_hdr; > + efi_capsule_header_t *cap_hdr; > + struct efi_manage_capsule_header *m_hdr; > + struct efi_manage_capsule_image_header *m_img_hdr; > + struct efi_image_auth *auth; > + int type, size; > + > + /* > + * Sanity check if the capsule image has a newer version > + * than current one. > + */ > + cap_hdr = (efi_capsule_header_t *)data; Why casting? > + size = cap_hdr->headersize; > + m_hdr = (struct efi_manage_capsule_header *)(data + size); > + /* > + * Current data structure size plus variable array indicated > + * by number of (emb_drv_cnt + payload_cnt) > + */ > + size += sizeof(struct efi_manage_capsule_header) + > + (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64); > + m_img_hdr = (struct efi_manage_capsule_image_header *)(data + size); > + > + type = get_image_type(m_img_hdr, pfru_dev); > + if (type < 0) > + return false; > + > + size = adjust_efi_size(m_img_hdr, size); > + if (size < 0) > + return false; > + > + auth = (struct efi_image_auth *)(data + size); > + size += sizeof(u64) + auth->auth_info.hdr.len; > + payload_hdr = (struct pfru_payload_hdr *)(data + size); > + > + /* finally compare the version */ > + if (type == CODE_INJECT_TYPE) > + return payload_hdr->rt_ver >= cap->code_rt_version; > + else > + return payload_hdr->rt_ver >= cap->drv_rt_version; > +} ... > +static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > +{ > + struct pfru_update_cap_info cap; > + struct pfru_device *pfru_dev; > + void __user *p; > + int ret, rev; > + pfru_dev = to_pfru_dev(file); > + p = (void __user *)arg; Can be combined with definitions above. Ditto for the rest cases in the code. > +} ... > + phy_addr = (phys_addr_t)(info.addr_lo | (info.addr_hi << 32)); Does it compile without warnings for 32-bit target? ... > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > + if (ret) > + return ret; > + > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > + if (ret) > + return ret; > + > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > + if (ret) > + return ret; Why do you need to keep zillions of copies of the data which seems is not going to be changed? Three global variables should be enough, no? > + ret = ida_alloc(&pfru_ida, GFP_KERNEL); > + if (ret < 0) > + return ret; > + > + pfru_dev->index = ret; ... > + /* default rev id is 1 */ Shouldn't you rather define this magic and drop this doubtful comment? > + pfru_dev->rev_id = 1; ... > +failed: Make you labeling consistent. The usual pattern is to explain what will be happened when goto to the certain label, for example, here is 'err_free_ida'. Also, add an empty line everywhere before labels. > + ida_free(&pfru_ida, pfru_dev->index); > + > + return ret; > +} ... > +#define UUID_SIZE 16 It must not be here at all. Or it should be properly namespaced.
On Wed, Oct 27, 2021 at 09:22:41AM +0200, Greg Kroah-Hartman wrote: > On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > > Introduce the pfru_update driver which can be used for Platform Firmware > > Runtime code injection and driver update [1]. The user is expected to > > provide the update firmware in the form of capsule file, and pass it to > > the driver via ioctl. Then the driver would hand this capsule file to the > > Platform Firmware Runtime Update via the ACPI device _DSM method. At last > > the low level Management Mode would do the firmware update. > > > > The corresponding userspace tool and man page will be introduced at > > tools/power/acpi/pfru. > > > > Link: https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf # [1] > > Signed-off-by: Chen Yu <yu.c.chen@intel.com> > > --- [snip...] > > +struct pfru_device { > > + guid_t uuid, code_uuid, drv_uuid; > > + int rev_id, index; > > shouldn't rev_id be a u32 as you are copying this to/from userspace? > Ok, will do. > > + struct device *dev; > > What device is this, the parent? Child? Sibling? need a hint... > It is the parent of the misc device, will add this hint. > > + struct miscdevice miscdev; > > +}; > > + > > +static DEFINE_IDA(pfru_ida); > > + > > +static inline struct pfru_device *to_pfru_dev(struct file *file) > > +{ > > + return container_of(file->private_data, struct pfru_device, miscdev); > > +} > > + > > + [snip...] > > +static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > > +{ > > + struct pfru_update_cap_info cap; > > You have a variable-length structure on the stack. What ever could go > wrong with that? > > {sigh} > > How was this tested??? > Ah, the risk of stack content overwrite. This symptom did not break out during previous test because currently the code would not manipulate on this variable-length array. I'll fix this in next version. > > + struct pfru_device *pfru_dev; > > + void __user *p; > > + int ret, rev; > > + > > + pfru_dev = to_pfru_dev(file); > > + p = (void __user *)arg; > > + > > + switch (cmd) { > > + case PFRU_IOC_QUERY_CAP: > > + ret = query_capability(&cap, pfru_dev); > > + if (ret) > > + return ret; > > + > > + if (copy_to_user(p, &cap, sizeof(cap))) > > But "sizeof" is not the real size of this structure, it's a variable > length structure, right? > > Again, how was this tested? > The variable length part was missing, and the userspace did not request to retrieve that part for now, so the bug was not triggered. I'll fix it. > > + return -EFAULT; > > + > > + return 0; > > + case PFRU_IOC_SET_REV: > > + if (copy_from_user(&rev, p, sizeof(unsigned int))) > > But this isn't an unsigned int, it's an "int" here. > > So again, make this explicit, probably u32, right? > Ok, I'll change it to u32. > > + return -EFAULT; > > + > > + if (!pfru_valid_revid(rev)) > > + return -EINVAL; > > + > > + pfru_dev->rev_id = rev; > > Why can't this just be a sysfs file? > > Heck, why can't all of these be sysfs files, why do you need a char > device at all? Who is going to use this thing? > > The reason why char device mode was introduced is that, this use case needs open/close/mmap/write and some extra actions(expose/adjust the parameters), which fits well into the special device file model. And it brings low coupling with other components. It could be sysfs based, then the capsule file could be put in a fixed path such as /lib/firmware/pfru, and load the capsule file from userspace via request_firmware(). The firmware update will be triggered if user touches /sys/firmware/acpi/pfru/start. > > + > > + return 0; > > + case PFRU_IOC_STAGE: > > + return start_acpi_update(START_STAGE, pfru_dev); > > + case PFRU_IOC_ACTIVATE: > > + return start_acpi_update(START_ACTIVATE, pfru_dev); > > + case PFRU_IOC_STAGE_ACTIVATE: > > + return start_acpi_update(START_STAGE_ACTIVATE, pfru_dev); > > + default: > > + return -ENOTTY; > > + } > > +} > > + > > +static int acpi_pfru_remove(struct platform_device *pdev) > > +{ > > + struct pfru_device *pfru_dev = platform_get_drvdata(pdev); > > + > > + misc_deregister(&pfru_dev->miscdev); > > + ida_free(&pfru_ida, pfru_dev->index); > > + > > + return 0; > > +} [snip] > > + .probe = acpi_pfru_probe, > > + .remove = acpi_pfru_remove, > > +}; > > +module_platform_driver(acpi_pfru_driver); > > Are you cleaning up your ida structure when the module is unloaded? > The ida index is released in acpi_pfru_remove(), while the ida stricture is a global variable, it should be cleared/invalid during module unload? > > + > > +MODULE_DESCRIPTION("Platform Firmware Runtime Update device driver"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h > > new file mode 100644 > > index 000000000000..d6f57a75afd2 > > --- /dev/null > > +++ b/include/uapi/linux/pfru.h > > @@ -0,0 +1,186 @@ > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > +/* > > + * Platform Firmware Runtime Update header > > + * > > + * Copyright(c) 2021 Intel Corporation. All rights reserved. > > + */ > > + > > +/** > > + * PFRU_IOC_SET_REV - _IOW(PFRU_MAGIC, 0x01, unsigned int) > > + * > > + * Return: 0 on success, -errno on failure > > + * > > + * Set the Revision ID for PFRU Runtime Update. It could be either 1 or 2. > > + */ > > +#define PFRU_IOC_SET_REV _IOW(PFRU_MAGIC, 0x01, unsigned int) > > +/** > > Please use blank lines between these, it's hard to read. > > Ok, will do. > > + * PFRU_IOC_STAGE - _IOW(PFRU_MAGIC, 0x02, unsigned int) > > + * > > + * Return: 0 on success, -errno on failure > > What -errno? > > Same for all of these, be explicit please. > Ok, will do. > > + * > > + * Stage a capsule image from communication buffer and perform authentication. > > + */ > > +#define PFRU_IOC_STAGE _IOW(PFRU_MAGIC, 0x02, unsigned int) > > +/** > > + * PFRU_IOC_ACTIVATE - _IOW(PFRU_MAGIC, 0x03, unsigned int) > > + * > > + * Return: 0 on success, -errno on failure > > + * > > + * Activate a previous staged capsule image. > > + */ > > +#define PFRU_IOC_ACTIVATE _IOW(PFRU_MAGIC, 0x03, unsigned int) > > +/** > > + * PFRU_IOC_STAGE_ACTIVATE - _IOW(PFRU_MAGIC, 0x04, unsigned int) > > + * > > + * Return: 0 on success, -errno on failure > > + * > > + * Perform both stage and activation actions. > > + */ > > +#define PFRU_IOC_STAGE_ACTIVATE _IOW(PFRU_MAGIC, 0x04, unsigned int) > > +/** > > + * PFRU_IOC_QUERY_CAP - _IOR(PFRU_MAGIC, 0x05, > > + * struct pfru_update_cap_info) > > + * > > + * Return: 0 on success, -errno on failure. > > + * > > + * Retrieve information about the PFRU Runtime Update capability. > > + * The information is a struct pfru_update_cap_info. > > + */ > > +#define PFRU_IOC_QUERY_CAP _IOR(PFRU_MAGIC, 0x05, struct pfru_update_cap_info) > > + > > +static inline int pfru_valid_revid(int id) > > +{ > > + return id == REVID_1 || id == REVID_2; > > Why is this in a uapi file? You just poluted the global namespace of > all userspace programs with this function :( > I'll remove this function in uapi header. > > +} > > + > > +/** > > + * struct pfru_payload_hdr - Capsule file payload header. > > + * > > + * @sig: Signature of this capsule file. > > + * @hdr_version: Revision of this header structure. > > + * @hdr_size: Size of this header, including the OemHeader bytes. > > + * @hw_ver: The supported firmware version. > > + * @rt_ver: Version of the code injection image. > > + * @platform_id: A platform specific GUID to specify the platform what > > + * this capsule image support. > > + */ > > +struct pfru_payload_hdr { > > + __u32 sig; > > + __u32 hdr_version; > > + __u32 hdr_size; > > + __u32 hw_ver; > > + __u32 rt_ver; > > + __u8 platform_id[UUID_SIZE]; > > +}; > > + > > +enum pfru_start_action { > > + START_STAGE, > > + START_ACTIVATE, > > + START_STAGE_ACTIVATE, > > +}; > > + > > +enum pfru_dsm_status { > > + DSM_SUCCEED, > > + DSM_FUNC_NOT_SUPPORT, > > + DSM_INVAL_INPUT, > > + DSM_HARDWARE_ERR, > > + DSM_RETRY_SUGGESTED, > > + DSM_UNKNOWN, > > + DSM_FUNC_SPEC_ERR, > > Do you need to be explicit about the numbers here? I never remember if > this is guaranteed by all C compilers or not... > Ok, will set them explicitly. > > +}; > > + > > +/** > > + * struct pfru_update_cap_info - Runtime update capability information. > > + * > > + * @status: Indicator of whether this query succeed. > > + * @update_cap: Bitmap to indicate whether the feature is supported. > > + * @code_type: A buffer containing an image type GUID. > > + * @fw_version: Platform firmware version. > > + * @code_rt_version: Code injection runtime version for anti-rollback. > > + * @drv_type: A buffer containing an image type GUID. > > + * @drv_rt_version: The version of the driver update runtime code. > > + * @drv_svn: The secure version number(SVN) of the driver update runtime code. > > + * @platform_id: A buffer containing a platform ID GUID. > > + * @oem_id: A buffer containing an OEM ID GUID. > > + * @oem_info: A buffer containing the vendor specific information. > > + */ > > +struct pfru_update_cap_info { > > + __u32 status; > > + __u32 update_cap; > > + > > + __u8 code_type[UUID_SIZE]; > > Don't we have a uuid structure that crosses the user/kernel boundry? > If guid_t is used then the header linux/uuid.h is needed in kernel space, while uuid/uuid.h is needed in the userspace. Then the include part in uAPI header becomes: #ifdef __KERNEL__ #include <linux/uuid.h> #else #include <uuid/uuid.h> #endif which looks more complicated than using the raw buffer. > > + __u32 fw_version; > > + __u32 code_rt_version; > > + > > + __u8 drv_type[UUID_SIZE]; > > + __u32 drv_rt_version; > > + __u32 drv_svn; > > + > > + __u8 platform_id[UUID_SIZE]; > > + __u8 oem_id[UUID_SIZE]; > > + > > + char oem_info[]; > > As mentioned above, this is variable length, right? Where are you > copying this extra data and how big do you know it is going to be? > Ah, right, will fix it in next version. Thanks, Chenyu
On Wed, Oct 27, 2021 at 01:20:33PM +0300, Andy Shevchenko wrote: > On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > > Introduce the pfru_update driver which can be used for Platform Firmware > > Runtime code injection and driver update [1]. The user is expected to > > provide the update firmware in the form of capsule file, and pass it to > > the driver via ioctl. Then the driver would hand this capsule file to the > > Platform Firmware Runtime Update via the ACPI device _DSM method. At last > > the low level Management Mode would do the firmware update. > > > > The corresponding userspace tool and man page will be introduced at > > tools/power/acpi/pfru. > > ... > > > +static int get_image_type(struct efi_manage_capsule_image_header *img_hdr, > > + struct pfru_device *pfru_dev) > > +{ > > + guid_t *image_type_id = &img_hdr->image_type_id; > > efi_guid_t ? > efi_guid_t is a 32-bit aligned guid_t, which is for the case when efi_guid_t* arguments are 32-bit aligned. And it is for 32-bit ARM. Since this code injection is only for 64-bit, the guid is not required to be strictly 32-bit aligned I suppose? > > + /* check whether this is a code injection or driver update */ > > + if (guid_equal(image_type_id, &pfru_dev->code_uuid)) > > + return CODE_INJECT_TYPE; > > + > > + if (guid_equal(image_type_id, &pfru_dev->drv_uuid)) > > + return DRIVER_UPDATE_TYPE; > > + > > + return -EINVAL; > > +} > > ... > > > +static bool valid_version(const void *data, struct pfru_update_cap_info *cap, > > + struct pfru_device *pfru_dev) > > +{ > > + struct pfru_payload_hdr *payload_hdr; > > + efi_capsule_header_t *cap_hdr; > > + struct efi_manage_capsule_header *m_hdr; > > + struct efi_manage_capsule_image_header *m_img_hdr; > > + struct efi_image_auth *auth; > > + int type, size; > > + > > + /* > > + * Sanity check if the capsule image has a newer version > > + * than current one. > > + */ > > + cap_hdr = (efi_capsule_header_t *)data; > > Why casting? > Will remove this in next version. > > + size = cap_hdr->headersize; > > + m_hdr = (struct efi_manage_capsule_header *)(data + size); > > + /* > > + * Current data structure size plus variable array indicated > > + * by number of (emb_drv_cnt + payload_cnt) > > + */ > > + size += sizeof(struct efi_manage_capsule_header) + > > + (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64); > > + m_img_hdr = (struct efi_manage_capsule_image_header *)(data + size); > > + > > + type = get_image_type(m_img_hdr, pfru_dev); > > + if (type < 0) > > + return false; > > + > > + size = adjust_efi_size(m_img_hdr, size); > > + if (size < 0) > > + return false; > > + > > + auth = (struct efi_image_auth *)(data + size); > > + size += sizeof(u64) + auth->auth_info.hdr.len; > > + payload_hdr = (struct pfru_payload_hdr *)(data + size); > > + > > + /* finally compare the version */ > > + if (type == CODE_INJECT_TYPE) > > + return payload_hdr->rt_ver >= cap->code_rt_version; > > + else > > + return payload_hdr->rt_ver >= cap->drv_rt_version; > > +} > > ... > > > +static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > > +{ > > + struct pfru_update_cap_info cap; > > + struct pfru_device *pfru_dev; > > + void __user *p; > > + int ret, rev; > > > + pfru_dev = to_pfru_dev(file); > > + p = (void __user *)arg; > > Can be combined with definitions above. Ditto for the rest cases in the code. > Ok, will do. > > +} > > ... > > > + phy_addr = (phys_addr_t)(info.addr_lo | (info.addr_hi << 32)); > > Does it compile without warnings for 32-bit target? > > ... > The code injection depends on Kconfig 64-bit and is not supposed to work on 32-bit AFAIK. > > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > > + if (ret) > > + return ret; > > + > > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > > + if (ret) > > + return ret; > > + > > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > > + if (ret) > > + return ret; > > Why do you need to keep zillions of copies of the data which seems > is not going to be changed? Three global variables should be enough, > no? > The guid information is embedded in each pfru_dev and only calculated once during probe. I thought people try to avoid using global variables if possible? > > + ret = ida_alloc(&pfru_ida, GFP_KERNEL); > > + if (ret < 0) > > + return ret; > > + > > + pfru_dev->index = ret; > > ... > > > + /* default rev id is 1 */ > > Shouldn't you rather define this magic and drop this doubtful comment? > Ok, will do. > > + pfru_dev->rev_id = 1; > > ... > > > +failed: > > Make you labeling consistent. The usual pattern is to explain what will be > happened when goto to the certain label, for example, here is 'err_free_ida'. > Also, add an empty line everywhere before labels. > Ok, got it, will do. > > + ida_free(&pfru_ida, pfru_dev->index); > > + > > + return ret; > > +} > > ... > > > +#define UUID_SIZE 16 > > It must not be here at all. > Or it should be properly namespaced. > Ok. Would __u8 uuid[16] applicable? There are some examples in uapi headers. thanks, Chenyu > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Nov 01, 2021 at 05:33:20PM +0800, Chen Yu wrote: > On Wed, Oct 27, 2021 at 01:20:33PM +0300, Andy Shevchenko wrote: > > On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: ... > > > + guid_t *image_type_id = &img_hdr->image_type_id; > > > > efi_guid_t ? > > > efi_guid_t is a 32-bit aligned guid_t, which is for the case when > efi_guid_t* arguments are 32-bit aligned. And it is for 32-bit ARM. > Since this code injection is only for 64-bit, the guid is not required > to be strictly 32-bit aligned I suppose? But the type of that member may not be guid_t. So, I expect not to see guid_t at all here or as a internal member with the export/import GUID API in place. See, for example d1c6e08e7503 ("libnvdimm/labels: Add uuid helpers") ... > > > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > > > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > > > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > > Why do you need to keep zillions of copies of the data which seems > > is not going to be changed? Three global variables should be enough, > > no? > > > The guid information is embedded in each pfru_dev and only calculated > once during probe. I thought people try to avoid using global variables > if possible? Use your common sense. You might have a lot of data duplication, and this data is definitely not something that needs any kind of serialization / personification / etc.
On Mon, Nov 01, 2021 at 01:21:36PM +0200, Andy Shevchenko wrote: > On Mon, Nov 01, 2021 at 05:33:20PM +0800, Chen Yu wrote: > > On Wed, Oct 27, 2021 at 01:20:33PM +0300, Andy Shevchenko wrote: > > > On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > > ... > > > > > + guid_t *image_type_id = &img_hdr->image_type_id; > > > > > > efi_guid_t ? > > > > > efi_guid_t is a 32-bit aligned guid_t, which is for the case when > > efi_guid_t* arguments are 32-bit aligned. And it is for 32-bit ARM. > > Since this code injection is only for 64-bit, the guid is not required > > to be strictly 32-bit aligned I suppose? > > But the type of that member may not be guid_t. So, I expect not to see guid_t > at all here or as a internal member with the export/import GUID API in place. > > See, for example > d1c6e08e7503 ("libnvdimm/labels: Add uuid helpers") > 8b03aa0e0e5a ("libnvdimm/labels: Add type-guid helpers") I suppose. Do you mean, since the label is not delcared as uuid type, then need to treat the data region as a uuid. But it is actually not a formal 'uuid' type, so uuid_t is not applicable here. And this also applys to the case in this patch? > ... > > > > > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > > > > > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > > > > > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > > > > Why do you need to keep zillions of copies of the data which seems > > > is not going to be changed? Three global variables should be enough, > > > no? > > > > > The guid information is embedded in each pfru_dev and only calculated > > once during probe. I thought people try to avoid using global variables > > if possible? > > Use your common sense. You might have a lot of data duplication, > and this data is definitely not something that needs any kind of > serialization / personification / etc. > Ok, I'll switch to global variables in next version. thanks, Chenyu > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Nov 01, 2021 at 09:14:34PM +0800, Chen Yu wrote: > On Mon, Nov 01, 2021 at 01:21:36PM +0200, Andy Shevchenko wrote: > > On Mon, Nov 01, 2021 at 05:33:20PM +0800, Chen Yu wrote: > > > On Wed, Oct 27, 2021 at 01:20:33PM +0300, Andy Shevchenko wrote: > > > > On Wed, Oct 27, 2021 at 03:07:51PM +0800, Chen Yu wrote: > > > > ... > > > > > > > + guid_t *image_type_id = &img_hdr->image_type_id; > > > > > > > > efi_guid_t ? > > > > > > > efi_guid_t is a 32-bit aligned guid_t, which is for the case when > > > efi_guid_t* arguments are 32-bit aligned. And it is for 32-bit ARM. > > > Since this code injection is only for 64-bit, the guid is not required > > > to be strictly 32-bit aligned I suppose? > > > > But the type of that member may not be guid_t. So, I expect not to see guid_t > > at all here or as a internal member with the export/import GUID API in place. > > > > See, for example > > d1c6e08e7503 ("libnvdimm/labels: Add uuid helpers") > > > 8b03aa0e0e5a ("libnvdimm/labels: Add type-guid helpers") I suppose. Do you > mean, since the label is not delcared as uuid type, then need to treat the > data region as a uuid. But it is actually not a formal 'uuid' type, so uuid_t > is not applicable here. And this also applys to the case in this patch? > > ... > > > > > > > + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); > > > > > > > + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); > > > > > > > + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); > > > > > > Why do you need to keep zillions of copies of the data which seems > > > > is not going to be changed? Three global variables should be enough, > > > > no? > > > > > > > The guid information is embedded in each pfru_dev and only calculated > > > once during probe. I thought people try to avoid using global variables > > > if possible? > > > > Use your common sense. You might have a lot of data duplication, > > and this data is definitely not something that needs any kind of > > serialization / personification / etc. > > > Ok, I'll switch to global variables in next version. Wait, no, why? Keep them per-device unless you can somehow be guaranteed there will never be more than one of these ACPI devices in a system. It's simpler this way, no need to worry about global state at all. thanks, greg k-h
On Mon, Nov 01, 2021 at 02:16:41PM +0100, Greg Kroah-Hartman wrote: > On Mon, Nov 01, 2021 at 09:14:34PM +0800, Chen Yu wrote: > > On Mon, Nov 01, 2021 at 01:21:36PM +0200, Andy Shevchenko wrote: ... > > Ok, I'll switch to global variables in next version. > > Wait, no, why? But why should we have a duplication of basically static data? > Keep them per-device unless you can somehow be > guaranteed there will never be more than one of these ACPI devices in a > system. I guess you missed my point. These definitions are _always_ the same. It does not matter how many devices in the system. Chen, is my perception correct? If no, then do what Greg says. > It's simpler this way, no need to worry about global state at > all. Actually I have no idea why we even have strings in and not raw buffers. Moreover, I haven't got why even we have them in uAPI.
On Mon, Nov 01, 2021 at 04:08:08PM +0200, Andy Shevchenko wrote: > On Mon, Nov 01, 2021 at 02:16:41PM +0100, Greg Kroah-Hartman wrote: > > On Mon, Nov 01, 2021 at 09:14:34PM +0800, Chen Yu wrote: > > > On Mon, Nov 01, 2021 at 01:21:36PM +0200, Andy Shevchenko wrote: > > ... > > > > Ok, I'll switch to global variables in next version. > > > > Wait, no, why? > > But why should we have a duplication of basically static data? > > > Keep them per-device unless you can somehow be > > guaranteed there will never be more than one of these ACPI devices in a > > system. > > I guess you missed my point. These definitions are _always_ the same. > It does not matter how many devices in the system. > > Chen, is my perception correct? > If no, then do what Greg says. > Yes, there would be only one PFRU ACPI object in the system. And for PFRU Telemetry ACPI object, I don't know if the specification would introduce separate telemetry log using different uuid in the future - Currently there are two telemetry log buffers share the same uuid). Maybe for scalability reason we can make them per device. > > It's simpler this way, no need to worry about global state at > > all. > > Actually I have no idea why we even have strings in and not raw buffers. > Moreover, I haven't got why even we have them in uAPI. I see. These uuid could be put into the .c and there is no need for the user to be aware of these values. thanks, Chenyu > > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Nov 01, 2021 at 10:57:57PM +0800, Chen Yu wrote: > On Mon, Nov 01, 2021 at 04:08:08PM +0200, Andy Shevchenko wrote: > > On Mon, Nov 01, 2021 at 02:16:41PM +0100, Greg Kroah-Hartman wrote: ... > > Actually I have no idea why we even have strings in and not raw buffers. > > Moreover, I haven't got why even we have them in uAPI. > I see. These uuid could be put into the .c and there is no need for the > user to be aware of these values. Right! Because I haven't found any use of UUID in uAPI (even in raw form).
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst index 2e8134059c87..6f7c86b6deb7 100644 --- a/Documentation/userspace-api/ioctl/ioctl-number.rst +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst @@ -365,6 +365,7 @@ Code Seq# Include File Comments <mailto:aherrman@de.ibm.com> 0xE5 00-3F linux/fuse.h 0xEC 00-01 drivers/platform/chrome/cros_ec_dev.h ChromeOS EC driver +0xEE 00-08 uapi/linux/pfru.h Platform Firmware Runtime Update and Telemetry 0xF3 00-3F drivers/usb/misc/sisusbvga/sisusb.h sisfb (in development) <mailto:thomas@winischhofer.net> 0xF6 all LTTng Linux Trace Toolkit Next Generation diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 1da360c51d66..1d8d2e2cefac 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -482,6 +482,7 @@ source "drivers/acpi/nfit/Kconfig" source "drivers/acpi/numa/Kconfig" source "drivers/acpi/apei/Kconfig" source "drivers/acpi/dptf/Kconfig" +source "drivers/acpi/pfru/Kconfig" config ACPI_WATCHDOG bool diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 3018714e87d9..9c2c5ddff6ec 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o obj-$(CONFIG_ACPI_SPCR_TABLE) += spcr.o obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o obj-$(CONFIG_ACPI_PPTT) += pptt.o +obj-$(CONFIG_ACPI_PFRU) += pfru/ # processor has its own "processor." module_param namespace processor-y := processor_driver.o diff --git a/drivers/acpi/pfru/Kconfig b/drivers/acpi/pfru/Kconfig new file mode 100644 index 000000000000..5b31675b173a --- /dev/null +++ b/drivers/acpi/pfru/Kconfig @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0 +config ACPI_PFRU + tristate "ACPI Platform Firmware Runtime Update (PFRU)" + depends on 64BIT + help + In order to reduce the system reboot times and update the platform firmware + in time, Platform Firmware Runtime Update is leveraged to patch the system + without reboot. This driver supports Platform Firmware Runtime Update, + which is composed of two parts: code injection and driver update. It also + allows telemetry data to be retrieved from the platform firmware. + + To compile this driver as a module, choose M here: + the module will be called pfru_update. diff --git a/drivers/acpi/pfru/Makefile b/drivers/acpi/pfru/Makefile new file mode 100644 index 000000000000..098cbe80cf3d --- /dev/null +++ b/drivers/acpi/pfru/Makefile @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +obj-$(CONFIG_ACPI_PFRU) += pfru_update.o diff --git a/drivers/acpi/pfru/pfru_update.c b/drivers/acpi/pfru/pfru_update.c new file mode 100644 index 000000000000..a9953eb6c2d5 --- /dev/null +++ b/drivers/acpi/pfru/pfru_update.c @@ -0,0 +1,572 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ACPI Platform Firmware Runtime Update Device Driver + * + * Copyright (C) 2021 Intel Corporation + * Author: Chen Yu <yu.c.chen@intel.com> + */ +#include <linux/acpi.h> +#include <linux/device.h> +#include <linux/efi.h> +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/file.h> +#include <linux/fs.h> +#include <linux/idr.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/string.h> +#include <linux/uaccess.h> +#include <linux/uio.h> +#include <linux/uuid.h> +#include <uapi/linux/pfru.h> + +enum cap_index { + CAP_STATUS_IDX, + CAP_UPDATE_IDX, + CAP_CODE_TYPE_IDX, + CAP_FW_VER_IDX, + CAP_CODE_RT_VER_IDX, + CAP_DRV_TYPE_IDX, + CAP_DRV_RT_VER_IDX, + CAP_DRV_SVN_IDX, + CAP_PLAT_ID_IDX, + CAP_OEM_ID_IDX, + CAP_OEM_INFO_IDX, + CAP_NR_IDX +}; + +enum buf_index { + BUF_STATUS_IDX, + BUF_EXT_STATUS_IDX, + BUF_ADDR_LOW_IDX, + BUF_ADDR_HI_IDX, + BUF_SIZE_IDX, + BUF_NR_IDX +}; + +enum update_index { + UPDATE_STATUS_IDX, + UPDATE_EXT_STATUS_IDX, + UPDATE_AUTH_TIME_LOW_IDX, + UPDATE_AUTH_TIME_HI_IDX, + UPDATE_EXEC_TIME_LOW_IDX, + UPDATE_EXEC_TIME_HI_IDX, + UPDATE_NR_IDX +}; + +struct pfru_device { + guid_t uuid, code_uuid, drv_uuid; + int rev_id, index; + struct device *dev; + struct miscdevice miscdev; +}; + +static DEFINE_IDA(pfru_ida); + +static inline struct pfru_device *to_pfru_dev(struct file *file) +{ + return container_of(file->private_data, struct pfru_device, miscdev); +} + +static int query_capability(struct pfru_update_cap_info *cap, + struct pfru_device *pfru_dev) +{ + union acpi_object *out_obj; + acpi_handle handle; + int ret = -EINVAL; + + handle = ACPI_HANDLE(pfru_dev->dev); + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, + pfru_dev->rev_id, + FUNC_QUERY_UPDATE_CAP, + NULL, ACPI_TYPE_PACKAGE); + if (!out_obj) + return ret; + + if (out_obj->package.count < CAP_NR_IDX) + goto free_acpi_buffer; + + if (out_obj->package.elements[CAP_STATUS_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->status = out_obj->package.elements[CAP_STATUS_IDX].integer.value; + + if (out_obj->package.elements[CAP_UPDATE_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->update_cap = out_obj->package.elements[CAP_UPDATE_IDX].integer.value; + + if (out_obj->package.elements[CAP_CODE_TYPE_IDX].type != ACPI_TYPE_BUFFER) + goto free_acpi_buffer; + + memcpy(&cap->code_type, + out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.pointer, + out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length); + + if (out_obj->package.elements[CAP_FW_VER_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->fw_version = + out_obj->package.elements[CAP_FW_VER_IDX].integer.value; + + if (out_obj->package.elements[CAP_CODE_RT_VER_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->code_rt_version = + out_obj->package.elements[CAP_CODE_RT_VER_IDX].integer.value; + + if (out_obj->package.elements[CAP_DRV_TYPE_IDX].type != ACPI_TYPE_BUFFER) + goto free_acpi_buffer; + + memcpy(&cap->drv_type, + out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.pointer, + out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length); + + if (out_obj->package.elements[CAP_DRV_RT_VER_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->drv_rt_version = + out_obj->package.elements[CAP_DRV_RT_VER_IDX].integer.value; + + if (out_obj->package.elements[CAP_DRV_SVN_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + cap->drv_svn = + out_obj->package.elements[CAP_DRV_SVN_IDX].integer.value; + + if (out_obj->package.elements[CAP_PLAT_ID_IDX].type != ACPI_TYPE_BUFFER) + goto free_acpi_buffer; + + memcpy(&cap->platform_id, + out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.pointer, + out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length); + + if (out_obj->package.elements[CAP_OEM_ID_IDX].type != ACPI_TYPE_BUFFER) + goto free_acpi_buffer; + + memcpy(&cap->oem_id, + out_obj->package.elements[CAP_OEM_ID_IDX].buffer.pointer, + out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length); + ret = 0; +free_acpi_buffer: + ACPI_FREE(out_obj); + + return ret; +} + +static int query_buffer(struct pfru_com_buf_info *info, + struct pfru_device *pfru_dev) +{ + union acpi_object *out_obj; + acpi_handle handle; + int ret = -EINVAL; + + handle = ACPI_HANDLE(pfru_dev->dev); + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, + pfru_dev->rev_id, FUNC_QUERY_BUF, + NULL, ACPI_TYPE_PACKAGE); + if (!out_obj) + return ret; + + if (out_obj->package.count < BUF_NR_IDX) + goto free_acpi_buffer; + + if (out_obj->package.elements[BUF_STATUS_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + info->status = out_obj->package.elements[BUF_STATUS_IDX].integer.value; + + if (out_obj->package.elements[BUF_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + info->ext_status = + out_obj->package.elements[BUF_EXT_STATUS_IDX].integer.value; + + if (out_obj->package.elements[BUF_ADDR_LOW_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + info->addr_lo = + out_obj->package.elements[BUF_ADDR_LOW_IDX].integer.value; + + if (out_obj->package.elements[BUF_ADDR_HI_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + info->addr_hi = + out_obj->package.elements[BUF_ADDR_HI_IDX].integer.value; + + if (out_obj->package.elements[BUF_SIZE_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + info->buf_size = out_obj->package.elements[BUF_SIZE_IDX].integer.value; + + ret = 0; +free_acpi_buffer: + ACPI_FREE(out_obj); + + return ret; +} + +static int get_image_type(struct efi_manage_capsule_image_header *img_hdr, + struct pfru_device *pfru_dev) +{ + guid_t *image_type_id = &img_hdr->image_type_id; + + /* check whether this is a code injection or driver update */ + if (guid_equal(image_type_id, &pfru_dev->code_uuid)) + return CODE_INJECT_TYPE; + + if (guid_equal(image_type_id, &pfru_dev->drv_uuid)) + return DRIVER_UPDATE_TYPE; + + return -EINVAL; +} + +static int adjust_efi_size(struct efi_manage_capsule_image_header *img_hdr, + int size) +{ + /* + * The (u64 hw_ins) was introduced in UEFI spec version 2, + * and (u64 capsule_support) was introduced in version 3. + * The size needs to be adjusted accordingly. That is to + * say, version 1 should subtract the size of hw_ins+capsule_support, + * and version 2 should sbstract the size of capsule_support. + */ + size += sizeof(struct efi_manage_capsule_image_header); + switch (img_hdr->ver) { + case 1: + return size - 2 * sizeof(u64); + case 2: + return size - sizeof(u64); + default: + /* only support version 1 and 2 */ + return -EINVAL; + } +} + +static bool valid_version(const void *data, struct pfru_update_cap_info *cap, + struct pfru_device *pfru_dev) +{ + struct pfru_payload_hdr *payload_hdr; + efi_capsule_header_t *cap_hdr; + struct efi_manage_capsule_header *m_hdr; + struct efi_manage_capsule_image_header *m_img_hdr; + struct efi_image_auth *auth; + int type, size; + + /* + * Sanity check if the capsule image has a newer version + * than current one. + */ + cap_hdr = (efi_capsule_header_t *)data; + size = cap_hdr->headersize; + m_hdr = (struct efi_manage_capsule_header *)(data + size); + /* + * Current data structure size plus variable array indicated + * by number of (emb_drv_cnt + payload_cnt) + */ + size += sizeof(struct efi_manage_capsule_header) + + (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64); + m_img_hdr = (struct efi_manage_capsule_image_header *)(data + size); + + type = get_image_type(m_img_hdr, pfru_dev); + if (type < 0) + return false; + + size = adjust_efi_size(m_img_hdr, size); + if (size < 0) + return false; + + auth = (struct efi_image_auth *)(data + size); + size += sizeof(u64) + auth->auth_info.hdr.len; + payload_hdr = (struct pfru_payload_hdr *)(data + size); + + /* finally compare the version */ + if (type == CODE_INJECT_TYPE) + return payload_hdr->rt_ver >= cap->code_rt_version; + else + return payload_hdr->rt_ver >= cap->drv_rt_version; +} + +static void dump_update_result(struct pfru_updated_result *result, + struct pfru_device *pfru_dev) +{ + dev_dbg(pfru_dev->dev, "Update result:\n"); + dev_dbg(pfru_dev->dev, "Status:%d\n", result->status); + dev_dbg(pfru_dev->dev, "Extended Status:%d\n", result->ext_status); + dev_dbg(pfru_dev->dev, "Authentication Time Low:%lld\n", + result->low_auth_time); + dev_dbg(pfru_dev->dev, "Authentication Time High:%lld\n", + result->high_auth_time); + dev_dbg(pfru_dev->dev, "Execution Time Low:%lld\n", + result->low_exec_time); + dev_dbg(pfru_dev->dev, "Execution Time High:%lld\n", + result->high_exec_time); +} + +static int start_acpi_update(int action, struct pfru_device *pfru_dev) +{ + union acpi_object *out_obj, in_obj, in_buf; + struct pfru_updated_result update_result; + acpi_handle handle; + int ret = -EINVAL; + + memset(&in_obj, 0, sizeof(in_obj)); + memset(&in_buf, 0, sizeof(in_buf)); + in_obj.type = ACPI_TYPE_PACKAGE; + in_obj.package.count = 1; + in_obj.package.elements = &in_buf; + in_buf.type = ACPI_TYPE_INTEGER; + in_buf.integer.value = action; + + handle = ACPI_HANDLE(pfru_dev->dev); + out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid, + pfru_dev->rev_id, FUNC_START, + &in_obj, ACPI_TYPE_PACKAGE); + if (!out_obj) + return ret; + + if (out_obj->package.count < UPDATE_NR_IDX) + goto free_acpi_buffer; + + if (out_obj->package.elements[UPDATE_STATUS_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.status = + out_obj->package.elements[UPDATE_STATUS_IDX].integer.value; + + if (out_obj->package.elements[UPDATE_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.ext_status = + out_obj->package.elements[UPDATE_EXT_STATUS_IDX].integer.value; + + if (out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.low_auth_time = + out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].integer.value; + + if (out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.high_auth_time = + out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].integer.value; + + if (out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.low_exec_time = + out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].integer.value; + + if (out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].type != ACPI_TYPE_INTEGER) + goto free_acpi_buffer; + + update_result.high_exec_time = + out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].integer.value; + + dump_update_result(&update_result, pfru_dev); + ret = 0; + +free_acpi_buffer: + ACPI_FREE(out_obj); + + return ret; +} + +static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct pfru_update_cap_info cap; + struct pfru_device *pfru_dev; + void __user *p; + int ret, rev; + + pfru_dev = to_pfru_dev(file); + p = (void __user *)arg; + + switch (cmd) { + case PFRU_IOC_QUERY_CAP: + ret = query_capability(&cap, pfru_dev); + if (ret) + return ret; + + if (copy_to_user(p, &cap, sizeof(cap))) + return -EFAULT; + + return 0; + case PFRU_IOC_SET_REV: + if (copy_from_user(&rev, p, sizeof(unsigned int))) + return -EFAULT; + + if (!pfru_valid_revid(rev)) + return -EINVAL; + + pfru_dev->rev_id = rev; + + return 0; + case PFRU_IOC_STAGE: + return start_acpi_update(START_STAGE, pfru_dev); + case PFRU_IOC_ACTIVATE: + return start_acpi_update(START_ACTIVATE, pfru_dev); + case PFRU_IOC_STAGE_ACTIVATE: + return start_acpi_update(START_STAGE_ACTIVATE, pfru_dev); + default: + return -ENOTTY; + } +} + +static ssize_t pfru_write(struct file *file, const char __user *buf, + size_t len, loff_t *ppos) +{ + struct pfru_update_cap_info cap; + struct pfru_com_buf_info info; + struct pfru_device *pfru_dev; + phys_addr_t phy_addr; + struct iov_iter iter; + struct iovec iov; + char *buf_ptr; + int ret; + + pfru_dev = to_pfru_dev(file); + + ret = query_buffer(&info, pfru_dev); + if (ret) + return ret; + + if (len > info.buf_size) + return -EINVAL; + + iov.iov_base = (void __user *)buf; + iov.iov_len = len; + iov_iter_init(&iter, WRITE, &iov, 1, len); + + /* map the communication buffer */ + phy_addr = (phys_addr_t)(info.addr_lo | (info.addr_hi << 32)); + buf_ptr = memremap(phy_addr, info.buf_size, MEMREMAP_WB); + if (IS_ERR(buf_ptr)) + return PTR_ERR(buf_ptr); + + if (!copy_from_iter_full(buf_ptr, len, &iter)) { + ret = -EINVAL; + goto unmap; + } + + /* check if the capsule header has a valid version number */ + ret = query_capability(&cap, pfru_dev); + if (ret) + goto unmap; + + if (cap.status != DSM_SUCCEED) + ret = -EBUSY; + else if (!valid_version(buf_ptr, &cap, pfru_dev)) + ret = -EINVAL; +unmap: + memunmap(buf_ptr); + + return ret ?: len; +} + +static const struct file_operations acpi_pfru_fops = { + .owner = THIS_MODULE, + .write = pfru_write, + .unlocked_ioctl = pfru_ioctl, + .llseek = noop_llseek, +}; + +static int acpi_pfru_remove(struct platform_device *pdev) +{ + struct pfru_device *pfru_dev = platform_get_drvdata(pdev); + + misc_deregister(&pfru_dev->miscdev); + ida_free(&pfru_ida, pfru_dev->index); + + return 0; +} + +static int acpi_pfru_probe(struct platform_device *pdev) +{ + struct pfru_device *pfru_dev; + acpi_handle handle; + int ret; + + handle = ACPI_HANDLE(&pdev->dev); + if (!acpi_has_method(handle, "_DSM")) { + dev_dbg(&pdev->dev, "Missing _DSM\n"); + return -ENODEV; + } + + pfru_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_dev), GFP_KERNEL); + if (!pfru_dev) + return -ENOMEM; + + ret = guid_parse(PFRU_UUID, &pfru_dev->uuid); + if (ret) + return ret; + + ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid); + if (ret) + return ret; + + ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid); + if (ret) + return ret; + + ret = ida_alloc(&pfru_ida, GFP_KERNEL); + if (ret < 0) + return ret; + + pfru_dev->index = ret; + /* default rev id is 1 */ + pfru_dev->rev_id = 1; + pfru_dev->dev = &pdev->dev; + + pfru_dev->miscdev.minor = MISC_DYNAMIC_MINOR; + pfru_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, + "pfru%d", pfru_dev->index); + if (!pfru_dev->miscdev.name) { + ret = -ENOMEM; + goto failed; + } + + pfru_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL, + "acpi_pfru%d", pfru_dev->index); + if (!pfru_dev->miscdev.nodename) { + ret = -ENOMEM; + goto failed; + } + + pfru_dev->miscdev.fops = &acpi_pfru_fops; + pfru_dev->miscdev.parent = &pdev->dev; + + ret = misc_register(&pfru_dev->miscdev); + if (ret) + goto failed; + + platform_set_drvdata(pdev, pfru_dev); + + return 0; +failed: + ida_free(&pfru_ida, pfru_dev->index); + + return ret; +} + +static const struct acpi_device_id acpi_pfru_ids[] = { + {"INTC1080", 0}, + {} +}; +MODULE_DEVICE_TABLE(acpi, acpi_pfru_ids); + +static struct platform_driver acpi_pfru_driver = { + .driver = { + .name = "pfru_update", + .acpi_match_table = acpi_pfru_ids, + }, + .probe = acpi_pfru_probe, + .remove = acpi_pfru_remove, +}; +module_platform_driver(acpi_pfru_driver); + +MODULE_DESCRIPTION("Platform Firmware Runtime Update device driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h new file mode 100644 index 000000000000..d6f57a75afd2 --- /dev/null +++ b/include/uapi/linux/pfru.h @@ -0,0 +1,186 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Platform Firmware Runtime Update header + * + * Copyright(c) 2021 Intel Corporation. All rights reserved. + */ +#ifndef __PFRU_H__ +#define __PFRU_H__ + +#include <linux/types.h> +#include <linux/ioctl.h> + +#define PFRU_UUID "ECF9533B-4A3C-4E89-939E-C77112601C6D" +#define PFRU_CODE_INJ_UUID "B2F84B79-7B6E-4E45-885F-3FB9BB185402" +#define PFRU_DRV_UPDATE_UUID "4569DD8C-75F1-429A-A3D6-24DE8097A0DF" + +#define FUNC_STANDARD_QUERY 0 +#define FUNC_QUERY_UPDATE_CAP 1 +#define FUNC_QUERY_BUF 2 +#define FUNC_START 3 + +#define CODE_INJECT_TYPE 1 +#define DRIVER_UPDATE_TYPE 2 + +#define REVID_1 1 +#define REVID_2 2 + +#define PFRU_MAGIC 0xEE + +#define UUID_SIZE 16 + +/** + * PFRU_IOC_SET_REV - _IOW(PFRU_MAGIC, 0x01, unsigned int) + * + * Return: 0 on success, -errno on failure + * + * Set the Revision ID for PFRU Runtime Update. It could be either 1 or 2. + */ +#define PFRU_IOC_SET_REV _IOW(PFRU_MAGIC, 0x01, unsigned int) +/** + * PFRU_IOC_STAGE - _IOW(PFRU_MAGIC, 0x02, unsigned int) + * + * Return: 0 on success, -errno on failure + * + * Stage a capsule image from communication buffer and perform authentication. + */ +#define PFRU_IOC_STAGE _IOW(PFRU_MAGIC, 0x02, unsigned int) +/** + * PFRU_IOC_ACTIVATE - _IOW(PFRU_MAGIC, 0x03, unsigned int) + * + * Return: 0 on success, -errno on failure + * + * Activate a previous staged capsule image. + */ +#define PFRU_IOC_ACTIVATE _IOW(PFRU_MAGIC, 0x03, unsigned int) +/** + * PFRU_IOC_STAGE_ACTIVATE - _IOW(PFRU_MAGIC, 0x04, unsigned int) + * + * Return: 0 on success, -errno on failure + * + * Perform both stage and activation actions. + */ +#define PFRU_IOC_STAGE_ACTIVATE _IOW(PFRU_MAGIC, 0x04, unsigned int) +/** + * PFRU_IOC_QUERY_CAP - _IOR(PFRU_MAGIC, 0x05, + * struct pfru_update_cap_info) + * + * Return: 0 on success, -errno on failure. + * + * Retrieve information about the PFRU Runtime Update capability. + * The information is a struct pfru_update_cap_info. + */ +#define PFRU_IOC_QUERY_CAP _IOR(PFRU_MAGIC, 0x05, struct pfru_update_cap_info) + +static inline int pfru_valid_revid(int id) +{ + return id == REVID_1 || id == REVID_2; +} + +/** + * struct pfru_payload_hdr - Capsule file payload header. + * + * @sig: Signature of this capsule file. + * @hdr_version: Revision of this header structure. + * @hdr_size: Size of this header, including the OemHeader bytes. + * @hw_ver: The supported firmware version. + * @rt_ver: Version of the code injection image. + * @platform_id: A platform specific GUID to specify the platform what + * this capsule image support. + */ +struct pfru_payload_hdr { + __u32 sig; + __u32 hdr_version; + __u32 hdr_size; + __u32 hw_ver; + __u32 rt_ver; + __u8 platform_id[UUID_SIZE]; +}; + +enum pfru_start_action { + START_STAGE, + START_ACTIVATE, + START_STAGE_ACTIVATE, +}; + +enum pfru_dsm_status { + DSM_SUCCEED, + DSM_FUNC_NOT_SUPPORT, + DSM_INVAL_INPUT, + DSM_HARDWARE_ERR, + DSM_RETRY_SUGGESTED, + DSM_UNKNOWN, + DSM_FUNC_SPEC_ERR, +}; + +/** + * struct pfru_update_cap_info - Runtime update capability information. + * + * @status: Indicator of whether this query succeed. + * @update_cap: Bitmap to indicate whether the feature is supported. + * @code_type: A buffer containing an image type GUID. + * @fw_version: Platform firmware version. + * @code_rt_version: Code injection runtime version for anti-rollback. + * @drv_type: A buffer containing an image type GUID. + * @drv_rt_version: The version of the driver update runtime code. + * @drv_svn: The secure version number(SVN) of the driver update runtime code. + * @platform_id: A buffer containing a platform ID GUID. + * @oem_id: A buffer containing an OEM ID GUID. + * @oem_info: A buffer containing the vendor specific information. + */ +struct pfru_update_cap_info { + __u32 status; + __u32 update_cap; + + __u8 code_type[UUID_SIZE]; + __u32 fw_version; + __u32 code_rt_version; + + __u8 drv_type[UUID_SIZE]; + __u32 drv_rt_version; + __u32 drv_svn; + + __u8 platform_id[UUID_SIZE]; + __u8 oem_id[UUID_SIZE]; + + char oem_info[]; +}; + +/** + * struct pfru_com_buf_info - Communication buffer information. + * + * @status: Indicator of whether this query succeed. + * @ext_status: Implementation specific query result. + * @addr_lo: Low 32bit physical address of the communication buffer to hold + * a runtime update package. + * @addr_hi: High 32bit physical address of the communication buffer to hold + * a runtime update package. + * @buf_size: Maximum size in bytes of the communication buffer. + */ +struct pfru_com_buf_info { + __u32 status; + __u32 ext_status; + __u64 addr_lo; + __u64 addr_hi; + __u32 buf_size; +}; + +/** + * struct pfru_updated_result - Platform firmware runtime update result information. + * @status: Indicator of whether this update succeed. + * @ext_status: Implementation specific update result. + * @low_auth_time: Low 32bit value of image authentication time in nanosecond. + * @high_auth_time: High 32bit value of image authentication time in nanosecond. + * @low_exec_time: Low 32bit value of image execution time in nanosecond. + * @high_exec_time: High 32bit value of image execution time in nanosecond. + */ +struct pfru_updated_result { + __u32 status; + __u32 ext_status; + __u64 low_auth_time; + __u64 high_auth_time; + __u64 low_exec_time; + __u64 high_exec_time; +}; + +#endif /* __PFRU_H__ */
Introduce the pfru_update driver which can be used for Platform Firmware Runtime code injection and driver update [1]. The user is expected to provide the update firmware in the form of capsule file, and pass it to the driver via ioctl. Then the driver would hand this capsule file to the Platform Firmware Runtime Update via the ACPI device _DSM method. At last the low level Management Mode would do the firmware update. The corresponding userspace tool and man page will be introduced at tools/power/acpi/pfru. Link: https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf # [1] Signed-off-by: Chen Yu <yu.c.chen@intel.com> --- v7: Use ida_alloc() to allocate a ID, and release the ID when device is removed. (Greg Kroah-Hartman) Check the _DSM method at early stage, before allocate or parse anything in acpi_pfru_probe(). (Greg Kroah-Hartman) Set the parent of the misc device. (Greg Kroah-Hartman) Use module_platform_driver() instead of platform_driver_register() in module_init(). Separate pfru driver and pfru_telemetry driver to two files. (Greg Kroah-Hartman) v6: Use Link: tag to add the specification download address. (Andy Shevchenko) Remove linux/uuid.h and use raw buffers to contain uuid. (Andy Shevchenko) Drop comma for each terminator entry in the enum structure. (Andy Shevchenko) Remove redundant 'else' in get_image_type(). (Andy Shevchenko) Directly return results from the switch cases in adjust_efi_size() and pfru_ioctl().(Andy Shevchenko) Keep comment style consistent by removing the period for one line comment. (Andy Shevchenko) Remove devm_kfree() if .probe() failed. (Andy Shevchenko) v5: Remove Documentation/ABI/pfru, and move the content to kernel doc in include/uapi/linux/pfru.h (Greg Kroah-Hartman) Shrink the range of ioctl numbers declared in Documentation/userspace-api/ioctl/ioctl-number.rst from 16 to 8. (Greg Kroah-Hartman) Change global variable struct pfru_device *pfru_dev to per ACPI device. (Greg Kroah-Hartman) Unregister the misc device in acpi_pfru_remove(). (Greg Kroah-Hartman) Convert the kzalloc() to devm_kzalloc() in the driver so as to avoid freeing the memory. (Greg Kroah-Hartman) Fix the compile error by declaring the pfru_log_ioctl() as static. (kernel test robot LKP) Change to global variable misc_device to per ACPI device. (Greg Kroah-Hartman) v4: Replace all pr_err() with dev_dbg() (Greg Kroah-Hartman, Rafael J. Wysocki) Returns ENOTTY rather than ENOIOCTLCMD if invalid ioctl command is provided. (Greg Kroah-Hartman) Remove compat ioctl. (Greg Kroah-Hartman) Rename /dev/pfru/pfru_update to /dev/acpi_pfru (Greg Kroah-Hartman) Simplify the check for element of the package in query_capability() (Rafael J. Wysocki) Remove the loop in query_capability(), query_buffer() and query the package elemenet directly. (Rafael J. Wysocki) Check the the number of elements in case the number of package elements is too small. (Rafael J. Wysocki) Doing the assignment as initialization in get_image_type(). Meanwhile, returns the type or a negative error code in get_image_type(). (Rafael J. Wysocki) Put the comments inside the function. (Rafael J. Wysocki) Returns the size or a negative error code in adjust_efi_size() (Rafael J. Wysocki) Fix the return value from EFAULT to EINVAL if pfru_valid_revid() does not pass. (Rafael J. Wysocki) Change the write() to be the code injection/update, the read() to be telemetry retrieval and all of the rest to be ioctl()s under one special device file.(Rafael J. Wysocki) Putting empty code lines after an if () statement that is not followed by a block. (Rafael J. Wysocki) Remove "goto" tags to make the code more readable. (Rafael J. Wysocki) v3: Use __u32 instead of int and __64 instead of unsigned long in include/uapi/linux/pfru.h (Greg Kroah-Hartman) Rename the structure in uapi to start with a prefix pfru so as to avoid confusing in the global namespace. (Greg Kroah-Hartman) v2: Add sanity check for duplicated instance of ACPI device. Update the driver to work with allocated pfru_device objects. (Mike Rapoport) For each switch case pair, get rid of the magic case numbers and add a default clause with the error handling. (Mike Rapoport) Move the obj->type checks outside the switch to reduce redundancy. (Mike Rapoport) Parse the code_inj_id and drv_update_id at driver initialization time to reduce the re-parsing at runtime.(Mike Rapoport) Explain in detail how the size needs to be adjusted when doing version check.(Mike Rapoport) Rename parse_update_result() to dump_update_result()(Mike Rapoport) Remove redundant return.(Mike Rapoport) Do not expose struct capsulate_buf_info to uapi, since it is not needed in userspace.(Mike Rapoport) --- .../userspace-api/ioctl/ioctl-number.rst | 1 + drivers/acpi/Kconfig | 1 + drivers/acpi/Makefile | 1 + drivers/acpi/pfru/Kconfig | 13 + drivers/acpi/pfru/Makefile | 2 + drivers/acpi/pfru/pfru_update.c | 572 ++++++++++++++++++ include/uapi/linux/pfru.h | 186 ++++++ 7 files changed, 776 insertions(+) create mode 100644 drivers/acpi/pfru/Kconfig create mode 100644 drivers/acpi/pfru/Makefile create mode 100644 drivers/acpi/pfru/pfru_update.c create mode 100644 include/uapi/linux/pfru.h