Introduce Platform Firmware Runtime Update and Telemetry drivers

PATCH Superseded Chen Yu 3 participants 15 messages Oct 25, 2021

Subsystems (4)

Documentation ACPI EFI ACPI component architecture (acpica)

lore

Share & tools

lore

Chen Yu

author

2021-10-25 06:25 UTC #0 lore raw

v6 cover 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

Signature · 1 line

2.25.1

Chen Yu

author

2021-10-25 06:25 UTC #1 lore raw

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(+)

include/linux/efi.h +50 -0

index 6b5d36babfcc..19ff834e1388 100644
@@ -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
  */

Signature · 1 line

2.25.1

Greg Kroah-Hartman

2021-10-25 06:44 UTC #2 lore raw

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

Quoted 33 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644

include/linux/efi.h typedef struct { · line 151

Quoted 4 lines

include/linux/efi.h +1 -0

@@ -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

Chen Yu

author

2021-10-25 11:45 UTC #3 lore raw

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

Quoted 37 lines · Greg Kroah-Hartman

Quoted 36 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644
@@ -148,6 +148,56 @@ typedef struct {
 	u32 imagesize;
 } efi_capsule_header_t;
 

Quoted 7 lines

include/linux/efi.h typedef struct { · line 151
Quoted 1 line
include/linux/efi.h +1 -0
+#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

Quoted 3 lines

thanks,
greg k-h

Greg Kroah-Hartman

2021-10-25 12:02 UTC #4 lore raw

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

Quoted 42 lines · Chen Yu

Quoted 41 lines · Greg Kroah-Hartman
Quoted 33 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644
include/linux/efi.h typedef struct { · line 151
Quoted 4 lines
include/linux/efi.h +1 -0
@@ -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?

Quoted 5 lines

Quoted 3 lines
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

Chen Yu

author

2021-10-25 12:47 UTC #5 lore raw

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

Quoted 43 lines · Greg Kroah-Hartman

Quoted 42 lines · Chen Yu
Quoted 41 lines · Greg Kroah-Hartman
Quoted 33 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644
include/linux/efi.h typedef struct { · line 151
Quoted 4 lines
include/linux/efi.h +1 -0
@@ -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?

Quoted 9 lines

Quoted 5 lines
Quoted 3 lines
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

Quoted 3 lines

thanks,
greg k-h

Ard Biesheuvel

maintainer

2021-10-25 13:12 UTC #6 lore raw

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

Quoted 57 lines · Chen Yu

Quoted 43 lines · Greg Kroah-Hartman
Quoted 42 lines · Chen Yu
Quoted 41 lines · Greg Kroah-Hartman
Quoted 33 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644
include/linux/efi.h typedef struct { · line 151
Quoted 4 lines
include/linux/efi.h +1 -0
@@ -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?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
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.

Quoted 9 lines

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.

Quoted 12 lines

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
Quoted 3 lines
thanks,
greg k-h

Chen Yu

author

2021-10-25 14:11 UTC #7 lore raw

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

Quoted 79 lines · Ard Biesheuvel

Quoted 57 lines · Chen Yu
Quoted 43 lines · Greg Kroah-Hartman
Quoted 42 lines · Chen Yu
Quoted 41 lines · Greg Kroah-Hartman
Quoted 33 lines · 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 <[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(+)
include/linux/efi.h +0 -0
index 6b5d36babfcc..19ff834e1388 100644
include/linux/efi.h typedef struct { · line 151
Quoted 4 lines
include/linux/efi.h +1 -0
@@ -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?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
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.
Quoted 9 lines
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.

Quoted 9 lines

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

Chen Yu

author

2021-10-25 06:25 UTC #8 lore raw

PATCH v6 2/4 drivers/acpi: Introduce Platform Firmware Runtime Update device driver

In reply to 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.

The corresponding userspace tool and man page will be introduced at tools/power/acpi/pfru.

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

Signed-off-by Chen Yu <[email protected]>

---
v6: Use Link: tag to add the specification download address.
(Andy Shevchenko)
Remove linux/uuid.h and use raw buffers to contain uuid. (Andy Shevchenko)
Drop comma for each terminator entry in the enum structure. (Andy Shevchenko)
Remove redundant 'else' in get_image_type().
(Andy Shevchenko)
Directly return results from the switch cases in adjust_efi_size() and pfru_ioctl().(Andy Shevchenko)
Keep comment style consistent by removing the period for one line comment.
(Andy Shevchenko)
Remove devm_kfree() if .probe() failed.
(Andy Shevchenko)
v5: Remove Documentation/ABI/pfru, and move the content to kernel doc
in include/uapi/linux/pfru.h (Greg Kroah-Hartman)
Shrink the range of ioctl numbers declared in Documentation/userspace-api/ioctl/ioctl-number.rst
from 16 to 8. (Greg Kroah-Hartman)
Change global variable struct pfru_device *pfru_dev to
per ACPI device. (Greg Kroah-Hartman)
Unregister the misc device in acpi_pfru_remove().
(Greg Kroah-Hartman)
Convert the kzalloc() to devm_kzalloc() in the driver so as to avoid freeing the memory. (Greg Kroah-Hartman)
Fix the compile error by declaring the pfru_log_ioctl() as static. (kernel test robot LKP)
Change to global variable misc_device to per ACPI device. (Greg Kroah-Hartman)
v4: Replace all pr_err() with dev_dbg() (Greg Kroah-Hartman,
Rafael J. Wysocki)
Returns ENOTTY rather than ENOIOCTLCMD if invalid ioctl command is provided. (Greg Kroah-Hartman)
Remove compat ioctl. (Greg Kroah-Hartman)
Rename /dev/pfru/pfru_update to /dev/acpi_pfru (Greg Kroah-Hartman) Simplify the check for element of the package in query_capability() (Rafael J. Wysocki)
Remove the loop in query_capability(), query_buffer() and query the package elemenet directly. (Rafael J. Wysocki)
Check the the number of elements in case the number of package elements is too small. (Rafael J. Wysocki)
Doing the assignment as initialization in get_image_type(). Meanwhile, returns the type or a negative error code in get_image_type(). (Rafael J. Wysocki)
Put the comments inside the function. (Rafael J. Wysocki) Returns the size or a negative error code in adjust_efi_size() (Rafael J. Wysocki)
Fix the return value from EFAULT to EINVAL if pfru_valid_revid() does not pass. (Rafael J. Wysocki)
Change the write() to be the code injection/update, the read() to be telemetry retrieval and all of the rest to be ioctl()s under one special device file.(Rafael J. Wysocki)
Putting empty code lines after an if () statement that is not followed by a block. (Rafael J. Wysocki)
Remove "goto" tags to make the code more readable. (Rafael J. Wysocki)
v3: Use __u32 instead of int and __64 instead of unsigned long
in include/uapi/linux/pfru.h (Greg Kroah-Hartman)
Rename the structure in uapi to start with a prefix pfru so as to avoid confusing in the global namespace. (Greg Kroah-Hartman)
v2: Add sanity check for duplicated instance of ACPI device.
Update the driver to work with allocated pfru_device objects. (Mike Rapoport)
For each switch case pair, get rid of the magic case numbers and add a default clause with the error handling.
(Mike Rapoport)
Move the obj->type checks outside the switch to reduce redundancy. (Mike Rapoport)
Parse the code_inj_id and drv_update_id at driver initialization time to reduce the re-parsing at runtime.(Mike Rapoport) Explain in detail how the size needs to be adjusted when doing version check.(Mike Rapoport)
Rename parse_update_result() to dump_update_result()(Mike Rapoport) Remove redundant return.(Mike Rapoport)
Do not expose struct capsulate_buf_info to uapi, since it is not needed in userspace.(Mike Rapoport)

---
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 drivers/acpi/Kconfig                          |   1 +
 drivers/acpi/Makefile                         |   1 +
 drivers/acpi/pfru/Kconfig                     |  13 +
 drivers/acpi/pfru/Makefile                    |   2 +
 drivers/acpi/pfru/pfru_update.c               | 567 ++++++++++++++++++
 include/uapi/linux/pfru.h                     | 186 ++++++
 7 files changed, 771 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

Documentation/userspace-api/ioctl/ioctl-number.rst +1 -0

index 2e8134059c87..6f7c86b6deb7 100644
@@ -365,6 +365,7 @@ Code  Seq#    Include File                                           Comments
                                                                      <mailto:[email protected]>
 0xE5  00-3F  linux/fuse.h
 0xEC  00-01  drivers/platform/chrome/cros_ec_dev.h                   ChromeOS EC driver
+0xEE  00-08  uapi/linux/pfru.h                                       Platform Firmware Runtime Update and Telemetry
 0xF3  00-3F  drivers/usb/misc/sisusbvga/sisusb.h                     sisfb (in development)
                                                                      <mailto:[email protected]>
 0xF6  all                                                            LTTng Linux Trace Toolkit Next Generation

drivers/acpi/Kconfig +1 -0

index 1da360c51d66..1d8d2e2cefac 100644
@@ -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

drivers/acpi/Makefile +1 -0

index 3018714e87d9..9c2c5ddff6ec 100644
@@ -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

drivers/acpi/pfru/Kconfig new file +13 -0

new file mode 100644
index 000000000000..5b31675b173a
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+config ACPI_PFRU
+	tristate "ACPI Platform Firmware Runtime Update (PFRU)"
+	depends on 64BIT
+	help
+	  In order to reduce the system reboot times and update the platform firmware
+	  in time, Platform Firmware Runtime Update is leveraged to patch the system
+	  without reboot. This driver supports Platform Firmware Runtime Update,
+	  which is composed of two parts: code injection and driver update. It also
+	  allows telemetry data to be retrieved from the platform firmware.
+
+	  To compile this driver as a module, choose M here:
+	  the module will be called pfru_update.

drivers/acpi/pfru/Makefile new file +2 -0

new file mode 100644
index 000000000000..098cbe80cf3d
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_ACPI_PFRU) += pfru_update.o

drivers/acpi/pfru/pfru_update.c new file +567 -0

567 lines not on this page · raw message

include/uapi/linux/pfru.h new file +186 -0

186 lines not on this page · raw message

Signature · 1 line

2.25.1

Greg Kroah-Hartman

2021-10-25 06:47 UTC #9 lore raw

Re: [PATCH v6 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver

Quoted 649 lines · Chen Yu

The corresponding userspace tool and man page will be introduced at tools/power/acpi/pfru.

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

Signed-off-by Chen Yu <[email protected]>

---
 .../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               | 567 ++++++++++++++++++
 include/uapi/linux/pfru.h                     | 186 ++++++
 7 files changed, 771 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

Documentation/userspace-api/ioctl/ioctl-number.rst +1 -0

index 2e8134059c87..6f7c86b6deb7 100644
@@ -365,6 +365,7 @@ Code  Seq#    Include File                                           Comments
                                                                      <mailto:[email protected]>
 0xE5  00-3F  linux/fuse.h
 0xEC  00-01  drivers/platform/chrome/cros_ec_dev.h                   ChromeOS EC driver
+0xEE  00-08  uapi/linux/pfru.h                                       Platform Firmware Runtime Update and Telemetry
 0xF3  00-3F  drivers/usb/misc/sisusbvga/sisusb.h                     sisfb (in development)
                                                                      <mailto:[email protected]>
 0xF6  all                                                            LTTng Linux Trace Toolkit Next Generation

drivers/acpi/Kconfig +1 -0

index 1da360c51d66..1d8d2e2cefac 100644
@@ -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

drivers/acpi/Makefile +1 -0

index 3018714e87d9..9c2c5ddff6ec 100644
@@ -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

drivers/acpi/pfru/Kconfig new file +13 -0

new file mode 100644
index 000000000000..5b31675b173a
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+config ACPI_PFRU
+	tristate "ACPI Platform Firmware Runtime Update (PFRU)"
+	depends on 64BIT
+	help
+	  In order to reduce the system reboot times and update the platform firmware
+	  in time, Platform Firmware Runtime Update is leveraged to patch the system
+	  without reboot. This driver supports Platform Firmware Runtime Update,
+	  which is composed of two parts: code injection and driver update. It also
+	  allows telemetry data to be retrieved from the platform firmware.
+
+	  To compile this driver as a module, choose M here:
+	  the module will be called pfru_update.

drivers/acpi/pfru/Makefile new file +2 -0

new file mode 100644
index 000000000000..098cbe80cf3d
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_ACPI_PFRU) += pfru_update.o

drivers/acpi/pfru/pfru_update.c new file +477 -0

new file mode 100644
index 000000000000..99a95b2ddc57
@@ -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 <[email protected]>
+ */
+#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;
+	struct miscdevice miscdev;
+};
+
+static inline struct pfru_device *to_pfru_dev(struct file *file)
+{
+	return container_of(file->private_data, struct pfru_device, miscdev);
+}
+
+static int query_capability(struct pfru_update_cap_info *cap,
+			    struct pfru_device *pfru_dev)
+{
+	union acpi_object *out_obj;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	handle = ACPI_HANDLE(pfru_dev->dev);
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid,
+					  pfru_dev->rev_id,
+					  FUNC_QUERY_UPDATE_CAP,
+					  NULL, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return ret;
+
+	if (out_obj->package.count < CAP_NR_IDX)
+		goto free_acpi_buffer;
+
+	if (out_obj->package.elements[CAP_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->status = out_obj->package.elements[CAP_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_UPDATE_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->update_cap = out_obj->package.elements[CAP_UPDATE_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_CODE_TYPE_IDX].type != ACPI_TYPE_BUFFER)
+		goto free_acpi_buffer;
+
+	memcpy(&cap->code_type,
+	       out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.pointer,
+	       out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length);
+
+	if (out_obj->package.elements[CAP_FW_VER_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->fw_version =
+		out_obj->package.elements[CAP_FW_VER_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_CODE_RT_VER_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->code_rt_version =
+		out_obj->package.elements[CAP_CODE_RT_VER_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_DRV_TYPE_IDX].type != ACPI_TYPE_BUFFER)
+		goto free_acpi_buffer;
+
+	memcpy(&cap->drv_type,
+	       out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.pointer,
+	       out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length);
+
+	if (out_obj->package.elements[CAP_DRV_RT_VER_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->drv_rt_version =
+		out_obj->package.elements[CAP_DRV_RT_VER_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_DRV_SVN_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	cap->drv_svn =
+		out_obj->package.elements[CAP_DRV_SVN_IDX].integer.value;
+
+	if (out_obj->package.elements[CAP_PLAT_ID_IDX].type != ACPI_TYPE_BUFFER)
+		goto free_acpi_buffer;
+
+	memcpy(&cap->platform_id,
+	       out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.pointer,
+	       out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length);
+
+	if (out_obj->package.elements[CAP_OEM_ID_IDX].type != ACPI_TYPE_BUFFER)
+		goto free_acpi_buffer;
+
+	memcpy(&cap->oem_id,
+	       out_obj->package.elements[CAP_OEM_ID_IDX].buffer.pointer,
+	       out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length);
+	ret = 0;
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static int query_buffer(struct pfru_com_buf_info *info,
+			struct pfru_device *pfru_dev)
+{
+	union acpi_object *out_obj;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	handle = ACPI_HANDLE(pfru_dev->dev);
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid,
+					  pfru_dev->rev_id, FUNC_QUERY_BUF,
+					  NULL, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return ret;
+
+	if (out_obj->package.count < BUF_NR_IDX)
+		goto free_acpi_buffer;
+
+	if (out_obj->package.elements[BUF_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	info->status = out_obj->package.elements[BUF_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[BUF_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	info->ext_status =
+		out_obj->package.elements[BUF_EXT_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[BUF_ADDR_LOW_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	info->addr_lo =
+		out_obj->package.elements[BUF_ADDR_LOW_IDX].integer.value;
+
+	if (out_obj->package.elements[BUF_ADDR_HI_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	info->addr_hi =
+		out_obj->package.elements[BUF_ADDR_HI_IDX].integer.value;
+
+	if (out_obj->package.elements[BUF_SIZE_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	info->buf_size = out_obj->package.elements[BUF_SIZE_IDX].integer.value;
+
+	ret = 0;
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static int get_image_type(efi_manage_capsule_image_header_t *img_hdr,
+			  struct pfru_device *pfru_dev)
+{
+	guid_t *image_type_id = &img_hdr->image_type_id;
+
+	/* check whether this is a code injection or driver update */
+	if (guid_equal(image_type_id, &pfru_dev->code_uuid))
+		return CODE_INJECT_TYPE;
+
+	if (guid_equal(image_type_id, &pfru_dev->drv_uuid))
+		return DRIVER_UPDATE_TYPE;
+
+	return -EINVAL;
+}
+
+static int adjust_efi_size(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:
+		return size - 2 * sizeof(u64);
+	case 2:
+		return size - sizeof(u64);
+	default:
+		/* only support version 1 and 2 */
+		return -EINVAL;
+	}
+}
+
+static bool valid_version(const void *data, struct pfru_update_cap_info *cap,
+			  struct pfru_device *pfru_dev)
+{
+	struct pfru_payload_hdr *payload_hdr;
+	efi_capsule_header_t *cap_hdr;
+	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, pfru_dev);
+	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,
+			       struct pfru_device *pfru_dev)
+{
+	dev_dbg(pfru_dev->dev, "Update result:\n");
+	dev_dbg(pfru_dev->dev, "Status:%d\n", result->status);
+	dev_dbg(pfru_dev->dev, "Extended Status:%d\n", result->ext_status);
+	dev_dbg(pfru_dev->dev, "Authentication Time Low:%lld\n",
+		result->low_auth_time);
+	dev_dbg(pfru_dev->dev, "Authentication Time High:%lld\n",
+		result->high_auth_time);
+	dev_dbg(pfru_dev->dev, "Execution Time Low:%lld\n",
+		result->low_exec_time);
+	dev_dbg(pfru_dev->dev, "Execution Time High:%lld\n",
+		result->high_exec_time);
+}
+
+static int start_acpi_update(int action, struct pfru_device *pfru_dev)
+{
+	union acpi_object *out_obj, in_obj, in_buf;
+	struct pfru_updated_result update_result;
+	acpi_handle handle;
+	int ret = -EINVAL;
+
+	memset(&in_obj, 0, sizeof(in_obj));
+	memset(&in_buf, 0, sizeof(in_buf));
+	in_obj.type = ACPI_TYPE_PACKAGE;
+	in_obj.package.count = 1;
+	in_obj.package.elements = &in_buf;
+	in_buf.type = ACPI_TYPE_INTEGER;
+	in_buf.integer.value = action;
+
+	handle = ACPI_HANDLE(pfru_dev->dev);
+	out_obj = acpi_evaluate_dsm_typed(handle, &pfru_dev->uuid,
+					  pfru_dev->rev_id, FUNC_START,
+					  &in_obj, ACPI_TYPE_PACKAGE);
+	if (!out_obj)
+		return ret;
+
+	if (out_obj->package.count < UPDATE_NR_IDX)
+		goto free_acpi_buffer;
+
+	if (out_obj->package.elements[UPDATE_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.status =
+		out_obj->package.elements[UPDATE_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[UPDATE_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.ext_status =
+		out_obj->package.elements[UPDATE_EXT_STATUS_IDX].integer.value;
+
+	if (out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.low_auth_time =
+		out_obj->package.elements[UPDATE_AUTH_TIME_LOW_IDX].integer.value;
+
+	if (out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.high_auth_time =
+		out_obj->package.elements[UPDATE_AUTH_TIME_HI_IDX].integer.value;
+
+	if (out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.low_exec_time =
+		out_obj->package.elements[UPDATE_EXEC_TIME_LOW_IDX].integer.value;
+
+	if (out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].type != ACPI_TYPE_INTEGER)
+		goto free_acpi_buffer;
+
+	update_result.high_exec_time =
+		out_obj->package.elements[UPDATE_EXEC_TIME_HI_IDX].integer.value;
+
+	dump_update_result(&update_result, pfru_dev);
+	ret = 0;
+
+free_acpi_buffer:
+	ACPI_FREE(out_obj);
+
+	return ret;
+}
+
+static long pfru_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct pfru_update_cap_info cap;
+	struct pfru_device *pfru_dev;
+	void __user *p;
+	int ret, rev;
+
+	pfru_dev = to_pfru_dev(file);
+	p = (void __user *)arg;
+
+	switch (cmd) {
+	case PFRU_IOC_QUERY_CAP:
+		ret = query_capability(&cap, pfru_dev);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &cap, sizeof(cap)))
+			return -EFAULT;
+
+		return 0;
+	case PFRU_IOC_SET_REV:
+		if (copy_from_user(&rev, p, sizeof(unsigned int)))
+			return -EFAULT;
+
+		if (!pfru_valid_revid(rev))
+			return -EINVAL;
+
+		pfru_dev->rev_id = rev;
+
+		return 0;
+	case PFRU_IOC_STAGE:
+		return start_acpi_update(START_STAGE, pfru_dev);
+	case PFRU_IOC_ACTIVATE:
+		return start_acpi_update(START_ACTIVATE, pfru_dev);
+	case PFRU_IOC_STAGE_ACTIVATE:
+		return start_acpi_update(START_STAGE_ACTIVATE, pfru_dev);
+	default:
+		return -ENOTTY;
+	}
+}
+
+static ssize_t pfru_write(struct file *file, const char __user *buf,
+			  size_t len, loff_t *ppos)
+{
+	struct pfru_update_cap_info cap;
+	struct pfru_com_buf_info info;
+	struct pfru_device *pfru_dev;
+	phys_addr_t phy_addr;
+	struct iov_iter iter;
+	struct iovec iov;
+	char *buf_ptr;
+	int ret;
+
+	pfru_dev = to_pfru_dev(file);
+
+	ret = query_buffer(&info, pfru_dev);
+	if (ret)
+		return ret;
+
+	if (len > info.buf_size)
+		return -EINVAL;
+
+	iov.iov_base = (void __user *)buf;
+	iov.iov_len = len;
+	iov_iter_init(&iter, WRITE, &iov, 1, len);
+
+	/* map the communication buffer */
+	phy_addr = (phys_addr_t)(info.addr_lo | (info.addr_hi << 32));
+	buf_ptr = memremap(phy_addr, info.buf_size, MEMREMAP_WB);
+	if (IS_ERR(buf_ptr))
+		return PTR_ERR(buf_ptr);
+
+	if (!copy_from_iter_full(buf_ptr, len, &iter)) {
+		ret = -EINVAL;
+		goto unmap;
+	}
+
+	/* check if the capsule header has a valid version number */
+	ret = query_capability(&cap, pfru_dev);
+	if (ret)
+		goto unmap;
+
+	if (cap.status != DSM_SUCCEED)
+		ret = -EBUSY;
+	else if (!valid_version(buf_ptr, &cap, pfru_dev))
+		ret = -EINVAL;
+unmap:
+	memunmap(buf_ptr);
+
+	return ret ?: len;
+}
+
+static const struct file_operations acpi_pfru_fops = {
+	.owner		= THIS_MODULE,
+	.write		= pfru_write,
+	.unlocked_ioctl = pfru_ioctl,
+	.llseek		= noop_llseek,
+};
+
+static int acpi_pfru_remove(struct platform_device *pdev)
+{
+	struct pfru_device *pfru_dev = platform_get_drvdata(pdev);
+

drivers/acpi/pfru/pfru_update.c line 487

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c new file +10 -0

+	misc_deregister(&pfru_dev->miscdev);
+
+	return 0;
+}
+
+static int acpi_pfru_probe(struct platform_device *pdev)
+{
+	struct pfru_device *pfru_dev;
+	acpi_handle handle;
+	static int pfru_idx;

Why not use an ida/idr structure for this? You never decrement this when the device is removed?

Quoted 16 lines

drivers/acpi/pfru/pfru_update.c new file +16 -0

+	int ret;
+
+	pfru_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_dev), GFP_KERNEL);
+	if (!pfru_dev)
+		return -ENOMEM;
+
+	ret = guid_parse(PFRU_UUID, &pfru_dev->uuid);
+	if (ret)
+		return ret;
+
+	ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid);
+	if (ret)
+		return ret;
+
+	ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid);
+	if (ret)

drivers/acpi/pfru/pfru_update.c line 513

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c new file +10 -0

+		return ret;
+
+	/* 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");
+		return -ENODEV;
+	}

Why not make this check first, before you allocate or parse anything?

Quoted 8 lines

drivers/acpi/pfru/pfru_update.c new file +8 -0

+
+	pfru_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
+	pfru_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						"pfru%d", pfru_idx);
+	if (!pfru_dev->miscdev.name)
+		return -ENOMEM;
+
+	pfru_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL,

drivers/acpi/pfru/pfru_update.c line 531

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c new file +10 -0

+						    "acpi_pfru%d", pfru_idx);
+	if (!pfru_dev->miscdev.nodename)
+		return -ENOMEM;
+
+	pfru_idx++;
+	pfru_dev->miscdev.fops = &acpi_pfru_fops;
+
+	ret = misc_register(&pfru_dev->miscdev);
+	if (ret)
+		return ret;

You forgot to set the parent of the misc device here, right? :(

Quoted 23 lines

drivers/acpi/pfru/pfru_update.c new file +23 -0

+
+	platform_set_drvdata(pdev, pfru_dev);
+
+	return 0;
+}
+
+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)
+{

drivers/acpi/pfru/pfru_update.c line 564

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c new file +10 -0

+	return platform_driver_register(&acpi_pfru_driver);
+}
+
+static void __exit pfru_exit(void)
+{
+	platform_driver_unregister(&acpi_pfru_driver);
+}
+
+module_init(pfru_init);
+module_exit(pfru_exit);

module_platform_driver()?

thanks,

greg k-h

Chen Yu

author

2021-10-25 14:11 UTC #10 lore raw

Re: [PATCH v6 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver

Quoted 1 line · Greg Kroah-Hartman

On Mon, Oct 25, 2021 at 02:25:16PM +0800, Chen Yu wrote:

[snip...]

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c line 487
Quoted 6 lines
drivers/acpi/pfru/pfru_update.c new file +6 -0
+
+static int acpi_pfru_probe(struct platform_device *pdev)
+{
+	struct pfru_device *pfru_dev;
+	acpi_handle handle;
+	static int pfru_idx;
Why not use an ida/idr structure for this? You never decrement this when the device is removed?

Will fix it and use ida_alloc() for this in next version.

Quoted 20 lines

drivers/acpi/pfru/pfru_update.c new file +20 -0

+	int ret;
+
+	pfru_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_dev), GFP_KERNEL);
+	if (!pfru_dev)
+		return -ENOMEM;
+
+	ret = guid_parse(PFRU_UUID, &pfru_dev->uuid);
+	if (ret)
+		return ret;
+
+	ret = guid_parse(PFRU_CODE_INJ_UUID, &pfru_dev->code_uuid);
+	if (ret)
+		return ret;
+
+	ret = guid_parse(PFRU_DRV_UPDATE_UUID, &pfru_dev->drv_uuid);
+	if (ret)
+		return ret;
+
+	/* default rev id is 1 */
+	pfru_dev->rev_id = 1;

Quoted 9 lines

drivers/acpi/pfru/pfru_update.c line 513
Quoted 6 lines
drivers/acpi/pfru/pfru_update.c new file +6 -0
+	pfru_dev->dev = &pdev->dev;
+	handle = ACPI_HANDLE(pfru_dev->dev);
+	if (!acpi_has_method(handle, "_DSM")) {
+		dev_dbg(&pdev->dev, "Missing _DSM\n");
+		return -ENODEV;
+	}
Why not make this check first, before you allocate or parse anything?

Will fix it in next version.

Quoted 12 lines

drivers/acpi/pfru/pfru_update.c new file +12 -0

+
+	pfru_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
+	pfru_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						"pfru%d", pfru_idx);
+	if (!pfru_dev->miscdev.name)
+		return -ENOMEM;
+
+	pfru_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						    "acpi_pfru%d", pfru_idx);
+	if (!pfru_dev->miscdev.nodename)
+		return -ENOMEM;
+

Quoted 10 lines

drivers/acpi/pfru/pfru_update.c line 531
Quoted 6 lines
drivers/acpi/pfru/pfru_update.c new file +6 -0
+	pfru_idx++;
+	pfru_dev->miscdev.fops = &acpi_pfru_fops;
+
+	ret = misc_register(&pfru_dev->miscdev);
+	if (ret)
+		return ret;
You forgot to set the parent of the misc device here, right? :(

Ah, yes, will fix it in next version.

Quoted 27 lines

drivers/acpi/pfru/pfru_update.c new file +27 -0

+
+	platform_set_drvdata(pdev, pfru_dev);
+
+	return 0;
+}
+
+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)
+{
+	return platform_driver_register(&acpi_pfru_driver);
+}
+
+static void __exit pfru_exit(void)

Quoted 9 lines

drivers/acpi/pfru/pfru_update.c line 564
Quoted 6 lines
drivers/acpi/pfru/pfru_update.c new file +6 -0
+{
+	platform_driver_unregister(&acpi_pfru_driver);
+}
+
+module_init(pfru_init);
+module_exit(pfru_exit);
module_platform_driver()?

Currently there are two platform drivers in this file, one is this platform driver, another one will be introduced in the subsequent patch for telemetry. Since the two platform drivers are treated as a whole, they are put into one file. Should I split them into two files?

thanks,
Chenyu

Quoted 3 lines

thanks,
greg k-h

Greg Kroah-Hartman

2021-10-25 14:59 UTC #11 lore raw

Re: [PATCH v6 2/4] drivers/acpi: Introduce Platform Firmware Runtime Update device driver

Quoted 10 lines · Chen Yu

Quoted 5 lines
drivers/acpi/pfru/pfru_update.c line 564
Quoted 2 lines
drivers/acpi/pfru/pfru_update.c new file +2 -0
+module_init(pfru_init);
+module_exit(pfru_exit);
module_platform_driver()?
Currently there are two platform drivers in this file, one is this platform driver, another one will be introduced in the subsequent patch for telemetry. Since the two platform drivers are treated as a whole, they are put into one file. Should I split them into two files?

If they bind to different hardware devices, then yes, they should be separate files as they are not sharing any common code here.

thanks,

greg k-h

Chen Yu

author

2021-10-25 06:26 UTC #12 lore raw

PATCH v6 3/4 drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry

In reply to 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.

The corresponding userspace tool and man page will be introduced at tools/power/acpi/pfru.

Signed-off-by Chen Yu <[email protected]>

---
v6: 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 on all possible architectures.
(Andy Shevchenko)
Directly return results from the switch cases in pfru_log_ioctl(). (Andy Shevchenko)
v5: Remove the log output sample 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) Fix the compile warning by declaring the pfru_log_ioctl() as static. (kernel test robot LKP)
v4: 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)
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 | 417 +++++++++++++++++++++++++++++++-
 include/uapi/linux/pfru.h       |  94 +++++++
 2 files changed, 510 insertions(+), 1 deletion(-)

drivers/acpi/pfru/pfru_update.c +416 -1

index 99a95b2ddc57..be018f36e759 100644
@@ -14,6 +14,7 @@
 #include <linux/fs.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
+#include <linux/mm.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>

@@ -55,6 +56,28 @@ 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 {
+	guid_t uuid;
+	struct pfru_log_info info;
+	struct device *dev;
+	struct miscdevice miscdev;
+};
+
 struct pfru_device {
 	guid_t uuid, code_uuid, drv_uuid;
 	int rev_id;

@@ -67,6 +90,386 @@ static inline struct pfru_device *to_pfru_dev(struct file *file)
 	return container_of(file->private_data, struct pfru_device, miscdev);
 }
 
+/************************** telemetry begin ************************/
+
+static inline struct pfru_log_device *to_pfru_log_dev(struct file *file)
+{
+	return container_of(file->private_data, struct pfru_log_device, miscdev);
+}
+
+static int get_pfru_log_data_info(struct pfru_log_data_info *data_info,
+				  struct pfru_log_device *pfru_log_dev)
+{
+	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 = pfru_log_dev->info.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, struct pfru_log_device *pfru_log_dev)
+{
+	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(struct pfru_log_device *pfru_log_dev)
+{
+	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;
+
+	ret = obj->integer.value;
+
+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;
+}
+
+static long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct pfru_log_device *pfru_log_dev;
+	struct pfru_log_data_info data_info;
+	struct pfru_log_info info;
+	void __user *p;
+	int ret = 0;
+
+	pfru_log_dev = to_pfru_log_dev(file);
+	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, pfru_log_dev);
+			if (ret < 0)
+				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;
+
+		return 0;
+	case PFRU_LOG_IOC_GET_INFO:
+		info.log_level = get_pfru_log_level(pfru_log_dev);
+		if (ret < 0)
+			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)))
+			return -EFAULT;
+
+		return 0;
+	case PFRU_LOG_IOC_GET_DATA_INFO:
+		ret = get_pfru_log_data_info(&data_info, pfru_log_dev);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &data_info, sizeof(struct pfru_log_data_info)))
+			return -EFAULT;
+
+		return 0;
+	default:
+		return -ENOTTY;
+	}
+}
+
+static int
+pfru_log_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct pfru_log_device *pfru_log_dev;
+	struct pfru_log_data_info info;
+	unsigned long psize, vsize;
+	phys_addr_t base_addr;
+	int ret;
+
+	if (vma->vm_flags & VM_WRITE)
+		return -EROFS;
+
+	/* changing from read to write with mprotect is not allowed */
+	vma->vm_flags &= ~VM_MAYWRITE;
+
+	pfru_log_dev = to_pfru_log_dev(file);
+
+	ret = get_pfru_log_data_info(&info, pfru_log_dev);
+	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 -ENODEV;
+
+	psize = info.max_data_size;
+	/* base address and total buffer size must be page aligned */
+	if (!PAGE_ALIGNED(base_addr) || !PAGE_ALIGNED(psize))
+		return -ENODEV;
+
+	vsize = vma->vm_end - vma->vm_start;
+	if (vsize > psize)
+		return -EINVAL;
+
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	if (io_remap_pfn_range(vma, vma->vm_start, PFN_DOWN(base_addr),
+			       vsize, vma->vm_page_prot))
+		return -EAGAIN;
+
+	return 0;
+}
+
+static const struct file_operations acpi_pfru_log_fops = {
+	.owner		= THIS_MODULE,
+	.mmap		= pfru_log_mmap,
+	.unlocked_ioctl = pfru_log_ioctl,
+	.llseek		= noop_llseek,
+};
+
+static int acpi_pfru_log_remove(struct platform_device *pdev)
+{
+	struct pfru_log_device *pfru_log_dev = platform_get_drvdata(pdev);
+
+	misc_deregister(&pfru_log_dev->miscdev);
+
+	return 0;
+}
+
+static int acpi_pfru_log_probe(struct platform_device *pdev)
+{
+	struct pfru_log_device *pfru_log_dev;
+	acpi_handle handle;
+	static int pfru_log_idx;
+	int ret;
+
+	pfru_log_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_log_dev), GFP_KERNEL);
+	if (!pfru_log_dev)
+		return -ENOMEM;
+
+	ret = guid_parse(PFRU_LOG_UUID, &pfru_log_dev->uuid);
+	if (ret)
+		return ret;
+
+	/* default rev id is 1 */
+	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")) {
+		dev_dbg(&pdev->dev, "Missing _DSM\n");
+		return -ENODEV;
+	}
+
+	pfru_log_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
+	pfru_log_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						    "pfru_telemetry%d",
+						    pfru_log_idx);
+	if (!pfru_log_dev->miscdev.name)
+		return -ENOMEM;
+
+	pfru_log_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+							"acpi_pfru_telemetry%d",
+							pfru_log_idx);
+	if (!pfru_log_dev->miscdev.nodename)
+		return -ENOMEM;
+
+	pfru_log_idx++;
+	pfru_log_dev->miscdev.fops = &acpi_pfru_log_fops;
+
+	ret = misc_register(&pfru_log_dev->miscdev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, pfru_log_dev);
+
+	return 0;
+}
+
+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_telemetry",
+		.acpi_match_table = acpi_pfru_log_ids,
+	},
+	.probe = acpi_pfru_log_probe,
+	.remove = acpi_pfru_log_remove,
+};
+
+/************************** telemetry end   *************************/
+
 static int query_capability(struct pfru_update_cap_info *cap,
 			    struct pfru_device *pfru_dev)
 {

@@ -552,11 +955,23 @@ static struct platform_driver acpi_pfru_driver = {
 
 static int __init pfru_init(void)
 {
-	return platform_driver_register(&acpi_pfru_driver);
+	int ret = platform_driver_register(&acpi_pfru_driver);
+
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&acpi_pfru_log_driver);
+	if (ret) {
+		platform_driver_unregister(&acpi_pfru_driver);
+		return ret;
+	}
+
+	return 0;
 }
 
 static void __exit pfru_exit(void)
 {
+	platform_driver_unregister(&acpi_pfru_log_driver);
 	platform_driver_unregister(&acpi_pfru_driver);
 }

include/uapi/linux/pfru.h +94 -0

98 lines not on this page · raw message

Signature · 1 line

2.25.1

Greg Kroah-Hartman

2021-10-25 06:48 UTC #13 lore raw

Re: [PATCH v6 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry

Quoted 4 lines · 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.

Quoted 8 lines

The linux provides interfaces for the user to query the parameters of telemetry data, and the user could read out the telemetry data accordingly.
The corresponding userspace tool and man page will be introduced at tools/power/acpi/pfru.
Signed-off-by Chen Yu <[email protected]>

The review comments I made on the previous patch in this series also apply here.

thanks,

greg k-h

Chen Yu

author

2021-10-25 06:26 UTC #14 lore raw

PATCH v6 4/4 tools: Introduce power/acpi/pfru/pfru

In reply to 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 <[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

tools/power/acpi/pfru/Makefile new file +25 -0

new file mode 100644
index 000000000000..54bf913b2a09
@@ -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

tools/power/acpi/pfru/pfru.8 new file +137 -0

137 lines not on this page · raw message

tools/power/acpi/pfru/pfru.c new file +404 -0

404 lines not on this page · raw message

Signature · 1 line

2.25.1