2021-10-25 07:01:59

by Chen Yu

[permalink] [raw]
Subject: [PATCH v6 0/4] Introduce Platform Firmware Runtime Update and Telemetry drivers

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
<https://uefi.org/sites/default/files/resources/Intel_MM_OS_Interface_Spec_Rev100.pdf>`
specification. 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.

=============
- Change from v5 to v6:
- Use Link: tag to add the specification download address.
(Andy Shevchenko)
- Drop comma for each terminator entry in the enum structure.
(Andy Shevchenko)
- Remove redundant 'else' in get_image_type().
(Andy Shevchenko)
- Directly return results from the switch cases in adjust_efi_size()
and pfru_ioctl().(Andy Shevchenko)
- Keep comment style consistency by removing the period for
one line comment.
(Andy Shevchenko)
- Remove devm_kfree() if .probe() failed.
(Andy Shevchenko)
- Remove linux/uuid.h and use raw buffers to contain uuid.
(Andy Shevchenko)
- Include types.h in pfru.h. (Andy Shevchenko)
- Use __u8[16] instead of uuid_t. (Andy Shevchenko)
- Replace enum in pfru.h with __u32 as enum size is not the
same size on all possible architectures.
(Andy Shevchenko)
- Simplify the userspace tool to use while loop for getopt_long().
(Andy Shevchenko)
- Change from v4 to v5:
- Remove Documentation/ABI/pfru, and move the content to kernel doc
in include/uapi/linux/pfru.h (Greg Kroah-Hartman)
- Shrink the range of ioctl numbers declared in
Documentation/userspace-api/ioctl/ioctl-number.rst
from 16 to 8. (Greg Kroah-Hartman)
- Change global variable struct pfru_device *pfru_dev to
per PFRU device. (Greg Kroah-Hartman)
- Unregister the misc device in acpi_pfru_remove().
(Greg Kroah-Hartman)
- Convert the kzalloc() to devm_kzalloc() in the driver so
as to avoid freeing the memory. (Greg Kroah-Hartman)
- Fix the compile warning by declaring the pfru_log_ioctl() as
static. (kernel test robot LKP)
- Change to global variable misc_device to per PFRU device.
(Greg Kroah-Hartman)
- Remove the telemetry output in commit log. (Greg Kroah-Hartman)
- Add link for corresponding userspace tool in the commit log.
(Greg Kroah-Hartman)
- Replace the telemetry .read() with .mmap() so that the userspace
could mmap once, and read multiple times. (Greg Kroah-Hartman)
- Change from v3 to v4:
- Add Documentation/ABI/testing/pfru to document the ABI and
remove Documentation/x86/pfru.rst (Rafael J. Wysocki)
- Replace all pr_err() with dev_dbg() (Greg Kroah-Hartman,
Rafael J. Wysocki)
- returns ENOTTY rather than ENOIOCTLCMD if invalid ioctl command
is provided. (Greg Kroah-Hartman)
- Remove compat ioctl. (Greg Kroah-Hartman)
- Rename /dev/pfru/pfru_update to /dev/acpi_pfru (Greg Kroah-Hartman)
- Simplify the check for element of the package in query_capability()
(Rafael J. Wysocki)
- Remove the loop in query_capability(), query_buffer() and query
the package elemenet directly. (Rafael J. Wysocki)
- Check the number of elements in case the number of package
elements is too small. (Rafael J. Wysocki)
- Doing the assignment as initialization in get_image_type().
Meanwhile, returns the type or a negative error code in
get_image_type(). (Rafael J. Wysocki)
- Put the comments inside the function. (Rafael J. Wysocki)
- Returns the size or a negative error code in adjust_efi_size()
(Rafael J. Wysocki)
- Fix the return value from EFAULT to EINVAL if pfru_valid_revid()
does not pass. (Rafael J. Wysocki)
- Change the write() to be the code injection/update, the read() to
be telemetry retrieval and all of the rest to be ioctl()s under
one special device file.(Rafael J. Wysocki)
- Remove redundant parens. (Rafael J. Wysocki)
- Putting empty code lines after an if () statement that is not
followed by a block. (Rafael J. Wysocki)
- Remove "goto" tags to make the code more readable. (Rafael J. Wysocki)
- Change from v2 to v3:
- Use valid types for structures that cross the user/kernel boundary
in the uapi header. (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)
- Change from v1 to v2:
- Add a spot in index.rst so it becomes part of the docs build
(Jonathan Corbet).
- Sticking to the 80-column limit(Jonathan Corbet).
- Underline lengths should match the title text(Jonathan Corbet).
- Use literal blocks for the code samples(Jonathan Corbet).
- 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)
- 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)

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

.../userspace-api/ioctl/ioctl-number.rst | 1 +
drivers/acpi/Kconfig | 1 +
drivers/acpi/Makefile | 1 +
drivers/acpi/pfru/Kconfig | 13 +
drivers/acpi/pfru/Makefile | 2 +
drivers/acpi/pfru/pfru_update.c | 982 ++++++++++++++++++
include/linux/efi.h | 50 +
include/uapi/linux/pfru.h | 280 +++++
tools/power/acpi/pfru/Makefile | 25 +
tools/power/acpi/pfru/pfru.8 | 137 +++
tools/power/acpi/pfru/pfru.c | 404 +++++++
11 files changed, 1896 insertions(+)
create mode 100644 drivers/acpi/pfru/Kconfig
create mode 100644 drivers/acpi/pfru/Makefile
create mode 100644 drivers/acpi/pfru/pfru_update.c
create mode 100644 include/uapi/linux/pfru.h
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


2021-10-25 10:44:53

by Chen Yu

[permalink] [raw]
Subject: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

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 <[email protected]>
---
v6: No change since v5.
v5: No change since v4.
v4: Revise the commit log to make it more clear. (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

2021-10-25 10:45:06

by Chen Yu

[permalink] [raw]
Subject: [PATCH v6 4/4] tools: Introduce power/acpi/pfru/pfru

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 <[email protected]>
---
v6: Simplify the userspace tool to use while loop for getopt_long().
(Andy Shevchenko)
v5: Replace the read() with mmap() so that the userspace
could mmap once, and read multiple times. (Greg Kroah-Hartman)
---
tools/power/acpi/pfru/Makefile | 25 ++
tools/power/acpi/pfru/pfru.8 | 137 +++++++++++
tools/power/acpi/pfru/pfru.c | 404 +++++++++++++++++++++++++++++++++
3 files changed, 566 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/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..d9cda7beaa3c
--- /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..f0795e06bf68
--- /dev/null
+++ b/tools/power/acpi/pfru/pfru.c
@@ -0,0 +1,404 @@
+// 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 <uuid/uuid.h>
+#include PFRU_HEADER
+
+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)
+{
+ int option_index = 0;
+ char *pathname;
+ int opt;
+
+ pathname = strdup(argv[0]);
+ progname = basename(pathname);
+
+ while ((opt = getopt_long_only(argc, argv, option_string,
+ long_options, &option_index)) != -1) {
+ switch (opt) {
+ 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;
+
+ 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_update_log, 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_pfru0", O_RDWR);
+ if (fd_update < 0) {
+ printf("PFRU device not supported - Quit...\n");
+ return 1;
+ }
+
+ fd_update_log = open("/dev/acpi_pfru_telemetry0", O_RDWR);
+ if (fd_update_log < 0) {
+ printf("PFRU telemetry 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.");
+ else
+ print_cap(&cap);
+
+ close(fd_update);
+ close(fd_update_log);
+
+ return ret;
+ }
+
+ if (log_getinfo) {
+ ret = ioctl(fd_update_log, PFRU_LOG_IOC_GET_DATA_INFO, &data_info);
+ if (ret) {
+ perror("Get telemetry data info failed.");
+
+ close(fd_update);
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ ret = ioctl(fd_update_log, PFRU_LOG_IOC_GET_INFO, &info);
+ if (ret) {
+ perror("Get telemetry info failed.");
+
+ close(fd_update);
+ close(fd_update_log);
+
+ 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, unchanged.\n",
+ log_revid);
+ } else {
+ info.log_revid = log_revid;
+ }
+ }
+
+ ret = ioctl(fd_update_log, PFRU_LOG_IOC_SET_INFO, &info);
+ if (ret) {
+ perror("Log information set failed.(log_level, log_type, log_revid)");
+ close(fd_update);
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ if (set_revid) {
+ ret = ioctl(fd_update, PFRU_IOC_SET_REV, &revid);
+ if (ret) {
+ perror("pfru update revid set failed");
+ close(fd_update);
+ close(fd_update_log);
+
+ 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...");
+ close(fd_update);
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ if (fstat(fd_capsule, &st) < 0) {
+ perror("Can not fstat capsule file...");
+ close(fd_capsule);
+ close(fd_update);
+ close(fd_update_log);
+
+ 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.");
+ close(fd_capsule);
+ close(fd_update);
+ close(fd_update_log);
+
+ 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");
+ close(fd_capsule);
+ close(fd_update);
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ munmap(addr_map_capsule, st.st_size);
+ close(fd_capsule);
+ 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 {
+ close(fd_update);
+ close(fd_update_log);
+
+ return 1;
+ }
+ printf("Update finished, return %d\n", ret);
+ }
+
+ close(fd_update);
+
+ if (log_read) {
+ void *p_mmap;
+ int max_data_sz;
+
+ ret = ioctl(fd_update_log, PFRU_LOG_IOC_GET_DATA_INFO, &data_info);
+ if (ret) {
+ perror("Get telemetry data info failed.");
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ max_data_sz = data_info.max_data_size;
+ if (!max_data_sz) {
+ printf("No telemetry data available.\n");
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ log_buf = malloc(max_data_sz + 1);
+ if (!log_buf) {
+ perror("log_buf allocate failed.");
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ p_mmap = mmap(NULL, max_data_sz, PROT_READ, MAP_SHARED, fd_update_log, 0);
+ if (p_mmap == MAP_FAILED) {
+ perror("mmap error.");
+ close(fd_update_log);
+
+ return 1;
+ }
+
+ memcpy(log_buf, p_mmap, max_data_sz);
+ log_buf[max_data_sz] = '\0';
+ printf("%s\n", log_buf);
+ free(log_buf);
+
+ munmap(p_mmap, max_data_sz);
+ }
+
+ close(fd_update_log);
+
+ return 0;
+}
--
2.25.1

2021-10-25 10:46:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> 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 <[email protected]>
> ---
> v6: No change since v5.
> v5: No change since v4.
> v4: Revise the commit log to make it more clear. (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)

Why is this pragma suddenly needed now in this file?

If you really need this for a specific structure, use the "__packed"
attribute please.

thanks,

greg k-h

2021-10-25 11:54:11

by Chen Yu

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, Oct 25, 2021 at 08:44:00AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> > 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 <[email protected]>
> > ---
> > v6: No change since v5.
> > v5: No change since v4.
> > v4: Revise the commit log to make it more clear. (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)
>
> Why is this pragma suddenly needed now in this file?
>
> If you really need this for a specific structure, use the "__packed"
> attribute please.
>
These two structures are required to be packed in the uefi spec, I'll change
them to "__packed".

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

2021-10-25 12:05:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, Oct 25, 2021 at 07:45:19PM +0800, Chen Yu wrote:
> On Mon, Oct 25, 2021 at 08:44:00AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> > > 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 <[email protected]>
> > > ---
> > > v6: No change since v5.
> > > v5: No change since v4.
> > > v4: Revise the commit log to make it more clear. (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)
> >
> > Why is this pragma suddenly needed now in this file?
> >
> > If you really need this for a specific structure, use the "__packed"
> > attribute please.
> >
> These two structures are required to be packed in the uefi spec, I'll change
> them to "__packed".

And they are the _only_ ones in this .h file that require this? I would
think that they all require this.

thanks,

greg k-h

2021-10-25 12:50:07

by Chen Yu

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, Oct 25, 2021 at 02:02:24PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 25, 2021 at 07:45:19PM +0800, Chen Yu wrote:
> > On Mon, Oct 25, 2021 at 08:44:00AM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> > > > 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 <[email protected]>
> > > > ---
> > > > v6: No change since v5.
> > > > v5: No change since v4.
> > > > v4: Revise the commit log to make it more clear. (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)
> > >
> > > Why is this pragma suddenly needed now in this file?
> > >
> > > If you really need this for a specific structure, use the "__packed"
> > > attribute please.
> > >
> > These two structures are required to be packed in the uefi spec, I'll change
> > them to "__packed".
>
> And they are the _only_ ones in this .h file that require this? I would
> think that they all require this.
>
I did a search in the uefi specification, and found 42 pack(1) structures,
while the other structures do not have pack(1) attribute.

It seems to me that whether the structures are required to be strictly packed
depends on the use case. Here's my understanding and I might be wrong: In this
patch, according to the skeleton of capsule file described in
[Figure 23-6 Firmware Management and Firmware Image Management headers]
in the uefi spec [1], the two structures are located at the beginning of
the capsule file, and followed by real payload. If these structure are packed
then the the adjacent binary payload could start on byte boundary without
padding, which might save space for capsule file.

For those structures that do not have strict pack requirement, the uefi spec
does not force to pack them.

Link: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_9_2021_03_18.pdf # [1]

thanks,
Chenyu


> thanks,
>
> greg k-h

2021-10-25 13:15:27

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, 25 Oct 2021 at 14:47, Chen Yu <[email protected]> wrote:
>
> On Mon, Oct 25, 2021 at 02:02:24PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 25, 2021 at 07:45:19PM +0800, Chen Yu wrote:
> > > On Mon, Oct 25, 2021 at 08:44:00AM +0200, Greg Kroah-Hartman wrote:
> > > > On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> > > > > 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 <[email protected]>
> > > > > ---
> > > > > v6: No change since v5.
> > > > > v5: No change since v4.
> > > > > v4: Revise the commit log to make it more clear. (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)
> > > >
> > > > Why is this pragma suddenly needed now in this file?
> > > >
> > > > If you really need this for a specific structure, use the "__packed"
> > > > attribute please.
> > > >
> > > These two structures are required to be packed in the uefi spec, I'll change
> > > them to "__packed".
> >
> > And they are the _only_ ones in this .h file that require this? I would
> > think that they all require this.
> >
> I did a search in the uefi specification, and found 42 pack(1) structures,
> while the other structures do not have pack(1) attribute.
>
> It seems to me that whether the structures are required to be strictly packed
> depends on the use case. Here's my understanding and I might be wrong: In this
> patch, according to the skeleton of capsule file described in
> [Figure 23-6 Firmware Management and Firmware Image Management headers]
> in the uefi spec [1], the two structures are located at the beginning of
> the capsule file, and followed by real payload. If these structure are packed
> then the the adjacent binary payload could start on byte boundary without
> padding, which might save space for capsule file.
>

Packing only affects internal padding, and a struct's size is never
padded to be a multiple of its alignment (which equals the largest
alignment of all its members). This of course assumes that you don't
abuse array indexing as a sizeof() operator.

However, the __packed attribute does indicate to the compiler that the
entire thing can appear misaligned in memory. So if one follows the
other in the capsule header, the __packed attribute may be appropriate
to ensure that the second one is not accessed using misaligned loads
and stores.

And then there is of course the ambiguity in alignment of uint64_t on
x86, which could be either 4 or 8 bytes depending on the context (and
UEFI targets all of them). So __packed may be used to disambiguate
between those if a uint64_t field appears on a boundary whose offset %
8 == 4.

So please use __packed rather than the pragma(), and apply it wherever
it is applied in the spec.



> For those structures that do not have strict pack requirement, the uefi spec
> does not force to pack them.
>
> Link: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_9_2021_03_18.pdf # [1]
>
> thanks,
> Chenyu
>
>
> > thanks,
> >
> > greg k-h

2021-10-25 14:33:49

by Chen Yu

[permalink] [raw]
Subject: Re: [PATCH v6 1/4] efi: Introduce EFI_FIRMWARE_MANAGEMENT_CAPSULE_HEADER and corresponding structures

On Mon, Oct 25, 2021 at 03:11:57PM +0200, Ard Biesheuvel wrote:
> On Mon, 25 Oct 2021 at 14:47, Chen Yu <[email protected]> wrote:
> >
> > On Mon, Oct 25, 2021 at 02:02:24PM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Oct 25, 2021 at 07:45:19PM +0800, Chen Yu wrote:
> > > > On Mon, Oct 25, 2021 at 08:44:00AM +0200, Greg Kroah-Hartman wrote:
> > > > > On Mon, Oct 25, 2021 at 02:25:04PM +0800, Chen Yu wrote:
> > > > > > 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 <[email protected]>
> > > > > > ---
> > > > > > v6: No change since v5.
> > > > > > v5: No change since v4.
> > > > > > v4: Revise the commit log to make it more clear. (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)
> > > > >
> > > > > Why is this pragma suddenly needed now in this file?
> > > > >
> > > > > If you really need this for a specific structure, use the "__packed"
> > > > > attribute please.
> > > > >
> > > > These two structures are required to be packed in the uefi spec, I'll change
> > > > them to "__packed".
> > >
> > > And they are the _only_ ones in this .h file that require this? I would
> > > think that they all require this.
> > >
> > I did a search in the uefi specification, and found 42 pack(1) structures,
> > while the other structures do not have pack(1) attribute.
> >
> > It seems to me that whether the structures are required to be strictly packed
> > depends on the use case. Here's my understanding and I might be wrong: In this
> > patch, according to the skeleton of capsule file described in
> > [Figure 23-6 Firmware Management and Firmware Image Management headers]
> > in the uefi spec [1], the two structures are located at the beginning of
> > the capsule file, and followed by real payload. If these structure are packed
> > then the the adjacent binary payload could start on byte boundary without
> > padding, which might save space for capsule file.
> >
>
> Packing only affects internal padding, and a struct's size is never
> padded to be a multiple of its alignment (which equals the largest
> alignment of all its members). This of course assumes that you don't
> abuse array indexing as a sizeof() operator.
>
> However, the __packed attribute does indicate to the compiler that the
> entire thing can appear misaligned in memory. So if one follows the
> other in the capsule header, the __packed attribute may be appropriate
> to ensure that the second one is not accessed using misaligned loads
> and stores.
>
> And then there is of course the ambiguity in alignment of uint64_t on
> x86, which could be either 4 or 8 bytes depending on the context (and
> UEFI targets all of them). So __packed may be used to disambiguate
> between those if a uint64_t field appears on a boundary whose offset %
> 8 == 4.
>
> So please use __packed rather than the pragma(), and apply it wherever
> it is applied in the spec.
>
Thanks for the explaination in detail! Ard. Will do in next version.

Thanks,
Chenyu