linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers
@ 2021-10-16 10:26 Chen Yu
  2021-10-16 10:31 ` [PATCH v4 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures Chen Yu
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-16 10:26 UTC (permalink / raw)
  To: linux-acpi
  Cc: Ard Biesheuvel, Rafael J. Wysocki, Len Brown, linux-kernel,
	Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Chen Yu

The PFRU(Platform Firmware Runtime Update) kernel interface is designed
to interact with the platform firmware interface defined in the Management
Mode Firmware Runtime Update specification[1]. The primary function of
PFRU is to carry out runtime updates of the platform firmware, which
doesn't require the system to be restarted.  It also allows telemetry data
to be retrieved from the platform firmware.

[1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf

Chen Yu (4):
  efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and
    corresponding structures
  drivers/acpi: Introduce Platform Firmware Runtime Update device driver
  drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  tools: Introduce power/acpi/pfru/pfru

 Documentation/ABI/testing/pfru                |  41 +
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 drivers/acpi/Kconfig                          |   1 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/pfru/Kconfig                     |  16 +
 drivers/acpi/pfru/Makefile                    |   2 +
 drivers/acpi/pfru/pfru_update.c               | 943 ++++++++++++++++++
 include/linux/efi.h                           |  50 +
 include/uapi/linux/pfru.h                     | 150 +++
 tools/power/acpi/pfru/Makefile                |  25 +
 tools/power/acpi/pfru/pfru.8                  | 139 +++
 tools/power/acpi/pfru/pfru.c                  | 336 +++++++
 12 files changed, 1705 insertions(+)
 create mode 100644 Documentation/ABI/testing/pfru
 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
 create mode 100644 tools/power/acpi/pfru/Makefile
 create mode 100644 tools/power/acpi/pfru/pfru.8
 create mode 100644 tools/power/acpi/pfru/pfru.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures
  2021-10-16 10:26 [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers Chen Yu
@ 2021-10-16 10:31 ` Chen Yu
  2021-10-16 10:40 ` [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver Chen Yu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-16 10:31 UTC (permalink / raw)
  To: linux-acpi
  Cc: Ard Biesheuvel, Rafael J. Wysocki, Len Brown, linux-kernel,
	Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Jonathan Corbet, linux-efi, Chen Yu

Platform Firmware Runtime Update image starts with UEFI headers, and the
headers are defined in UEFI specification, but some of them have not been
defined in the kernel yet.

For example, the header layout of a capsule file looks like this:

EFI_CAPSULE_HEADER
EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER
EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER
EFI_FIRMWARE_IMAGE_AUTHENTICATION

These structures would be used by the Platform Firmware Runtime Update
driver to parse the format of capsule file to verify if the corresponding
version number is valid. The EFI_CAPSULE_HEADER has been defined in the
kernel, however the rest are not, thus introduce corresponding UEFI
structures accordingly. Besides, EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER
and EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER need not be aligned and
so the corresponding data types should be packed.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v4: Revise the commit log to better describe the packing.
    (Rafael J. Wysocki)
---
 include/linux/efi.h | 50 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 6b5d36babfcc..19ff834e1388 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -148,6 +148,56 @@ typedef struct {
 	u32 imagesize;
 } efi_capsule_header_t;
 
+#pragma pack(1)
+
+/* EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER */
+typedef struct {
+	u32	ver;
+	u16	emb_drv_cnt;
+	u16	payload_cnt;
+	/*
+	 * Variable array indicated by number of
+	 * (emb_drv_cnt + payload_cnt)
+	 */
+	u64	offset_list[];
+} efi_manage_capsule_header_t;
+
+/* EFI_FIRMWARE_MANAGEMENT_CAPSULE_IMAGE_HEADER */
+typedef struct {
+	u32	ver;
+	guid_t	image_type_id;
+	u8	image_index;
+	u8	reserved_bytes[3];
+	u32	image_size;
+	u32	vendor_code_size;
+	/* ver = 2. */
+	u64	hw_ins;
+	/* ver = v3. */
+	u64	capsule_support;
+} efi_manage_capsule_image_header_t;
+
+#pragma pack()
+
+/* WIN_CERTIFICATE */
+typedef struct {
+	u32	len;
+	u16	rev;
+	u16	cert_type;
+} win_cert_t;
+
+/* WIN_CERTIFICATE_UEFI_GUID */
+typedef struct {
+	win_cert_t	hdr;
+	guid_t		cert_type;
+	u8		cert_data[];
+} win_cert_uefi_guid_t;
+
+/* EFI_FIRMWARE_IMAGE_AUTHENTICATIO */
+typedef struct {
+	u64				mon_count;
+	win_cert_uefi_guid_t		auth_info;
+} efi_image_auth_t;
+
 /*
  * EFI capsule flags
  */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver
  2021-10-16 10:26 [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers Chen Yu
  2021-10-16 10:31 ` [PATCH v4 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures Chen Yu
@ 2021-10-16 10:40 ` Chen Yu
  2021-10-16 15:16   ` Greg Kroah-Hartman
  2021-10-16 10:44 ` [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry Chen Yu
  2021-10-16 10:47 ` [PATCH v4 4/4] tools: Introduce power/acpi/pfru/pfru Chen Yu
  3 siblings, 1 reply; 14+ messages in thread
From: Chen Yu @ 2021-10-16 10:40 UTC (permalink / raw)
  To: linux-acpi
  Cc: Ard Biesheuvel, Rafael J. Wysocki, Len Brown, linux-kernel,
	Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Jonathan Corbet, Chen Yu

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.

[1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v4: Add Documentation/ABI/testing/pfru (Rafael J. Wysocki)
    Change all pr_debug() to dev_dbg() (Greg Kroah-Hartman,
    Rafael J. Wysocki)
    Change the error code ENOIOCTLCMD to ENOTTY in ioctl.
    (Greg Kroah-Hartman)
    Remove compat ioctl. (Greg Kroah-Hartman)
    Change /dev/pfru/update to /dev/acpi_pfru (Greg Kroah-Hartman)
    Remove valid_cap_type() and do sanity check in query_capability().
    (Rafael J. Wysocki)
    Remove the loop in query_capability().
    (Rafael J. Wysocki)
    Do not fail if the package has more elements than expected,
    and return error if the number of package elements is too
    small. (Rafael J. Wysocki)
    Return the type or a negative error code in get_image_type()
    (Rafael J. Wysocki)
    Put the comment inside the function rather than outside.
    (Rafael J. Wysocki)
    Return the size or a negative error code adjust_efi_size()
    (Rafael J. Wysocki)
    Return -EINVAL rather than -EFAULT if revison id is incorrect.
    (Rafael J. Wysocki)
    Move the an read() of pfru into ioctl(), and using read() for
    the telemetry retrieval. So as to avoid the telemetry device
    file, the write() will be the code injection/update, the read()
    will be telemetry retrieval and all of the rest can be ioctl()s
    under one special device file.
    (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)
---
 Documentation/ABI/testing/pfru                |  41 ++
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 drivers/acpi/Kconfig                          |   1 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/pfru/Kconfig                     |  16 +
 drivers/acpi/pfru/Makefile                    |   2 +
 drivers/acpi/pfru/pfru_update.c               | 567 ++++++++++++++++++
 include/uapi/linux/pfru.h                     | 102 ++++
 8 files changed, 731 insertions(+)
 create mode 100644 Documentation/ABI/testing/pfru
 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/ABI/testing/pfru b/Documentation/ABI/testing/pfru
new file mode 100644
index 000000000000..b8bc81703f46
--- /dev/null
+++ b/Documentation/ABI/testing/pfru
@@ -0,0 +1,41 @@
+What:		/dev/acpi_pfru
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	Chen Yu <yu.c.chen@intel.com>
+Description:
+		The ioctl interface to drivers for platform firmware runtime
+		update(PFRU). Following actions are supported:
+
+		* PFRU_IOC_QUERY_CAP: Read the PFRU Runtime Update capability.
+		  The value is a structure of pfru_update_cap_info.
+		  See include/uapi/linux/pfru.h for definition.
+
+		* PFRU_SET_REV: Set the Revision ID for PFRU Runtime Update.
+		  It could be either 1 or 2.
+
+		* PFRU_IOC_STAGE: Stage a capsule image from communication
+		  buffer and perform authentication.
+
+		* PFRU_IOC_ACTIVATE: Activate a previous staged capsule image.
+
+		* PFRU_IOC_STAGE_ACTIVATE: Perform both stage and activation
+		  actions.
+
+		* PFRU_LOG_IOC_SET_INFO: set log information in Telemetry
+		  Service. The input is a structure of pfru_log_info.
+		  This structure includes log revision id(1 or 2),
+		  log level(0 : Error Message, 1 : Warning Message,
+		  2 : Informational Message, 4 : Verbose), log data type
+		  (0 : Execution Log, 1 : History Information).
+		  See include/uapi/linux/pfru.h for definition.
+
+		* PFRU_LOG_IOC_GET_INFO: get log information in Telemetry.
+		  The output is a structure of pfru_log_info.
+
+		* PFRU_LOG_IOC_GET_DATA_INFO: get log data information in
+		  Telemetry. The output is a structure of pfru_log_data_info.
+		  See include/uapi/linux/pfru.h for definition.
+
+		Besides ioctl interface, write() and read() are supported on
+		/dev/acpi_pfru. The write() will be the code injection/update,
+		and the read() will be telemetry retrieval.
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 2e8134059c87..6e5a82fff408 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-1F  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..87388a46e760
--- /dev/null
+++ b/drivers/acpi/pfru/Kconfig
@@ -0,0 +1,16 @@
+# 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.
+
+	  For more information, see:
+	  <file:Documentation/ABI/testing/pfru>
+
+	  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..f57a39e79808
--- /dev/null
+++ b/drivers/acpi/pfru/pfru_update.c
@@ -0,0 +1,567 @@
+// 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/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;
+	struct device *dev;
+};
+
+static struct pfru_device *pfru_dev;
+
+static int query_capability(struct pfru_update_cap_info *cap)
+{
+	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)
+{
+	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(efi_manage_capsule_image_header_t *img_hdr)
+{
+	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;
+	else if (guid_equal(image_type_id, &pfru_dev->drv_uuid))
+		return DRIVER_UPDATE_TYPE;
+	else
+		return -EINVAL;
+}
+
+static int adjust_efi_size(efi_manage_capsule_image_header_t *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(efi_manage_capsule_image_header_t);
+	switch (img_hdr->ver) {
+	case 1:
+		size -= 2 * sizeof(u64);
+		break;
+	case 2:
+		size -= sizeof(u64);
+		break;
+	default:
+		/* only support version 1 and 2 */
+		return -EINVAL;
+	}
+
+	return size;
+}
+
+static bool valid_version(const void *data, struct pfru_update_cap_info *cap)
+{
+	struct pfru_payload_hdr *payload_hdr;
+	efi_capsule_header_t *cap_hdr;
+	efi_manage_capsule_header_t *m_hdr;
+	efi_manage_capsule_image_header_t *m_img_hdr;
+	efi_image_auth_t *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 = (efi_manage_capsule_header_t *)(data + size);
+	/*
+	 * Current data structure size plus variable array indicated
+	 * by number of (emb_drv_cnt + payload_cnt)
+	 */
+	size += sizeof(efi_manage_capsule_header_t) +
+		      (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64);
+	m_img_hdr = (efi_manage_capsule_image_header_t *)(data + size);
+
+	type = get_image_type(m_img_hdr);
+	if (type < 0)
+		return false;
+
+	size = adjust_efi_size(m_img_hdr, size);
+	if (size < 0)
+		return false;
+
+	auth = (efi_image_auth_t *)(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)
+{
+	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)
+{
+	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);
+	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;
+	void __user *p;
+	int ret = 0, rev;
+
+	if (!pfru_dev)
+		return -ENODEV;
+
+	p = (void __user *)arg;
+
+	switch (cmd) {
+	case PFRU_IOC_QUERY_CAP:
+		ret = query_capability(&cap);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &cap, sizeof(cap)))
+			return -EFAULT;
+
+		break;
+	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;
+		break;
+	case PFRU_IOC_STAGE:
+		ret = start_acpi_update(START_STAGE);
+		break;
+	case PFRU_IOC_ACTIVATE:
+		ret = start_acpi_update(START_ACTIVATE);
+		break;
+	case PFRU_IOC_STAGE_ACTIVATE:
+		ret = start_acpi_update(START_STAGE_ACTIVATE);
+		break;
+	default:
+		ret = -ENOTTY;
+		break;
+	}
+
+	return ret;
+}
+
+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;
+	phys_addr_t phy_addr;
+	struct iov_iter iter;
+	struct iovec iov;
+	char *buf_ptr;
+	int ret;
+
+	if (!pfru_dev)
+		return -ENODEV;
+
+	ret = query_buffer(&info);
+	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);
+	if (ret)
+		goto unmap;
+
+	if (cap.status != DSM_SUCCEED)
+		ret = -EBUSY;
+	else if (!valid_version(buf_ptr, &cap))
+		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 struct miscdevice pfru_misc_dev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "pfru",
+	.nodename = "acpi_pfru",
+	.fops = &acpi_pfru_fops,
+};
+
+static int acpi_pfru_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int acpi_pfru_probe(struct platform_device *pdev)
+{
+	acpi_handle handle;
+	int ret;
+
+	/* Only one instance is allowed. */
+	if (pfru_dev)
+		return 0;
+
+	pfru_dev = kzalloc(sizeof(*pfru_dev), GFP_KERNEL);
+	if (!pfru_dev)
+		return -ENOMEM;
+
+	ret = guid_parse(PFRU_UUID, &pfru_dev->uuid);
+	if (ret)
+		goto out;
+
+	ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid);
+	if (ret)
+		goto out;
+
+	ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid);
+	if (ret)
+		goto out;
+
+	/* default rev id is 1 */
+	pfru_dev->rev_id = 1;
+	pfru_dev->dev = &pdev->dev;
+	handle = ACPI_HANDLE(pfru_dev->dev);
+	if (!acpi_has_method(handle, "_DSM")) {
+		dev_dbg(&pdev->dev, "Missing _DSM\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
+	return 0;
+out:
+	kfree(pfru_dev);
+	pfru_dev = NULL;
+
+	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,
+};
+
+static int __init pfru_init(void)
+{
+	int ret;
+
+	ret = misc_register(&pfru_misc_dev);
+	if (ret)
+		return ret;
+
+	return platform_driver_register(&acpi_pfru_driver);
+}
+
+static void __exit pfru_exit(void)
+{
+	platform_driver_unregister(&acpi_pfru_driver);
+	misc_deregister(&pfru_misc_dev);
+	kfree(pfru_dev);
+	pfru_dev = NULL;
+}
+
+module_init(pfru_init);
+module_exit(pfru_exit);
+
+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..127fc38638cb
--- /dev/null
+++ b/include/uapi/linux/pfru.h
@@ -0,0 +1,102 @@
+/* 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/ioctl.h>
+#include <linux/uuid.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 PFRU_IOC_SET_REV _IOW(PFRU_MAGIC, 0x01, unsigned int)
+#define PFRU_IOC_STAGE _IOW(PFRU_MAGIC, 0x02, unsigned int)
+#define PFRU_IOC_ACTIVATE _IOW(PFRU_MAGIC, 0x03, unsigned int)
+#define PFRU_IOC_STAGE_ACTIVATE _IOW(PFRU_MAGIC, 0x04, unsigned int)
+#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;
+}
+
+/* Capsule file payload header */
+struct pfru_payload_hdr {
+	__u32	sig;
+	__u32	hdr_version;
+	__u32	hdr_size;
+	__u32	hw_ver;
+	__u32	rt_ver;
+	uuid_t	platform_id;
+};
+
+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 {
+	enum pfru_dsm_status status;
+	__u32 update_cap;
+
+	uuid_t code_type;
+	__u32 fw_version;
+	__u32 code_rt_version;
+
+	uuid_t drv_type;
+	__u32 drv_rt_version;
+	__u32 drv_svn;
+
+	uuid_t platform_id;
+	uuid_t oem_id;
+
+	char oem_info[];
+};
+
+struct pfru_com_buf_info {
+	enum pfru_dsm_status status;
+	enum pfru_dsm_status ext_status;
+	__u64 addr_lo;
+	__u64 addr_hi;
+	__u32 buf_size;
+};
+
+struct pfru_updated_result {
+	enum pfru_dsm_status status;
+	enum pfru_dsm_status ext_status;
+	__u64 low_auth_time;
+	__u64 high_auth_time;
+	__u64 low_exec_time;
+	__u64 high_exec_time;
+};
+
+#endif /* __PFRU_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-16 10:26 [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers Chen Yu
  2021-10-16 10:31 ` [PATCH v4 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures Chen Yu
  2021-10-16 10:40 ` [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver Chen Yu
@ 2021-10-16 10:44 ` Chen Yu
  2021-10-16 15:10   ` Greg Kroah-Hartman
  2021-10-21  6:37   ` kernel test robot
  2021-10-16 10:47 ` [PATCH v4 4/4] tools: Introduce power/acpi/pfru/pfru Chen Yu
  3 siblings, 2 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-16 10:44 UTC (permalink / raw)
  To: linux-acpi
  Cc: Ard Biesheuvel, Rafael J. Wysocki, Len Brown, linux-kernel,
	Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Chen Yu

Platform Firmware Runtime Update(PFRU) Telemetry Service is part of RoT
(Root of Trust), which allows PFRU handler and other PFRU drivers to
produce telemetry data to upper layer OS consumer at runtime.

The linux provides interfaces for the user to query the parameters of
telemetry data, and the user could read out the telemetry data
accordingly.

Also add the ABI documentation.

Typical log looks like:
[SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
ProcessSmmRuntimeUpdate = START, Action = 2
[SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
FwVersion = 0, CodeInjectionVersion = 1
[ShadowSmmRuntimeUpdateImage]
Image = 0x74D9B000, ImageSize = 0x1172
[ProcessSmmRuntimeUpdate]
ShadowSmmRuntimeUpdateImage.Status = Success
[ValidateSmmRuntimeUpdateImage]
CapsuleHeader.CapsuleGuid = 6DCBD5ED-E82D-4C44-BDA1-7194199AD92A
[ValidateSmmRuntimeUpdateImage]
FmpCapHeader.Version = 1
[ValidateSmmRuntimeUpdateImage]
FmpCapImageHeader.UpdateImageTypeId = B2F84B79-7B6E-4E45-885F-3FB9BB185402
[ValidateSmmRuntimeUpdateImage]
SmmRuntimeUpdateVerifyImageWithDenylist.Status = Success
[ValidateSmmRuntimeUpdateImage]
SmmRuntimeUpdateVerifyImageWithAllowlist.Status = Success
[SmmCodeInjectionVerifyPayloadHeader]
PayloadHeader.Signature = 0x31494353
[SmmCodeInjectionVerifyPayloadHeader]
PayloadHeader.PlatformId = 63462139-A8B1-AA4E-9024-F2BB53EA4723
[SmmCodeInjectionVerifyPayloadHeader]
PayloadHeader.SupportedSmmFirmwareVersion = 0,
PayloadHeader.SmmCodeInjectionRuntimeVersion = 1
[ProcessSmmRuntimeUpdate]
ValidateSmmRuntimeUpdateImage.Status = Success
CPU CSR[0B102D28] Before = 7FBF830E
CPU CSR[0B102D28] After = 7FBF8310
[ProcessSmmRuntimeUpdate] ProcessSmmCodeInjection.Status = Success
[SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
ProcessSmmRuntimeUpdate = End, Status = Success

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v4: Remove the telemetry kernel Kconfig and combine it with pfru
    (Rafael J. Wysocki)
    Remove redundant parens. (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: Do similar clean up as pfru_update driver:
    Add sanity check for duplicated instance of ACPI device.
    Update the driver to work with allocated telem_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)
---
 drivers/acpi/pfru/pfru_update.c | 380 +++++++++++++++++++++++++++++++-
 include/uapi/linux/pfru.h       |  43 ++++
 2 files changed, 421 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/pfru/pfru_update.c b/drivers/acpi/pfru/pfru_update.c
index f57a39e79808..fe5b1bf0aeb3 100644
--- a/drivers/acpi/pfru/pfru_update.c
+++ b/drivers/acpi/pfru/pfru_update.c
@@ -55,6 +55,27 @@ enum update_index {
 	UPDATE_NR_IDX,
 };
 
+enum log_index {
+	LOG_STATUS_IDX,
+	LOG_EXT_STATUS_IDX,
+	LOG_MAX_SZ_IDX,
+	LOG_CHUNK1_LO_IDX,
+	LOG_CHUNK1_HI_IDX,
+	LOG_CHUNK1_SZ_IDX,
+	LOG_CHUNK2_LO_IDX,
+	LOG_CHUNK2_HI_IDX,
+	LOG_CHUNK2_SZ_IDX,
+	LOG_ROLLOVER_CNT_IDX,
+	LOG_RESET_CNT_IDX,
+	LOG_NR_IDX,
+};
+
+struct pfru_log_device {
+	struct device *dev;
+	guid_t uuid;
+	struct pfru_log_info info;
+};
+
 struct pfru_device {
 	guid_t uuid, code_uuid, drv_uuid;
 	int rev_id;
@@ -62,6 +83,299 @@ struct pfru_device {
 };
 
 static struct pfru_device *pfru_dev;
+static struct pfru_log_device *pfru_log_dev;
+
+static int get_pfru_log_data_info(struct pfru_log_data_info *data_info,
+				  int log_type)
+{
+	union acpi_object *out_obj, in_obj, in_buf;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	handle = ACPI_HANDLE(pfru_log_dev->dev);
+
+	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 = log_type;
+
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
+					  pfru_log_dev->info.log_revid, FUNC_GET_DATA,
+					  &in_obj, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return ret;
+
+	if (out_obj->package.count < LOG_NR_IDX)
+		goto free_acpi_buffer;
+
+	if (out_obj->package.elements[LOG_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->status = out_obj->package.elements[LOG_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->ext_status =
+		out_obj->package.elements[LOG_EXT_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_MAX_SZ_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->max_data_size =
+		out_obj->package.elements[LOG_MAX_SZ_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK1_LO_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk1_addr_lo =
+		out_obj->package.elements[LOG_CHUNK1_LO_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK1_HI_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk1_addr_hi =
+		out_obj->package.elements[LOG_CHUNK1_HI_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK1_SZ_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk1_size =
+		out_obj->package.elements[LOG_CHUNK1_SZ_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK2_LO_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk2_addr_lo =
+		out_obj->package.elements[LOG_CHUNK2_LO_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK2_HI_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk2_addr_hi =
+		out_obj->package.elements[LOG_CHUNK2_HI_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_CHUNK2_SZ_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->chunk2_size =
+		out_obj->package.elements[LOG_CHUNK2_SZ_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->rollover_cnt =
+		out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].integer.value;
+
+	if (out_obj->package.elements[LOG_RESET_CNT_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	data_info->reset_cnt =
+		out_obj->package.elements[LOG_RESET_CNT_IDX].integer.value;
+
+	ret = 0;
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static int set_pfru_log_level(int level)
+{
+	union acpi_object *out_obj, *obj, in_obj, in_buf;
+	enum pfru_dsm_status status;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	handle = ACPI_HANDLE(pfru_log_dev->dev);
+
+	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 = level;
+
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
+					  pfru_log_dev->info.log_revid, FUNC_SET_LEV,
+					  &in_obj, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return -EINVAL;
+
+	obj = &out_obj->package.elements[0];
+	status = obj->integer.value;
+	if (status)
+		goto free_acpi_buffer;
+
+	obj = &out_obj->package.elements[1];
+	status = obj->integer.value;
+	if (status)
+		goto free_acpi_buffer;
+
+	ret = 0;
+
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static int get_pfru_log_level(int *level)
+{
+	union acpi_object *out_obj, *obj;
+	enum pfru_dsm_status status;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	handle = ACPI_HANDLE(pfru_log_dev->dev);
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
+					  pfru_log_dev->info.log_revid, FUNC_GET_LEV,
+					  NULL, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return -EINVAL;
+
+	obj = &out_obj->package.elements[0];
+	if (obj->type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	status = obj->integer.value;
+	if (status)
+		goto free_acpi_buffer;
+
+	obj = &out_obj->package.elements[1];
+	if (obj->type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	status = obj->integer.value;
+	if (status)
+		goto free_acpi_buffer;
+
+	obj = &out_obj->package.elements[2];
+	if (obj->type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	*level = obj->integer.value;
+	ret = 0;
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static int valid_log_level(int level)
+{
+	return level == LOG_ERR || level == LOG_WARN ||
+		level == LOG_INFO || level == LOG_VERB;
+}
+
+static int valid_log_type(int type)
+{
+	return type == LOG_EXEC_IDX || type == LOG_HISTORY_IDX;
+}
+
+long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct pfru_log_data_info data_info;
+	struct pfru_log_info info;
+	void __user *p;
+	int ret = 0;
+
+	if (!pfru_log_dev)
+		return -ENODEV;
+
+	p = (void __user *)arg;
+
+	switch (cmd) {
+	case PFRU_LOG_IOC_SET_INFO:
+		if (copy_from_user(&info, p, sizeof(info)))
+			return -EFAULT;
+
+		if (pfru_valid_revid(info.log_revid))
+			pfru_log_dev->info.log_revid = info.log_revid;
+
+		if (valid_log_level(info.log_level)) {
+			ret = set_pfru_log_level(info.log_level);
+			if (ret)
+				return ret;
+			pfru_log_dev->info.log_level = info.log_level;
+		}
+
+		if (valid_log_type(info.log_type))
+			pfru_log_dev->info.log_type = info.log_type;
+
+		break;
+	case PFRU_LOG_IOC_GET_INFO:
+		ret = get_pfru_log_level(&info.log_level);
+		if (ret)
+			return ret;
+
+		info.log_type = pfru_log_dev->info.log_type;
+		info.log_revid = pfru_log_dev->info.log_revid;
+		if (copy_to_user(p, &info, sizeof(info)))
+			ret = -EFAULT;
+
+		break;
+	case PFRU_LOG_IOC_GET_DATA_INFO:
+		ret = get_pfru_log_data_info(&data_info, pfru_log_dev->info.log_type);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &data_info, sizeof(struct pfru_log_data_info)))
+			ret = -EFAULT;
+
+		break;
+	default:
+		ret = -ENOTTY;
+		break;
+	}
+	return ret;
+}
+
+ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
+		      size_t size, loff_t *off)
+{
+	struct pfru_log_data_info info;
+	phys_addr_t base_addr;
+	int buf_size, ret;
+	char *buf_ptr;
+
+	if (!pfru_log_dev)
+		return -ENODEV;
+
+	if (*off < 0)
+		return -EINVAL;
+
+	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
+	if (ret)
+		return ret;
+
+	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
+	/* pfru update has not been launched yet.*/
+	if (!base_addr)
+		return -EBUSY;
+
+	buf_size = info.max_data_size;
+	if (*off >= buf_size)
+		return 0;
+
+	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
+	if (IS_ERR(buf_ptr))
+		return PTR_ERR(buf_ptr);
+
+	size = min_t(size_t, size, buf_size - *off);
+	if (copy_to_user(ubuf, buf_ptr + *off, size))
+		ret = -EFAULT;
+	else
+		ret = 0;
+
+	memunmap(buf_ptr);
+
+	return ret ? ret : size;
+}
 
 static int query_capability(struct pfru_update_cap_info *cap)
 {
@@ -406,7 +720,7 @@ static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		ret = start_acpi_update(START_STAGE_ACTIVATE);
 		break;
 	default:
-		ret = -ENOTTY;
+		ret = pfru_log_ioctl(file, cmd, arg);
 		break;
 	}
 
@@ -467,6 +781,7 @@ static ssize_t pfru_write(struct file *file, const char __user *buf,
 static const struct file_operations acpi_pfru_fops = {
 	.owner		= THIS_MODULE,
 	.write		= pfru_write,
+	.read		= pfru_log_read,
 	.unlocked_ioctl = pfru_ioctl,
 	.llseek		= noop_llseek,
 };
@@ -478,6 +793,44 @@ static struct miscdevice pfru_misc_dev = {
 	.fops = &acpi_pfru_fops,
 };
 
+static int acpi_pfru_log_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static int acpi_pfru_log_probe(struct platform_device *pdev)
+{
+	acpi_handle handle;
+	int ret;
+
+	/* One instance is expected. */
+	if (pfru_log_dev)
+		return 0;
+
+	pfru_log_dev = kzalloc(sizeof(*pfru_log_dev), GFP_KERNEL);
+	if (!pfru_log_dev)
+		return -ENOMEM;
+
+	ret = guid_parse(PFRU_LOG_UUID, &pfru_log_dev->uuid);
+	if (ret)
+		goto out;
+
+	pfru_log_dev->info.log_revid = 1;
+	pfru_log_dev->dev = &pdev->dev;
+	handle = ACPI_HANDLE(pfru_log_dev->dev);
+	if (!acpi_has_method(handle, "_DSM")) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	return 0;
+out:
+	kfree(pfru_log_dev);
+	pfru_log_dev = NULL;
+
+	return ret;
+}
+
 static int acpi_pfru_remove(struct platform_device *pdev)
 {
 	return 0;
@@ -526,6 +879,21 @@ static int acpi_pfru_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static const struct acpi_device_id acpi_pfru_log_ids[] = {
+	{"INTC1081", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, acpi_pfru_log_ids);
+
+static struct platform_driver acpi_pfru_log_driver = {
+	.driver = {
+		.name = "pfru_log",
+		.acpi_match_table = acpi_pfru_log_ids,
+	},
+	.probe = acpi_pfru_log_probe,
+	.remove = acpi_pfru_log_remove,
+};
+
 static const struct acpi_device_id acpi_pfru_ids[] = {
 	{"INTC1080", 0},
 	{}
@@ -549,7 +917,11 @@ static int __init pfru_init(void)
 	if (ret)
 		return ret;
 
-	return platform_driver_register(&acpi_pfru_driver);
+	ret = platform_driver_register(&acpi_pfru_driver);
+	if (ret)
+		misc_deregister(&pfru_misc_dev);
+
+	return platform_driver_register(&acpi_pfru_log_driver);
 }
 
 static void __exit pfru_exit(void)
@@ -558,6 +930,10 @@ static void __exit pfru_exit(void)
 	misc_deregister(&pfru_misc_dev);
 	kfree(pfru_dev);
 	pfru_dev = NULL;
+
+	platform_driver_unregister(&acpi_pfru_log_driver);
+	kfree(pfru_log_dev);
+	pfru_log_dev = NULL;
 }
 
 module_init(pfru_init);
diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h
index 127fc38638cb..9ab74d9cd21a 100644
--- a/include/uapi/linux/pfru.h
+++ b/include/uapi/linux/pfru.h
@@ -99,4 +99,47 @@ struct pfru_updated_result {
 	__u64 high_exec_time;
 };
 
+#define PFRU_LOG_UUID	"75191659-8178-4D9D-B88F-AC5E5E93E8BF"
+
+/* Telemetry structures. */
+struct pfru_log_data_info {
+	enum pfru_dsm_status status;
+	enum pfru_dsm_status ext_status;
+	__u64 chunk1_addr_lo;
+	__u64 chunk1_addr_hi;
+	__u64 chunk2_addr_lo;
+	__u64 chunk2_addr_hi;
+	__u32 max_data_size;
+	__u32 chunk1_size;
+	__u32 chunk2_size;
+	__u32 rollover_cnt;
+	__u32 reset_cnt;
+};
+
+struct pfru_log_info {
+	__u32 log_level;
+	__u32 log_type;
+	__u32 log_revid;
+};
+
+/* Two logs: history and execution log */
+#define LOG_EXEC_IDX	0
+#define LOG_HISTORY_IDX	1
+#define NR_LOG_TYPE	2
+
+#define LOG_ERR		0
+#define LOG_WARN	1
+#define LOG_INFO	2
+#define LOG_VERB	4
+
+#define FUNC_SET_LEV		1
+#define FUNC_GET_LEV		2
+#define FUNC_GET_DATA		3
+
+#define LOG_NAME_SIZE		10
+
+#define PFRU_LOG_IOC_SET_INFO _IOW(PFRU_MAGIC, 0x05, struct pfru_log_info)
+#define PFRU_LOG_IOC_GET_INFO _IOR(PFRU_MAGIC, 0x06, struct pfru_log_info)
+#define PFRU_LOG_IOC_GET_DATA_INFO _IOR(PFRU_MAGIC, 0x07, struct pfru_log_data_info)
+
 #endif /* __PFRU_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4 4/4] tools: Introduce power/acpi/pfru/pfru
  2021-10-16 10:26 [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers Chen Yu
                   ` (2 preceding siblings ...)
  2021-10-16 10:44 ` [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry Chen Yu
@ 2021-10-16 10:47 ` Chen Yu
  3 siblings, 0 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-16 10:47 UTC (permalink / raw)
  To: linux-acpi
  Cc: Ard Biesheuvel, Rafael J. Wysocki, Len Brown, linux-kernel,
	Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Robert Moore, devel, Chen Yu

Introduce a user space tool to make use of the interface exposed by
Platform Firmware Runtime Update and Telemetry drivers. The users
can use this tool to do firmware code injection, driver update and
to retrieve the telemetry data.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
v4: Move the tool from tools/testings to tools/power/acpi.
    (Rafael J. Wysocki)
    Add corresponding man page for this tool.
    (Rafael J. Wysocki)
v3: No change since v2.
v2: Do not allow non-root user to run this test.
    (Shuah Khan)
    Test runs on platform without pfru_telemetry should skip
    instead of reporting failure/error.
    (Shuah Khan)
    Reuse uapi/linux/pfru.h instead of copying it into
    the test directory.
    (Mike Rapoport)
---
 include/uapi/linux/pfru.h      |   5 +
 tools/power/acpi/pfru/Makefile |  25 +++
 tools/power/acpi/pfru/pfru.8   | 137 ++++++++++++++
 tools/power/acpi/pfru/pfru.c   | 336 +++++++++++++++++++++++++++++++++
 4 files changed, 505 insertions(+)
 create mode 100644 tools/power/acpi/pfru/Makefile
 create mode 100644 tools/power/acpi/pfru/pfru.8
 create mode 100644 tools/power/acpi/pfru/pfru.c

diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h
index 9ab74d9cd21a..e7b3e17ef38c 100644
--- a/include/uapi/linux/pfru.h
+++ b/include/uapi/linux/pfru.h
@@ -8,7 +8,12 @@
 #define __PFRU_H__
 
 #include <linux/ioctl.h>
+#ifdef __KERNEL__
 #include <linux/uuid.h>
+#else
+#include <uuid/uuid.h>
+#include <linux/types.h>
+#endif
 
 #define PFRU_UUID		"ECF9533B-4A3C-4E89-939E-C77112601C6D"
 #define PFRU_CODE_INJ_UUID		"B2F84B79-7B6E-4E45-885F-3FB9BB185402"
diff --git a/tools/power/acpi/pfru/Makefile b/tools/power/acpi/pfru/Makefile
new file mode 100644
index 000000000000..54bf913b2a09
--- /dev/null
+++ b/tools/power/acpi/pfru/Makefile
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+CFLAGS += -Wall -O2
+CFLAGS += -DPFRU_HEADER='"../../../../include/uapi/linux/pfru.h"'
+BUILD_OUTPUT	:= $(CURDIR)
+
+ifeq ("$(origin O)", "command line")
+	BUILD_OUTPUT := $(O)
+endif
+
+pfru : pfru.c
+
+%: %.c
+	@mkdir -p $(BUILD_OUTPUT)
+	$(CC) $(CFLAGS) $< -o $(BUILD_OUTPUT)/$@ $(LDFLAGS) -luuid
+
+.PHONY : clean
+clean :
+	@rm -f $(BUILD_OUTPUT)/pfru
+
+install : pfru
+	install -d  $(DESTDIR)$(PREFIX)/bin
+	install $(BUILD_OUTPUT)/pfru $(DESTDIR)$(PREFIX)/bin/pfru
+	install -d  $(DESTDIR)$(PREFIX)/share/man/man8
+	install -m 644 pfru.8 $(DESTDIR)$(PREFIX)/share/man/man8
diff --git a/tools/power/acpi/pfru/pfru.8 b/tools/power/acpi/pfru/pfru.8
new file mode 100644
index 000000000000..2322b59ebada
--- /dev/null
+++ b/tools/power/acpi/pfru/pfru.8
@@ -0,0 +1,137 @@
+.TH "PFRU" "8" "October 2021" "pfru 1.0" ""
+.hy
+.SH Name
+.PP
+pfru \- Platform Firmware Runtime Update tool
+.SH SYNOPSIS
+.PP
+\f[B]pfru\f[R] [\f[I]Options\f[R]]
+.SH DESCRIPTION
+.PP
+The PFRU(Platform Firmware Runtime Update) kernel interface is designed
+to
+.PD 0
+.P
+.PD
+interact with the platform firmware interface defined in the
+.PD 0
+.P
+.PD
+Management Mode Firmware Runtime
+Update (https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf)
+.PD 0
+.P
+.PD
+\f[B]pfru\f[R] is the tool to interact with the kernel interface.
+.PD 0
+.P
+.PD
+.SH OPTIONS
+.TP
+.B \f[B]\-h\f[R], \f[B]\-\-help\f[R]
+Display helper information.
+.TP
+.B \f[B]\-l\f[R], \f[B]\-\-load\f[R]
+Load the capsule file into the system.
+To be more specific, the capsule file will be copied to the
+communication buffer.
+.TP
+.B \f[B]\-s\f[R], \f[B]\-\-stage\f[R]
+Stage the capsule image from communication buffer into Management Mode
+and perform authentication.
+.TP
+.B \f[B]\-a\f[R], \f[B]\-\-activate\f[R]
+Activate a previous staged capsule image.
+.TP
+.B \f[B]\-u\f[R], \f[B]\-\-update\f[R]
+Perform both stage and activation actions.
+.TP
+.B \f[B]\-q\f[R], \f[B]\-\-query\f[R]
+Query the update capability.
+.TP
+.B \f[B]\-d\f[R], \f[B]\-\-setrev\f[R]
+Set the revision ID of code injection/driver update.
+.TP
+.B \f[B]\-D\f[R], \f[B]\-\-setrevlog\f[R]
+Set the revision ID of telemetry.
+.TP
+.B \f[B]\-G\f[R], \f[B]\-\-getloginfo\f[R]
+Get telemetry log information and print it out.
+.TP
+.B \f[B]\-T\f[R], \f[B]\-\-type\f[R]
+Set the telemetry log data type.
+.TP
+.B \f[B]\-L\f[R], \f[B]\-\-level\f[R]
+Set the telemetry log level.
+.TP
+.B \f[B]\-R\f[R], \f[B]\-\-read\f[R]
+Read all the telemetry data and print it out.
+.SH EXAMPLES
+.PP
+\f[B]pfru \-G\f[R]
+.PP
+log_level:4
+.PD 0
+.P
+.PD
+log_type:0
+.PD 0
+.P
+.PD
+log_revid:2
+.PD 0
+.P
+.PD
+max_data_size:65536
+.PD 0
+.P
+.PD
+chunk1_size:0
+.PD 0
+.P
+.PD
+chunk2_size:1401
+.PD 0
+.P
+.PD
+rollover_cnt:0
+.PD 0
+.P
+.PD
+reset_cnt:4
+.PP
+\f[B]pfru \-q\f[R]
+.PP
+code injection image type:794bf8b2\-6e7b\-454e\-885f\-3fb9bb185402
+.PD 0
+.P
+.PD
+fw_version:0
+.PD 0
+.P
+.PD
+code_rt_version:1
+.PD 0
+.P
+.PD
+driver update image type:0e5f0b14\-f849\-7945\-ad81\-bc7b6d2bb245
+.PD 0
+.P
+.PD
+drv_rt_version:0
+.PD 0
+.P
+.PD
+drv_svn:0
+.PD 0
+.P
+.PD
+platform id:39214663\-b1a8\-4eaa\-9024\-f2bb53ea4723
+.PD 0
+.P
+.PD
+oem id:a36db54f\-ea2a\-e14e\-b7c4\-b5780e51ba3d
+.PP
+\f[B]pfru \-l yours.cap \-u \-T 1 \-L 4\f[R]
+.SH AUTHORS
+Chen Yu.
diff --git a/tools/power/acpi/pfru/pfru.c b/tools/power/acpi/pfru/pfru.c
new file mode 100644
index 000000000000..a65f895d856e
--- /dev/null
+++ b/tools/power/acpi/pfru/pfru.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Platform Firmware Runtime Update tool to do Management
+ * Mode code injection/driver update and telemetry retrieval.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include PFRU_HEADER
+
+#define MAX_LOG_SIZE 65536
+
+char *capsule_name;
+int action, query_cap, log_type, log_level, log_read, log_getinfo,
+	revid, log_revid;
+int set_log_level, set_log_type,
+	set_revid, set_log_revid;
+
+char *progname;
+
+static int valid_log_level(int level)
+{
+	return level == LOG_ERR || level == LOG_WARN ||
+		level == LOG_INFO || level == LOG_VERB;
+}
+
+static int valid_log_type(int type)
+{
+	return type == LOG_EXEC_IDX || type == LOG_HISTORY_IDX;
+}
+
+static void help(void)
+{
+	fprintf(stderr,
+		"usage: %s [OPTIONS]\n"
+		" code injection:\n"
+		"  -l, --load\n"
+		"  -s, --stage\n"
+		"  -a, --activate\n"
+		"  -u, --update [stage and activate]\n"
+		"  -q, --query\n"
+		"  -d, --revid update\n"
+		" telemetry:\n"
+		"  -G, --getloginfo\n"
+		"  -T, --type(0:execution, 1:history)\n"
+		"  -L, --level(0, 1, 2, 4)\n"
+		"  -R, --read\n"
+		"  -D, --revid log\n",
+		progname);
+}
+
+char *option_string = "l:sauqd:GT:L:RD:h";
+static struct option long_options[] = {
+	{"load", required_argument, 0, 'l'},
+	{"stage", no_argument, 0, 's'},
+	{"activate", no_argument, 0, 'a'},
+	{"update", no_argument, 0, 'u'},
+	{"query", no_argument, 0, 'q'},
+	{"getloginfo", no_argument, 0, 'G'},
+	{"type", required_argument, 0, 'T'},
+	{"level", required_argument, 0, 'L'},
+	{"read", no_argument, 0, 'R'},
+	{"setrev", required_argument, 0, 'd'},
+	{"setrevlog", required_argument, 0, 'D'},
+	{"help", no_argument, 0, 'h'},
+	{}
+};
+
+static void parse_options(int argc, char **argv)
+{
+	char *pathname;
+	int c;
+
+	pathname = strdup(argv[0]);
+	progname = basename(pathname);
+
+	while (1) {
+		int option_index = 0;
+
+		c = getopt_long(argc, argv, option_string,
+				long_options, &option_index);
+		if (c == -1)
+			break;
+
+		switch (c) {
+		case 'l':
+			capsule_name = optarg;
+			break;
+		case 's':
+			action = 1;
+			break;
+		case 'a':
+			action = 2;
+			break;
+		case 'u':
+			action = 3;
+			break;
+		case 'q':
+			query_cap = 1;
+			break;
+		case 'G':
+			log_getinfo = 1;
+			break;
+		case 'T':
+			log_type = atoi(optarg);
+			set_log_type = 1;
+			break;
+		case 'L':
+			log_level = atoi(optarg);
+			set_log_level = 1;
+			break;
+		case 'R':
+			log_read = 1;
+			break;
+		case 'd':
+			revid = atoi(optarg);
+			set_revid = 1;
+			break;
+		case 'D':
+			log_revid = atoi(optarg);
+			set_log_revid = 1;
+			break;
+		case 'h':
+			help();
+			break;
+		default:
+			break;
+		}
+	}
+}
+
+void print_cap(struct pfru_update_cap_info *cap)
+{
+	char *uuid = malloc(37);
+
+	if (!uuid) {
+		perror("Can not allocate uuid buffer\n");
+		exit(1);
+	}
+
+	uuid_unparse(cap->code_type, uuid);
+	printf("code injection image type:%s\n", uuid);
+	printf("fw_version:%d\n", cap->fw_version);
+	printf("code_rt_version:%d\n", cap->code_rt_version);
+
+	uuid_unparse(cap->drv_type, uuid);
+	printf("driver update image type:%s\n", uuid);
+	printf("drv_rt_version:%d\n", cap->drv_rt_version);
+	printf("drv_svn:%d\n", cap->drv_svn);
+
+	uuid_unparse(cap->platform_id, uuid);
+	printf("platform id:%s\n", uuid);
+	uuid_unparse(cap->oem_id, uuid);
+	printf("oem id:%s\n", uuid);
+
+	free(uuid);
+}
+
+int main(int argc, char *argv[])
+{
+	int fd_update, fd_capsule;
+	struct pfru_log_data_info data_info;
+	struct pfru_log_info info;
+	struct pfru_update_cap_info cap;
+	void *addr_map_capsule;
+	struct stat st;
+	char *log_buf;
+	int ret = 0;
+
+	if (getuid() != 0) {
+		printf("Please run the tool as root - Exiting.\n");
+		return 1;
+	}
+
+	parse_options(argc, argv);
+
+	fd_update = open("/dev/acpi_pfru", O_RDWR);
+	if (fd_update < 0) {
+		printf("PFRU device not supported - Quit...\n");
+		return 1;
+	}
+
+	if (query_cap) {
+		ret = ioctl(fd_update, PFRU_IOC_QUERY_CAP, &cap);
+		if (ret) {
+			perror("Query Update Capability info failed.");
+			return 1;
+		}
+
+		print_cap(&cap);
+	}
+
+	if (log_getinfo) {
+		ret = ioctl(fd_update, PFRU_LOG_IOC_GET_DATA_INFO, &data_info);
+		if (ret) {
+			perror("Get telemetry data info failed.");
+			return 1;
+		}
+
+		ret = ioctl(fd_update, PFRU_LOG_IOC_GET_INFO, &info);
+		if (ret) {
+			perror("Get telemetry info failed.");
+			return 1;
+		}
+
+		printf("log_level:%d\n", info.log_level);
+		printf("log_type:%d\n", info.log_type);
+		printf("log_revid:%d\n", info.log_revid);
+		printf("max_data_size:%d\n", data_info.max_data_size);
+		printf("chunk1_size:%d\n", data_info.chunk1_size);
+		printf("chunk2_size:%d\n", data_info.chunk2_size);
+		printf("rollover_cnt:%d\n", data_info.rollover_cnt);
+		printf("reset_cnt:%d\n", data_info.reset_cnt);
+
+		return 0;
+	}
+
+	info.log_level = -1;
+	info.log_type = -1;
+	info.log_revid = -1;
+
+	if (set_log_level) {
+		if (!valid_log_level(log_level)) {
+			printf("Invalid log level %d\n",
+			       log_level);
+		} else {
+			info.log_level = log_level;
+		}
+	}
+
+	if (set_log_type) {
+		if (!valid_log_type(log_type)) {
+			printf("Invalid log type %d\n",
+			       log_type);
+		} else {
+			info.log_type = log_type;
+		}
+	}
+
+	if (set_log_revid) {
+		if (!pfru_valid_revid(log_revid)) {
+			printf("Invalid log revid %d\n",
+			       log_revid);
+		} else {
+			info.log_revid = log_revid;
+		}
+	}
+
+	ret = ioctl(fd_update, PFRU_LOG_IOC_SET_INFO, &info);
+	if (ret) {
+		perror("Log information set failed.(log_level, log_type, log_revid)");
+		return 1;
+	}
+
+	if (set_revid) {
+		ret = ioctl(fd_update, PFRU_IOC_SET_REV, &revid);
+		if (ret) {
+			perror("pfru update revid set failed");
+			return 1;
+		}
+
+		printf("pfru update revid set to %d\n", revid);
+	}
+
+	if (capsule_name) {
+		fd_capsule = open(capsule_name, O_RDONLY);
+		if (fd_capsule < 0) {
+			perror("Can not open capsule file...");
+			return 1;
+		}
+
+		if (fstat(fd_capsule, &st) < 0) {
+			perror("Can not fstat capsule file...");
+			return 1;
+		}
+
+		addr_map_capsule = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED,
+					fd_capsule, 0);
+		if (addr_map_capsule == MAP_FAILED) {
+			perror("Failed to mmap capsule file.");
+			return 1;
+		}
+
+		ret = write(fd_update, (char *)addr_map_capsule, st.st_size);
+		printf("Load %d bytes of capsule file into the system\n",
+		       ret);
+
+		if (ret == -1) {
+			perror("Failed to load capsule file");
+			return 1;
+		}
+
+		munmap(addr_map_capsule, st.st_size);
+		printf("Load done.\n");
+	}
+
+	if (action) {
+		if (action == 1)
+			ret = ioctl(fd_update, PFRU_IOC_STAGE, NULL);
+		else if (action == 2)
+			ret = ioctl(fd_update, PFRU_IOC_ACTIVATE, NULL);
+		else if (action == 3)
+			ret = ioctl(fd_update, PFRU_IOC_STAGE_ACTIVATE, NULL);
+		else
+			return 1;
+		printf("Update finished, return %d\n", ret);
+	}
+
+	if (log_read) {
+		log_buf = malloc(MAX_LOG_SIZE + 1);
+		if (!log_buf) {
+			perror("log_buf allocate failed.");
+			return 1;
+		}
+
+		ret = read(fd_update, log_buf, MAX_LOG_SIZE);
+		if (ret == -1) {
+			perror("Read error.");
+			return 1;
+		}
+
+		log_buf[ret] = '\0';
+		printf("%s\n", log_buf);
+		free(log_buf);
+	}
+
+	return 0;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-16 10:44 ` [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry Chen Yu
@ 2021-10-16 15:10   ` Greg Kroah-Hartman
  2021-10-20  8:29     ` Chen Yu
  2021-10-21  6:37   ` kernel test robot
  1 sibling, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-16 15:10 UTC (permalink / raw)
  To: Chen Yu
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li

On Sat, Oct 16, 2021 at 06:44:31PM +0800, Chen Yu wrote:
> Platform Firmware Runtime Update(PFRU) Telemetry Service is part of RoT
> (Root of Trust), which allows PFRU handler and other PFRU drivers to
> produce telemetry data to upper layer OS consumer at runtime.
> 
> The linux provides interfaces for the user to query the parameters of
> telemetry data, and the user could read out the telemetry data
> accordingly.

What type of interface is this?  How does userspace interact with it?

> 
> Also add the ABI documentation.

Add it where?

> 
> Typical log looks like:
> [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> ProcessSmmRuntimeUpdate = START, Action = 2
> [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> FwVersion = 0, CodeInjectionVersion = 1
> [ShadowSmmRuntimeUpdateImage]
> Image = 0x74D9B000, ImageSize = 0x1172
> [ProcessSmmRuntimeUpdate]
> ShadowSmmRuntimeUpdateImage.Status = Success
> [ValidateSmmRuntimeUpdateImage]
> CapsuleHeader.CapsuleGuid = 6DCBD5ED-E82D-4C44-BDA1-7194199AD92A
> [ValidateSmmRuntimeUpdateImage]
> FmpCapHeader.Version = 1
> [ValidateSmmRuntimeUpdateImage]
> FmpCapImageHeader.UpdateImageTypeId = B2F84B79-7B6E-4E45-885F-3FB9BB185402
> [ValidateSmmRuntimeUpdateImage]
> SmmRuntimeUpdateVerifyImageWithDenylist.Status = Success
> [ValidateSmmRuntimeUpdateImage]
> SmmRuntimeUpdateVerifyImageWithAllowlist.Status = Success
> [SmmCodeInjectionVerifyPayloadHeader]
> PayloadHeader.Signature = 0x31494353
> [SmmCodeInjectionVerifyPayloadHeader]
> PayloadHeader.PlatformId = 63462139-A8B1-AA4E-9024-F2BB53EA4723
> [SmmCodeInjectionVerifyPayloadHeader]
> PayloadHeader.SupportedSmmFirmwareVersion = 0,
> PayloadHeader.SmmCodeInjectionRuntimeVersion = 1
> [ProcessSmmRuntimeUpdate]
> ValidateSmmRuntimeUpdateImage.Status = Success
> CPU CSR[0B102D28] Before = 7FBF830E
> CPU CSR[0B102D28] After = 7FBF8310
> [ProcessSmmRuntimeUpdate] ProcessSmmCodeInjection.Status = Success
> [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> ProcessSmmRuntimeUpdate = End, Status = Success

This log does not make any sense to me, where is it from?  Why the odd
line-wrapping?

> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> v4: Remove the telemetry kernel Kconfig and combine it with pfru
>     (Rafael J. Wysocki)
>     Remove redundant parens. (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: Do similar clean up as pfru_update driver:
>     Add sanity check for duplicated instance of ACPI device.
>     Update the driver to work with allocated telem_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)
> ---
>  drivers/acpi/pfru/pfru_update.c | 380 +++++++++++++++++++++++++++++++-
>  include/uapi/linux/pfru.h       |  43 ++++
>  2 files changed, 421 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/pfru/pfru_update.c b/drivers/acpi/pfru/pfru_update.c
> index f57a39e79808..fe5b1bf0aeb3 100644
> --- a/drivers/acpi/pfru/pfru_update.c
> +++ b/drivers/acpi/pfru/pfru_update.c
> @@ -55,6 +55,27 @@ enum update_index {
>  	UPDATE_NR_IDX,
>  };
>  
> +enum log_index {
> +	LOG_STATUS_IDX,
> +	LOG_EXT_STATUS_IDX,
> +	LOG_MAX_SZ_IDX,
> +	LOG_CHUNK1_LO_IDX,
> +	LOG_CHUNK1_HI_IDX,
> +	LOG_CHUNK1_SZ_IDX,
> +	LOG_CHUNK2_LO_IDX,
> +	LOG_CHUNK2_HI_IDX,
> +	LOG_CHUNK2_SZ_IDX,
> +	LOG_ROLLOVER_CNT_IDX,
> +	LOG_RESET_CNT_IDX,
> +	LOG_NR_IDX,
> +};
> +
> +struct pfru_log_device {
> +	struct device *dev;
> +	guid_t uuid;
> +	struct pfru_log_info info;
> +};
> +
>  struct pfru_device {
>  	guid_t uuid, code_uuid, drv_uuid;
>  	int rev_id;
> @@ -62,6 +83,299 @@ struct pfru_device {
>  };
>  
>  static struct pfru_device *pfru_dev;
> +static struct pfru_log_device *pfru_log_dev;
> +
> +static int get_pfru_log_data_info(struct pfru_log_data_info *data_info,
> +				  int log_type)
> +{
> +	union acpi_object *out_obj, in_obj, in_buf;
> +	acpi_handle handle;
> +	int ret = -EINVAL;
> +
> +	handle = ACPI_HANDLE(pfru_log_dev->dev);
> +
> +	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 = log_type;
> +
> +	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
> +					  pfru_log_dev->info.log_revid, FUNC_GET_DATA,
> +					  &in_obj, ACPI_TYPE_PACKAGE);
> +	if (!out_obj)
> +		return ret;
> +
> +	if (out_obj->package.count < LOG_NR_IDX)
> +		goto free_acpi_buffer;
> +
> +	if (out_obj->package.elements[LOG_STATUS_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->status = out_obj->package.elements[LOG_STATUS_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->ext_status =
> +		out_obj->package.elements[LOG_EXT_STATUS_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_MAX_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->max_data_size =
> +		out_obj->package.elements[LOG_MAX_SZ_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK1_LO_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk1_addr_lo =
> +		out_obj->package.elements[LOG_CHUNK1_LO_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK1_HI_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk1_addr_hi =
> +		out_obj->package.elements[LOG_CHUNK1_HI_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK1_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk1_size =
> +		out_obj->package.elements[LOG_CHUNK1_SZ_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK2_LO_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk2_addr_lo =
> +		out_obj->package.elements[LOG_CHUNK2_LO_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK2_HI_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk2_addr_hi =
> +		out_obj->package.elements[LOG_CHUNK2_HI_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_CHUNK2_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->chunk2_size =
> +		out_obj->package.elements[LOG_CHUNK2_SZ_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->rollover_cnt =
> +		out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].integer.value;
> +
> +	if (out_obj->package.elements[LOG_RESET_CNT_IDX].type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	data_info->reset_cnt =
> +		out_obj->package.elements[LOG_RESET_CNT_IDX].integer.value;
> +
> +	ret = 0;
> +free_acpi_buffer:
> +	ACPI_FREE(out_obj);
> +
> +	return ret;
> +}
> +
> +static int set_pfru_log_level(int level)
> +{
> +	union acpi_object *out_obj, *obj, in_obj, in_buf;
> +	enum pfru_dsm_status status;
> +	acpi_handle handle;
> +	int ret = -EINVAL;
> +
> +	handle = ACPI_HANDLE(pfru_log_dev->dev);
> +
> +	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 = level;
> +
> +	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
> +					  pfru_log_dev->info.log_revid, FUNC_SET_LEV,
> +					  &in_obj, ACPI_TYPE_PACKAGE);
> +	if (!out_obj)
> +		return -EINVAL;
> +
> +	obj = &out_obj->package.elements[0];
> +	status = obj->integer.value;
> +	if (status)
> +		goto free_acpi_buffer;
> +
> +	obj = &out_obj->package.elements[1];
> +	status = obj->integer.value;
> +	if (status)
> +		goto free_acpi_buffer;
> +
> +	ret = 0;
> +
> +free_acpi_buffer:
> +	ACPI_FREE(out_obj);
> +
> +	return ret;
> +}
> +
> +static int get_pfru_log_level(int *level)
> +{
> +	union acpi_object *out_obj, *obj;
> +	enum pfru_dsm_status status;
> +	acpi_handle handle;
> +	int ret = -EINVAL;
> +
> +	handle = ACPI_HANDLE(pfru_log_dev->dev);
> +	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_dev->uuid,
> +					  pfru_log_dev->info.log_revid, FUNC_GET_LEV,
> +					  NULL, ACPI_TYPE_PACKAGE);
> +	if (!out_obj)
> +		return -EINVAL;
> +
> +	obj = &out_obj->package.elements[0];
> +	if (obj->type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	status = obj->integer.value;
> +	if (status)
> +		goto free_acpi_buffer;
> +
> +	obj = &out_obj->package.elements[1];
> +	if (obj->type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	status = obj->integer.value;
> +	if (status)
> +		goto free_acpi_buffer;
> +
> +	obj = &out_obj->package.elements[2];
> +	if (obj->type != ACPI_TYPE_INTEGER)
> +		goto free_acpi_buffer;
> +
> +	*level = obj->integer.value;
> +	ret = 0;
> +free_acpi_buffer:
> +	ACPI_FREE(out_obj);
> +
> +	return ret;
> +}
> +
> +static int valid_log_level(int level)
> +{
> +	return level == LOG_ERR || level == LOG_WARN ||
> +		level == LOG_INFO || level == LOG_VERB;
> +}
> +
> +static int valid_log_type(int type)
> +{
> +	return type == LOG_EXEC_IDX || type == LOG_HISTORY_IDX;
> +}
> +
> +long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct pfru_log_data_info data_info;
> +	struct pfru_log_info info;
> +	void __user *p;
> +	int ret = 0;
> +
> +	if (!pfru_log_dev)
> +		return -ENODEV;
> +
> +	p = (void __user *)arg;
> +
> +	switch (cmd) {
> +	case PFRU_LOG_IOC_SET_INFO:
> +		if (copy_from_user(&info, p, sizeof(info)))
> +			return -EFAULT;
> +
> +		if (pfru_valid_revid(info.log_revid))
> +			pfru_log_dev->info.log_revid = info.log_revid;
> +
> +		if (valid_log_level(info.log_level)) {
> +			ret = set_pfru_log_level(info.log_level);
> +			if (ret)
> +				return ret;
> +			pfru_log_dev->info.log_level = info.log_level;
> +		}
> +
> +		if (valid_log_type(info.log_type))
> +			pfru_log_dev->info.log_type = info.log_type;
> +
> +		break;
> +	case PFRU_LOG_IOC_GET_INFO:
> +		ret = get_pfru_log_level(&info.log_level);
> +		if (ret)
> +			return ret;
> +
> +		info.log_type = pfru_log_dev->info.log_type;
> +		info.log_revid = pfru_log_dev->info.log_revid;
> +		if (copy_to_user(p, &info, sizeof(info)))
> +			ret = -EFAULT;
> +
> +		break;
> +	case PFRU_LOG_IOC_GET_DATA_INFO:
> +		ret = get_pfru_log_data_info(&data_info, pfru_log_dev->info.log_type);
> +		if (ret)
> +			return ret;
> +
> +		if (copy_to_user(p, &data_info, sizeof(struct pfru_log_data_info)))
> +			ret = -EFAULT;
> +
> +		break;
> +	default:
> +		ret = -ENOTTY;
> +		break;
> +	}
> +	return ret;
> +}
> +
> +ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
> +		      size_t size, loff_t *off)
> +{
> +	struct pfru_log_data_info info;
> +	phys_addr_t base_addr;
> +	int buf_size, ret;
> +	char *buf_ptr;
> +
> +	if (!pfru_log_dev)
> +		return -ENODEV;
> +
> +	if (*off < 0)
> +		return -EINVAL;
> +
> +	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
> +	if (ret)
> +		return ret;
> +
> +	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
> +	/* pfru update has not been launched yet.*/
> +	if (!base_addr)
> +		return -EBUSY;
> +
> +	buf_size = info.max_data_size;
> +	if (*off >= buf_size)
> +		return 0;
> +
> +	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
> +	if (IS_ERR(buf_ptr))
> +		return PTR_ERR(buf_ptr);
> +
> +	size = min_t(size_t, size, buf_size - *off);
> +	if (copy_to_user(ubuf, buf_ptr + *off, size))
> +		ret = -EFAULT;
> +	else
> +		ret = 0;

As all you are doing is mapping some memory and reading from it, why do
you need a read() file operation at all?  Why not just use mmap?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver
  2021-10-16 10:40 ` [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver Chen Yu
@ 2021-10-16 15:16   ` Greg Kroah-Hartman
  2021-10-19 16:33     ` Rafael J. Wysocki
  2021-10-20  8:00     ` Chen Yu
  0 siblings, 2 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-16 15:16 UTC (permalink / raw)
  To: Chen Yu
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Jonathan Corbet

On Sat, Oct 16, 2021 at 06:40: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.
> 
> [1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> v4: Add Documentation/ABI/testing/pfru (Rafael J. Wysocki)
>     Change all pr_debug() to dev_dbg() (Greg Kroah-Hartman,
>     Rafael J. Wysocki)
>     Change the error code ENOIOCTLCMD to ENOTTY in ioctl.
>     (Greg Kroah-Hartman)
>     Remove compat ioctl. (Greg Kroah-Hartman)
>     Change /dev/pfru/update to /dev/acpi_pfru (Greg Kroah-Hartman)
>     Remove valid_cap_type() and do sanity check in query_capability().
>     (Rafael J. Wysocki)
>     Remove the loop in query_capability().
>     (Rafael J. Wysocki)
>     Do not fail if the package has more elements than expected,
>     and return error if the number of package elements is too
>     small. (Rafael J. Wysocki)
>     Return the type or a negative error code in get_image_type()
>     (Rafael J. Wysocki)
>     Put the comment inside the function rather than outside.
>     (Rafael J. Wysocki)
>     Return the size or a negative error code adjust_efi_size()
>     (Rafael J. Wysocki)
>     Return -EINVAL rather than -EFAULT if revison id is incorrect.
>     (Rafael J. Wysocki)
>     Move the an read() of pfru into ioctl(), and using read() for
>     the telemetry retrieval. So as to avoid the telemetry device
>     file, the write() will be the code injection/update, the read()
>     will be telemetry retrieval and all of the rest can be ioctl()s
>     under one special device file.
>     (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)
> ---
>  Documentation/ABI/testing/pfru                |  41 ++
>  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
>  drivers/acpi/Kconfig                          |   1 +
>  drivers/acpi/Makefile                         |   1 +
>  drivers/acpi/pfru/Kconfig                     |  16 +
>  drivers/acpi/pfru/Makefile                    |   2 +
>  drivers/acpi/pfru/pfru_update.c               | 567 ++++++++++++++++++
>  include/uapi/linux/pfru.h                     | 102 ++++
>  8 files changed, 731 insertions(+)
>  create mode 100644 Documentation/ABI/testing/pfru
>  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/ABI/testing/pfru b/Documentation/ABI/testing/pfru
> new file mode 100644
> index 000000000000..b8bc81703f46
> --- /dev/null
> +++ b/Documentation/ABI/testing/pfru
> @@ -0,0 +1,41 @@
> +What:		/dev/acpi_pfru
> +Date:		October 2021
> +KernelVersion:	5.15
> +Contact:	Chen Yu <yu.c.chen@intel.com>
> +Description:
> +		The ioctl interface to drivers for platform firmware runtime
> +		update(PFRU). Following actions are supported:
> +
> +		* PFRU_IOC_QUERY_CAP: Read the PFRU Runtime Update capability.
> +		  The value is a structure of pfru_update_cap_info.
> +		  See include/uapi/linux/pfru.h for definition.
> +
> +		* PFRU_SET_REV: Set the Revision ID for PFRU Runtime Update.
> +		  It could be either 1 or 2.
> +
> +		* PFRU_IOC_STAGE: Stage a capsule image from communication
> +		  buffer and perform authentication.
> +
> +		* PFRU_IOC_ACTIVATE: Activate a previous staged capsule image.
> +
> +		* PFRU_IOC_STAGE_ACTIVATE: Perform both stage and activation
> +		  actions.
> +
> +		* PFRU_LOG_IOC_SET_INFO: set log information in Telemetry
> +		  Service. The input is a structure of pfru_log_info.
> +		  This structure includes log revision id(1 or 2),
> +		  log level(0 : Error Message, 1 : Warning Message,
> +		  2 : Informational Message, 4 : Verbose), log data type
> +		  (0 : Execution Log, 1 : History Information).
> +		  See include/uapi/linux/pfru.h for definition.
> +
> +		* PFRU_LOG_IOC_GET_INFO: get log information in Telemetry.
> +		  The output is a structure of pfru_log_info.
> +
> +		* PFRU_LOG_IOC_GET_DATA_INFO: get log data information in
> +		  Telemetry. The output is a structure of pfru_log_data_info.
> +		  See include/uapi/linux/pfru.h for definition.
> +
> +		Besides ioctl interface, write() and read() are supported on
> +		/dev/acpi_pfru. The write() will be the code injection/update,
> +		and the read() will be telemetry retrieval.

Do we normally describe ioctl interfaces in Documentation/ABI/?  Why not
just add this to the kernel doc with the structures you are creating?
Wouldn't that be easier?

Or are other acpi ioctl interfaces documented here already?

> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 2e8134059c87..6e5a82fff408 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-1F  uapi/linux/pfru.h                                       Platform Firmware Runtime Update and Telemetry

You are not using all of those values, right?

>  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..87388a46e760
> --- /dev/null
> +++ b/drivers/acpi/pfru/Kconfig
> @@ -0,0 +1,16 @@
> +# 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.
> +
> +	  For more information, see:
> +	  <file:Documentation/ABI/testing/pfru>
> +
> +	  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..f57a39e79808
> --- /dev/null
> +++ b/drivers/acpi/pfru/pfru_update.c
> @@ -0,0 +1,567 @@
> +// 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/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;
> +	struct device *dev;
> +};
> +
> +static struct pfru_device *pfru_dev;

Why is this a single variable?  Shouldn't this be per-device as the bus
provides it to you?

> +
> +static int query_capability(struct pfru_update_cap_info *cap)
> +{
> +	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)
> +{
> +	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(efi_manage_capsule_image_header_t *img_hdr)
> +{
> +	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;
> +	else if (guid_equal(image_type_id, &pfru_dev->drv_uuid))
> +		return DRIVER_UPDATE_TYPE;
> +	else
> +		return -EINVAL;
> +}
> +
> +static int adjust_efi_size(efi_manage_capsule_image_header_t *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(efi_manage_capsule_image_header_t);
> +	switch (img_hdr->ver) {
> +	case 1:
> +		size -= 2 * sizeof(u64);
> +		break;
> +	case 2:
> +		size -= sizeof(u64);
> +		break;
> +	default:
> +		/* only support version 1 and 2 */
> +		return -EINVAL;
> +	}
> +
> +	return size;
> +}
> +
> +static bool valid_version(const void *data, struct pfru_update_cap_info *cap)
> +{
> +	struct pfru_payload_hdr *payload_hdr;
> +	efi_capsule_header_t *cap_hdr;
> +	efi_manage_capsule_header_t *m_hdr;
> +	efi_manage_capsule_image_header_t *m_img_hdr;
> +	efi_image_auth_t *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 = (efi_manage_capsule_header_t *)(data + size);
> +	/*
> +	 * Current data structure size plus variable array indicated
> +	 * by number of (emb_drv_cnt + payload_cnt)
> +	 */
> +	size += sizeof(efi_manage_capsule_header_t) +
> +		      (m_hdr->emb_drv_cnt + m_hdr->payload_cnt) * sizeof(u64);
> +	m_img_hdr = (efi_manage_capsule_image_header_t *)(data + size);
> +
> +	type = get_image_type(m_img_hdr);
> +	if (type < 0)
> +		return false;
> +
> +	size = adjust_efi_size(m_img_hdr, size);
> +	if (size < 0)
> +		return false;
> +
> +	auth = (efi_image_auth_t *)(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)
> +{
> +	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)
> +{
> +	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);
> +	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;
> +	void __user *p;
> +	int ret = 0, rev;
> +
> +	if (!pfru_dev)
> +		return -ENODEV;
> +
> +	p = (void __user *)arg;
> +
> +	switch (cmd) {
> +	case PFRU_IOC_QUERY_CAP:
> +		ret = query_capability(&cap);
> +		if (ret)
> +			return ret;
> +
> +		if (copy_to_user(p, &cap, sizeof(cap)))
> +			return -EFAULT;
> +
> +		break;
> +	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;
> +		break;
> +	case PFRU_IOC_STAGE:
> +		ret = start_acpi_update(START_STAGE);
> +		break;
> +	case PFRU_IOC_ACTIVATE:
> +		ret = start_acpi_update(START_ACTIVATE);
> +		break;
> +	case PFRU_IOC_STAGE_ACTIVATE:
> +		ret = start_acpi_update(START_STAGE_ACTIVATE);
> +		break;
> +	default:
> +		ret = -ENOTTY;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +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;
> +	phys_addr_t phy_addr;
> +	struct iov_iter iter;
> +	struct iovec iov;
> +	char *buf_ptr;
> +	int ret;
> +
> +	if (!pfru_dev)
> +		return -ENODEV;
> +
> +	ret = query_buffer(&info);
> +	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);
> +	if (ret)
> +		goto unmap;
> +
> +	if (cap.status != DSM_SUCCEED)
> +		ret = -EBUSY;
> +	else if (!valid_version(buf_ptr, &cap))
> +		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 struct miscdevice pfru_misc_dev = {
> +	.minor = MISC_DYNAMIC_MINOR,
> +	.name = "pfru",
> +	.nodename = "acpi_pfru",
> +	.fops = &acpi_pfru_fops,
> +};
> +
> +static int acpi_pfru_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}

You do not free any of your memory???

> +
> +static int acpi_pfru_probe(struct platform_device *pdev)
> +{
> +	acpi_handle handle;
> +	int ret;
> +
> +	/* Only one instance is allowed. */
> +	if (pfru_dev)
> +		return 0;

Why is only one instance allowed?  Why add extra work to do this when it
really is not needed at all?  It is simpler and less code to make it so
that there is no restriction like this at all.

Also, the return value is incorrect, so your implementaion of trying to
keep only one instance does not work properly :(

> +	pfru_dev = kzalloc(sizeof(*pfru_dev), GFP_KERNEL);
> +	if (!pfru_dev)
> +		return -ENOMEM;
> +
> +	ret = guid_parse(PFRU_UUID, &pfru_dev->uuid);
> +	if (ret)
> +		goto out;
> +
> +	ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid);
> +	if (ret)
> +		goto out;
> +
> +	ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid);
> +	if (ret)
> +		goto out;
> +
> +	/* default rev id is 1 */
> +	pfru_dev->rev_id = 1;
> +	pfru_dev->dev = &pdev->dev;
> +	handle = ACPI_HANDLE(pfru_dev->dev);
> +	if (!acpi_has_method(handle, "_DSM")) {
> +		dev_dbg(&pdev->dev, "Missing _DSM\n");
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	return 0;
> +out:
> +	kfree(pfru_dev);
> +	pfru_dev = NULL;
> +
> +	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,
> +};
> +
> +static int __init pfru_init(void)
> +{
> +	int ret;
> +
> +	ret = misc_register(&pfru_misc_dev);
> +	if (ret)
> +		return ret;
> +

Why register this here, BEFORE you have a real device?  That looks like
a big race condition here :(

Register it per device you have in the system please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver
  2021-10-16 15:16   ` Greg Kroah-Hartman
@ 2021-10-19 16:33     ` Rafael J. Wysocki
  2021-10-20  8:00     ` Chen Yu
  1 sibling, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2021-10-19 16:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Chen Yu, ACPI Devel Maling List, Ard Biesheuvel,
	Rafael J. Wysocki, Len Brown, Linux Kernel Mailing List,
	Ashok Raj, Andy Shevchenko, Mike Rapoport, Aubrey Li,
	Jonathan Corbet

On Sat, Oct 16, 2021 at 5:16 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, Oct 16, 2021 at 06:40: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.
> >
> > [1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf
> >
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> > v4: Add Documentation/ABI/testing/pfru (Rafael J. Wysocki)
> >     Change all pr_debug() to dev_dbg() (Greg Kroah-Hartman,
> >     Rafael J. Wysocki)
> >     Change the error code ENOIOCTLCMD to ENOTTY in ioctl.
> >     (Greg Kroah-Hartman)
> >     Remove compat ioctl. (Greg Kroah-Hartman)
> >     Change /dev/pfru/update to /dev/acpi_pfru (Greg Kroah-Hartman)
> >     Remove valid_cap_type() and do sanity check in query_capability().
> >     (Rafael J. Wysocki)
> >     Remove the loop in query_capability().
> >     (Rafael J. Wysocki)
> >     Do not fail if the package has more elements than expected,
> >     and return error if the number of package elements is too
> >     small. (Rafael J. Wysocki)
> >     Return the type or a negative error code in get_image_type()
> >     (Rafael J. Wysocki)
> >     Put the comment inside the function rather than outside.
> >     (Rafael J. Wysocki)
> >     Return the size or a negative error code adjust_efi_size()
> >     (Rafael J. Wysocki)
> >     Return -EINVAL rather than -EFAULT if revison id is incorrect.
> >     (Rafael J. Wysocki)
> >     Move the an read() of pfru into ioctl(), and using read() for
> >     the telemetry retrieval. So as to avoid the telemetry device
> >     file, the write() will be the code injection/update, the read()
> >     will be telemetry retrieval and all of the rest can be ioctl()s
> >     under one special device file.
> >     (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)
> > ---
> >  Documentation/ABI/testing/pfru                |  41 ++
> >  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
> >  drivers/acpi/Kconfig                          |   1 +
> >  drivers/acpi/Makefile                         |   1 +
> >  drivers/acpi/pfru/Kconfig                     |  16 +
> >  drivers/acpi/pfru/Makefile                    |   2 +
> >  drivers/acpi/pfru/pfru_update.c               | 567 ++++++++++++++++++
> >  include/uapi/linux/pfru.h                     | 102 ++++
> >  8 files changed, 731 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/pfru
> >  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/ABI/testing/pfru b/Documentation/ABI/testing/pfru
> > new file mode 100644
> > index 000000000000..b8bc81703f46
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/pfru
> > @@ -0,0 +1,41 @@
> > +What:                /dev/acpi_pfru
> > +Date:                October 2021
> > +KernelVersion:       5.15
> > +Contact:     Chen Yu <yu.c.chen@intel.com>
> > +Description:
> > +             The ioctl interface to drivers for platform firmware runtime
> > +             update(PFRU). Following actions are supported:
> > +
> > +             * PFRU_IOC_QUERY_CAP: Read the PFRU Runtime Update capability.
> > +               The value is a structure of pfru_update_cap_info.
> > +               See include/uapi/linux/pfru.h for definition.
> > +
> > +             * PFRU_SET_REV: Set the Revision ID for PFRU Runtime Update.
> > +               It could be either 1 or 2.
> > +
> > +             * PFRU_IOC_STAGE: Stage a capsule image from communication
> > +               buffer and perform authentication.
> > +
> > +             * PFRU_IOC_ACTIVATE: Activate a previous staged capsule image.
> > +
> > +             * PFRU_IOC_STAGE_ACTIVATE: Perform both stage and activation
> > +               actions.
> > +
> > +             * PFRU_LOG_IOC_SET_INFO: set log information in Telemetry
> > +               Service. The input is a structure of pfru_log_info.
> > +               This structure includes log revision id(1 or 2),
> > +               log level(0 : Error Message, 1 : Warning Message,
> > +               2 : Informational Message, 4 : Verbose), log data type
> > +               (0 : Execution Log, 1 : History Information).
> > +               See include/uapi/linux/pfru.h for definition.
> > +
> > +             * PFRU_LOG_IOC_GET_INFO: get log information in Telemetry.
> > +               The output is a structure of pfru_log_info.
> > +
> > +             * PFRU_LOG_IOC_GET_DATA_INFO: get log data information in
> > +               Telemetry. The output is a structure of pfru_log_data_info.
> > +               See include/uapi/linux/pfru.h for definition.
> > +
> > +             Besides ioctl interface, write() and read() are supported on
> > +             /dev/acpi_pfru. The write() will be the code injection/update,
> > +             and the read() will be telemetry retrieval.
>
> Do we normally describe ioctl interfaces in Documentation/ABI/?  Why not
> just add this to the kernel doc with the structures you are creating?
> Wouldn't that be easier?

It would work I suppose.

> Or are other acpi ioctl interfaces documented here already?

No, they aren't, but there is Documentation/ABI/testing/gpio-cdev, for
example, so there is some practice there.  Whether it is good or bad
is a separate topic though.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver
  2021-10-16 15:16   ` Greg Kroah-Hartman
  2021-10-19 16:33     ` Rafael J. Wysocki
@ 2021-10-20  8:00     ` Chen Yu
  1 sibling, 0 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-20  8:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li, Jonathan Corbet

On Sat, Oct 16, 2021 at 05:16:17PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Oct 16, 2021 at 06:40: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.
> > 
> > [1] https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf
> > 
[snip...]
> 
> Do we normally describe ioctl interfaces in Documentation/ABI/?  Why not
> just add this to the kernel doc with the structures you are creating?
> Wouldn't that be easier?
> 
Ok, will move these comments into kernel doc in pfru.h.
> Or are other acpi ioctl interfaces documented here already?
> 
No other acpi ioctl interfaces, but there are some non-acpi
ioctl interfaces, such as rtc-cdev.
> > diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> > index 2e8134059c87..6e5a82fff408 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-1F  uapi/linux/pfru.h                                       Platform Firmware Runtime Update and Telemetry
> 
> You are not using all of those values, right?
> 
Not using all of them, will shrink the range to 8 in next
version.
> >  0xF3  00-3F  drivers/usb/misc/sisusbvga/sisusb.h                     sisfb (in development)
> >
[snip...]                                                                       <mailto:thomas@winischhofer.net>
> > +
> > +struct pfru_device {
> > +	guid_t uuid, code_uuid, drv_uuid;
> > +	int rev_id;
> > +	struct device *dev;
> > +};
> > +
> > +static struct pfru_device *pfru_dev;
> 
> Why is this a single variable?  Shouldn't this be per-device as the bus
> provides it to you?
>
[snip...] 
> > +
> > +static int acpi_pfru_probe(struct platform_device *pdev)
> > +{
> > +	acpi_handle handle;
> > +	int ret;
> > +
> > +	/* Only one instance is allowed. */
> > +	if (pfru_dev)
> > +		return 0;
> 
> Why is only one instance allowed?  Why add extra work to do this when it
> really is not needed at all?  It is simpler and less code to make it so
> that there is no restriction like this at all.
> 
> Also, the return value is incorrect, so your implementaion of trying to
> keep only one instance does not work properly :(
> 
Ok, I'll change it to per-device in next version. And the motivation of
using a single variable was that:
There would be only one instance of PFRU ACPI object and one PFRU Telemetry
ACPI object provided by BIOS, otherwise it is regarded as a BIOS bug for now.
But since per-device variable is more acceptable and scalable, will change
it to per-device in next version.
[snip...]
> > +};
> > +
[snip...]
> > +static int __init pfru_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = misc_register(&pfru_misc_dev);
> > +	if (ret)
> > +		return ret;
> > +
> 
> Why register this here, BEFORE you have a real device?  That looks like
> a big race condition here :(
> 
> Register it per device you have in the system please.
>
Ok.
Previously the pfru_misc_dev is shared between the PFRU device and
PFRU Telemetry device, so that the PFRU device is accessed via
pfru_misc_dev.write() and PFRU device is accessed via pfru_misc_dev.read().
The benefit of doing this is that, the user only deals with one misc_dev node
rather than two. Changing this to per-device scope would generate two misc_dev
nodes, and the user needs to deal with them respectively, but with better
scalability and less race condition. I'll revise it in next version.

Thanks,
Chenyu
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-20  8:29     ` Chen Yu
@ 2021-10-20  8:27       ` Greg Kroah-Hartman
  2021-10-20  8:53         ` Mike Rapoport
  2021-10-20  9:17         ` Chen Yu
  0 siblings, 2 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-10-20  8:27 UTC (permalink / raw)
  To: Chen Yu
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li

On Wed, Oct 20, 2021 at 04:29:39PM +0800, Chen Yu wrote:
> > > +ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
> > > +		      size_t size, loff_t *off)
> > > +{
> > > +	struct pfru_log_data_info info;
> > > +	phys_addr_t base_addr;
> > > +	int buf_size, ret;
> > > +	char *buf_ptr;
> > > +
> > > +	if (!pfru_log_dev)
> > > +		return -ENODEV;
> > > +
> > > +	if (*off < 0)
> > > +		return -EINVAL;
> > > +
> > > +	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
> > > +	/* pfru update has not been launched yet.*/
> > > +	if (!base_addr)
> > > +		return -EBUSY;
> > > +
> > > +	buf_size = info.max_data_size;
> > > +	if (*off >= buf_size)
> > > +		return 0;
> > > +
> > > +	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
> > > +	if (IS_ERR(buf_ptr))
> > > +		return PTR_ERR(buf_ptr);
> > > +
> > > +	size = min_t(size_t, size, buf_size - *off);
> > > +	if (copy_to_user(ubuf, buf_ptr + *off, size))
> > > +		ret = -EFAULT;
> > > +	else
> > > +		ret = 0;
> > 
> > As all you are doing is mapping some memory and reading from it, why do
> > you need a read() file operation at all?  Why not just use mmap?
> > 
> In the beginning mmap() interface was provided to the user. Then it was
> realized that there is no guarantee in the spec that, the physical address
> provided by the BIOS would remain unchanged. So instead of asking the user
> to mmap the file each time before reading the log, the read() is leveraged
> here to always memremap() the latest address.

So you are forced to memremap on _EVERY_ read call because the BIOS can
change things underneath us without the kernel knowing about it?  How
does the chunk2_addr_lo and _hi values change while the system is
running?  Where does that happen, and isn't this going to be a very slow
and expensive read call all the time?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-16 15:10   ` Greg Kroah-Hartman
@ 2021-10-20  8:29     ` Chen Yu
  2021-10-20  8:27       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Chen Yu @ 2021-10-20  8:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li

On Sat, Oct 16, 2021 at 05:10:25PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Oct 16, 2021 at 06:44:31PM +0800, Chen Yu wrote:
> > Platform Firmware Runtime Update(PFRU) Telemetry Service is part of RoT
> > (Root of Trust), which allows PFRU handler and other PFRU drivers to
> > produce telemetry data to upper layer OS consumer at runtime.
> > 
> > The linux provides interfaces for the user to query the parameters of
> > telemetry data, and the user could read out the telemetry data
> > accordingly.
> 
> What type of interface is this?  How does userspace interact with it?
> 
> > 
> > Also add the ABI documentation.
> 
> Add it where?
>
They are all ioctl interfaces, and was introduced in previous patch in
Documentation/ABI/testing/pfru. The way userspace interace with it is
introduced in next patch in the man page. I'll revise the commit log to
better describe how user could use it.
> > 
> > Typical log looks like:
> > [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> > ProcessSmmRuntimeUpdate = START, Action = 2
> > [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> > FwVersion = 0, CodeInjectionVersion = 1
> > [ShadowSmmRuntimeUpdateImage]
> > Image = 0x74D9B000, ImageSize = 0x1172
> > [ProcessSmmRuntimeUpdate]
> > ShadowSmmRuntimeUpdateImage.Status = Success
> > [ValidateSmmRuntimeUpdateImage]
> > CapsuleHeader.CapsuleGuid = 6DCBD5ED-E82D-4C44-BDA1-7194199AD92A
> > [ValidateSmmRuntimeUpdateImage]
> > FmpCapHeader.Version = 1
> > [ValidateSmmRuntimeUpdateImage]
> > FmpCapImageHeader.UpdateImageTypeId = B2F84B79-7B6E-4E45-885F-3FB9BB185402
> > [ValidateSmmRuntimeUpdateImage]
> > SmmRuntimeUpdateVerifyImageWithDenylist.Status = Success
> > [ValidateSmmRuntimeUpdateImage]
> > SmmRuntimeUpdateVerifyImageWithAllowlist.Status = Success
> > [SmmCodeInjectionVerifyPayloadHeader]
> > PayloadHeader.Signature = 0x31494353
> > [SmmCodeInjectionVerifyPayloadHeader]
> > PayloadHeader.PlatformId = 63462139-A8B1-AA4E-9024-F2BB53EA4723
> > [SmmCodeInjectionVerifyPayloadHeader]
> > PayloadHeader.SupportedSmmFirmwareVersion = 0,
> > PayloadHeader.SmmCodeInjectionRuntimeVersion = 1
> > [ProcessSmmRuntimeUpdate]
> > ValidateSmmRuntimeUpdateImage.Status = Success
> > CPU CSR[0B102D28] Before = 7FBF830E
> > CPU CSR[0B102D28] After = 7FBF8310
> > [ProcessSmmRuntimeUpdate] ProcessSmmCodeInjection.Status = Success
> > [SmmRuntimeUpdateHandler.ProcessSmmRuntimeUpdate]
> > ProcessSmmRuntimeUpdate = End, Status = Success
> 
> This log does not make any sense to me, where is it from?  Why the odd
> line-wrapping?
>
It is from the telemetry log generated by the BIOS. Since this content is
platform specific, I'll remove the log in next version.
> > +};
> > +
> > +
[snip...]
> > +ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
> > +		      size_t size, loff_t *off)
> > +{
> > +	struct pfru_log_data_info info;
> > +	phys_addr_t base_addr;
> > +	int buf_size, ret;
> > +	char *buf_ptr;
> > +
> > +	if (!pfru_log_dev)
> > +		return -ENODEV;
> > +
> > +	if (*off < 0)
> > +		return -EINVAL;
> > +
> > +	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
> > +	if (ret)
> > +		return ret;
> > +
> > +	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
> > +	/* pfru update has not been launched yet.*/
> > +	if (!base_addr)
> > +		return -EBUSY;
> > +
> > +	buf_size = info.max_data_size;
> > +	if (*off >= buf_size)
> > +		return 0;
> > +
> > +	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
> > +	if (IS_ERR(buf_ptr))
> > +		return PTR_ERR(buf_ptr);
> > +
> > +	size = min_t(size_t, size, buf_size - *off);
> > +	if (copy_to_user(ubuf, buf_ptr + *off, size))
> > +		ret = -EFAULT;
> > +	else
> > +		ret = 0;
> 
> As all you are doing is mapping some memory and reading from it, why do
> you need a read() file operation at all?  Why not just use mmap?
> 
In the beginning mmap() interface was provided to the user. Then it was
realized that there is no guarantee in the spec that, the physical address
provided by the BIOS would remain unchanged. So instead of asking the user
to mmap the file each time before reading the log, the read() is leveraged
here to always memremap() the latest address.

thanks,
Chenyu
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-20  8:27       ` Greg Kroah-Hartman
@ 2021-10-20  8:53         ` Mike Rapoport
  2021-10-20  9:17         ` Chen Yu
  1 sibling, 0 replies; 14+ messages in thread
From: Mike Rapoport @ 2021-10-20  8:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Chen Yu, linux-acpi, Ard Biesheuvel, Rafael J. Wysocki,
	Len Brown, linux-kernel, Ashok Raj, Andy Shevchenko, Aubrey Li

On Wed, Oct 20, 2021 at 10:27:28AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 20, 2021 at 04:29:39PM +0800, Chen Yu wrote:
> > > > +ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
> > > > +		      size_t size, loff_t *off)
> > > > +{
> > > > +	struct pfru_log_data_info info;
> > > > +	phys_addr_t base_addr;
> > > > +	int buf_size, ret;
> > > > +	char *buf_ptr;
> > > > +
> > > > +	if (!pfru_log_dev)
> > > > +		return -ENODEV;
> > > > +
> > > > +	if (*off < 0)
> > > > +		return -EINVAL;
> > > > +
> > > > +	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
> > > > +	/* pfru update has not been launched yet.*/
> > > > +	if (!base_addr)
> > > > +		return -EBUSY;
> > > > +
> > > > +	buf_size = info.max_data_size;
> > > > +	if (*off >= buf_size)
> > > > +		return 0;
> > > > +
> > > > +	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
> > > > +	if (IS_ERR(buf_ptr))
> > > > +		return PTR_ERR(buf_ptr);
> > > > +
> > > > +	size = min_t(size_t, size, buf_size - *off);
> > > > +	if (copy_to_user(ubuf, buf_ptr + *off, size))
> > > > +		ret = -EFAULT;
> > > > +	else
> > > > +		ret = 0;
> > > 
> > > As all you are doing is mapping some memory and reading from it, why do
> > > you need a read() file operation at all?  Why not just use mmap?
> > > 
> > In the beginning mmap() interface was provided to the user. Then it was
> > realized that there is no guarantee in the spec that, the physical address
> > provided by the BIOS would remain unchanged. So instead of asking the user
> > to mmap the file each time before reading the log, the read() is leveraged
> > here to always memremap() the latest address.
> 
> So you are forced to memremap on _EVERY_ read call because the BIOS can
> change things underneath us without the kernel knowing about it?  How
> does the chunk2_addr_lo and _hi values change while the system is
> running?  Where does that happen, and isn't this going to be a very slow
> and expensive read call all the time?

And maybe it is something that can be fixed in the spec so that the address
will remain constant?

(I realise it's wishful thinking, but sill...)

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-20  8:27       ` Greg Kroah-Hartman
  2021-10-20  8:53         ` Mike Rapoport
@ 2021-10-20  9:17         ` Chen Yu
  1 sibling, 0 replies; 14+ messages in thread
From: Chen Yu @ 2021-10-20  9:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-acpi, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Ashok Raj, Andy Shevchenko, Mike Rapoport,
	Aubrey Li

On Wed, Oct 20, 2021 at 10:27:28AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 20, 2021 at 04:29:39PM +0800, Chen Yu wrote:
> > > > +ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
> > > > +		      size_t size, loff_t *off)
> > > > +{
> > > > +	struct pfru_log_data_info info;
> > > > +	phys_addr_t base_addr;
> > > > +	int buf_size, ret;
> > > > +	char *buf_ptr;
> > > > +
> > > > +	if (!pfru_log_dev)
> > > > +		return -ENODEV;
> > > > +
> > > > +	if (*off < 0)
> > > > +		return -EINVAL;
> > > > +
> > > > +	ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
> > > > +	/* pfru update has not been launched yet.*/
> > > > +	if (!base_addr)
> > > > +		return -EBUSY;
> > > > +
> > > > +	buf_size = info.max_data_size;
> > > > +	if (*off >= buf_size)
> > > > +		return 0;
> > > > +
> > > > +	buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
> > > > +	if (IS_ERR(buf_ptr))
> > > > +		return PTR_ERR(buf_ptr);
> > > > +
> > > > +	size = min_t(size_t, size, buf_size - *off);
> > > > +	if (copy_to_user(ubuf, buf_ptr + *off, size))
> > > > +		ret = -EFAULT;
> > > > +	else
> > > > +		ret = 0;
> > > 
> > > As all you are doing is mapping some memory and reading from it, why do
> > > you need a read() file operation at all?  Why not just use mmap?
> > > 
> > In the beginning mmap() interface was provided to the user. Then it was
> > realized that there is no guarantee in the spec that, the physical address
> > provided by the BIOS would remain unchanged. So instead of asking the user
> > to mmap the file each time before reading the log, the read() is leveraged
> > here to always memremap() the latest address.
> 
> So you are forced to memremap on _EVERY_ read call because the BIOS can
> change things underneath us without the kernel knowing about it?  How
> does the chunk2_addr_lo and _hi values change while the system is
> running?  Where does that happen, and isn't this going to be a very slow
> and expensive read call all the time?
>
It was not documented in the spec whether the chunk address will change or not,
for safety I changed it from mmap() to read(). I'll try to reach the spec designer
and check if the address will change or not.

thanks,
Chenyu
 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry
  2021-10-16 10:44 ` [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry Chen Yu
  2021-10-16 15:10   ` Greg Kroah-Hartman
@ 2021-10-21  6:37   ` kernel test robot
  1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-10-21  6:37 UTC (permalink / raw)
  To: Chen Yu, linux-acpi
  Cc: llvm, kbuild-all, Ard Biesheuvel, Rafael J. Wysocki, Len Brown,
	linux-kernel, Greg Kroah-Hartman, Ashok Raj, Andy Shevchenko,
	Mike Rapoport, Aubrey Li

[-- Attachment #1: Type: text/plain, Size: 5495 bytes --]

Hi Chen,

I love your patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on efi/next linus/master v5.15-rc6 next-20211020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Chen-Yu/Introduce-Platform-Firmware-Runtime-Update-and-Telemetry-drivers/20211016-184349
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-c007-20211021 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 3cea2505fd8d99a9ba0cb625aecfe28a47c4e3f8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/93acf331ef196b860f6b34a5e9f8b3a249f1a0ce
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Chen-Yu/Introduce-Platform-Firmware-Runtime-Update-and-Telemetry-drivers/20211016-184349
        git checkout 93acf331ef196b860f6b34a5e9f8b3a249f1a0ce
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/acpi/pfru/pfru_update.c:280:6: warning: no previous prototype for function 'pfru_log_ioctl' [-Wmissing-prototypes]
   long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
        ^
   drivers/acpi/pfru/pfru_update.c:280:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
   ^
   static 
>> drivers/acpi/pfru/pfru_update.c:338:9: warning: no previous prototype for function 'pfru_log_read' [-Wmissing-prototypes]
   ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
           ^
   drivers/acpi/pfru/pfru_update.c:338:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
   ^
   static 
   2 warnings generated.


vim +/pfru_log_ioctl +280 drivers/acpi/pfru/pfru_update.c

   279	
 > 280	long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
   281	{
   282		struct pfru_log_data_info data_info;
   283		struct pfru_log_info info;
   284		void __user *p;
   285		int ret = 0;
   286	
   287		if (!pfru_log_dev)
   288			return -ENODEV;
   289	
   290		p = (void __user *)arg;
   291	
   292		switch (cmd) {
   293		case PFRU_LOG_IOC_SET_INFO:
   294			if (copy_from_user(&info, p, sizeof(info)))
   295				return -EFAULT;
   296	
   297			if (pfru_valid_revid(info.log_revid))
   298				pfru_log_dev->info.log_revid = info.log_revid;
   299	
   300			if (valid_log_level(info.log_level)) {
   301				ret = set_pfru_log_level(info.log_level);
   302				if (ret)
   303					return ret;
   304				pfru_log_dev->info.log_level = info.log_level;
   305			}
   306	
   307			if (valid_log_type(info.log_type))
   308				pfru_log_dev->info.log_type = info.log_type;
   309	
   310			break;
   311		case PFRU_LOG_IOC_GET_INFO:
   312			ret = get_pfru_log_level(&info.log_level);
   313			if (ret)
   314				return ret;
   315	
   316			info.log_type = pfru_log_dev->info.log_type;
   317			info.log_revid = pfru_log_dev->info.log_revid;
   318			if (copy_to_user(p, &info, sizeof(info)))
   319				ret = -EFAULT;
   320	
   321			break;
   322		case PFRU_LOG_IOC_GET_DATA_INFO:
   323			ret = get_pfru_log_data_info(&data_info, pfru_log_dev->info.log_type);
   324			if (ret)
   325				return ret;
   326	
   327			if (copy_to_user(p, &data_info, sizeof(struct pfru_log_data_info)))
   328				ret = -EFAULT;
   329	
   330			break;
   331		default:
   332			ret = -ENOTTY;
   333			break;
   334		}
   335		return ret;
   336	}
   337	
 > 338	ssize_t pfru_log_read(struct file *filp, char __user *ubuf,
   339			      size_t size, loff_t *off)
   340	{
   341		struct pfru_log_data_info info;
   342		phys_addr_t base_addr;
   343		int buf_size, ret;
   344		char *buf_ptr;
   345	
   346		if (!pfru_log_dev)
   347			return -ENODEV;
   348	
   349		if (*off < 0)
   350			return -EINVAL;
   351	
   352		ret = get_pfru_log_data_info(&info, pfru_log_dev->info.log_type);
   353		if (ret)
   354			return ret;
   355	
   356		base_addr = (phys_addr_t)(info.chunk2_addr_lo | (info.chunk2_addr_hi << 32));
   357		/* pfru update has not been launched yet.*/
   358		if (!base_addr)
   359			return -EBUSY;
   360	
   361		buf_size = info.max_data_size;
   362		if (*off >= buf_size)
   363			return 0;
   364	
   365		buf_ptr = memremap(base_addr, buf_size, MEMREMAP_WB);
   366		if (IS_ERR(buf_ptr))
   367			return PTR_ERR(buf_ptr);
   368	
   369		size = min_t(size_t, size, buf_size - *off);
   370		if (copy_to_user(ubuf, buf_ptr + *off, size))
   371			ret = -EFAULT;
   372		else
   373			ret = 0;
   374	
   375		memunmap(buf_ptr);
   376	
   377		return ret ? ret : size;
   378	}
   379	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35320 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-10-21  6:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-16 10:26 [PATCH v4 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers Chen Yu
2021-10-16 10:31 ` [PATCH v4 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures Chen Yu
2021-10-16 10:40 ` [PATCH v4 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver Chen Yu
2021-10-16 15:16   ` Greg Kroah-Hartman
2021-10-19 16:33     ` Rafael J. Wysocki
2021-10-20  8:00     ` Chen Yu
2021-10-16 10:44 ` [PATCH v4 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry Chen Yu
2021-10-16 15:10   ` Greg Kroah-Hartman
2021-10-20  8:29     ` Chen Yu
2021-10-20  8:27       ` Greg Kroah-Hartman
2021-10-20  8:53         ` Mike Rapoport
2021-10-20  9:17         ` Chen Yu
2021-10-21  6:37   ` kernel test robot
2021-10-16 10:47 ` [PATCH v4 4/4] tools: Introduce power/acpi/pfru/pfru Chen Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).