All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL 00/17] EFI updates for v4.18
@ 2018-05-04  5:59 Ard Biesheuvel
  2018-05-04  5:59 ` [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot Ard Biesheuvel
                   ` (16 more replies)
  0 siblings, 17 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Ard Biesheuvel, linux-kernel, Borislav Petkov, Daniel Kiper,
	Hans de Goede, Jarkko Sakkinen, Joey Zheng, Kim Phillips,
	Lukas Wunner, Mark Rutland, Shunyong Yang, Wei Yongjun,
	Yazen Ghannam

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-next

for you to fetch changes up to a0e781fedc0e975d2f33fda94e26d3f76331ee3d:

  efi/libstub/arm64: handle randomized TEXT_OFFSET (2018-05-03 19:00:27 +0200)

----------------------------------------------------------------
First batch of EFI changes for v4.18:
- Initialize UEFI secure boot state during Xen dom0 boot (Daniel Kiper)
- Decode x86 CPER data (Yazen Ghannam)
- Ignore unrealistically large option roms (Hans de Goede)
- Additional minor tweaks
- A fix for arm64 kernels with CONFIG_RANDOMIZE_TEXT_OFFSET=y and running
  with KASLR enabled. I was on the fence whether to send it as a fix or
  cc it to stable, so I chose the latter. (TEXT_OFFSET randomization is a
  debug feature that the distros don't enable)

----------------------------------------------------------------
Ard Biesheuvel (3):
      efi: fix efi_pci_io_protocol32 prototype for mixed mode
      efi: align efi_pci_io_protocol typedefs to type naming convention
      efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one

Borislav Petkov (1):
      efi/cper: Remove the INDENT_SP silliness

Daniel Kiper (1):
      x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

Hans de Goede (1):
      efi/x86: Ignore unrealistically large option roms

Mark Rutland (1):
      efi/libstub/arm64: handle randomized TEXT_OFFSET

Shunyong Yang (1):
      efi/capsule-loader: Don't output reset log when reset flags are not set

Wei Yongjun (1):
      efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

Yazen Ghannam (8):
      efi: Fix IA32/X64 Processor Error Record definition
      efi: Decode IA32/X64 Processor Error Section
      efi: Decode IA32/X64 Processor Error Info Structure
      efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs
      efi: Decode IA32/X64 Cache, TLB, and Bus Check structures
      efi: Decode additional IA32/X64 Bus Check fields
      efi: Decode IA32/X64 MS Check structure
      efi: Decode IA32/X64 Context Info structure

 arch/x86/boot/compressed/eboot.c          | 112 +++-------
 arch/x86/xen/efi.c                        |  57 +++++
 drivers/firmware/efi/Kconfig              |   5 +
 drivers/firmware/efi/Makefile             |   1 +
 drivers/firmware/efi/capsule-loader.c     |  14 +-
 drivers/firmware/efi/cper-arm.c           |   6 +-
 drivers/firmware/efi/cper-x86.c           | 356 ++++++++++++++++++++++++++++++
 drivers/firmware/efi/cper.c               |  16 +-
 drivers/firmware/efi/libstub/arm64-stub.c |   7 +
 drivers/firmware/efi/libstub/secureboot.c |   3 +
 drivers/firmware/efi/libstub/tpm.c        |   2 +-
 include/linux/cper.h                      |   4 +-
 include/linux/efi.h                       |  14 +-
 13 files changed, 496 insertions(+), 101 deletions(-)
 create mode 100644 drivers/firmware/efi/cper-x86.c

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

* [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:43   ` [tip:efi/core] " tip-bot for Daniel Kiper
  2018-05-04  5:59 ` [PATCH 02/17] efi/cper: Remove the INDENT_SP silliness Ard Biesheuvel
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Daniel Kiper, Ard Biesheuvel, linux-kernel

From: Daniel Kiper <daniel.kiper@oracle.com>

Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.

Note that part of drivers/firmware/efi/libstub/secureboot.c is duplicated
by this patch, only in this case, it runs in the context of the kernel
proper rather than UEFI boot context. The reason for the duplication is
that maintaining the original code to run correctly on ARM/arm64 as well
as on all the quirky x86 firmware we support is enough of a burden as it
is, and adding the x86/Xen execution context to that mix just so we can
reuse a single routine just isn't worth it.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
[ardb: explain rationale for code duplication]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/xen/efi.c                        | 57 +++++++++++++++++++++++
 drivers/firmware/efi/libstub/secureboot.c |  3 ++
 2 files changed, 60 insertions(+)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703be9ead..1804b27f9632 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
 	return &efi_systab_xen;
 }
 
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+	static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+	static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+	efi_status_t status;
+	u8 moksbstate, secboot, setupmode;
+	unsigned long size;
+
+	size = sizeof(secboot);
+	status = efi.get_variable(L"SecureBoot", &efi_variable_guid,
+				  NULL, &size, &secboot);
+
+	if (status == EFI_NOT_FOUND)
+		return efi_secureboot_mode_disabled;
+
+	if (status != EFI_SUCCESS)
+		goto out_efi_err;
+
+	size = sizeof(setupmode);
+	status = efi.get_variable(L"SetupMode", &efi_variable_guid,
+				  NULL, &size, &setupmode);
+
+	if (status != EFI_SUCCESS)
+		goto out_efi_err;
+
+	if (secboot == 0 || setupmode == 1)
+		return efi_secureboot_mode_disabled;
+
+	/* See if a user has put the shim into insecure mode. */
+	size = sizeof(moksbstate);
+	status = efi.get_variable(L"MokSBStateRT", &shim_guid,
+				  NULL, &size, &moksbstate);
+
+	/* If it fails, we don't care why. Default to secure. */
+	if (status != EFI_SUCCESS)
+		goto secure_boot_enabled;
+
+	if (moksbstate == 1)
+		return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+	pr_info("UEFI Secure Boot is enabled.\n");
+	return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+	pr_err("Could not determine UEFI Secure Boot status.\n");
+	return efi_secureboot_mode_unknown;
+}
+
 void __init xen_efi_init(void)
 {
 	efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
 	boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
 	boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
 
+	boot_params.secure_boot = xen_efi_get_secureboot();
+
 	set_bit(EFI_BOOT, &efi.flags);
 	set_bit(EFI_PARAVIRT, &efi.flags);
 	set_bit(EFI_64BIT, &efi.flags);
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb414c00..72d9dfbebf08 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@ static const efi_char16_t shim_MokSBState_name[] = L"MokSBState";
 
 /*
  * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
  */
 enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
 {
-- 
2.17.0

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

* [PATCH 02/17] efi/cper: Remove the INDENT_SP silliness
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
  2018-05-04  5:59 ` [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:44   ` [tip:efi/core] " tip-bot for Borislav Petkov
  2018-05-04  5:59 ` [PATCH 03/17] efi: Fix IA32/X64 Processor Error Record definition Ard Biesheuvel
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Borislav Petkov, Ard Biesheuvel, linux-kernel

From: Borislav Petkov <bp@suse.de>

A separate define just to print a space character is silly and
completely unneeded. Remove it.

Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-arm.c | 6 ++----
 drivers/firmware/efi/cper.c     | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
index 698e5c8e0c8d..502811344e81 100644
--- a/drivers/firmware/efi/cper-arm.c
+++ b/drivers/firmware/efi/cper-arm.c
@@ -30,8 +30,6 @@
 #include <acpi/ghes.h>
 #include <ras/ras_event.h>
 
-#define INDENT_SP	" "
-
 static const char * const arm_reg_ctx_strs[] = {
 	"AArch32 general purpose registers",
 	"AArch32 EL1 context registers",
@@ -283,7 +281,7 @@ void cper_print_proc_arm(const char *pfx,
 			pfx, proc->psci_state);
 	}
 
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 
 	err_info = (struct cper_arm_err_info *)(proc + 1);
 	for (i = 0; i < proc->err_info_num; i++) {
@@ -310,7 +308,7 @@ void cper_print_proc_arm(const char *pfx,
 		if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) {
 			printk("%serror_info: 0x%016llx\n", newpfx,
 			       err_info->error_info);
-			snprintf(infopfx, sizeof(infopfx), "%s%s", newpfx, INDENT_SP);
+			snprintf(infopfx, sizeof(infopfx), "%s ", newpfx);
 			cper_print_arm_err_info(infopfx, err_info->type,
 						err_info->error_info);
 		}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index c165933ebf38..ab21f1614007 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -37,8 +37,6 @@
 #include <acpi/ghes.h>
 #include <ras/ras_event.h>
 
-#define INDENT_SP	" "
-
 static char rcd_decode_str[CPER_REC_LEN];
 
 /*
@@ -433,7 +431,7 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
 		printk("%s""fru_text: %.20s\n", pfx, gdata->fru_text);
 
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 	if (guid_equal(sec_type, &CPER_SEC_PROC_GENERIC)) {
 		struct cper_sec_proc_generic *proc_err = acpi_hest_get_payload(gdata);
 
@@ -500,7 +498,7 @@ void cper_estatus_print(const char *pfx,
 		       "It has been corrected by h/w "
 		       "and requires no further action");
 	printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 
 	apei_estatus_for_each_section(estatus, gdata) {
 		cper_estatus_print_section(newpfx, gdata, sec_no);
-- 
2.17.0

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

* [PATCH 03/17] efi: Fix IA32/X64 Processor Error Record definition
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
  2018-05-04  5:59 ` [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot Ard Biesheuvel
  2018-05-04  5:59 ` [PATCH 02/17] efi/cper: Remove the INDENT_SP silliness Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:44   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 04/17] efi: Decode IA32/X64 Processor Error Section Ard Biesheuvel
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

Based on UEFI 2.7 Table 255. Processor Error Record, the "Local APIC_ID"
field is 8 bytes but Linux defines this field as 1 byte.

Fix this in the struct cper_sec_proc_ia definition.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 include/linux/cper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cper.h b/include/linux/cper.h
index d14ef4e77c8a..4b5f8459b403 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -381,7 +381,7 @@ struct cper_sec_proc_generic {
 /* IA32/X64 Processor Error Section */
 struct cper_sec_proc_ia {
 	__u64	validation_bits;
-	__u8	lapic_id;
+	__u64	lapic_id;
 	__u8	cpuid[48];
 };
 
-- 
2.17.0

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

* [PATCH 04/17] efi: Decode IA32/X64 Processor Error Section
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 03/17] efi: Fix IA32/X64 Processor Error Record definition Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:45   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 05/17] efi: Decode IA32/X64 Processor Error Info Structure Ard Biesheuvel
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

Recognize the IA32/X64 Processor Error Section.

Do the section decoding in a new "cper-x86.c" file and add this to the
Makefile depending on a new "UEFI_CPER_X86" config option.

Print the Local APIC ID and CPUID info from the Processor Error Record.

The "Processor Error Info" and "Processor Context" fields will be
decoded in following patches.

Based on UEFI 2.7 Table 252. Processor Error Record.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/Kconfig    |  5 +++++
 drivers/firmware/efi/Makefile   |  1 +
 drivers/firmware/efi/cper-x86.c | 23 +++++++++++++++++++++++
 drivers/firmware/efi/cper.c     | 10 ++++++++++
 include/linux/cper.h            |  2 ++
 5 files changed, 41 insertions(+)
 create mode 100644 drivers/firmware/efi/cper-x86.c

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 3098410abad8..781a4a337557 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -174,6 +174,11 @@ config UEFI_CPER_ARM
 	depends on UEFI_CPER && ( ARM || ARM64 )
 	default y
 
+config UEFI_CPER_X86
+	bool
+	depends on UEFI_CPER && X86
+	default y
+
 config EFI_DEV_PATH_PARSER
 	bool
 	depends on ACPI
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index cb805374f4bc..5f9f5039de50 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_ARM)			+= $(arm-obj-y)
 obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
+obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
new file mode 100644
index 000000000000..863f0cd2a0ff
--- /dev/null
+++ b/drivers/firmware/efi/cper-x86.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018, Advanced Micro Devices, Inc.
+
+#include <linux/cper.h>
+
+/*
+ * We don't need a "CPER_IA" prefix since these are all locally defined.
+ * This will save us a lot of line space.
+ */
+#define VALID_LAPIC_ID			BIT_ULL(0)
+#define VALID_CPUID_INFO		BIT_ULL(1)
+
+void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
+{
+	if (proc->validation_bits & VALID_LAPIC_ID)
+		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
+
+	if (proc->validation_bits & VALID_CPUID_INFO) {
+		printk("%sCPUID Info:\n", pfx);
+		print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
+			       sizeof(proc->cpuid), 0);
+	}
+}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index ab21f1614007..3bf0dca378a6 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -467,6 +467,16 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 			cper_print_proc_arm(newpfx, arm_err);
 		else
 			goto err_section_too_small;
+#endif
+#if defined(CONFIG_UEFI_CPER_X86)
+	} else if (guid_equal(sec_type, &CPER_SEC_PROC_IA)) {
+		struct cper_sec_proc_ia *ia_err = acpi_hest_get_payload(gdata);
+
+		printk("%ssection_type: IA32/X64 processor error\n", newpfx);
+		if (gdata->error_data_length >= sizeof(*ia_err))
+			cper_print_proc_ia(newpfx, ia_err);
+		else
+			goto err_section_too_small;
 #endif
 	} else {
 		const void *err = acpi_hest_get_payload(gdata);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 4b5f8459b403..9c703a0abe6e 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -551,5 +551,7 @@ const char *cper_mem_err_unpack(struct trace_seq *,
 				struct cper_mem_err_compact *);
 void cper_print_proc_arm(const char *pfx,
 			 const struct cper_sec_proc_arm *proc);
+void cper_print_proc_ia(const char *pfx,
+			const struct cper_sec_proc_ia *proc);
 
 #endif
-- 
2.17.0

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

* [PATCH 05/17] efi: Decode IA32/X64 Processor Error Info Structure
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 04/17] efi: Decode IA32/X64 Processor Error Section Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:45   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 06/17] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs Ard Biesheuvel
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

Print the fields in the IA32/X64 Processor Error Info Structure.

Based on UEFI 2.7 Table 253. IA32/X64 Processor Error Information
Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 48 +++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 863f0cd2a0ff..e0633a103fcf 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -9,9 +9,20 @@
  */
 #define VALID_LAPIC_ID			BIT_ULL(0)
 #define VALID_CPUID_INFO		BIT_ULL(1)
+#define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
+
+#define INFO_VALID_CHECK_INFO		BIT_ULL(0)
+#define INFO_VALID_TARGET_ID		BIT_ULL(1)
+#define INFO_VALID_REQUESTOR_ID		BIT_ULL(2)
+#define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
+#define INFO_VALID_IP			BIT_ULL(4)
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
+	int i;
+	struct cper_ia_err_info *err_info;
+	char newpfx[64];
+
 	if (proc->validation_bits & VALID_LAPIC_ID)
 		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
 
@@ -20,4 +31,41 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 		print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
 			       sizeof(proc->cpuid), 0);
 	}
+
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
+
+	err_info = (struct cper_ia_err_info *)(proc + 1);
+	for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
+		printk("%sError Information Structure %d:\n", pfx, i);
+
+		printk("%sError Structure Type: %pUl\n", newpfx,
+		       &err_info->err_type);
+
+		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
+			printk("%sCheck Information: 0x%016llx\n", newpfx,
+			       err_info->check_info);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
+			printk("%sTarget Identifier: 0x%016llx\n",
+			       newpfx, err_info->target_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
+			printk("%sRequestor Identifier: 0x%016llx\n",
+			       newpfx, err_info->requestor_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
+			printk("%sResponder Identifier: 0x%016llx\n",
+			       newpfx, err_info->responder_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_IP) {
+			printk("%sInstruction Pointer: 0x%016llx\n",
+			       newpfx, err_info->ip);
+		}
+
+		err_info++;
+	}
 }
-- 
2.17.0

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

* [PATCH 06/17] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 05/17] efi: Decode IA32/X64 Processor Error Info Structure Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:46   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 07/17] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures Ard Biesheuvel
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

For easier handling, match the known IA32/X64 error structure GUIDs to
enums.

Also, print out the name of the matching Error Structure Type.

Only print the GUID for unknown types.

GUIDs taken from UEFI 2.7 section N.2.4.2.1 IA32/X64 Processor Error
Information Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 47 +++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index e0633a103fcf..5438097b93ac 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -11,17 +11,53 @@
 #define VALID_CPUID_INFO		BIT_ULL(1)
 #define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
 
+#define INFO_ERR_STRUCT_TYPE_CACHE					\
+	GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B,	\
+		  0x57, 0x3F, 0xAD, 0x2C)
+#define INFO_ERR_STRUCT_TYPE_TLB					\
+	GUID_INIT(0xFC06B535, 0x5E1F, 0x4562, 0x9F, 0x25, 0x0A, 0x3B,	\
+		  0x9A, 0xDB, 0x63, 0xC3)
+#define INFO_ERR_STRUCT_TYPE_BUS					\
+	GUID_INIT(0x1CF3F8B3, 0xC5B1, 0x49a2, 0xAA, 0x59, 0x5E, 0xEF,	\
+		  0x92, 0xFF, 0xA6, 0x3C)
+#define INFO_ERR_STRUCT_TYPE_MS						\
+	GUID_INIT(0x48AB7F57, 0xDC34, 0x4f6c, 0xA7, 0xD3, 0xB0, 0xB5,	\
+		  0xB0, 0xA7, 0x43, 0x14)
+
 #define INFO_VALID_CHECK_INFO		BIT_ULL(0)
 #define INFO_VALID_TARGET_ID		BIT_ULL(1)
 #define INFO_VALID_REQUESTOR_ID		BIT_ULL(2)
 #define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
 #define INFO_VALID_IP			BIT_ULL(4)
 
+enum err_types {
+	ERR_TYPE_CACHE = 0,
+	ERR_TYPE_TLB,
+	ERR_TYPE_BUS,
+	ERR_TYPE_MS,
+	N_ERR_TYPES
+};
+
+static enum err_types cper_get_err_type(const guid_t *err_type)
+{
+	if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_CACHE))
+		return ERR_TYPE_CACHE;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_TLB))
+		return ERR_TYPE_TLB;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_BUS))
+		return ERR_TYPE_BUS;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_MS))
+		return ERR_TYPE_MS;
+	else
+		return N_ERR_TYPES;
+}
+
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
 	char newpfx[64];
+	u8 err_type;
 
 	if (proc->validation_bits & VALID_LAPIC_ID)
 		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
@@ -38,8 +74,15 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 	for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
 		printk("%sError Information Structure %d:\n", pfx, i);
 
-		printk("%sError Structure Type: %pUl\n", newpfx,
-		       &err_info->err_type);
+		err_type = cper_get_err_type(&err_info->err_type);
+		printk("%sError Structure Type: %s\n", newpfx,
+		       err_type < ARRAY_SIZE(cper_proc_error_type_strs) ?
+		       cper_proc_error_type_strs[err_type] : "unknown");
+
+		if (err_type >= N_ERR_TYPES) {
+			printk("%sError Structure Type: %pUl\n", newpfx,
+			       &err_info->err_type);
+		}
 
 		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
 			printk("%sCheck Information: 0x%016llx\n", newpfx,
-- 
2.17.0

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

* [PATCH 07/17] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 06/17] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:46   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 08/17] efi: Decode additional IA32/X64 Bus Check fields Ard Biesheuvel
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

Print the common fields of the Cache, TLB, and Bus check structures.The
fields of these three check types are the same except for a few more
fields in the Bus check structure. The remaining Bus check structure
fields will be decoded in a following patch.

Based on UEFI 2.7,
Table 254. IA32/X64 Cache Check Structure
Table 255. IA32/X64 TLB Check Structure
Table 256. IA32/X64 Bus Check Structure

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 99 ++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 5438097b93ac..f70c46f7a4db 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -30,6 +30,25 @@
 #define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
 #define INFO_VALID_IP			BIT_ULL(4)
 
+#define CHECK_VALID_TRANS_TYPE		BIT_ULL(0)
+#define CHECK_VALID_OPERATION		BIT_ULL(1)
+#define CHECK_VALID_LEVEL		BIT_ULL(2)
+#define CHECK_VALID_PCC			BIT_ULL(3)
+#define CHECK_VALID_UNCORRECTED		BIT_ULL(4)
+#define CHECK_VALID_PRECISE_IP		BIT_ULL(5)
+#define CHECK_VALID_RESTARTABLE_IP	BIT_ULL(6)
+#define CHECK_VALID_OVERFLOW		BIT_ULL(7)
+
+#define CHECK_VALID_BITS(check)		(((check) & GENMASK_ULL(15, 0)))
+#define CHECK_TRANS_TYPE(check)		(((check) & GENMASK_ULL(17, 16)) >> 16)
+#define CHECK_OPERATION(check)		(((check) & GENMASK_ULL(21, 18)) >> 18)
+#define CHECK_LEVEL(check)		(((check) & GENMASK_ULL(24, 22)) >> 22)
+#define CHECK_PCC			BIT_ULL(25)
+#define CHECK_UNCORRECTED		BIT_ULL(26)
+#define CHECK_PRECISE_IP		BIT_ULL(27)
+#define CHECK_RESTARTABLE_IP		BIT_ULL(28)
+#define CHECK_OVERFLOW			BIT_ULL(29)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -52,11 +71,81 @@ static enum err_types cper_get_err_type(const guid_t *err_type)
 		return N_ERR_TYPES;
 }
 
+static const char * const ia_check_trans_type_strs[] = {
+	"Instruction",
+	"Data Access",
+	"Generic",
+};
+
+static const char * const ia_check_op_strs[] = {
+	"generic error",
+	"generic read",
+	"generic write",
+	"data read",
+	"data write",
+	"instruction fetch",
+	"prefetch",
+	"eviction",
+	"snoop",
+};
+
+static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
+{
+	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
+}
+
+static void print_err_info(const char *pfx, u8 err_type, u64 check)
+{
+	u16 validation_bits = CHECK_VALID_BITS(check);
+
+	if (err_type == ERR_TYPE_MS)
+		return;
+
+	if (validation_bits & CHECK_VALID_TRANS_TYPE) {
+		u8 trans_type = CHECK_TRANS_TYPE(check);
+
+		printk("%sTransaction Type: %u, %s\n", pfx, trans_type,
+		       trans_type < ARRAY_SIZE(ia_check_trans_type_strs) ?
+		       ia_check_trans_type_strs[trans_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_OPERATION) {
+		u8 op = CHECK_OPERATION(check);
+
+		/*
+		 * CACHE has more operation types than TLB or BUS, though the
+		 * name and the order are the same.
+		 */
+		u8 max_ops = (err_type == ERR_TYPE_CACHE) ? 9 : 7;
+
+		printk("%sOperation: %u, %s\n", pfx, op,
+		       op < max_ops ? ia_check_op_strs[op] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_LEVEL)
+		printk("%sLevel: %llu\n", pfx, CHECK_LEVEL(check));
+
+	if (validation_bits & CHECK_VALID_PCC)
+		print_bool("Processor Context Corrupt", pfx, check, CHECK_PCC);
+
+	if (validation_bits & CHECK_VALID_UNCORRECTED)
+		print_bool("Uncorrected", pfx, check, CHECK_UNCORRECTED);
+
+	if (validation_bits & CHECK_VALID_PRECISE_IP)
+		print_bool("Precise IP", pfx, check, CHECK_PRECISE_IP);
+
+	if (validation_bits & CHECK_VALID_RESTARTABLE_IP)
+		print_bool("Restartable IP", pfx, check, CHECK_RESTARTABLE_IP);
+
+	if (validation_bits & CHECK_VALID_OVERFLOW)
+		print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+}
+
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
-	char newpfx[64];
+	char newpfx[64], infopfx[64];
 	u8 err_type;
 
 	if (proc->validation_bits & VALID_LAPIC_ID)
@@ -87,6 +176,14 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
 			printk("%sCheck Information: 0x%016llx\n", newpfx,
 			       err_info->check_info);
+
+			if (err_type < N_ERR_TYPES) {
+				snprintf(infopfx, sizeof(infopfx), "%s ",
+					 newpfx);
+
+				print_err_info(infopfx, err_type,
+					       err_info->check_info);
+			}
 		}
 
 		if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
-- 
2.17.0

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

* [PATCH 08/17] efi: Decode additional IA32/X64 Bus Check fields
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 07/17] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:47   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 09/17] efi: Decode IA32/X64 MS Check structure Ard Biesheuvel
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

The "Participation Type", "Time Out", and "Address Space" fields are
unique to the IA32/X64 Bus Check structure. Print these fields.

Based on UEFI 2.7 Table 256. IA32/X64 Bus Check Structure

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index f70c46f7a4db..5e6716564dba 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -39,6 +39,10 @@
 #define CHECK_VALID_RESTARTABLE_IP	BIT_ULL(6)
 #define CHECK_VALID_OVERFLOW		BIT_ULL(7)
 
+#define CHECK_VALID_BUS_PART_TYPE	BIT_ULL(8)
+#define CHECK_VALID_BUS_TIME_OUT	BIT_ULL(9)
+#define CHECK_VALID_BUS_ADDR_SPACE	BIT_ULL(10)
+
 #define CHECK_VALID_BITS(check)		(((check) & GENMASK_ULL(15, 0)))
 #define CHECK_TRANS_TYPE(check)		(((check) & GENMASK_ULL(17, 16)) >> 16)
 #define CHECK_OPERATION(check)		(((check) & GENMASK_ULL(21, 18)) >> 18)
@@ -49,6 +53,10 @@
 #define CHECK_RESTARTABLE_IP		BIT_ULL(28)
 #define CHECK_OVERFLOW			BIT_ULL(29)
 
+#define CHECK_BUS_PART_TYPE(check)	(((check) & GENMASK_ULL(31, 30)) >> 30)
+#define CHECK_BUS_TIME_OUT		BIT_ULL(32)
+#define CHECK_BUS_ADDR_SPACE(check)	(((check) & GENMASK_ULL(34, 33)) >> 33)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -89,6 +97,20 @@ static const char * const ia_check_op_strs[] = {
 	"snoop",
 };
 
+static const char * const ia_check_bus_part_type_strs[] = {
+	"Local Processor originated request",
+	"Local Processor responded to request",
+	"Local Processor observed",
+	"Generic",
+};
+
+static const char * const ia_check_bus_addr_space_strs[] = {
+	"Memory Access",
+	"Reserved",
+	"I/O",
+	"Other Transaction",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -139,6 +161,28 @@ static void print_err_info(const char *pfx, u8 err_type, u64 check)
 
 	if (validation_bits & CHECK_VALID_OVERFLOW)
 		print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+
+	if (err_type != ERR_TYPE_BUS)
+		return;
+
+	if (validation_bits & CHECK_VALID_BUS_PART_TYPE) {
+		u8 part_type = CHECK_BUS_PART_TYPE(check);
+
+		printk("%sParticipation Type: %u, %s\n", pfx, part_type,
+		       part_type < ARRAY_SIZE(ia_check_bus_part_type_strs) ?
+		       ia_check_bus_part_type_strs[part_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_BUS_TIME_OUT)
+		print_bool("Time Out", pfx, check, CHECK_BUS_TIME_OUT);
+
+	if (validation_bits & CHECK_VALID_BUS_ADDR_SPACE) {
+		u8 addr_space = CHECK_BUS_ADDR_SPACE(check);
+
+		printk("%sAddress Space: %u, %s\n", pfx, addr_space,
+		       addr_space < ARRAY_SIZE(ia_check_bus_addr_space_strs) ?
+		       ia_check_bus_addr_space_strs[addr_space] : "unknown");
+	}
 }
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
-- 
2.17.0

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

* [PATCH 09/17] efi: Decode IA32/X64 MS Check structure
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (7 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 08/17] efi: Decode additional IA32/X64 Bus Check fields Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:47   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 10/17] efi: Decode IA32/X64 Context Info structure Ard Biesheuvel
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

The IA32/X64 MS Check structure varies from the other Check structures
in the the bit positions of its fields, and it includes an additional
"Error Type" field.

Decode the MS Check structure in a separate function.

Based on UEFI 2.7 Table 257. IA32/X64 MS Check Field Description.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 55 ++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 5e6716564dba..356b8d326219 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -57,6 +57,20 @@
 #define CHECK_BUS_TIME_OUT		BIT_ULL(32)
 #define CHECK_BUS_ADDR_SPACE(check)	(((check) & GENMASK_ULL(34, 33)) >> 33)
 
+#define CHECK_VALID_MS_ERR_TYPE		BIT_ULL(0)
+#define CHECK_VALID_MS_PCC		BIT_ULL(1)
+#define CHECK_VALID_MS_UNCORRECTED	BIT_ULL(2)
+#define CHECK_VALID_MS_PRECISE_IP	BIT_ULL(3)
+#define CHECK_VALID_MS_RESTARTABLE_IP	BIT_ULL(4)
+#define CHECK_VALID_MS_OVERFLOW		BIT_ULL(5)
+
+#define CHECK_MS_ERR_TYPE(check)	(((check) & GENMASK_ULL(18, 16)) >> 16)
+#define CHECK_MS_PCC			BIT_ULL(19)
+#define CHECK_MS_UNCORRECTED		BIT_ULL(20)
+#define CHECK_MS_PRECISE_IP		BIT_ULL(21)
+#define CHECK_MS_RESTARTABLE_IP		BIT_ULL(22)
+#define CHECK_MS_OVERFLOW		BIT_ULL(23)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -111,17 +125,56 @@ static const char * const ia_check_bus_addr_space_strs[] = {
 	"Other Transaction",
 };
 
+static const char * const ia_check_ms_error_type_strs[] = {
+	"No Error",
+	"Unclassified",
+	"Microcode ROM Parity Error",
+	"External Error",
+	"FRC Error",
+	"Internal Unclassified",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
 }
 
+static void print_err_info_ms(const char *pfx, u16 validation_bits, u64 check)
+{
+	if (validation_bits & CHECK_VALID_MS_ERR_TYPE) {
+		u8 err_type = CHECK_MS_ERR_TYPE(check);
+
+		printk("%sError Type: %u, %s\n", pfx, err_type,
+		       err_type < ARRAY_SIZE(ia_check_ms_error_type_strs) ?
+		       ia_check_ms_error_type_strs[err_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_MS_PCC)
+		print_bool("Processor Context Corrupt", pfx, check, CHECK_MS_PCC);
+
+	if (validation_bits & CHECK_VALID_MS_UNCORRECTED)
+		print_bool("Uncorrected", pfx, check, CHECK_MS_UNCORRECTED);
+
+	if (validation_bits & CHECK_VALID_MS_PRECISE_IP)
+		print_bool("Precise IP", pfx, check, CHECK_MS_PRECISE_IP);
+
+	if (validation_bits & CHECK_VALID_MS_RESTARTABLE_IP)
+		print_bool("Restartable IP", pfx, check, CHECK_MS_RESTARTABLE_IP);
+
+	if (validation_bits & CHECK_VALID_MS_OVERFLOW)
+		print_bool("Overflow", pfx, check, CHECK_MS_OVERFLOW);
+}
+
 static void print_err_info(const char *pfx, u8 err_type, u64 check)
 {
 	u16 validation_bits = CHECK_VALID_BITS(check);
 
+	/*
+	 * The MS Check structure varies a lot from the others, so use a
+	 * separate function for decoding.
+	 */
 	if (err_type == ERR_TYPE_MS)
-		return;
+		return print_err_info_ms(pfx, validation_bits, check);
 
 	if (validation_bits & CHECK_VALID_TRANS_TYPE) {
 		u8 trans_type = CHECK_TRANS_TYPE(check);
-- 
2.17.0

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

* [PATCH 10/17] efi: Decode IA32/X64 Context Info structure
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (8 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 09/17] efi: Decode IA32/X64 MS Check structure Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:48   ` [tip:efi/core] " tip-bot for Yazen Ghannam
  2018-05-04  5:59 ` [PATCH 11/17] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static Ard Biesheuvel
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Yazen Ghannam, Ard Biesheuvel, linux-kernel

From: Yazen Ghannam <yazen.ghannam@amd.com>

Print the fields of the IA32/X64 Context Information structure.

Print the "Register Array" as raw values. Some context types are defined
in the UEFI spec, so more detailed decoded may be added in the future.

Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
Information Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/cper-x86.c | 48 +++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 356b8d326219..2531de49f56c 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -10,6 +10,7 @@
 #define VALID_LAPIC_ID			BIT_ULL(0)
 #define VALID_CPUID_INFO		BIT_ULL(1)
 #define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
+#define VALID_PROC_CXT_INFO_NUM(bits)	(((bits) & GENMASK_ULL(13, 8)) >> 8)
 
 #define INFO_ERR_STRUCT_TYPE_CACHE					\
 	GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B,	\
@@ -71,6 +72,9 @@
 #define CHECK_MS_RESTARTABLE_IP		BIT_ULL(22)
 #define CHECK_MS_OVERFLOW		BIT_ULL(23)
 
+#define CTX_TYPE_MSR			1
+#define CTX_TYPE_MMREG			7
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -134,6 +138,17 @@ static const char * const ia_check_ms_error_type_strs[] = {
 	"Internal Unclassified",
 };
 
+static const char * const ia_reg_ctx_strs[] = {
+	"Unclassified Data",
+	"MSR Registers (Machine Check and other MSRs)",
+	"32-bit Mode Execution Context",
+	"64-bit Mode Execution Context",
+	"FXSAVE Context",
+	"32-bit Mode Debug Registers (DR0-DR7)",
+	"64-bit Mode Debug Registers (DR0-DR7)",
+	"Memory Mapped Registers",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -242,6 +257,7 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
+	struct cper_ia_proc_ctx *ctx_info;
 	char newpfx[64], infopfx[64];
 	u8 err_type;
 
@@ -305,4 +321,36 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 
 		err_info++;
 	}
+
+	ctx_info = (struct cper_ia_proc_ctx *)err_info;
+	for (i = 0; i < VALID_PROC_CXT_INFO_NUM(proc->validation_bits); i++) {
+		int size = sizeof(*ctx_info) + ctx_info->reg_arr_size;
+		int groupsize = 4;
+
+		printk("%sContext Information Structure %d:\n", pfx, i);
+
+		printk("%sRegister Context Type: %s\n", newpfx,
+		       ctx_info->reg_ctx_type < ARRAY_SIZE(ia_reg_ctx_strs) ?
+		       ia_reg_ctx_strs[ctx_info->reg_ctx_type] : "unknown");
+
+		printk("%sRegister Array Size: 0x%04x\n", newpfx,
+		       ctx_info->reg_arr_size);
+
+		if (ctx_info->reg_ctx_type == CTX_TYPE_MSR) {
+			groupsize = 8; /* MSRs are 8 bytes wide. */
+			printk("%sMSR Address: 0x%08x\n", newpfx,
+			       ctx_info->msr_addr);
+		}
+
+		if (ctx_info->reg_ctx_type == CTX_TYPE_MMREG) {
+			printk("%sMM Register Address: 0x%016llx\n", newpfx,
+			       ctx_info->mm_reg_addr);
+		}
+
+		printk("%sRegister Array:\n", newpfx);
+		print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, groupsize,
+			       (ctx_info + 1), ctx_info->reg_arr_size, 0);
+
+		ctx_info = (struct cper_ia_proc_ctx *)((long)ctx_info + size);
+	}
 }
-- 
2.17.0

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

* [PATCH 11/17] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (9 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 10/17] efi: Decode IA32/X64 Context Info structure Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:48   ` [tip:efi/core] " tip-bot for Wei Yongjun
  2018-05-04  5:59 ` [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode Ard Biesheuvel
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Wei Yongjun, Ard Biesheuvel, linux-kernel

From: Wei Yongjun <weiyongjun1@huawei.com>

Fixes the following sparse warning:

drivers/firmware/efi/libstub/tpm.c:62:6: warning:
 symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 9d08cea3f1b0..caa37a6dd9d4 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -59,7 +59,7 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
 
 #endif
 
-void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
+static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 {
 	efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
 	efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
-- 
2.17.0

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

* [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (10 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 11/17] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  6:57   ` Ingo Molnar
  2018-05-14  7:42   ` [tip:efi/core] efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition " tip-bot for Ard Biesheuvel
  2018-05-04  5:59 ` [PATCH 13/17] efi: align efi_pci_io_protocol typedefs to type naming convention Ard Biesheuvel
                   ` (4 subsequent siblings)
  16 siblings, 2 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner; +Cc: Ard Biesheuvel, linux-kernel

Mixed mode allows a kernel built for x86_64 to interact with 32-bit
EFI firmware, but requires us to define all struct definitions carefully
when it comes to pointer sizes. efi_pci_io_protocol32 currently uses a
void* for the 'romimage' field, which will be interpreted as a 64-bit
field on such kernels, potentially resulting in bogus memory references
and subsequent crashes.

Cc: <stable@vger.kernel.org>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/boot/compressed/eboot.c | 6 ++++--
 include/linux/efi.h              | 8 ++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 47d3efff6805..09f36c0d9d4f 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -163,7 +163,8 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, pci->romimage, pci->romsize);
+	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
+	       pci->romsize);
 	return status;
 
 free_struct:
@@ -269,7 +270,8 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, pci->romimage, pci->romsize);
+	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
+	       pci->romsize);
 	return status;
 
 free_struct:
diff --git a/include/linux/efi.h b/include/linux/efi.h
index f1b7d68ac460..3016d8c456bc 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -395,8 +395,8 @@ typedef struct {
 	u32 attributes;
 	u32 get_bar_attributes;
 	u32 set_bar_attributes;
-	uint64_t romsize;
-	void *romimage;
+	u64 romsize;
+	u32 romimage;
 } efi_pci_io_protocol_32;
 
 typedef struct {
@@ -415,8 +415,8 @@ typedef struct {
 	u64 attributes;
 	u64 get_bar_attributes;
 	u64 set_bar_attributes;
-	uint64_t romsize;
-	void *romimage;
+	u64 romsize;
+	u64 romimage;
 } efi_pci_io_protocol_64;
 
 typedef struct {
-- 
2.17.0

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

* [PATCH 13/17] efi: align efi_pci_io_protocol typedefs to type naming convention
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (11 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode Ard Biesheuvel
@ 2018-05-04  5:59 ` Ard Biesheuvel
  2018-05-14  7:49   ` [tip:efi/core] efi: Align " tip-bot for Ard Biesheuvel
  2018-05-04  6:00 ` [PATCH 14/17] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one Ard Biesheuvel
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  5:59 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner; +Cc: Ard Biesheuvel, linux-kernel

In order to use the helper macros that perform type mangling with the
EFI PCI I/O protocol struct typedefs, align their Linux typenames with
the convention we use for definitionns that originate in the UEFI spec,
and add the trailing _t to each.

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/boot/compressed/eboot.c | 8 ++++----
 include/linux/efi.h              | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 09f36c0d9d4f..3994f48c4043 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -109,7 +109,7 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
 }
 
 static efi_status_t
-__setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom = NULL;
 	efi_status_t status;
@@ -176,7 +176,7 @@ static void
 setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_32 *pci = NULL;
+	efi_pci_io_protocol_32_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u32 *handles = (u32 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -218,7 +218,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 }
 
 static efi_status_t
-__setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom;
 	efi_status_t status;
@@ -284,7 +284,7 @@ static void
 setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_64 *pci = NULL;
+	efi_pci_io_protocol_64_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u64 *handles = (u64 *)(unsigned long)pci_handle;
 	efi_status_t status;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3016d8c456bc..56add823f190 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -397,7 +397,7 @@ typedef struct {
 	u32 set_bar_attributes;
 	u64 romsize;
 	u32 romimage;
-} efi_pci_io_protocol_32;
+} efi_pci_io_protocol_32_t;
 
 typedef struct {
 	u64 poll_mem;
@@ -417,7 +417,7 @@ typedef struct {
 	u64 set_bar_attributes;
 	u64 romsize;
 	u64 romimage;
-} efi_pci_io_protocol_64;
+} efi_pci_io_protocol_64_t;
 
 typedef struct {
 	void *poll_mem;
@@ -437,7 +437,7 @@ typedef struct {
 	void *set_bar_attributes;
 	uint64_t romsize;
 	void *romimage;
-} efi_pci_io_protocol;
+} efi_pci_io_protocol_t;
 
 #define EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO 0x0001
 #define EFI_PCI_IO_ATTRIBUTE_ISA_IO 0x0002
-- 
2.17.0

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

* [PATCH 14/17] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (12 preceding siblings ...)
  2018-05-04  5:59 ` [PATCH 13/17] efi: align efi_pci_io_protocol typedefs to type naming convention Ard Biesheuvel
@ 2018-05-04  6:00 ` Ard Biesheuvel
  2018-05-14  7:49   ` [tip:efi/core] efi/x86: Fold __setup_efi_pci32() and __setup_efi_pci64() into one function tip-bot for Ard Biesheuvel
  2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  6:00 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner; +Cc: Ard Biesheuvel, linux-kernel

As suggested by Lukas, use his efi_call_proto() and efi_table_attr()
macros to merge __setup_efi_pci32() and __setup_efi_pci64() into a
single function, removing the need to duplicate changes made in
subsequent patches across both.

Cc: Lukas Wunner <lukas@wunner.de>
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/boot/compressed/eboot.c | 107 ++++++++-----------------------
 1 file changed, 25 insertions(+), 82 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 3994f48c4043..dadf32312082 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -109,23 +109,27 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
 }
 
 static efi_status_t
-__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom = NULL;
 	efi_status_t status;
 	unsigned long size;
-	uint64_t attributes;
+	uint64_t attributes, romsize;
+	void *romimage;
 
-	status = efi_early->call(pci->attributes, pci,
-				 EfiPciIoAttributeOperationGet, 0, 0,
-				 &attributes);
+	status = efi_call_proto(efi_pci_io_protocol, attributes, pci,
+				EfiPciIoAttributeOperationGet, 0, 0,
+				&attributes);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	if (!pci->romimage || !pci->romsize)
+	romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol,
+							 romimage, pci);
+	romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci);
+	if (!romimage || !romsize)
 		return EFI_INVALID_PARAMETER;
 
-	size = pci->romsize + sizeof(*rom);
+	size = romsize + sizeof(*rom);
 
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
 	if (status != EFI_SUCCESS) {
@@ -141,30 +145,32 @@ __setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
 	rom->pcilen = pci->romsize;
 	*__rom = rom;
 
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_VENDOR_ID, 1, &(rom->vendor));
+	status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
+				EfiPciIoWidthUint16, PCI_VENDOR_ID, 1,
+				&rom->vendor);
 
 	if (status != EFI_SUCCESS) {
 		efi_printk(sys_table, "Failed to read rom->vendor\n");
 		goto free_struct;
 	}
 
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_DEVICE_ID, 1, &(rom->devid));
+	status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
+				EfiPciIoWidthUint16, PCI_DEVICE_ID, 1,
+				&rom->devid);
 
 	if (status != EFI_SUCCESS) {
 		efi_printk(sys_table, "Failed to read rom->devid\n");
 		goto free_struct;
 	}
 
-	status = efi_early->call(pci->get_location, pci, &(rom->segment),
-				 &(rom->bus), &(rom->device), &(rom->function));
+	status = efi_call_proto(efi_pci_io_protocol, get_location, pci,
+				&rom->segment, &rom->bus, &rom->device,
+				&rom->function);
 
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
-	       pci->romsize);
+	memcpy(rom->romdata, romimage, romsize);
 	return status;
 
 free_struct:
@@ -176,7 +182,7 @@ static void
 setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_32_t *pci = NULL;
+	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u32 *handles = (u32 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -203,7 +209,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		if (!pci)
 			continue;
 
-		status = __setup_efi_pci32(pci, &rom);
+		status = __setup_efi_pci(pci, &rom);
 		if (status != EFI_SUCCESS)
 			continue;
 
@@ -217,74 +223,11 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 	}
 }
 
-static efi_status_t
-__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom)
-{
-	struct pci_setup_rom *rom;
-	efi_status_t status;
-	unsigned long size;
-	uint64_t attributes;
-
-	status = efi_early->call(pci->attributes, pci,
-				 EfiPciIoAttributeOperationGet, 0,
-				 &attributes);
-	if (status != EFI_SUCCESS)
-		return status;
-
-	if (!pci->romimage || !pci->romsize)
-		return EFI_INVALID_PARAMETER;
-
-	size = pci->romsize + sizeof(*rom);
-
-	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for rom\n");
-		return status;
-	}
-
-	rom->data.type = SETUP_PCI;
-	rom->data.len = size - sizeof(struct setup_data);
-	rom->data.next = 0;
-	rom->pcilen = pci->romsize;
-	*__rom = rom;
-
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_VENDOR_ID, 1, &(rom->vendor));
-
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to read rom->vendor\n");
-		goto free_struct;
-	}
-
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_DEVICE_ID, 1, &(rom->devid));
-
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to read rom->devid\n");
-		goto free_struct;
-	}
-
-	status = efi_early->call(pci->get_location, pci, &(rom->segment),
-				 &(rom->bus), &(rom->device), &(rom->function));
-
-	if (status != EFI_SUCCESS)
-		goto free_struct;
-
-	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
-	       pci->romsize);
-	return status;
-
-free_struct:
-	efi_call_early(free_pool, rom);
-	return status;
-
-}
-
 static void
 setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_64_t *pci = NULL;
+	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u64 *handles = (u64 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -311,7 +254,7 @@ setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		if (!pci)
 			continue;
 
-		status = __setup_efi_pci64(pci, &rom);
+		status = __setup_efi_pci(pci, &rom);
 		if (status != EFI_SUCCESS)
 			continue;
 
-- 
2.17.0

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

* [PATCH 15/17] efi/x86: Ignore unrealistically large option roms
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (13 preceding siblings ...)
  2018-05-04  6:00 ` [PATCH 14/17] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one Ard Biesheuvel
@ 2018-05-04  6:00 ` Ard Biesheuvel
  2018-05-14  6:40   ` Ingo Molnar
                     ` (2 more replies)
  2018-05-04  6:00 ` [PATCH 16/17] efi/capsule-loader: Don't output reset log when reset flags are not set Ard Biesheuvel
  2018-05-04  6:00 ` [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET Ard Biesheuvel
  16 siblings, 3 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  6:00 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Hans de Goede, Ard Biesheuvel, linux-kernel

From: Hans de Goede <hdegoede@redhat.com>

setup_efi_pci() tries to save a copy of each PCI option ROM as this may
be necessary for the device driver for the PCI device to have access too.

On some systems the efi_pci_io_protocol's romimage and romsize fields
contain invalid data, which looks a bit like pointers pointing back into
other EFI code or data. Interpreting these pointers as romsize leads to
a very large value and if we then try to alloc this amount of memory to
save a copy the alloc call fails.

This leads to a "Failed to alloc mem for rom" error being printed on the
EFI console for each PCI device.

This commit avoids the printing of these errors, by checking romsize before
doing the alloc and if it is larger then the EFI spec limit of 16 MiB
silently ignore the ROM fields instead of trying to alloc mem and fail.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/boot/compressed/eboot.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index dadf32312082..720b06e86698 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -123,10 +123,17 @@ __setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		return status;
 
+	/*
+	 * Some firmwares contain EFI function pointers at the place where the
+	 * romimage and romsize fields are supposed to be. Typically the EFI
+	 * code is mapped at high addresses, translating to an unrealistically
+	 * large romsize. The UEFI spec limits the size of option ROMs to 16
+	 * MiB so we reject any roms over 16 MiB in size to catch this.
+	 */
 	romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol,
 							 romimage, pci);
 	romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci);
-	if (!romimage || !romsize)
+	if (!romimage || !romsize || romsize > SZ_16M)
 		return EFI_INVALID_PARAMETER;
 
 	size = romsize + sizeof(*rom);
-- 
2.17.0

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

* [PATCH 16/17] efi/capsule-loader: Don't output reset log when reset flags are not set
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (14 preceding siblings ...)
  2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
@ 2018-05-04  6:00 ` Ard Biesheuvel
  2018-05-14  7:50   ` [tip:efi/core] " tip-bot for Shunyong Yang
  2018-05-04  6:00 ` [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET Ard Biesheuvel
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  6:00 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Shunyong Yang, Ard Biesheuvel, linux-kernel

From: Shunyong Yang <shunyong.yang@hxt-semitech.com>

When reset flags in capsule header are not set, it means firmware
attempts to immediately process or launch the capsule. Moreover, reset
is not needed in this case. The current code will output log to indicate
reset.

This patch adds a branch to avoid reset log output when the flags are not
set.

Cc: Joey Zheng <yu.zheng@hxt-semitech.com>
Signed-off-by: Shunyong Yang <shunyong.yang@hxt-semitech.com>
[ardb: use braces in multi-line 'if', clarify comment and commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/capsule-loader.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index e456f4602df1..96688986da56 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -134,10 +134,16 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 
 	/* Indicate capsule binary uploading is done */
 	cap_info->index = NO_FURTHER_WRITE_ACTION;
-	pr_info("Successfully upload capsule file with reboot type '%s'\n",
-		!cap_info->reset_type ? "RESET_COLD" :
-		cap_info->reset_type == 1 ? "RESET_WARM" :
-		"RESET_SHUTDOWN");
+
+	if (cap_info->header.flags & EFI_CAPSULE_PERSIST_ACROSS_RESET) {
+		pr_info("Successfully uploaded capsule file with reboot type '%s'\n",
+			!cap_info->reset_type ? "RESET_COLD" :
+			cap_info->reset_type == 1 ? "RESET_WARM" :
+			"RESET_SHUTDOWN");
+	} else {
+		pr_info("Successfully processed capsule file\n");
+	}
+
 	return 0;
 }
 
-- 
2.17.0

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

* [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET
  2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
                   ` (15 preceding siblings ...)
  2018-05-04  6:00 ` [PATCH 16/17] efi/capsule-loader: Don't output reset log when reset flags are not set Ard Biesheuvel
@ 2018-05-04  6:00 ` Ard Biesheuvel
  2018-05-14  6:47   ` Ingo Molnar
  16 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-04  6:00 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner
  Cc: Mark Rutland, Ard Biesheuvel, linux-kernel

From: Mark Rutland <mark.rutland@arm.com>

When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an
arbitrary multiple of PAGE_SIZE in the interval [0, 2MB).

The EFI stub does not account for the potential misalignment of
TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized
physical offset which is always a round multiple of EFI_KIMG_ALIGN.
This may result in statically allocated objects whose alignment exceeds
PAGE_SIZE to appear misaligned in memory. This has been observed to
result in spurious stack overflow reports and failure to make use of
the IRQ stacks, and theoretically could result in a number of other
issues.

We can OR in the low bits of TEXT_OFFSET to ensure that we have the
necessary offset (and hence preserve the misalignment of TEXT_OFFSET
relative to EFI_KIMG_ALIGN), so let's do that.

Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity")
Cc: <stable@vger.kernel.org> # v4.7+
Reported-by: Kim Phillips <kim.phillips@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Kim Phillips <kim.phillips@arm.com>
[ardb: clarify commit log]
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index b9bd827caa22..541b82fdc8a2 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
 		u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
 			     (phys_seed >> 32) & mask : TEXT_OFFSET;
 
+		/*
+		 * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a
+		 * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply
+		 * the offset below EFI_KIMG_ALIGN.
+		 */
+		offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN);
+
 		/*
 		 * If KASLR is enabled, and we have some randomness available,
 		 * locate the kernel at a randomized offset in physical memory.
-- 
2.17.0

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

* Re: [PATCH 15/17] efi/x86: Ignore unrealistically large option roms
  2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
@ 2018-05-14  6:40   ` Ingo Molnar
  2018-05-14  6:43   ` [PATCH] efi/x86: Clean up the eboot code a bit Ingo Molnar
  2018-05-14  7:50   ` [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs tip-bot for Hans de Goede
  2 siblings, 0 replies; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  6:40 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner, Hans de Goede, linux-kernel


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> +	/*
> +	 * Some firmwares contain EFI function pointers at the place where the
> +	 * romimage and romsize fields are supposed to be. Typically the EFI
> +	 * code is mapped at high addresses, translating to an unrealistically
> +	 * large romsize. The UEFI spec limits the size of option ROMs to 16
> +	 * MiB so we reject any roms over 16 MiB in size to catch this.
> +	 */

JFYI, I fixed this:

 s/Some firmwares contain
  /Some firmware images contain

and:

 s/roms
  /ROMs

(Looks good otherwise, no need to resend.)

Thanks,

	Ingo

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

* [PATCH] efi/x86: Clean up the eboot code a bit
  2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
  2018-05-14  6:40   ` Ingo Molnar
@ 2018-05-14  6:43   ` Ingo Molnar
  2018-05-14  6:47     ` Ard Biesheuvel
  2018-05-14  7:50   ` [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs tip-bot for Hans de Goede
  2 siblings, 1 reply; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  6:43 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner, Hans de Goede, linux-kernel


So I looked at arch/x86/boot/compressed/eboot.c to improve a printk message and 
ended up with the cleanups below.

Only build tested.

Thanks,

	Ingo

=================>
Subject: efi/x86: Clean up the eboot code
From: Ingo Molnar <mingo@kernel.org>
Date: Mon May 14 08:33:40 CEST 2018

Various small cleanups:

 - Standardize printk messages:

     'alloc' => 'allocate'
     'mem'   => 'memory'

   also put variable names in printk messages between quotes.

 - Align mass-assignments vertically for better readability

 - Break multi-line function prototypes at the name where possible,
   not in the middle of the parameter list

 - Use a newline before return statements consistently.

 - Use curly braces in a balanced fashion.

 - Remove stray newlines.

No change in functionality.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c |  247 +++++++++++++++++++--------------------
 1 file changed, 126 insertions(+), 121 deletions(-)

Index: tip/arch/x86/boot/compressed/eboot.c
===================================================================
--- tip.orig/arch/x86/boot/compressed/eboot.c
+++ tip/arch/x86/boot/compressed/eboot.c
@@ -34,9 +34,9 @@ static void setup_boot_services##bits(st
 									\
 	table = (typeof(table))sys_table;				\
 									\
-	c->runtime_services = table->runtime;				\
-	c->boot_services = table->boottime;				\
-	c->text_output = table->con_out;				\
+	c->runtime_services	= table->runtime;			\
+	c->boot_services	= table->boottime;			\
+	c->text_output		= table->con_out;			\
 }
 BOOT_SERVICES(32);
 BOOT_SERVICES(64);
@@ -64,6 +64,7 @@ static inline efi_status_t __open_volume
 		efi_printk(sys_table, "Failed to open volume\n");
 
 	*__fh = fh;
+
 	return status;
 }
 
@@ -90,6 +91,7 @@ static inline efi_status_t __open_volume
 		efi_printk(sys_table, "Failed to open volume\n");
 
 	*__fh = fh;
+
 	return status;
 }
 
@@ -140,16 +142,16 @@ __setup_efi_pci(efi_pci_io_protocol_t *p
 
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
 	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for rom\n");
+		efi_printk(sys_table, "Failed to allocate memory for 'rom'\n");
 		return status;
 	}
 
 	memset(rom, 0, sizeof(*rom));
 
-	rom->data.type = SETUP_PCI;
-	rom->data.len = size - sizeof(struct setup_data);
-	rom->data.next = 0;
-	rom->pcilen = pci->romsize;
+	rom->data.type	= SETUP_PCI;
+	rom->data.len	= size - sizeof(struct setup_data);
+	rom->data.next	= 0;
+	rom->pcilen	= pci->romsize;
 	*__rom = rom;
 
 	status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
@@ -186,8 +188,7 @@ free_struct:
 }
 
 static void
-setup_efi_pci32(struct boot_params *params, void **pci_handle,
-		unsigned long size)
+setup_efi_pci32(struct boot_params *params, void **pci_handle, unsigned long size)
 {
 	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
@@ -226,13 +227,11 @@ setup_efi_pci32(struct boot_params *para
 			params->hdr.setup_data = (unsigned long)rom;
 
 		data = (struct setup_data *)rom;
-
 	}
 }
 
 static void
-setup_efi_pci64(struct boot_params *params, void **pci_handle,
-		unsigned long size)
+setup_efi_pci64(struct boot_params *params, void **pci_handle, unsigned long size)
 {
 	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
@@ -271,7 +270,6 @@ setup_efi_pci64(struct boot_params *para
 			params->hdr.setup_data = (unsigned long)rom;
 
 		data = (struct setup_data *)rom;
-
 	}
 }
 
@@ -301,7 +299,7 @@ static void setup_efi_pci(struct boot_pa
 					size, (void **)&pci_handle);
 
 		if (status != EFI_SUCCESS) {
-			efi_printk(sys_table, "Failed to alloc mem for pci_handle\n");
+			efi_printk(sys_table, "Failed to allocate memory for 'pci_handle'\n");
 			return;
 		}
 
@@ -347,8 +345,7 @@ static void retrieve_apple_device_proper
 		status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
 					size + sizeof(struct setup_data), &new);
 		if (status != EFI_SUCCESS) {
-			efi_printk(sys_table,
-					"Failed to alloc mem for properties\n");
+			efi_printk(sys_table, "Failed to allocate memory for 'properties'\n");
 			return;
 		}
 
@@ -364,9 +361,9 @@ static void retrieve_apple_device_proper
 	new->next = 0;
 
 	data = (struct setup_data *)(unsigned long)boot_params->hdr.setup_data;
-	if (!data)
+	if (!data) {
 		boot_params->hdr.setup_data = (unsigned long)new;
-	else {
+	} else {
 		while (data->next)
 			data = (struct setup_data *)(unsigned long)data->next;
 		data->next = (unsigned long)new;
@@ -479,8 +476,8 @@ setup_uga64(void **uga_handle, unsigned
 /*
  * See if we have Universal Graphics Adapter (UGA) protocol
  */
-static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,
-			      unsigned long size)
+static efi_status_t
+setup_uga(struct screen_info *si, efi_guid_t *uga_proto, unsigned long size)
 {
 	efi_status_t status;
 	u32 width, height;
@@ -509,23 +506,24 @@ static efi_status_t setup_uga(struct scr
 		goto free_handle;
 
 	/* EFI framebuffer */
-	si->orig_video_isVGA = VIDEO_TYPE_EFI;
+	si->orig_video_isVGA	= VIDEO_TYPE_EFI;
 
-	si->lfb_depth = 32;
-	si->lfb_width = width;
-	si->lfb_height = height;
-
-	si->red_size = 8;
-	si->red_pos = 16;
-	si->green_size = 8;
-	si->green_pos = 8;
-	si->blue_size = 8;
-	si->blue_pos = 0;
-	si->rsvd_size = 8;
-	si->rsvd_pos = 24;
+	si->lfb_depth		= 32;
+	si->lfb_width		= width;
+	si->lfb_height		= height;
+
+	si->red_size		= 8;
+	si->red_pos		= 16;
+	si->green_size		= 8;
+	si->green_pos		= 8;
+	si->blue_size		= 8;
+	si->blue_pos		= 0;
+	si->rsvd_size		= 8;
+	si->rsvd_pos		= 24;
 
 free_handle:
 	efi_call_early(free_pool, uga_handle);
+
 	return status;
 }
 
@@ -607,7 +605,7 @@ struct boot_params *make_boot_params(str
 	status = efi_low_alloc(sys_table, 0x4000, 1,
 			       (unsigned long *)&boot_params);
 	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc lowmem for boot params\n");
+		efi_printk(sys_table, "Failed to allocate lowmem for boot params\n");
 		return NULL;
 	}
 
@@ -623,9 +621,9 @@ struct boot_params *make_boot_params(str
 	 * Fill out some of the header fields ourselves because the
 	 * EFI firmware loader doesn't load the first sector.
 	 */
-	hdr->root_flags = 1;
-	hdr->vid_mode = 0xffff;
-	hdr->boot_flag = 0xAA55;
+	hdr->root_flags	= 1;
+	hdr->vid_mode	= 0xffff;
+	hdr->boot_flag	= 0xAA55;
 
 	hdr->type_of_loader = 0x21;
 
@@ -633,6 +631,7 @@ struct boot_params *make_boot_params(str
 	cmdline_ptr = efi_convert_cmdline(sys_table, image, &options_size);
 	if (!cmdline_ptr)
 		goto fail;
+
 	hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
 	/* Fill in upper bits of command line address, NOP on 32 bit  */
 	boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
@@ -669,10 +668,12 @@ struct boot_params *make_boot_params(str
 	boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
 
 	return boot_params;
+
 fail2:
 	efi_free(sys_table, options_size, hdr->cmd_line_ptr);
 fail:
 	efi_free(sys_table, 0x4000, (unsigned long)boot_params);
+
 	return NULL;
 }
 
@@ -684,7 +685,7 @@ static void add_e820ext(struct boot_para
 	unsigned long size;
 
 	e820ext->type = SETUP_E820_EXT;
-	e820ext->len = nr_entries * sizeof(struct boot_e820_entry);
+	e820ext->len  = nr_entries * sizeof(struct boot_e820_entry);
 	e820ext->next = 0;
 
 	data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
@@ -698,8 +699,8 @@ static void add_e820ext(struct boot_para
 		params->hdr.setup_data = (unsigned long)e820ext;
 }
 
-static efi_status_t setup_e820(struct boot_params *params,
-			       struct setup_data *e820ext, u32 e820ext_size)
+static efi_status_t
+setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_size)
 {
 	struct boot_e820_entry *entry = params->e820_table;
 	struct efi_info *efi = &params->efi_info;
@@ -820,11 +821,11 @@ static efi_status_t alloc_e820ext(u32 nr
 }
 
 struct exit_boot_struct {
-	struct boot_params *boot_params;
-	struct efi_info *efi;
-	struct setup_data *e820ext;
-	__u32 e820ext_size;
-	bool is64;
+	struct boot_params	*boot_params;
+	struct efi_info		*efi;
+	struct setup_data	*e820ext;
+	__u32			e820ext_size;
+	bool			is64;
 };
 
 static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -854,15 +855,15 @@ static efi_status_t exit_boot_func(efi_s
 	signature = p->is64 ? EFI64_LOADER_SIGNATURE : EFI32_LOADER_SIGNATURE;
 	memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
 
-	p->efi->efi_systab = (unsigned long)sys_table_arg;
-	p->efi->efi_memdesc_size = *map->desc_size;
-	p->efi->efi_memdesc_version = *map->desc_ver;
-	p->efi->efi_memmap = (unsigned long)*map->map;
-	p->efi->efi_memmap_size = *map->map_size;
+	p->efi->efi_systab		= (unsigned long)sys_table_arg;
+	p->efi->efi_memdesc_size	= *map->desc_size;
+	p->efi->efi_memdesc_version	= *map->desc_ver;
+	p->efi->efi_memmap		= (unsigned long)*map->map;
+	p->efi->efi_memmap_size		= *map->map_size;
 
 #ifdef CONFIG_X86_64
-	p->efi->efi_systab_hi = (unsigned long)sys_table_arg >> 32;
-	p->efi->efi_memmap_hi = (unsigned long)*map->map >> 32;
+	p->efi->efi_systab_hi		= (unsigned long)sys_table_arg >> 32;
+	p->efi->efi_memmap_hi		= (unsigned long)*map->map >> 32;
 #endif
 
 	return EFI_SUCCESS;
@@ -880,17 +881,17 @@ static efi_status_t exit_boot(struct boo
 	struct efi_boot_memmap map;
 	struct exit_boot_struct priv;
 
-	map.map =		&mem_map;
-	map.map_size =		&map_sz;
-	map.desc_size =		&desc_size;
-	map.desc_ver =		&desc_version;
-	map.key_ptr =		&key;
-	map.buff_size =		&buff_size;
-	priv.boot_params =	boot_params;
-	priv.efi =		&boot_params->efi_info;
-	priv.e820ext =		NULL;
-	priv.e820ext_size =	0;
-	priv.is64 =		is64;
+	map.map			= &mem_map;
+	map.map_size		= &map_sz;
+	map.desc_size		= &desc_size;
+	map.desc_ver		= &desc_version;
+	map.key_ptr		= &key;
+	map.buff_size		= &buff_size;
+	priv.boot_params	= boot_params;
+	priv.efi		= &boot_params->efi_info;
+	priv.e820ext		= NULL;
+	priv.e820ext_size	= 0;
+	priv.is64		= is64;
 
 	/* Might as well exit boot services now */
 	status = efi_exit_boot_services(sys_table, handle, &map, &priv,
@@ -898,10 +899,11 @@ static efi_status_t exit_boot(struct boo
 	if (status != EFI_SUCCESS)
 		return status;
 
-	e820ext = priv.e820ext;
-	e820ext_size = priv.e820ext_size;
+	e820ext			= priv.e820ext;
+	e820ext_size		= priv.e820ext_size;
+
 	/* Historic? */
-	boot_params->alt_mem_k = 32 * 1024;
+	boot_params->alt_mem_k	= 32 * 1024;
 
 	status = setup_e820(boot_params, e820ext, e820ext_size);
 	if (status != EFI_SUCCESS)
@@ -914,8 +916,8 @@ static efi_status_t exit_boot(struct boo
  * On success we return a pointer to a boot_params structure, and NULL
  * on failure.
  */
-struct boot_params *efi_main(struct efi_config *c,
-			     struct boot_params *boot_params)
+struct boot_params *
+efi_main(struct efi_config *c, struct boot_params *boot_params)
 {
 	struct desc_ptr *gdt = NULL;
 	efi_loaded_image_t *image;
@@ -963,7 +965,7 @@ struct boot_params *efi_main(struct efi_
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
 				sizeof(*gdt), (void **)&gdt);
 	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for gdt structure\n");
+		efi_printk(sys_table, "Failed to allocate memory for 'gdt' structure\n");
 		goto fail;
 	}
 
@@ -971,7 +973,7 @@ struct boot_params *efi_main(struct efi_
 	status = efi_low_alloc(sys_table, gdt->size, 8,
 			   (unsigned long *)&gdt->address);
 	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for gdt\n");
+		efi_printk(sys_table, "Failed to allocate memory for 'gdt'\n");
 		goto fail;
 	}
 
@@ -1008,19 +1010,20 @@ struct boot_params *efi_main(struct efi_
 
 	if (IS_ENABLED(CONFIG_X86_64)) {
 		/* __KERNEL32_CS */
-		desc->limit0 = 0xffff;
-		desc->base0 = 0x0000;
-		desc->base1 = 0x0000;
-		desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
-		desc->s = DESC_TYPE_CODE_DATA;
-		desc->dpl = 0;
-		desc->p = 1;
-		desc->limit1 = 0xf;
-		desc->avl = 0;
-		desc->l = 0;
-		desc->d = SEG_OP_SIZE_32BIT;
-		desc->g = SEG_GRANULARITY_4KB;
-		desc->base2 = 0x00;
+		desc->limit0	= 0xffff;
+		desc->base0	= 0x0000;
+		desc->base1	= 0x0000;
+		desc->type	= SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
+		desc->s		= DESC_TYPE_CODE_DATA;
+		desc->dpl	= 0;
+		desc->p		= 1;
+		desc->limit1	= 0xf;
+		desc->avl	= 0;
+		desc->l		= 0;
+		desc->d		= SEG_OP_SIZE_32BIT;
+		desc->g		= SEG_GRANULARITY_4KB;
+		desc->base2	= 0x00;
+
 		desc++;
 	} else {
 		/* Second entry is unused on 32-bit */
@@ -1028,15 +1031,16 @@ struct boot_params *efi_main(struct efi_
 	}
 
 	/* __KERNEL_CS */
-	desc->limit0 = 0xffff;
-	desc->base0 = 0x0000;
-	desc->base1 = 0x0000;
-	desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
-	desc->s = DESC_TYPE_CODE_DATA;
-	desc->dpl = 0;
-	desc->p = 1;
-	desc->limit1 = 0xf;
-	desc->avl = 0;
+	desc->limit0	= 0xffff;
+	desc->base0	= 0x0000;
+	desc->base1	= 0x0000;
+	desc->type	= SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
+	desc->s		= DESC_TYPE_CODE_DATA;
+	desc->dpl	= 0;
+	desc->p		= 1;
+	desc->limit1	= 0xf;
+	desc->avl	= 0;
+
 	if (IS_ENABLED(CONFIG_X86_64)) {
 		desc->l = 1;
 		desc->d = 0;
@@ -1044,41 +1048,41 @@ struct boot_params *efi_main(struct efi_
 		desc->l = 0;
 		desc->d = SEG_OP_SIZE_32BIT;
 	}
-	desc->g = SEG_GRANULARITY_4KB;
-	desc->base2 = 0x00;
+	desc->g		= SEG_GRANULARITY_4KB;
+	desc->base2	= 0x00;
 	desc++;
 
 	/* __KERNEL_DS */
-	desc->limit0 = 0xffff;
-	desc->base0 = 0x0000;
-	desc->base1 = 0x0000;
-	desc->type = SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
-	desc->s = DESC_TYPE_CODE_DATA;
-	desc->dpl = 0;
-	desc->p = 1;
-	desc->limit1 = 0xf;
-	desc->avl = 0;
-	desc->l = 0;
-	desc->d = SEG_OP_SIZE_32BIT;
-	desc->g = SEG_GRANULARITY_4KB;
-	desc->base2 = 0x00;
+	desc->limit0	= 0xffff;
+	desc->base0	= 0x0000;
+	desc->base1	= 0x0000;
+	desc->type	= SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
+	desc->s		= DESC_TYPE_CODE_DATA;
+	desc->dpl	= 0;
+	desc->p		= 1;
+	desc->limit1	= 0xf;
+	desc->avl	= 0;
+	desc->l		= 0;
+	desc->d		= SEG_OP_SIZE_32BIT;
+	desc->g		= SEG_GRANULARITY_4KB;
+	desc->base2	= 0x00;
 	desc++;
 
 	if (IS_ENABLED(CONFIG_X86_64)) {
 		/* Task segment value */
-		desc->limit0 = 0x0000;
-		desc->base0 = 0x0000;
-		desc->base1 = 0x0000;
-		desc->type = SEG_TYPE_TSS;
-		desc->s = 0;
-		desc->dpl = 0;
-		desc->p = 1;
-		desc->limit1 = 0x0;
-		desc->avl = 0;
-		desc->l = 0;
-		desc->d = 0;
-		desc->g = SEG_GRANULARITY_4KB;
-		desc->base2 = 0x00;
+		desc->limit0	= 0x0000;
+		desc->base0	= 0x0000;
+		desc->base1	= 0x0000;
+		desc->type	= SEG_TYPE_TSS;
+		desc->s		= 0;
+		desc->dpl	= 0;
+		desc->p		= 1;
+		desc->limit1	= 0x0;
+		desc->avl	= 0;
+		desc->l		= 0;
+		desc->d		= 0;
+		desc->g		= SEG_GRANULARITY_4KB;
+		desc->base2	= 0x00;
 		desc++;
 	}
 
@@ -1088,5 +1092,6 @@ struct boot_params *efi_main(struct efi_
 	return boot_params;
 fail:
 	efi_printk(sys_table, "efi_main() failed!\n");
+
 	return NULL;
 }

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

* Re: [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET
  2018-05-04  6:00 ` [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET Ard Biesheuvel
@ 2018-05-14  6:47   ` Ingo Molnar
  2018-05-14  6:48     ` Ard Biesheuvel
  0 siblings, 1 reply; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  6:47 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner, Mark Rutland, linux-kernel


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> From: Mark Rutland <mark.rutland@arm.com>
> 
> When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an
> arbitrary multiple of PAGE_SIZE in the interval [0, 2MB).
> 
> The EFI stub does not account for the potential misalignment of
> TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized
> physical offset which is always a round multiple of EFI_KIMG_ALIGN.
> This may result in statically allocated objects whose alignment exceeds
> PAGE_SIZE to appear misaligned in memory. This has been observed to
> result in spurious stack overflow reports and failure to make use of
> the IRQ stacks, and theoretically could result in a number of other
> issues.
> 
> We can OR in the low bits of TEXT_OFFSET to ensure that we have the
> necessary offset (and hence preserve the misalignment of TEXT_OFFSET
> relative to EFI_KIMG_ALIGN), so let's do that.
> 
> Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity")
> Cc: <stable@vger.kernel.org> # v4.7+
> Reported-by: Kim Phillips <kim.phillips@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Kim Phillips <kim.phillips@arm.com>
> [ardb: clarify commit log]
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> index b9bd827caa22..541b82fdc8a2 100644
> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
>  		u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
>  			     (phys_seed >> 32) & mask : TEXT_OFFSET;
>  
> +		/*
> +		 * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a
> +		 * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply
> +		 * the offset below EFI_KIMG_ALIGN.
> +		 */

When referring to config variables in comments and changelogs I'd suggest a bit 
more verbosity:

  s/CONFIG_RANDOMIZE_TEXT_OFFSET
   /CONFIG_RANDOMIZE_TEXT_OFFSET=y

... because at first I thought (based on the name) that 
CONFIG_RANDOMIZE_TEXT_OFFSET is an actual integer offset value - while it's a 
bool. The =y makes the bool nature obvious.

( Similarly, when negated the canonical way to refer to it is 
  !CONFIG_RANDOMIZE_TEXT_OFFSET. )

> +		offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN);

The parentheses are not needed here I think.

Thanks,

	Ingo

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

* Re: [PATCH] efi/x86: Clean up the eboot code a bit
  2018-05-14  6:43   ` [PATCH] efi/x86: Clean up the eboot code a bit Ingo Molnar
@ 2018-05-14  6:47     ` Ard Biesheuvel
  2018-05-14  6:58       ` Ingo Molnar
  0 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-14  6:47 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-efi, Thomas Gleixner, Hans de Goede, Linux Kernel Mailing List

On 14 May 2018 at 08:43, Ingo Molnar <mingo@kernel.org> wrote:
>
> So I looked at arch/x86/boot/compressed/eboot.c to improve a printk message and
> ended up with the cleanups below.
>
> Only build tested.
>
> Thanks,
>
>         Ingo
>
> =================>
> Subject: efi/x86: Clean up the eboot code
> From: Ingo Molnar <mingo@kernel.org>
> Date: Mon May 14 08:33:40 CEST 2018
>
> Various small cleanups:
>
>  - Standardize printk messages:
>
>      'alloc' => 'allocate'
>      'mem'   => 'memory'
>
>    also put variable names in printk messages between quotes.
>
>  - Align mass-assignments vertically for better readability
>
>  - Break multi-line function prototypes at the name where possible,
>    not in the middle of the parameter list
>
>  - Use a newline before return statements consistently.
>
>  - Use curly braces in a balanced fashion.
>
>  - Remove stray newlines.
>
> No change in functionality.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-efi@vger.kernel.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>

Thanks Ingo

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  arch/x86/boot/compressed/eboot.c |  247 +++++++++++++++++++--------------------
>  1 file changed, 126 insertions(+), 121 deletions(-)
>
> Index: tip/arch/x86/boot/compressed/eboot.c
> ===================================================================
> --- tip.orig/arch/x86/boot/compressed/eboot.c
> +++ tip/arch/x86/boot/compressed/eboot.c
> @@ -34,9 +34,9 @@ static void setup_boot_services##bits(st
>                                                                         \
>         table = (typeof(table))sys_table;                               \
>                                                                         \
> -       c->runtime_services = table->runtime;                           \
> -       c->boot_services = table->boottime;                             \
> -       c->text_output = table->con_out;                                \
> +       c->runtime_services     = table->runtime;                       \
> +       c->boot_services        = table->boottime;                      \
> +       c->text_output          = table->con_out;                       \
>  }
>  BOOT_SERVICES(32);
>  BOOT_SERVICES(64);
> @@ -64,6 +64,7 @@ static inline efi_status_t __open_volume
>                 efi_printk(sys_table, "Failed to open volume\n");
>
>         *__fh = fh;
> +
>         return status;
>  }
>
> @@ -90,6 +91,7 @@ static inline efi_status_t __open_volume
>                 efi_printk(sys_table, "Failed to open volume\n");
>
>         *__fh = fh;
> +
>         return status;
>  }
>
> @@ -140,16 +142,16 @@ __setup_efi_pci(efi_pci_io_protocol_t *p
>
>         status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
>         if (status != EFI_SUCCESS) {
> -               efi_printk(sys_table, "Failed to alloc mem for rom\n");
> +               efi_printk(sys_table, "Failed to allocate memory for 'rom'\n");
>                 return status;
>         }
>
>         memset(rom, 0, sizeof(*rom));
>
> -       rom->data.type = SETUP_PCI;
> -       rom->data.len = size - sizeof(struct setup_data);
> -       rom->data.next = 0;
> -       rom->pcilen = pci->romsize;
> +       rom->data.type  = SETUP_PCI;
> +       rom->data.len   = size - sizeof(struct setup_data);
> +       rom->data.next  = 0;
> +       rom->pcilen     = pci->romsize;
>         *__rom = rom;
>
>         status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
> @@ -186,8 +188,7 @@ free_struct:
>  }
>
>  static void
> -setup_efi_pci32(struct boot_params *params, void **pci_handle,
> -               unsigned long size)
> +setup_efi_pci32(struct boot_params *params, void **pci_handle, unsigned long size)
>  {
>         efi_pci_io_protocol_t *pci = NULL;
>         efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
> @@ -226,13 +227,11 @@ setup_efi_pci32(struct boot_params *para
>                         params->hdr.setup_data = (unsigned long)rom;
>
>                 data = (struct setup_data *)rom;
> -
>         }
>  }
>
>  static void
> -setup_efi_pci64(struct boot_params *params, void **pci_handle,
> -               unsigned long size)
> +setup_efi_pci64(struct boot_params *params, void **pci_handle, unsigned long size)
>  {
>         efi_pci_io_protocol_t *pci = NULL;
>         efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
> @@ -271,7 +270,6 @@ setup_efi_pci64(struct boot_params *para
>                         params->hdr.setup_data = (unsigned long)rom;
>
>                 data = (struct setup_data *)rom;
> -
>         }
>  }
>
> @@ -301,7 +299,7 @@ static void setup_efi_pci(struct boot_pa
>                                         size, (void **)&pci_handle);
>
>                 if (status != EFI_SUCCESS) {
> -                       efi_printk(sys_table, "Failed to alloc mem for pci_handle\n");
> +                       efi_printk(sys_table, "Failed to allocate memory for 'pci_handle'\n");
>                         return;
>                 }
>
> @@ -347,8 +345,7 @@ static void retrieve_apple_device_proper
>                 status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>                                         size + sizeof(struct setup_data), &new);
>                 if (status != EFI_SUCCESS) {
> -                       efi_printk(sys_table,
> -                                       "Failed to alloc mem for properties\n");
> +                       efi_printk(sys_table, "Failed to allocate memory for 'properties'\n");
>                         return;
>                 }
>
> @@ -364,9 +361,9 @@ static void retrieve_apple_device_proper
>         new->next = 0;
>
>         data = (struct setup_data *)(unsigned long)boot_params->hdr.setup_data;
> -       if (!data)
> +       if (!data) {
>                 boot_params->hdr.setup_data = (unsigned long)new;
> -       else {
> +       } else {
>                 while (data->next)
>                         data = (struct setup_data *)(unsigned long)data->next;
>                 data->next = (unsigned long)new;
> @@ -479,8 +476,8 @@ setup_uga64(void **uga_handle, unsigned
>  /*
>   * See if we have Universal Graphics Adapter (UGA) protocol
>   */
> -static efi_status_t setup_uga(struct screen_info *si, efi_guid_t *uga_proto,
> -                             unsigned long size)
> +static efi_status_t
> +setup_uga(struct screen_info *si, efi_guid_t *uga_proto, unsigned long size)
>  {
>         efi_status_t status;
>         u32 width, height;
> @@ -509,23 +506,24 @@ static efi_status_t setup_uga(struct scr
>                 goto free_handle;
>
>         /* EFI framebuffer */
> -       si->orig_video_isVGA = VIDEO_TYPE_EFI;
> +       si->orig_video_isVGA    = VIDEO_TYPE_EFI;
>
> -       si->lfb_depth = 32;
> -       si->lfb_width = width;
> -       si->lfb_height = height;
> -
> -       si->red_size = 8;
> -       si->red_pos = 16;
> -       si->green_size = 8;
> -       si->green_pos = 8;
> -       si->blue_size = 8;
> -       si->blue_pos = 0;
> -       si->rsvd_size = 8;
> -       si->rsvd_pos = 24;
> +       si->lfb_depth           = 32;
> +       si->lfb_width           = width;
> +       si->lfb_height          = height;
> +
> +       si->red_size            = 8;
> +       si->red_pos             = 16;
> +       si->green_size          = 8;
> +       si->green_pos           = 8;
> +       si->blue_size           = 8;
> +       si->blue_pos            = 0;
> +       si->rsvd_size           = 8;
> +       si->rsvd_pos            = 24;
>
>  free_handle:
>         efi_call_early(free_pool, uga_handle);
> +
>         return status;
>  }
>
> @@ -607,7 +605,7 @@ struct boot_params *make_boot_params(str
>         status = efi_low_alloc(sys_table, 0x4000, 1,
>                                (unsigned long *)&boot_params);
>         if (status != EFI_SUCCESS) {
> -               efi_printk(sys_table, "Failed to alloc lowmem for boot params\n");
> +               efi_printk(sys_table, "Failed to allocate lowmem for boot params\n");
>                 return NULL;
>         }
>
> @@ -623,9 +621,9 @@ struct boot_params *make_boot_params(str
>          * Fill out some of the header fields ourselves because the
>          * EFI firmware loader doesn't load the first sector.
>          */
> -       hdr->root_flags = 1;
> -       hdr->vid_mode = 0xffff;
> -       hdr->boot_flag = 0xAA55;
> +       hdr->root_flags = 1;
> +       hdr->vid_mode   = 0xffff;
> +       hdr->boot_flag  = 0xAA55;
>
>         hdr->type_of_loader = 0x21;
>
> @@ -633,6 +631,7 @@ struct boot_params *make_boot_params(str
>         cmdline_ptr = efi_convert_cmdline(sys_table, image, &options_size);
>         if (!cmdline_ptr)
>                 goto fail;
> +
>         hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
>         /* Fill in upper bits of command line address, NOP on 32 bit  */
>         boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;
> @@ -669,10 +668,12 @@ struct boot_params *make_boot_params(str
>         boot_params->ext_ramdisk_size  = (u64)ramdisk_size >> 32;
>
>         return boot_params;
> +
>  fail2:
>         efi_free(sys_table, options_size, hdr->cmd_line_ptr);
>  fail:
>         efi_free(sys_table, 0x4000, (unsigned long)boot_params);
> +
>         return NULL;
>  }
>
> @@ -684,7 +685,7 @@ static void add_e820ext(struct boot_para
>         unsigned long size;
>
>         e820ext->type = SETUP_E820_EXT;
> -       e820ext->len = nr_entries * sizeof(struct boot_e820_entry);
> +       e820ext->len  = nr_entries * sizeof(struct boot_e820_entry);
>         e820ext->next = 0;
>
>         data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
> @@ -698,8 +699,8 @@ static void add_e820ext(struct boot_para
>                 params->hdr.setup_data = (unsigned long)e820ext;
>  }
>
> -static efi_status_t setup_e820(struct boot_params *params,
> -                              struct setup_data *e820ext, u32 e820ext_size)
> +static efi_status_t
> +setup_e820(struct boot_params *params, struct setup_data *e820ext, u32 e820ext_size)
>  {
>         struct boot_e820_entry *entry = params->e820_table;
>         struct efi_info *efi = &params->efi_info;
> @@ -820,11 +821,11 @@ static efi_status_t alloc_e820ext(u32 nr
>  }
>
>  struct exit_boot_struct {
> -       struct boot_params *boot_params;
> -       struct efi_info *efi;
> -       struct setup_data *e820ext;
> -       __u32 e820ext_size;
> -       bool is64;
> +       struct boot_params      *boot_params;
> +       struct efi_info         *efi;
> +       struct setup_data       *e820ext;
> +       __u32                   e820ext_size;
> +       bool                    is64;
>  };
>
>  static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
> @@ -854,15 +855,15 @@ static efi_status_t exit_boot_func(efi_s
>         signature = p->is64 ? EFI64_LOADER_SIGNATURE : EFI32_LOADER_SIGNATURE;
>         memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
>
> -       p->efi->efi_systab = (unsigned long)sys_table_arg;
> -       p->efi->efi_memdesc_size = *map->desc_size;
> -       p->efi->efi_memdesc_version = *map->desc_ver;
> -       p->efi->efi_memmap = (unsigned long)*map->map;
> -       p->efi->efi_memmap_size = *map->map_size;
> +       p->efi->efi_systab              = (unsigned long)sys_table_arg;
> +       p->efi->efi_memdesc_size        = *map->desc_size;
> +       p->efi->efi_memdesc_version     = *map->desc_ver;
> +       p->efi->efi_memmap              = (unsigned long)*map->map;
> +       p->efi->efi_memmap_size         = *map->map_size;
>
>  #ifdef CONFIG_X86_64
> -       p->efi->efi_systab_hi = (unsigned long)sys_table_arg >> 32;
> -       p->efi->efi_memmap_hi = (unsigned long)*map->map >> 32;
> +       p->efi->efi_systab_hi           = (unsigned long)sys_table_arg >> 32;
> +       p->efi->efi_memmap_hi           = (unsigned long)*map->map >> 32;
>  #endif
>
>         return EFI_SUCCESS;
> @@ -880,17 +881,17 @@ static efi_status_t exit_boot(struct boo
>         struct efi_boot_memmap map;
>         struct exit_boot_struct priv;
>
> -       map.map =               &mem_map;
> -       map.map_size =          &map_sz;
> -       map.desc_size =         &desc_size;
> -       map.desc_ver =          &desc_version;
> -       map.key_ptr =           &key;
> -       map.buff_size =         &buff_size;
> -       priv.boot_params =      boot_params;
> -       priv.efi =              &boot_params->efi_info;
> -       priv.e820ext =          NULL;
> -       priv.e820ext_size =     0;
> -       priv.is64 =             is64;
> +       map.map                 = &mem_map;
> +       map.map_size            = &map_sz;
> +       map.desc_size           = &desc_size;
> +       map.desc_ver            = &desc_version;
> +       map.key_ptr             = &key;
> +       map.buff_size           = &buff_size;
> +       priv.boot_params        = boot_params;
> +       priv.efi                = &boot_params->efi_info;
> +       priv.e820ext            = NULL;
> +       priv.e820ext_size       = 0;
> +       priv.is64               = is64;
>
>         /* Might as well exit boot services now */
>         status = efi_exit_boot_services(sys_table, handle, &map, &priv,
> @@ -898,10 +899,11 @@ static efi_status_t exit_boot(struct boo
>         if (status != EFI_SUCCESS)
>                 return status;
>
> -       e820ext = priv.e820ext;
> -       e820ext_size = priv.e820ext_size;
> +       e820ext                 = priv.e820ext;
> +       e820ext_size            = priv.e820ext_size;
> +
>         /* Historic? */
> -       boot_params->alt_mem_k = 32 * 1024;
> +       boot_params->alt_mem_k  = 32 * 1024;
>
>         status = setup_e820(boot_params, e820ext, e820ext_size);
>         if (status != EFI_SUCCESS)
> @@ -914,8 +916,8 @@ static efi_status_t exit_boot(struct boo
>   * On success we return a pointer to a boot_params structure, and NULL
>   * on failure.
>   */
> -struct boot_params *efi_main(struct efi_config *c,
> -                            struct boot_params *boot_params)
> +struct boot_params *
> +efi_main(struct efi_config *c, struct boot_params *boot_params)
>  {
>         struct desc_ptr *gdt = NULL;
>         efi_loaded_image_t *image;
> @@ -963,7 +965,7 @@ struct boot_params *efi_main(struct efi_
>         status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
>                                 sizeof(*gdt), (void **)&gdt);
>         if (status != EFI_SUCCESS) {
> -               efi_printk(sys_table, "Failed to alloc mem for gdt structure\n");
> +               efi_printk(sys_table, "Failed to allocate memory for 'gdt' structure\n");
>                 goto fail;
>         }
>
> @@ -971,7 +973,7 @@ struct boot_params *efi_main(struct efi_
>         status = efi_low_alloc(sys_table, gdt->size, 8,
>                            (unsigned long *)&gdt->address);
>         if (status != EFI_SUCCESS) {
> -               efi_printk(sys_table, "Failed to alloc mem for gdt\n");
> +               efi_printk(sys_table, "Failed to allocate memory for 'gdt'\n");
>                 goto fail;
>         }
>
> @@ -1008,19 +1010,20 @@ struct boot_params *efi_main(struct efi_
>
>         if (IS_ENABLED(CONFIG_X86_64)) {
>                 /* __KERNEL32_CS */
> -               desc->limit0 = 0xffff;
> -               desc->base0 = 0x0000;
> -               desc->base1 = 0x0000;
> -               desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
> -               desc->s = DESC_TYPE_CODE_DATA;
> -               desc->dpl = 0;
> -               desc->p = 1;
> -               desc->limit1 = 0xf;
> -               desc->avl = 0;
> -               desc->l = 0;
> -               desc->d = SEG_OP_SIZE_32BIT;
> -               desc->g = SEG_GRANULARITY_4KB;
> -               desc->base2 = 0x00;
> +               desc->limit0    = 0xffff;
> +               desc->base0     = 0x0000;
> +               desc->base1     = 0x0000;
> +               desc->type      = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
> +               desc->s         = DESC_TYPE_CODE_DATA;
> +               desc->dpl       = 0;
> +               desc->p         = 1;
> +               desc->limit1    = 0xf;
> +               desc->avl       = 0;
> +               desc->l         = 0;
> +               desc->d         = SEG_OP_SIZE_32BIT;
> +               desc->g         = SEG_GRANULARITY_4KB;
> +               desc->base2     = 0x00;
> +
>                 desc++;
>         } else {
>                 /* Second entry is unused on 32-bit */
> @@ -1028,15 +1031,16 @@ struct boot_params *efi_main(struct efi_
>         }
>
>         /* __KERNEL_CS */
> -       desc->limit0 = 0xffff;
> -       desc->base0 = 0x0000;
> -       desc->base1 = 0x0000;
> -       desc->type = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
> -       desc->s = DESC_TYPE_CODE_DATA;
> -       desc->dpl = 0;
> -       desc->p = 1;
> -       desc->limit1 = 0xf;
> -       desc->avl = 0;
> +       desc->limit0    = 0xffff;
> +       desc->base0     = 0x0000;
> +       desc->base1     = 0x0000;
> +       desc->type      = SEG_TYPE_CODE | SEG_TYPE_EXEC_READ;
> +       desc->s         = DESC_TYPE_CODE_DATA;
> +       desc->dpl       = 0;
> +       desc->p         = 1;
> +       desc->limit1    = 0xf;
> +       desc->avl       = 0;
> +
>         if (IS_ENABLED(CONFIG_X86_64)) {
>                 desc->l = 1;
>                 desc->d = 0;
> @@ -1044,41 +1048,41 @@ struct boot_params *efi_main(struct efi_
>                 desc->l = 0;
>                 desc->d = SEG_OP_SIZE_32BIT;
>         }
> -       desc->g = SEG_GRANULARITY_4KB;
> -       desc->base2 = 0x00;
> +       desc->g         = SEG_GRANULARITY_4KB;
> +       desc->base2     = 0x00;
>         desc++;
>
>         /* __KERNEL_DS */
> -       desc->limit0 = 0xffff;
> -       desc->base0 = 0x0000;
> -       desc->base1 = 0x0000;
> -       desc->type = SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
> -       desc->s = DESC_TYPE_CODE_DATA;
> -       desc->dpl = 0;
> -       desc->p = 1;
> -       desc->limit1 = 0xf;
> -       desc->avl = 0;
> -       desc->l = 0;
> -       desc->d = SEG_OP_SIZE_32BIT;
> -       desc->g = SEG_GRANULARITY_4KB;
> -       desc->base2 = 0x00;
> +       desc->limit0    = 0xffff;
> +       desc->base0     = 0x0000;
> +       desc->base1     = 0x0000;
> +       desc->type      = SEG_TYPE_DATA | SEG_TYPE_READ_WRITE;
> +       desc->s         = DESC_TYPE_CODE_DATA;
> +       desc->dpl       = 0;
> +       desc->p         = 1;
> +       desc->limit1    = 0xf;
> +       desc->avl       = 0;
> +       desc->l         = 0;
> +       desc->d         = SEG_OP_SIZE_32BIT;
> +       desc->g         = SEG_GRANULARITY_4KB;
> +       desc->base2     = 0x00;
>         desc++;
>
>         if (IS_ENABLED(CONFIG_X86_64)) {
>                 /* Task segment value */
> -               desc->limit0 = 0x0000;
> -               desc->base0 = 0x0000;
> -               desc->base1 = 0x0000;
> -               desc->type = SEG_TYPE_TSS;
> -               desc->s = 0;
> -               desc->dpl = 0;
> -               desc->p = 1;
> -               desc->limit1 = 0x0;
> -               desc->avl = 0;
> -               desc->l = 0;
> -               desc->d = 0;
> -               desc->g = SEG_GRANULARITY_4KB;
> -               desc->base2 = 0x00;
> +               desc->limit0    = 0x0000;
> +               desc->base0     = 0x0000;
> +               desc->base1     = 0x0000;
> +               desc->type      = SEG_TYPE_TSS;
> +               desc->s         = 0;
> +               desc->dpl       = 0;
> +               desc->p         = 1;
> +               desc->limit1    = 0x0;
> +               desc->avl       = 0;
> +               desc->l         = 0;
> +               desc->d         = 0;
> +               desc->g         = SEG_GRANULARITY_4KB;
> +               desc->base2     = 0x00;
>                 desc++;
>         }
>
> @@ -1088,5 +1092,6 @@ struct boot_params *efi_main(struct efi_
>         return boot_params;
>  fail:
>         efi_printk(sys_table, "efi_main() failed!\n");
> +
>         return NULL;
>  }

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

* Re: [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET
  2018-05-14  6:47   ` Ingo Molnar
@ 2018-05-14  6:48     ` Ard Biesheuvel
  2018-05-14  7:00       ` Ingo Molnar
  0 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-14  6:48 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-efi, Thomas Gleixner, Mark Rutland, Linux Kernel Mailing List

On 14 May 2018 at 08:47, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> From: Mark Rutland <mark.rutland@arm.com>
>>
>> When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an
>> arbitrary multiple of PAGE_SIZE in the interval [0, 2MB).
>>
>> The EFI stub does not account for the potential misalignment of
>> TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized
>> physical offset which is always a round multiple of EFI_KIMG_ALIGN.
>> This may result in statically allocated objects whose alignment exceeds
>> PAGE_SIZE to appear misaligned in memory. This has been observed to
>> result in spurious stack overflow reports and failure to make use of
>> the IRQ stacks, and theoretically could result in a number of other
>> issues.
>>
>> We can OR in the low bits of TEXT_OFFSET to ensure that we have the
>> necessary offset (and hence preserve the misalignment of TEXT_OFFSET
>> relative to EFI_KIMG_ALIGN), so let's do that.
>>
>> Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity")
>> Cc: <stable@vger.kernel.org> # v4.7+
>> Reported-by: Kim Phillips <kim.phillips@arm.com>
>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>> Tested-by: Kim Phillips <kim.phillips@arm.com>
>> [ardb: clarify commit log]
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
>> index b9bd827caa22..541b82fdc8a2 100644
>> --- a/drivers/firmware/efi/libstub/arm64-stub.c
>> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
>> @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
>>               u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
>>                            (phys_seed >> 32) & mask : TEXT_OFFSET;
>>
>> +             /*
>> +              * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a
>> +              * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply
>> +              * the offset below EFI_KIMG_ALIGN.
>> +              */
>
> When referring to config variables in comments and changelogs I'd suggest a bit
> more verbosity:
>
>   s/CONFIG_RANDOMIZE_TEXT_OFFSET
>    /CONFIG_RANDOMIZE_TEXT_OFFSET=y
>
> ... because at first I thought (based on the name) that
> CONFIG_RANDOMIZE_TEXT_OFFSET is an actual integer offset value - while it's a
> bool. The =y makes the bool nature obvious.
>
> ( Similarly, when negated the canonical way to refer to it is
>   !CONFIG_RANDOMIZE_TEXT_OFFSET. )
>

Fair enough.

>> +             offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN);
>
> The parentheses are not needed here I think.
>

Nope.

Will you fix this up when applying? Or should I resend?

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

* Re: [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode
  2018-05-04  5:59 ` [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode Ard Biesheuvel
@ 2018-05-14  6:57   ` Ingo Molnar
  2018-05-14  7:02     ` Ard Biesheuvel
  2018-05-14  7:42   ` [tip:efi/core] efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition " tip-bot for Ard Biesheuvel
  1 sibling, 1 reply; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  6:57 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Thomas Gleixner, linux-kernel


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> Mixed mode allows a kernel built for x86_64 to interact with 32-bit
> EFI firmware, but requires us to define all struct definitions carefully
> when it comes to pointer sizes. efi_pci_io_protocol32 currently uses a
> void* for the 'romimage' field, which will be interpreted as a 64-bit
> field on such kernels, potentially resulting in bogus memory references
> and subsequent crashes.

Yeah, so the first confusion I ran into is:

 s/efi_pci_io_protocol32
  /efi_pci_io_protocol_32

Once I found it in the code I made this change:

 s/efi: fix efi_pci_io_protocol32 prototype for mixed mode
  /efi: Fix 'struct efi_pci_io_protocol32' definition for mixed mode

Because we normally use the 'prototype' name for function declarations, not for 
data type definitions. Adding 'struct' and putting it between quotes makes it 
obvious at a glance that we are talking about a structure definition here.

BTW., since it's marked -stable, due to:

 > potentially resulting in bogus memory references
 > and subsequent crashes.

I'm moving it to efi/urgent: the principle here is that if a patch is urgent 
enough for -stable then it should generally not wait for the next merge window.

Also, because this actually fixes a crash, I extended the title to spell this out 
more clearly:

  Subject: efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition for mixed mode

... which also makes it easier for maintainers of older stable kernels to decide 
whether to backport the patch or not.

Anyway, the patch is looking good otherwise, no need to resend.

Thanks,

	Ingo

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

* Re: [PATCH] efi/x86: Clean up the eboot code a bit
  2018-05-14  6:47     ` Ard Biesheuvel
@ 2018-05-14  6:58       ` Ingo Molnar
  2018-05-14  6:59         ` Ard Biesheuvel
  0 siblings, 1 reply; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  6:58 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Thomas Gleixner, Hans de Goede, Linux Kernel Mailing List


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 14 May 2018 at 08:43, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > So I looked at arch/x86/boot/compressed/eboot.c to improve a printk message and
> > ended up with the cleanups below.
> >
> > Only build tested.
> >
> > Thanks,
> >
> >         Ingo
> >
> > =================>
> > Subject: efi/x86: Clean up the eboot code
> > From: Ingo Molnar <mingo@kernel.org>
> > Date: Mon May 14 08:33:40 CEST 2018
> >
> > Various small cleanups:
> >
> >  - Standardize printk messages:
> >
> >      'alloc' => 'allocate'
> >      'mem'   => 'memory'
> >
> >    also put variable names in printk messages between quotes.
> >
> >  - Align mass-assignments vertically for better readability
> >
> >  - Break multi-line function prototypes at the name where possible,
> >    not in the middle of the parameter list
> >
> >  - Use a newline before return statements consistently.
> >
> >  - Use curly braces in a balanced fashion.
> >
> >  - Remove stray newlines.
> >
> > No change in functionality.
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: linux-efi@vger.kernel.org
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> 
> Thanks Ingo
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thanks - mind picking it up into your tree? I only did minimal testing and don't 
want to interfere with your patch flow either.

Thanks,

	Ingo

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

* Re: [PATCH] efi/x86: Clean up the eboot code a bit
  2018-05-14  6:58       ` Ingo Molnar
@ 2018-05-14  6:59         ` Ard Biesheuvel
  0 siblings, 0 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-14  6:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-efi, Thomas Gleixner, Hans de Goede, Linux Kernel Mailing List

On 14 May 2018 at 08:58, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 14 May 2018 at 08:43, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > So I looked at arch/x86/boot/compressed/eboot.c to improve a printk message and
>> > ended up with the cleanups below.
>> >
>> > Only build tested.
>> >
>> > Thanks,
>> >
>> >         Ingo
>> >
>> > =================>
>> > Subject: efi/x86: Clean up the eboot code
>> > From: Ingo Molnar <mingo@kernel.org>
>> > Date: Mon May 14 08:33:40 CEST 2018
>> >
>> > Various small cleanups:
>> >
>> >  - Standardize printk messages:
>> >
>> >      'alloc' => 'allocate'
>> >      'mem'   => 'memory'
>> >
>> >    also put variable names in printk messages between quotes.
>> >
>> >  - Align mass-assignments vertically for better readability
>> >
>> >  - Break multi-line function prototypes at the name where possible,
>> >    not in the middle of the parameter list
>> >
>> >  - Use a newline before return statements consistently.
>> >
>> >  - Use curly braces in a balanced fashion.
>> >
>> >  - Remove stray newlines.
>> >
>> > No change in functionality.
>> >
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
>> > Cc: Peter Zijlstra <peterz@infradead.org>
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: linux-efi@vger.kernel.org
>> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
>>
>> Thanks Ingo
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Thanks - mind picking it up into your tree? I only did minimal testing and don't
> want to interfere with your patch flow either.
>

Will do, thanks.

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

* Re: [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET
  2018-05-14  6:48     ` Ard Biesheuvel
@ 2018-05-14  7:00       ` Ingo Molnar
  2018-05-14  7:01         ` Ard Biesheuvel
  0 siblings, 1 reply; 47+ messages in thread
From: Ingo Molnar @ 2018-05-14  7:00 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Thomas Gleixner, Mark Rutland, Linux Kernel Mailing List


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 14 May 2018 at 08:47, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >> From: Mark Rutland <mark.rutland@arm.com>
> >>
> >> When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an
> >> arbitrary multiple of PAGE_SIZE in the interval [0, 2MB).
> >>
> >> The EFI stub does not account for the potential misalignment of
> >> TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized
> >> physical offset which is always a round multiple of EFI_KIMG_ALIGN.
> >> This may result in statically allocated objects whose alignment exceeds
> >> PAGE_SIZE to appear misaligned in memory. This has been observed to
> >> result in spurious stack overflow reports and failure to make use of
> >> the IRQ stacks, and theoretically could result in a number of other
> >> issues.
> >>
> >> We can OR in the low bits of TEXT_OFFSET to ensure that we have the
> >> necessary offset (and hence preserve the misalignment of TEXT_OFFSET
> >> relative to EFI_KIMG_ALIGN), so let's do that.
> >>
> >> Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity")
> >> Cc: <stable@vger.kernel.org> # v4.7+
> >> Reported-by: Kim Phillips <kim.phillips@arm.com>
> >> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> >> Tested-by: Kim Phillips <kim.phillips@arm.com>
> >> [ardb: clarify commit log]
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
> >> index b9bd827caa22..541b82fdc8a2 100644
> >> --- a/drivers/firmware/efi/libstub/arm64-stub.c
> >> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
> >> @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
> >>               u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
> >>                            (phys_seed >> 32) & mask : TEXT_OFFSET;
> >>
> >> +             /*
> >> +              * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a
> >> +              * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply
> >> +              * the offset below EFI_KIMG_ALIGN.
> >> +              */
> >
> > When referring to config variables in comments and changelogs I'd suggest a bit
> > more verbosity:
> >
> >   s/CONFIG_RANDOMIZE_TEXT_OFFSET
> >    /CONFIG_RANDOMIZE_TEXT_OFFSET=y
> >
> > ... because at first I thought (based on the name) that
> > CONFIG_RANDOMIZE_TEXT_OFFSET is an actual integer offset value - while it's a
> > bool. The =y makes the bool nature obvious.
> >
> > ( Similarly, when negated the canonical way to refer to it is
> >   !CONFIG_RANDOMIZE_TEXT_OFFSET. )
> >
> 
> Fair enough.
> 
> >> +             offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN);
> >
> > The parentheses are not needed here I think.
> >
> 
> Nope.
> 
> Will you fix this up when applying? Or should I resend?

Since this was at the tail with no dependencies I'll skip this for now I think - 
mind sending the refreshed version in the next batch?

Thanks,

	Ingo

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

* Re: [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET
  2018-05-14  7:00       ` Ingo Molnar
@ 2018-05-14  7:01         ` Ard Biesheuvel
  0 siblings, 0 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-14  7:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-efi, Thomas Gleixner, Mark Rutland, Linux Kernel Mailing List

On 14 May 2018 at 09:00, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 14 May 2018 at 08:47, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >
>> >> From: Mark Rutland <mark.rutland@arm.com>
>> >>
>> >> When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an
>> >> arbitrary multiple of PAGE_SIZE in the interval [0, 2MB).
>> >>
>> >> The EFI stub does not account for the potential misalignment of
>> >> TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized
>> >> physical offset which is always a round multiple of EFI_KIMG_ALIGN.
>> >> This may result in statically allocated objects whose alignment exceeds
>> >> PAGE_SIZE to appear misaligned in memory. This has been observed to
>> >> result in spurious stack overflow reports and failure to make use of
>> >> the IRQ stacks, and theoretically could result in a number of other
>> >> issues.
>> >>
>> >> We can OR in the low bits of TEXT_OFFSET to ensure that we have the
>> >> necessary offset (and hence preserve the misalignment of TEXT_OFFSET
>> >> relative to EFI_KIMG_ALIGN), so let's do that.
>> >>
>> >> Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity")
>> >> Cc: <stable@vger.kernel.org> # v4.7+
>> >> Reported-by: Kim Phillips <kim.phillips@arm.com>
>> >> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>> >> Tested-by: Kim Phillips <kim.phillips@arm.com>
>> >> [ardb: clarify commit log]
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >>  drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++
>> >>  1 file changed, 7 insertions(+)
>> >>
>> >> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
>> >> index b9bd827caa22..541b82fdc8a2 100644
>> >> --- a/drivers/firmware/efi/libstub/arm64-stub.c
>> >> +++ b/drivers/firmware/efi/libstub/arm64-stub.c
>> >> @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg,
>> >>               u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ?
>> >>                            (phys_seed >> 32) & mask : TEXT_OFFSET;
>> >>
>> >> +             /*
>> >> +              * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a
>> >> +              * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply
>> >> +              * the offset below EFI_KIMG_ALIGN.
>> >> +              */
>> >
>> > When referring to config variables in comments and changelogs I'd suggest a bit
>> > more verbosity:
>> >
>> >   s/CONFIG_RANDOMIZE_TEXT_OFFSET
>> >    /CONFIG_RANDOMIZE_TEXT_OFFSET=y
>> >
>> > ... because at first I thought (based on the name) that
>> > CONFIG_RANDOMIZE_TEXT_OFFSET is an actual integer offset value - while it's a
>> > bool. The =y makes the bool nature obvious.
>> >
>> > ( Similarly, when negated the canonical way to refer to it is
>> >   !CONFIG_RANDOMIZE_TEXT_OFFSET. )
>> >
>>
>> Fair enough.
>>
>> >> +             offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN);
>> >
>> > The parentheses are not needed here I think.
>> >
>>
>> Nope.
>>
>> Will you fix this up when applying? Or should I resend?
>
> Since this was at the tail with no dependencies I'll skip this for now I think -
> mind sending the refreshed version in the next batch?
>

Actually, by the reasoning in your other reply, I'll fix this up and
resend it for -urgent instead.

Thanks,
Ard.

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

* Re: [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode
  2018-05-14  6:57   ` Ingo Molnar
@ 2018-05-14  7:02     ` Ard Biesheuvel
  0 siblings, 0 replies; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-14  7:02 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-efi, Thomas Gleixner, Linux Kernel Mailing List

On 14 May 2018 at 08:57, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> Mixed mode allows a kernel built for x86_64 to interact with 32-bit
>> EFI firmware, but requires us to define all struct definitions carefully
>> when it comes to pointer sizes. efi_pci_io_protocol32 currently uses a
>> void* for the 'romimage' field, which will be interpreted as a 64-bit
>> field on such kernels, potentially resulting in bogus memory references
>> and subsequent crashes.
>
> Yeah, so the first confusion I ran into is:
>
>  s/efi_pci_io_protocol32
>   /efi_pci_io_protocol_32
>
> Once I found it in the code I made this change:
>
>  s/efi: fix efi_pci_io_protocol32 prototype for mixed mode
>   /efi: Fix 'struct efi_pci_io_protocol32' definition for mixed mode
>
> Because we normally use the 'prototype' name for function declarations, not for
> data type definitions. Adding 'struct' and putting it between quotes makes it
> obvious at a glance that we are talking about a structure definition here.
>

OK.

> BTW., since it's marked -stable, due to:
>
>  > potentially resulting in bogus memory references
>  > and subsequent crashes.
>
> I'm moving it to efi/urgent: the principle here is that if a patch is urgent
> enough for -stable then it should generally not wait for the next merge window.
>

The issue was found by inspection rather than a crash report, so I was
in doubt about this.

> Also, because this actually fixes a crash, I extended the title to spell this out
> more clearly:
>
>   Subject: efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition for mixed mode
>
> ... which also makes it easier for maintainers of older stable kernels to decide
> whether to backport the patch or not.
>

OK.

> Anyway, the patch is looking good otherwise, no need to resend.
>
> Thanks,
>
>         Ingo

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

* [tip:efi/core] efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition for mixed mode
  2018-05-04  5:59 ` [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode Ard Biesheuvel
  2018-05-14  6:57   ` Ingo Molnar
@ 2018-05-14  7:42   ` tip-bot for Ard Biesheuvel
  1 sibling, 0 replies; 47+ messages in thread
From: tip-bot for Ard Biesheuvel @ 2018-05-14  7:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, matt, linux-kernel, hdegoede, tglx, ard.biesheuvel, peterz,
	stable, torvalds, mingo

Commit-ID:  0b3225ab9407f557a8e20f23f37aa7236c10a9b1
Gitweb:     https://git.kernel.org/tip/0b3225ab9407f557a8e20f23f37aa7236c10a9b1
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Fri, 4 May 2018 07:59:58 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:56:29 +0200

efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition for mixed mode

Mixed mode allows a kernel built for x86_64 to interact with 32-bit
EFI firmware, but requires us to define all struct definitions carefully
when it comes to pointer sizes.

'struct efi_pci_io_protocol_32' currently uses a 'void *' for the
'romimage' field, which will be interpreted as a 64-bit field
on such kernels, potentially resulting in bogus memory references
and subsequent crashes.

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: <stable@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-13-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 6 ++++--
 include/linux/efi.h              | 8 ++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 47d3efff6805..09f36c0d9d4f 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -163,7 +163,8 @@ __setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, pci->romimage, pci->romsize);
+	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
+	       pci->romsize);
 	return status;
 
 free_struct:
@@ -269,7 +270,8 @@ __setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, pci->romimage, pci->romsize);
+	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
+	       pci->romsize);
 	return status;
 
 free_struct:
diff --git a/include/linux/efi.h b/include/linux/efi.h
index f1b7d68ac460..3016d8c456bc 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -395,8 +395,8 @@ typedef struct {
 	u32 attributes;
 	u32 get_bar_attributes;
 	u32 set_bar_attributes;
-	uint64_t romsize;
-	void *romimage;
+	u64 romsize;
+	u32 romimage;
 } efi_pci_io_protocol_32;
 
 typedef struct {
@@ -415,8 +415,8 @@ typedef struct {
 	u64 attributes;
 	u64 get_bar_attributes;
 	u64 set_bar_attributes;
-	uint64_t romsize;
-	void *romimage;
+	u64 romsize;
+	u64 romimage;
 } efi_pci_io_protocol_64;
 
 typedef struct {

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

* [tip:efi/core] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
  2018-05-04  5:59 ` [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot Ard Biesheuvel
@ 2018-05-14  7:43   ` tip-bot for Daniel Kiper
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Daniel Kiper @ 2018-05-14  7:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, daniel.kiper, matt, linux-kernel, peterz, mingo, torvalds,
	ard.biesheuvel, hpa

Commit-ID:  a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Gitweb:     https://git.kernel.org/tip/a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Author:     Daniel Kiper <daniel.kiper@oracle.com>
AuthorDate: Fri, 4 May 2018 07:59:47 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:46 +0200

x86/xen/efi: Initialize UEFI secure boot state during dom0 boot

Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.

Note that part of drivers/firmware/efi/libstub/secureboot.c is duplicated
by this patch, only in this case, it runs in the context of the kernel
proper rather than UEFI boot context. The reason for the duplication is
that maintaining the original code to run correctly on ARM/arm64 as well
as on all the quirky x86 firmware we support is enough of a burden as it
is, and adding the x86/Xen execution context to that mix just so we can
reuse a single routine just isn't worth it.

[ardb: explain rationale for code duplication]
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-2-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/xen/efi.c                        | 57 +++++++++++++++++++++++++++++++
 drivers/firmware/efi/libstub/secureboot.c |  3 ++
 2 files changed, 60 insertions(+)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703be9ead..1804b27f9632 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
 	return &efi_systab_xen;
 }
 
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+	static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+	static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+	efi_status_t status;
+	u8 moksbstate, secboot, setupmode;
+	unsigned long size;
+
+	size = sizeof(secboot);
+	status = efi.get_variable(L"SecureBoot", &efi_variable_guid,
+				  NULL, &size, &secboot);
+
+	if (status == EFI_NOT_FOUND)
+		return efi_secureboot_mode_disabled;
+
+	if (status != EFI_SUCCESS)
+		goto out_efi_err;
+
+	size = sizeof(setupmode);
+	status = efi.get_variable(L"SetupMode", &efi_variable_guid,
+				  NULL, &size, &setupmode);
+
+	if (status != EFI_SUCCESS)
+		goto out_efi_err;
+
+	if (secboot == 0 || setupmode == 1)
+		return efi_secureboot_mode_disabled;
+
+	/* See if a user has put the shim into insecure mode. */
+	size = sizeof(moksbstate);
+	status = efi.get_variable(L"MokSBStateRT", &shim_guid,
+				  NULL, &size, &moksbstate);
+
+	/* If it fails, we don't care why. Default to secure. */
+	if (status != EFI_SUCCESS)
+		goto secure_boot_enabled;
+
+	if (moksbstate == 1)
+		return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+	pr_info("UEFI Secure Boot is enabled.\n");
+	return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+	pr_err("Could not determine UEFI Secure Boot status.\n");
+	return efi_secureboot_mode_unknown;
+}
+
 void __init xen_efi_init(void)
 {
 	efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
 	boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
 	boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 32);
 
+	boot_params.secure_boot = xen_efi_get_secureboot();
+
 	set_bit(EFI_BOOT, &efi.flags);
 	set_bit(EFI_PARAVIRT, &efi.flags);
 	set_bit(EFI_64BIT, &efi.flags);
diff --git a/drivers/firmware/efi/libstub/secureboot.c b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb414c00..72d9dfbebf08 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@ static const efi_char16_t shim_MokSBState_name[] = L"MokSBState";
 
 /*
  * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
  */
 enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
 {

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

* [tip:efi/core] efi/cper: Remove the INDENT_SP silliness
  2018-05-04  5:59 ` [PATCH 02/17] efi/cper: Remove the INDENT_SP silliness Ard Biesheuvel
@ 2018-05-14  7:44   ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Borislav Petkov @ 2018-05-14  7:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, bp, mingo, torvalds, matt, peterz,
	ard.biesheuvel, tglx

Commit-ID:  75e4fd31ce90e7f268365cb455381581896e0bca
Gitweb:     https://git.kernel.org/tip/75e4fd31ce90e7f268365cb455381581896e0bca
Author:     Borislav Petkov <bp@suse.de>
AuthorDate: Fri, 4 May 2018 07:59:48 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:47 +0200

efi/cper: Remove the INDENT_SP silliness

A separate define just to print a space character is silly and
completely unneeded. Remove it.

Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-3-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-arm.c | 6 ++----
 drivers/firmware/efi/cper.c     | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
index 698e5c8e0c8d..502811344e81 100644
--- a/drivers/firmware/efi/cper-arm.c
+++ b/drivers/firmware/efi/cper-arm.c
@@ -30,8 +30,6 @@
 #include <acpi/ghes.h>
 #include <ras/ras_event.h>
 
-#define INDENT_SP	" "
-
 static const char * const arm_reg_ctx_strs[] = {
 	"AArch32 general purpose registers",
 	"AArch32 EL1 context registers",
@@ -283,7 +281,7 @@ void cper_print_proc_arm(const char *pfx,
 			pfx, proc->psci_state);
 	}
 
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 
 	err_info = (struct cper_arm_err_info *)(proc + 1);
 	for (i = 0; i < proc->err_info_num; i++) {
@@ -310,7 +308,7 @@ void cper_print_proc_arm(const char *pfx,
 		if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) {
 			printk("%serror_info: 0x%016llx\n", newpfx,
 			       err_info->error_info);
-			snprintf(infopfx, sizeof(infopfx), "%s%s", newpfx, INDENT_SP);
+			snprintf(infopfx, sizeof(infopfx), "%s ", newpfx);
 			cper_print_arm_err_info(infopfx, err_info->type,
 						err_info->error_info);
 		}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index c165933ebf38..ab21f1614007 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -37,8 +37,6 @@
 #include <acpi/ghes.h>
 #include <ras/ras_event.h>
 
-#define INDENT_SP	" "
-
 static char rcd_decode_str[CPER_REC_LEN];
 
 /*
@@ -433,7 +431,7 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 	if (gdata->validation_bits & CPER_SEC_VALID_FRU_TEXT)
 		printk("%s""fru_text: %.20s\n", pfx, gdata->fru_text);
 
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 	if (guid_equal(sec_type, &CPER_SEC_PROC_GENERIC)) {
 		struct cper_sec_proc_generic *proc_err = acpi_hest_get_payload(gdata);
 
@@ -500,7 +498,7 @@ void cper_estatus_print(const char *pfx,
 		       "It has been corrected by h/w "
 		       "and requires no further action");
 	printk("%s""event severity: %s\n", pfx, cper_severity_str(severity));
-	snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
 
 	apei_estatus_for_each_section(estatus, gdata) {
 		cper_estatus_print_section(newpfx, gdata, sec_no);

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

* [tip:efi/core] efi: Fix IA32/X64 Processor Error Record definition
  2018-05-04  5:59 ` [PATCH 03/17] efi: Fix IA32/X64 Processor Error Record definition Ard Biesheuvel
@ 2018-05-14  7:44   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ard.biesheuvel, yazen.ghannam, torvalds, matt, tglx,
	linux-kernel, peterz, hpa, mingo

Commit-ID:  742632d237ce180439ab4af31e9891df0df81233
Gitweb:     https://git.kernel.org/tip/742632d237ce180439ab4af31e9891df0df81233
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:49 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:47 +0200

efi: Fix IA32/X64 Processor Error Record definition

Based on UEFI 2.7 Table 255. Processor Error Record, the "Local APIC_ID"
field is 8 bytes but Linux defines this field as 1 byte.

Fix this in the struct cper_sec_proc_ia definition.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-4-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/cper.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cper.h b/include/linux/cper.h
index d14ef4e77c8a..4b5f8459b403 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -381,7 +381,7 @@ struct cper_sec_proc_generic {
 /* IA32/X64 Processor Error Section */
 struct cper_sec_proc_ia {
 	__u64	validation_bits;
-	__u8	lapic_id;
+	__u64	lapic_id;
 	__u8	cpuid[48];
 };
 

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

* [tip:efi/core] efi: Decode IA32/X64 Processor Error Section
  2018-05-04  5:59 ` [PATCH 04/17] efi: Decode IA32/X64 Processor Error Section Ard Biesheuvel
@ 2018-05-14  7:45   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: matt, hpa, peterz, torvalds, linux-kernel, mingo, yazen.ghannam,
	tglx, ard.biesheuvel

Commit-ID:  f9e1bdb9f35f4f5cfa7c9025ac68c02909b6d3b1
Gitweb:     https://git.kernel.org/tip/f9e1bdb9f35f4f5cfa7c9025ac68c02909b6d3b1
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:50 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:47 +0200

efi: Decode IA32/X64 Processor Error Section

Recognize the IA32/X64 Processor Error Section.

Do the section decoding in a new "cper-x86.c" file and add this to the
Makefile depending on a new "UEFI_CPER_X86" config option.

Print the Local APIC ID and CPUID info from the Processor Error Record.

The "Processor Error Info" and "Processor Context" fields will be
decoded in following patches.

Based on UEFI 2.7 Table 252. Processor Error Record.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-5-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/Kconfig    |  5 +++++
 drivers/firmware/efi/Makefile   |  1 +
 drivers/firmware/efi/cper-x86.c | 23 +++++++++++++++++++++++
 drivers/firmware/efi/cper.c     | 10 ++++++++++
 include/linux/cper.h            |  2 ++
 5 files changed, 41 insertions(+)

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 3098410abad8..781a4a337557 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -174,6 +174,11 @@ config UEFI_CPER_ARM
 	depends on UEFI_CPER && ( ARM || ARM64 )
 	default y
 
+config UEFI_CPER_X86
+	bool
+	depends on UEFI_CPER && X86
+	default y
+
 config EFI_DEV_PATH_PARSER
 	bool
 	depends on ACPI
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index cb805374f4bc..5f9f5039de50 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -31,3 +31,4 @@ obj-$(CONFIG_ARM)			+= $(arm-obj-y)
 obj-$(CONFIG_ARM64)			+= $(arm-obj-y)
 obj-$(CONFIG_EFI_CAPSULE_LOADER)	+= capsule-loader.o
 obj-$(CONFIG_UEFI_CPER_ARM)		+= cper-arm.o
+obj-$(CONFIG_UEFI_CPER_X86)		+= cper-x86.o
diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
new file mode 100644
index 000000000000..863f0cd2a0ff
--- /dev/null
+++ b/drivers/firmware/efi/cper-x86.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018, Advanced Micro Devices, Inc.
+
+#include <linux/cper.h>
+
+/*
+ * We don't need a "CPER_IA" prefix since these are all locally defined.
+ * This will save us a lot of line space.
+ */
+#define VALID_LAPIC_ID			BIT_ULL(0)
+#define VALID_CPUID_INFO		BIT_ULL(1)
+
+void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
+{
+	if (proc->validation_bits & VALID_LAPIC_ID)
+		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
+
+	if (proc->validation_bits & VALID_CPUID_INFO) {
+		printk("%sCPUID Info:\n", pfx);
+		print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
+			       sizeof(proc->cpuid), 0);
+	}
+}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index ab21f1614007..3bf0dca378a6 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -467,6 +467,16 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
 			cper_print_proc_arm(newpfx, arm_err);
 		else
 			goto err_section_too_small;
+#endif
+#if defined(CONFIG_UEFI_CPER_X86)
+	} else if (guid_equal(sec_type, &CPER_SEC_PROC_IA)) {
+		struct cper_sec_proc_ia *ia_err = acpi_hest_get_payload(gdata);
+
+		printk("%ssection_type: IA32/X64 processor error\n", newpfx);
+		if (gdata->error_data_length >= sizeof(*ia_err))
+			cper_print_proc_ia(newpfx, ia_err);
+		else
+			goto err_section_too_small;
 #endif
 	} else {
 		const void *err = acpi_hest_get_payload(gdata);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 4b5f8459b403..9c703a0abe6e 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -551,5 +551,7 @@ const char *cper_mem_err_unpack(struct trace_seq *,
 				struct cper_mem_err_compact *);
 void cper_print_proc_arm(const char *pfx,
 			 const struct cper_sec_proc_arm *proc);
+void cper_print_proc_ia(const char *pfx,
+			const struct cper_sec_proc_ia *proc);
 
 #endif

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

* [tip:efi/core] efi: Decode IA32/X64 Processor Error Info Structure
  2018-05-04  5:59 ` [PATCH 05/17] efi: Decode IA32/X64 Processor Error Info Structure Ard Biesheuvel
@ 2018-05-14  7:45   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: matt, tglx, peterz, mingo, ard.biesheuvel, hpa, yazen.ghannam,
	linux-kernel, torvalds

Commit-ID:  7c9449b8c8a59511b7d749afb193c96353451c82
Gitweb:     https://git.kernel.org/tip/7c9449b8c8a59511b7d749afb193c96353451c82
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:51 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:47 +0200

efi: Decode IA32/X64 Processor Error Info Structure

Print the fields in the IA32/X64 Processor Error Info Structure.

Based on UEFI 2.7 Table 253. IA32/X64 Processor Error Information
Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-6-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 48 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 863f0cd2a0ff..e0633a103fcf 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -9,9 +9,20 @@
  */
 #define VALID_LAPIC_ID			BIT_ULL(0)
 #define VALID_CPUID_INFO		BIT_ULL(1)
+#define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
+
+#define INFO_VALID_CHECK_INFO		BIT_ULL(0)
+#define INFO_VALID_TARGET_ID		BIT_ULL(1)
+#define INFO_VALID_REQUESTOR_ID		BIT_ULL(2)
+#define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
+#define INFO_VALID_IP			BIT_ULL(4)
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
+	int i;
+	struct cper_ia_err_info *err_info;
+	char newpfx[64];
+
 	if (proc->validation_bits & VALID_LAPIC_ID)
 		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
 
@@ -20,4 +31,41 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 		print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
 			       sizeof(proc->cpuid), 0);
 	}
+
+	snprintf(newpfx, sizeof(newpfx), "%s ", pfx);
+
+	err_info = (struct cper_ia_err_info *)(proc + 1);
+	for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
+		printk("%sError Information Structure %d:\n", pfx, i);
+
+		printk("%sError Structure Type: %pUl\n", newpfx,
+		       &err_info->err_type);
+
+		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
+			printk("%sCheck Information: 0x%016llx\n", newpfx,
+			       err_info->check_info);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
+			printk("%sTarget Identifier: 0x%016llx\n",
+			       newpfx, err_info->target_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
+			printk("%sRequestor Identifier: 0x%016llx\n",
+			       newpfx, err_info->requestor_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
+			printk("%sResponder Identifier: 0x%016llx\n",
+			       newpfx, err_info->responder_id);
+		}
+
+		if (err_info->validation_bits & INFO_VALID_IP) {
+			printk("%sInstruction Pointer: 0x%016llx\n",
+			       newpfx, err_info->ip);
+		}
+
+		err_info++;
+	}
 }

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

* [tip:efi/core] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs
  2018-05-04  5:59 ` [PATCH 06/17] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs Ard Biesheuvel
@ 2018-05-14  7:46   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: yazen.ghannam, ard.biesheuvel, matt, mingo, linux-kernel, peterz,
	torvalds, hpa, tglx

Commit-ID:  dc2d26e4b667c8005c58669e71de3efd17f4390f
Gitweb:     https://git.kernel.org/tip/dc2d26e4b667c8005c58669e71de3efd17f4390f
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:52 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:47 +0200

efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs

For easier handling, match the known IA32/X64 error structure GUIDs to
enums.

Also, print out the name of the matching Error Structure Type.

Only print the GUID for unknown types.

GUIDs taken from UEFI 2.7 section N.2.4.2.1 IA32/X64 Processor Error
Information Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-7-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 47 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index e0633a103fcf..5438097b93ac 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -11,17 +11,53 @@
 #define VALID_CPUID_INFO		BIT_ULL(1)
 #define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
 
+#define INFO_ERR_STRUCT_TYPE_CACHE					\
+	GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B,	\
+		  0x57, 0x3F, 0xAD, 0x2C)
+#define INFO_ERR_STRUCT_TYPE_TLB					\
+	GUID_INIT(0xFC06B535, 0x5E1F, 0x4562, 0x9F, 0x25, 0x0A, 0x3B,	\
+		  0x9A, 0xDB, 0x63, 0xC3)
+#define INFO_ERR_STRUCT_TYPE_BUS					\
+	GUID_INIT(0x1CF3F8B3, 0xC5B1, 0x49a2, 0xAA, 0x59, 0x5E, 0xEF,	\
+		  0x92, 0xFF, 0xA6, 0x3C)
+#define INFO_ERR_STRUCT_TYPE_MS						\
+	GUID_INIT(0x48AB7F57, 0xDC34, 0x4f6c, 0xA7, 0xD3, 0xB0, 0xB5,	\
+		  0xB0, 0xA7, 0x43, 0x14)
+
 #define INFO_VALID_CHECK_INFO		BIT_ULL(0)
 #define INFO_VALID_TARGET_ID		BIT_ULL(1)
 #define INFO_VALID_REQUESTOR_ID		BIT_ULL(2)
 #define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
 #define INFO_VALID_IP			BIT_ULL(4)
 
+enum err_types {
+	ERR_TYPE_CACHE = 0,
+	ERR_TYPE_TLB,
+	ERR_TYPE_BUS,
+	ERR_TYPE_MS,
+	N_ERR_TYPES
+};
+
+static enum err_types cper_get_err_type(const guid_t *err_type)
+{
+	if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_CACHE))
+		return ERR_TYPE_CACHE;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_TLB))
+		return ERR_TYPE_TLB;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_BUS))
+		return ERR_TYPE_BUS;
+	else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_MS))
+		return ERR_TYPE_MS;
+	else
+		return N_ERR_TYPES;
+}
+
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
 	char newpfx[64];
+	u8 err_type;
 
 	if (proc->validation_bits & VALID_LAPIC_ID)
 		printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
@@ -38,8 +74,15 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 	for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
 		printk("%sError Information Structure %d:\n", pfx, i);
 
-		printk("%sError Structure Type: %pUl\n", newpfx,
-		       &err_info->err_type);
+		err_type = cper_get_err_type(&err_info->err_type);
+		printk("%sError Structure Type: %s\n", newpfx,
+		       err_type < ARRAY_SIZE(cper_proc_error_type_strs) ?
+		       cper_proc_error_type_strs[err_type] : "unknown");
+
+		if (err_type >= N_ERR_TYPES) {
+			printk("%sError Structure Type: %pUl\n", newpfx,
+			       &err_info->err_type);
+		}
 
 		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
 			printk("%sCheck Information: 0x%016llx\n", newpfx,

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

* [tip:efi/core] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures
  2018-05-04  5:59 ` [PATCH 07/17] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures Ard Biesheuvel
@ 2018-05-14  7:46   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, ard.biesheuvel, torvalds, tglx, hpa, mingo, matt,
	peterz, yazen.ghannam

Commit-ID:  a9c1e3e791409e35207277b7873efc756b6fb625
Gitweb:     https://git.kernel.org/tip/a9c1e3e791409e35207277b7873efc756b6fb625
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:53 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi: Decode IA32/X64 Cache, TLB, and Bus Check structures

Print the common fields of the Cache, TLB, and Bus check structures.The
fields of these three check types are the same except for a few more
fields in the Bus check structure. The remaining Bus check structure
fields will be decoded in a following patch.

Based on UEFI 2.7,
Table 254. IA32/X64 Cache Check Structure
Table 255. IA32/X64 TLB Check Structure
Table 256. IA32/X64 Bus Check Structure

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-8-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 99 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 98 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 5438097b93ac..f70c46f7a4db 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -30,6 +30,25 @@
 #define INFO_VALID_RESPONDER_ID		BIT_ULL(3)
 #define INFO_VALID_IP			BIT_ULL(4)
 
+#define CHECK_VALID_TRANS_TYPE		BIT_ULL(0)
+#define CHECK_VALID_OPERATION		BIT_ULL(1)
+#define CHECK_VALID_LEVEL		BIT_ULL(2)
+#define CHECK_VALID_PCC			BIT_ULL(3)
+#define CHECK_VALID_UNCORRECTED		BIT_ULL(4)
+#define CHECK_VALID_PRECISE_IP		BIT_ULL(5)
+#define CHECK_VALID_RESTARTABLE_IP	BIT_ULL(6)
+#define CHECK_VALID_OVERFLOW		BIT_ULL(7)
+
+#define CHECK_VALID_BITS(check)		(((check) & GENMASK_ULL(15, 0)))
+#define CHECK_TRANS_TYPE(check)		(((check) & GENMASK_ULL(17, 16)) >> 16)
+#define CHECK_OPERATION(check)		(((check) & GENMASK_ULL(21, 18)) >> 18)
+#define CHECK_LEVEL(check)		(((check) & GENMASK_ULL(24, 22)) >> 22)
+#define CHECK_PCC			BIT_ULL(25)
+#define CHECK_UNCORRECTED		BIT_ULL(26)
+#define CHECK_PRECISE_IP		BIT_ULL(27)
+#define CHECK_RESTARTABLE_IP		BIT_ULL(28)
+#define CHECK_OVERFLOW			BIT_ULL(29)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -52,11 +71,81 @@ static enum err_types cper_get_err_type(const guid_t *err_type)
 		return N_ERR_TYPES;
 }
 
+static const char * const ia_check_trans_type_strs[] = {
+	"Instruction",
+	"Data Access",
+	"Generic",
+};
+
+static const char * const ia_check_op_strs[] = {
+	"generic error",
+	"generic read",
+	"generic write",
+	"data read",
+	"data write",
+	"instruction fetch",
+	"prefetch",
+	"eviction",
+	"snoop",
+};
+
+static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
+{
+	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
+}
+
+static void print_err_info(const char *pfx, u8 err_type, u64 check)
+{
+	u16 validation_bits = CHECK_VALID_BITS(check);
+
+	if (err_type == ERR_TYPE_MS)
+		return;
+
+	if (validation_bits & CHECK_VALID_TRANS_TYPE) {
+		u8 trans_type = CHECK_TRANS_TYPE(check);
+
+		printk("%sTransaction Type: %u, %s\n", pfx, trans_type,
+		       trans_type < ARRAY_SIZE(ia_check_trans_type_strs) ?
+		       ia_check_trans_type_strs[trans_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_OPERATION) {
+		u8 op = CHECK_OPERATION(check);
+
+		/*
+		 * CACHE has more operation types than TLB or BUS, though the
+		 * name and the order are the same.
+		 */
+		u8 max_ops = (err_type == ERR_TYPE_CACHE) ? 9 : 7;
+
+		printk("%sOperation: %u, %s\n", pfx, op,
+		       op < max_ops ? ia_check_op_strs[op] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_LEVEL)
+		printk("%sLevel: %llu\n", pfx, CHECK_LEVEL(check));
+
+	if (validation_bits & CHECK_VALID_PCC)
+		print_bool("Processor Context Corrupt", pfx, check, CHECK_PCC);
+
+	if (validation_bits & CHECK_VALID_UNCORRECTED)
+		print_bool("Uncorrected", pfx, check, CHECK_UNCORRECTED);
+
+	if (validation_bits & CHECK_VALID_PRECISE_IP)
+		print_bool("Precise IP", pfx, check, CHECK_PRECISE_IP);
+
+	if (validation_bits & CHECK_VALID_RESTARTABLE_IP)
+		print_bool("Restartable IP", pfx, check, CHECK_RESTARTABLE_IP);
+
+	if (validation_bits & CHECK_VALID_OVERFLOW)
+		print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+}
+
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
-	char newpfx[64];
+	char newpfx[64], infopfx[64];
 	u8 err_type;
 
 	if (proc->validation_bits & VALID_LAPIC_ID)
@@ -87,6 +176,14 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 		if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
 			printk("%sCheck Information: 0x%016llx\n", newpfx,
 			       err_info->check_info);
+
+			if (err_type < N_ERR_TYPES) {
+				snprintf(infopfx, sizeof(infopfx), "%s ",
+					 newpfx);
+
+				print_err_info(infopfx, err_type,
+					       err_info->check_info);
+			}
 		}
 
 		if (err_info->validation_bits & INFO_VALID_TARGET_ID) {

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

* [tip:efi/core] efi: Decode additional IA32/X64 Bus Check fields
  2018-05-04  5:59 ` [PATCH 08/17] efi: Decode additional IA32/X64 Bus Check fields Ard Biesheuvel
@ 2018-05-14  7:47   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ard.biesheuvel, hpa, linux-kernel, tglx, peterz, mingo, torvalds,
	yazen.ghannam, matt

Commit-ID:  c6bc4ac0aadede7a5c5260bcc315cd2b18c6b471
Gitweb:     https://git.kernel.org/tip/c6bc4ac0aadede7a5c5260bcc315cd2b18c6b471
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:54 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi: Decode additional IA32/X64 Bus Check fields

The "Participation Type", "Time Out", and "Address Space" fields are
unique to the IA32/X64 Bus Check structure. Print these fields.

Based on UEFI 2.7 Table 256. IA32/X64 Bus Check Structure

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-9-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 44 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index f70c46f7a4db..5e6716564dba 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -39,6 +39,10 @@
 #define CHECK_VALID_RESTARTABLE_IP	BIT_ULL(6)
 #define CHECK_VALID_OVERFLOW		BIT_ULL(7)
 
+#define CHECK_VALID_BUS_PART_TYPE	BIT_ULL(8)
+#define CHECK_VALID_BUS_TIME_OUT	BIT_ULL(9)
+#define CHECK_VALID_BUS_ADDR_SPACE	BIT_ULL(10)
+
 #define CHECK_VALID_BITS(check)		(((check) & GENMASK_ULL(15, 0)))
 #define CHECK_TRANS_TYPE(check)		(((check) & GENMASK_ULL(17, 16)) >> 16)
 #define CHECK_OPERATION(check)		(((check) & GENMASK_ULL(21, 18)) >> 18)
@@ -49,6 +53,10 @@
 #define CHECK_RESTARTABLE_IP		BIT_ULL(28)
 #define CHECK_OVERFLOW			BIT_ULL(29)
 
+#define CHECK_BUS_PART_TYPE(check)	(((check) & GENMASK_ULL(31, 30)) >> 30)
+#define CHECK_BUS_TIME_OUT		BIT_ULL(32)
+#define CHECK_BUS_ADDR_SPACE(check)	(((check) & GENMASK_ULL(34, 33)) >> 33)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -89,6 +97,20 @@ static const char * const ia_check_op_strs[] = {
 	"snoop",
 };
 
+static const char * const ia_check_bus_part_type_strs[] = {
+	"Local Processor originated request",
+	"Local Processor responded to request",
+	"Local Processor observed",
+	"Generic",
+};
+
+static const char * const ia_check_bus_addr_space_strs[] = {
+	"Memory Access",
+	"Reserved",
+	"I/O",
+	"Other Transaction",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -139,6 +161,28 @@ static void print_err_info(const char *pfx, u8 err_type, u64 check)
 
 	if (validation_bits & CHECK_VALID_OVERFLOW)
 		print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+
+	if (err_type != ERR_TYPE_BUS)
+		return;
+
+	if (validation_bits & CHECK_VALID_BUS_PART_TYPE) {
+		u8 part_type = CHECK_BUS_PART_TYPE(check);
+
+		printk("%sParticipation Type: %u, %s\n", pfx, part_type,
+		       part_type < ARRAY_SIZE(ia_check_bus_part_type_strs) ?
+		       ia_check_bus_part_type_strs[part_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_BUS_TIME_OUT)
+		print_bool("Time Out", pfx, check, CHECK_BUS_TIME_OUT);
+
+	if (validation_bits & CHECK_VALID_BUS_ADDR_SPACE) {
+		u8 addr_space = CHECK_BUS_ADDR_SPACE(check);
+
+		printk("%sAddress Space: %u, %s\n", pfx, addr_space,
+		       addr_space < ARRAY_SIZE(ia_check_bus_addr_space_strs) ?
+		       ia_check_bus_addr_space_strs[addr_space] : "unknown");
+	}
 }
 
 void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)

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

* [tip:efi/core] efi: Decode IA32/X64 MS Check structure
  2018-05-04  5:59 ` [PATCH 09/17] efi: Decode IA32/X64 MS Check structure Ard Biesheuvel
@ 2018-05-14  7:47   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: yazen.ghannam, mingo, peterz, matt, hpa, linux-kernel,
	ard.biesheuvel, tglx, torvalds

Commit-ID:  a32bc29ed19776ef6827d6336847de9a0b7a8dc5
Gitweb:     https://git.kernel.org/tip/a32bc29ed19776ef6827d6336847de9a0b7a8dc5
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:55 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi: Decode IA32/X64 MS Check structure

The IA32/X64 MS Check structure varies from the other Check structures
in the the bit positions of its fields, and it includes an additional
"Error Type" field.

Decode the MS Check structure in a separate function.

Based on UEFI 2.7 Table 257. IA32/X64 MS Check Field Description.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-10-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 55 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 5e6716564dba..356b8d326219 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -57,6 +57,20 @@
 #define CHECK_BUS_TIME_OUT		BIT_ULL(32)
 #define CHECK_BUS_ADDR_SPACE(check)	(((check) & GENMASK_ULL(34, 33)) >> 33)
 
+#define CHECK_VALID_MS_ERR_TYPE		BIT_ULL(0)
+#define CHECK_VALID_MS_PCC		BIT_ULL(1)
+#define CHECK_VALID_MS_UNCORRECTED	BIT_ULL(2)
+#define CHECK_VALID_MS_PRECISE_IP	BIT_ULL(3)
+#define CHECK_VALID_MS_RESTARTABLE_IP	BIT_ULL(4)
+#define CHECK_VALID_MS_OVERFLOW		BIT_ULL(5)
+
+#define CHECK_MS_ERR_TYPE(check)	(((check) & GENMASK_ULL(18, 16)) >> 16)
+#define CHECK_MS_PCC			BIT_ULL(19)
+#define CHECK_MS_UNCORRECTED		BIT_ULL(20)
+#define CHECK_MS_PRECISE_IP		BIT_ULL(21)
+#define CHECK_MS_RESTARTABLE_IP		BIT_ULL(22)
+#define CHECK_MS_OVERFLOW		BIT_ULL(23)
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -111,17 +125,56 @@ static const char * const ia_check_bus_addr_space_strs[] = {
 	"Other Transaction",
 };
 
+static const char * const ia_check_ms_error_type_strs[] = {
+	"No Error",
+	"Unclassified",
+	"Microcode ROM Parity Error",
+	"External Error",
+	"FRC Error",
+	"Internal Unclassified",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
 }
 
+static void print_err_info_ms(const char *pfx, u16 validation_bits, u64 check)
+{
+	if (validation_bits & CHECK_VALID_MS_ERR_TYPE) {
+		u8 err_type = CHECK_MS_ERR_TYPE(check);
+
+		printk("%sError Type: %u, %s\n", pfx, err_type,
+		       err_type < ARRAY_SIZE(ia_check_ms_error_type_strs) ?
+		       ia_check_ms_error_type_strs[err_type] : "unknown");
+	}
+
+	if (validation_bits & CHECK_VALID_MS_PCC)
+		print_bool("Processor Context Corrupt", pfx, check, CHECK_MS_PCC);
+
+	if (validation_bits & CHECK_VALID_MS_UNCORRECTED)
+		print_bool("Uncorrected", pfx, check, CHECK_MS_UNCORRECTED);
+
+	if (validation_bits & CHECK_VALID_MS_PRECISE_IP)
+		print_bool("Precise IP", pfx, check, CHECK_MS_PRECISE_IP);
+
+	if (validation_bits & CHECK_VALID_MS_RESTARTABLE_IP)
+		print_bool("Restartable IP", pfx, check, CHECK_MS_RESTARTABLE_IP);
+
+	if (validation_bits & CHECK_VALID_MS_OVERFLOW)
+		print_bool("Overflow", pfx, check, CHECK_MS_OVERFLOW);
+}
+
 static void print_err_info(const char *pfx, u8 err_type, u64 check)
 {
 	u16 validation_bits = CHECK_VALID_BITS(check);
 
+	/*
+	 * The MS Check structure varies a lot from the others, so use a
+	 * separate function for decoding.
+	 */
 	if (err_type == ERR_TYPE_MS)
-		return;
+		return print_err_info_ms(pfx, validation_bits, check);
 
 	if (validation_bits & CHECK_VALID_TRANS_TYPE) {
 		u8 trans_type = CHECK_TRANS_TYPE(check);

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

* [tip:efi/core] efi: Decode IA32/X64 Context Info structure
  2018-05-04  5:59 ` [PATCH 10/17] efi: Decode IA32/X64 Context Info structure Ard Biesheuvel
@ 2018-05-14  7:48   ` tip-bot for Yazen Ghannam
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Yazen Ghannam @ 2018-05-14  7:48 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, matt, yazen.ghannam, linux-kernel, torvalds, tglx, hpa,
	ard.biesheuvel, mingo

Commit-ID:  9c178663cbf2e754be322505078306b4a380a697
Gitweb:     https://git.kernel.org/tip/9c178663cbf2e754be322505078306b4a380a697
Author:     Yazen Ghannam <yazen.ghannam@amd.com>
AuthorDate: Fri, 4 May 2018 07:59:56 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi: Decode IA32/X64 Context Info structure

Print the fields of the IA32/X64 Context Information structure.

Print the "Register Array" as raw values. Some context types are defined
in the UEFI spec, so more detailed decoded may be added in the future.

Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
Information Structure.

Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-11-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/cper-x86.c | 48 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 356b8d326219..2531de49f56c 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -10,6 +10,7 @@
 #define VALID_LAPIC_ID			BIT_ULL(0)
 #define VALID_CPUID_INFO		BIT_ULL(1)
 #define VALID_PROC_ERR_INFO_NUM(bits)	(((bits) & GENMASK_ULL(7, 2)) >> 2)
+#define VALID_PROC_CXT_INFO_NUM(bits)	(((bits) & GENMASK_ULL(13, 8)) >> 8)
 
 #define INFO_ERR_STRUCT_TYPE_CACHE					\
 	GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B,	\
@@ -71,6 +72,9 @@
 #define CHECK_MS_RESTARTABLE_IP		BIT_ULL(22)
 #define CHECK_MS_OVERFLOW		BIT_ULL(23)
 
+#define CTX_TYPE_MSR			1
+#define CTX_TYPE_MMREG			7
+
 enum err_types {
 	ERR_TYPE_CACHE = 0,
 	ERR_TYPE_TLB,
@@ -134,6 +138,17 @@ static const char * const ia_check_ms_error_type_strs[] = {
 	"Internal Unclassified",
 };
 
+static const char * const ia_reg_ctx_strs[] = {
+	"Unclassified Data",
+	"MSR Registers (Machine Check and other MSRs)",
+	"32-bit Mode Execution Context",
+	"64-bit Mode Execution Context",
+	"FXSAVE Context",
+	"32-bit Mode Debug Registers (DR0-DR7)",
+	"64-bit Mode Debug Registers (DR0-DR7)",
+	"Memory Mapped Registers",
+};
+
 static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
 {
 	printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -242,6 +257,7 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 {
 	int i;
 	struct cper_ia_err_info *err_info;
+	struct cper_ia_proc_ctx *ctx_info;
 	char newpfx[64], infopfx[64];
 	u8 err_type;
 
@@ -305,4 +321,36 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
 
 		err_info++;
 	}
+
+	ctx_info = (struct cper_ia_proc_ctx *)err_info;
+	for (i = 0; i < VALID_PROC_CXT_INFO_NUM(proc->validation_bits); i++) {
+		int size = sizeof(*ctx_info) + ctx_info->reg_arr_size;
+		int groupsize = 4;
+
+		printk("%sContext Information Structure %d:\n", pfx, i);
+
+		printk("%sRegister Context Type: %s\n", newpfx,
+		       ctx_info->reg_ctx_type < ARRAY_SIZE(ia_reg_ctx_strs) ?
+		       ia_reg_ctx_strs[ctx_info->reg_ctx_type] : "unknown");
+
+		printk("%sRegister Array Size: 0x%04x\n", newpfx,
+		       ctx_info->reg_arr_size);
+
+		if (ctx_info->reg_ctx_type == CTX_TYPE_MSR) {
+			groupsize = 8; /* MSRs are 8 bytes wide. */
+			printk("%sMSR Address: 0x%08x\n", newpfx,
+			       ctx_info->msr_addr);
+		}
+
+		if (ctx_info->reg_ctx_type == CTX_TYPE_MMREG) {
+			printk("%sMM Register Address: 0x%016llx\n", newpfx,
+			       ctx_info->mm_reg_addr);
+		}
+
+		printk("%sRegister Array:\n", newpfx);
+		print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, groupsize,
+			       (ctx_info + 1), ctx_info->reg_arr_size, 0);
+
+		ctx_info = (struct cper_ia_proc_ctx *)((long)ctx_info + size);
+	}
 }

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

* [tip:efi/core] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static
  2018-05-04  5:59 ` [PATCH 11/17] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static Ard Biesheuvel
@ 2018-05-14  7:48   ` tip-bot for Wei Yongjun
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Wei Yongjun @ 2018-05-14  7:48 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, mingo, weiyongjun1, linux-kernel, torvalds, tglx,
	jarkko.sakkinen, matt, peterz, ard.biesheuvel

Commit-ID:  0add16c13f49bda5455d9418d479d6c89f7ab272
Gitweb:     https://git.kernel.org/tip/0add16c13f49bda5455d9418d479d6c89f7ab272
Author:     Wei Yongjun <weiyongjun1@huawei.com>
AuthorDate: Fri, 4 May 2018 07:59:57 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static

Fixes the following sparse warning:

drivers/firmware/efi/libstub/tpm.c:62:6: warning:
 symbol 'efi_retrieve_tpm2_eventlog_1_2' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-12-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/libstub/tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index 9d08cea3f1b0..caa37a6dd9d4 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -59,7 +59,7 @@ void efi_enable_reset_attack_mitigation(efi_system_table_t *sys_table_arg)
 
 #endif
 
-void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
+static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
 {
 	efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
 	efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;

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

* [tip:efi/core] efi: Align efi_pci_io_protocol typedefs to type naming convention
  2018-05-04  5:59 ` [PATCH 13/17] efi: align efi_pci_io_protocol typedefs to type naming convention Ard Biesheuvel
@ 2018-05-14  7:49   ` tip-bot for Ard Biesheuvel
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Ard Biesheuvel @ 2018-05-14  7:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: ard.biesheuvel, torvalds, linux-kernel, tglx, hpa, matt,
	hdegoede, mingo, peterz

Commit-ID:  cb0ba793525788e40e7a9ee82de8f3b017ca4459
Gitweb:     https://git.kernel.org/tip/cb0ba793525788e40e7a9ee82de8f3b017ca4459
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Fri, 4 May 2018 07:59:59 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:48 +0200

efi: Align efi_pci_io_protocol typedefs to type naming convention

In order to use the helper macros that perform type mangling with the
EFI PCI I/O protocol struct typedefs, align their Linux typenames with
the convention we use for definitionns that originate in the UEFI spec,
and add the trailing _t to each.

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-14-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 8 ++++----
 include/linux/efi.h              | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 09f36c0d9d4f..3994f48c4043 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -109,7 +109,7 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
 }
 
 static efi_status_t
-__setup_efi_pci32(efi_pci_io_protocol_32 *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom = NULL;
 	efi_status_t status;
@@ -176,7 +176,7 @@ static void
 setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_32 *pci = NULL;
+	efi_pci_io_protocol_32_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u32 *handles = (u32 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -218,7 +218,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 }
 
 static efi_status_t
-__setup_efi_pci64(efi_pci_io_protocol_64 *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom;
 	efi_status_t status;
@@ -284,7 +284,7 @@ static void
 setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_64 *pci = NULL;
+	efi_pci_io_protocol_64_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u64 *handles = (u64 *)(unsigned long)pci_handle;
 	efi_status_t status;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3016d8c456bc..56add823f190 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -397,7 +397,7 @@ typedef struct {
 	u32 set_bar_attributes;
 	u64 romsize;
 	u32 romimage;
-} efi_pci_io_protocol_32;
+} efi_pci_io_protocol_32_t;
 
 typedef struct {
 	u64 poll_mem;
@@ -417,7 +417,7 @@ typedef struct {
 	u64 set_bar_attributes;
 	u64 romsize;
 	u64 romimage;
-} efi_pci_io_protocol_64;
+} efi_pci_io_protocol_64_t;
 
 typedef struct {
 	void *poll_mem;
@@ -437,7 +437,7 @@ typedef struct {
 	void *set_bar_attributes;
 	uint64_t romsize;
 	void *romimage;
-} efi_pci_io_protocol;
+} efi_pci_io_protocol_t;
 
 #define EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO 0x0001
 #define EFI_PCI_IO_ATTRIBUTE_ISA_IO 0x0002

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

* [tip:efi/core] efi/x86: Fold __setup_efi_pci32() and __setup_efi_pci64() into one function
  2018-05-04  6:00 ` [PATCH 14/17] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one Ard Biesheuvel
@ 2018-05-14  7:49   ` tip-bot for Ard Biesheuvel
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Ard Biesheuvel @ 2018-05-14  7:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hdegoede, linux-kernel, tglx, mingo, torvalds, peterz, hpa,
	ard.biesheuvel, lukas, matt

Commit-ID:  2c3625cb9fa2c477d5877a1819e29d6a902e5fef
Gitweb:     https://git.kernel.org/tip/2c3625cb9fa2c477d5877a1819e29d6a902e5fef
Author:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
AuthorDate: Fri, 4 May 2018 08:00:00 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:49 +0200

efi/x86: Fold __setup_efi_pci32() and __setup_efi_pci64() into one function

As suggested by Lukas, use his efi_call_proto() and efi_table_attr()
macros to merge __setup_efi_pci32() and __setup_efi_pci64() into a
single function, removing the need to duplicate changes made in
subsequent patches across both.

Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-15-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 107 +++++++++------------------------------
 1 file changed, 25 insertions(+), 82 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 3994f48c4043..dadf32312082 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -109,23 +109,27 @@ void efi_char16_printk(efi_system_table_t *table, efi_char16_t *str)
 }
 
 static efi_status_t
-__setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
+__setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 {
 	struct pci_setup_rom *rom = NULL;
 	efi_status_t status;
 	unsigned long size;
-	uint64_t attributes;
+	uint64_t attributes, romsize;
+	void *romimage;
 
-	status = efi_early->call(pci->attributes, pci,
-				 EfiPciIoAttributeOperationGet, 0, 0,
-				 &attributes);
+	status = efi_call_proto(efi_pci_io_protocol, attributes, pci,
+				EfiPciIoAttributeOperationGet, 0, 0,
+				&attributes);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	if (!pci->romimage || !pci->romsize)
+	romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol,
+							 romimage, pci);
+	romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci);
+	if (!romimage || !romsize)
 		return EFI_INVALID_PARAMETER;
 
-	size = pci->romsize + sizeof(*rom);
+	size = romsize + sizeof(*rom);
 
 	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
 	if (status != EFI_SUCCESS) {
@@ -141,30 +145,32 @@ __setup_efi_pci32(efi_pci_io_protocol_32_t *pci, struct pci_setup_rom **__rom)
 	rom->pcilen = pci->romsize;
 	*__rom = rom;
 
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_VENDOR_ID, 1, &(rom->vendor));
+	status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
+				EfiPciIoWidthUint16, PCI_VENDOR_ID, 1,
+				&rom->vendor);
 
 	if (status != EFI_SUCCESS) {
 		efi_printk(sys_table, "Failed to read rom->vendor\n");
 		goto free_struct;
 	}
 
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_DEVICE_ID, 1, &(rom->devid));
+	status = efi_call_proto(efi_pci_io_protocol, pci.read, pci,
+				EfiPciIoWidthUint16, PCI_DEVICE_ID, 1,
+				&rom->devid);
 
 	if (status != EFI_SUCCESS) {
 		efi_printk(sys_table, "Failed to read rom->devid\n");
 		goto free_struct;
 	}
 
-	status = efi_early->call(pci->get_location, pci, &(rom->segment),
-				 &(rom->bus), &(rom->device), &(rom->function));
+	status = efi_call_proto(efi_pci_io_protocol, get_location, pci,
+				&rom->segment, &rom->bus, &rom->device,
+				&rom->function);
 
 	if (status != EFI_SUCCESS)
 		goto free_struct;
 
-	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
-	       pci->romsize);
+	memcpy(rom->romdata, romimage, romsize);
 	return status;
 
 free_struct:
@@ -176,7 +182,7 @@ static void
 setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_32_t *pci = NULL;
+	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u32 *handles = (u32 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -203,7 +209,7 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 		if (!pci)
 			continue;
 
-		status = __setup_efi_pci32(pci, &rom);
+		status = __setup_efi_pci(pci, &rom);
 		if (status != EFI_SUCCESS)
 			continue;
 
@@ -217,74 +223,11 @@ setup_efi_pci32(struct boot_params *params, void **pci_handle,
 	}
 }
 
-static efi_status_t
-__setup_efi_pci64(efi_pci_io_protocol_64_t *pci, struct pci_setup_rom **__rom)
-{
-	struct pci_setup_rom *rom;
-	efi_status_t status;
-	unsigned long size;
-	uint64_t attributes;
-
-	status = efi_early->call(pci->attributes, pci,
-				 EfiPciIoAttributeOperationGet, 0,
-				 &attributes);
-	if (status != EFI_SUCCESS)
-		return status;
-
-	if (!pci->romimage || !pci->romsize)
-		return EFI_INVALID_PARAMETER;
-
-	size = pci->romsize + sizeof(*rom);
-
-	status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to alloc mem for rom\n");
-		return status;
-	}
-
-	rom->data.type = SETUP_PCI;
-	rom->data.len = size - sizeof(struct setup_data);
-	rom->data.next = 0;
-	rom->pcilen = pci->romsize;
-	*__rom = rom;
-
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_VENDOR_ID, 1, &(rom->vendor));
-
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to read rom->vendor\n");
-		goto free_struct;
-	}
-
-	status = efi_early->call(pci->pci.read, pci, EfiPciIoWidthUint16,
-				 PCI_DEVICE_ID, 1, &(rom->devid));
-
-	if (status != EFI_SUCCESS) {
-		efi_printk(sys_table, "Failed to read rom->devid\n");
-		goto free_struct;
-	}
-
-	status = efi_early->call(pci->get_location, pci, &(rom->segment),
-				 &(rom->bus), &(rom->device), &(rom->function));
-
-	if (status != EFI_SUCCESS)
-		goto free_struct;
-
-	memcpy(rom->romdata, (void *)(unsigned long)pci->romimage,
-	       pci->romsize);
-	return status;
-
-free_struct:
-	efi_call_early(free_pool, rom);
-	return status;
-
-}
-
 static void
 setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		unsigned long size)
 {
-	efi_pci_io_protocol_64_t *pci = NULL;
+	efi_pci_io_protocol_t *pci = NULL;
 	efi_guid_t pci_proto = EFI_PCI_IO_PROTOCOL_GUID;
 	u64 *handles = (u64 *)(unsigned long)pci_handle;
 	efi_status_t status;
@@ -311,7 +254,7 @@ setup_efi_pci64(struct boot_params *params, void **pci_handle,
 		if (!pci)
 			continue;
 
-		status = __setup_efi_pci64(pci, &rom);
+		status = __setup_efi_pci(pci, &rom);
 		if (status != EFI_SUCCESS)
 			continue;
 

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

* [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs
  2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
  2018-05-14  6:40   ` Ingo Molnar
  2018-05-14  6:43   ` [PATCH] efi/x86: Clean up the eboot code a bit Ingo Molnar
@ 2018-05-14  7:50   ` tip-bot for Hans de Goede
  2018-05-15  9:18     ` Ard Biesheuvel
  2 siblings, 1 reply; 47+ messages in thread
From: tip-bot for Hans de Goede @ 2018-05-14  7:50 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: matt, torvalds, hdegoede, peterz, mingo, tglx, hpa, linux-kernel,
	ard.biesheuvel

Commit-ID:  1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
Gitweb:     https://git.kernel.org/tip/1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
Author:     Hans de Goede <hdegoede@redhat.com>
AuthorDate: Fri, 4 May 2018 08:00:01 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:49 +0200

efi/x86: Ignore unrealistically large option ROMs

setup_efi_pci() tries to save a copy of each PCI option ROM as this may
be necessary for the device driver for the PCI device to have access too.

On some systems the efi_pci_io_protocol's romimage and romsize fields
contain invalid data, which looks a bit like pointers pointing back into
other EFI code or data. Interpreting these pointers as romsize leads to
a very large value and if we then try to alloc this amount of memory to
save a copy the alloc call fails.

This leads to a "Failed to alloc mem for rom" error being printed on the
EFI console for each PCI device.

This commit avoids the printing of these errors, by checking romsize before
doing the alloc and if it is larger then the EFI spec limit of 16 MiB
silently ignore the ROM fields instead of trying to alloc mem and fail.

Tested-by: Hans de Goede <hdegoede@redhat.com>
[ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-16-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index dadf32312082..a8a8642d2b0b 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -123,10 +123,17 @@ __setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
 	if (status != EFI_SUCCESS)
 		return status;
 
+	/*
+	 * Some firmware images contain EFI function pointers at the place where the
+	 * romimage and romsize fields are supposed to be. Typically the EFI
+	 * code is mapped at high addresses, translating to an unrealistically
+	 * large romsize. The UEFI spec limits the size of option ROMs to 16
+	 * MiB so we reject any ROMs over 16 MiB in size to catch this.
+	 */
 	romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol,
 							 romimage, pci);
 	romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci);
-	if (!romimage || !romsize)
+	if (!romimage || !romsize || romsize > SZ_16M)
 		return EFI_INVALID_PARAMETER;
 
 	size = romsize + sizeof(*rom);

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

* [tip:efi/core] efi/capsule-loader: Don't output reset log when reset flags are not set
  2018-05-04  6:00 ` [PATCH 16/17] efi/capsule-loader: Don't output reset log when reset flags are not set Ard Biesheuvel
@ 2018-05-14  7:50   ` tip-bot for Shunyong Yang
  0 siblings, 0 replies; 47+ messages in thread
From: tip-bot for Shunyong Yang @ 2018-05-14  7:50 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: yu.zheng, peterz, shunyong.yang, hpa, mingo, linux-kernel, matt,
	tglx, torvalds, ard.biesheuvel

Commit-ID:  83f0a7c7b265a56d757f7e3a80622e5b0b7ebc46
Gitweb:     https://git.kernel.org/tip/83f0a7c7b265a56d757f7e3a80622e5b0b7ebc46
Author:     Shunyong Yang <shunyong.yang@hxt-semitech.com>
AuthorDate: Fri, 4 May 2018 08:00:02 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:49 +0200

efi/capsule-loader: Don't output reset log when reset flags are not set

When reset flags in capsule header are not set, it means firmware
attempts to immediately process or launch the capsule. Moreover, reset
is not needed in this case. The current code will output log to indicate
reset.

This patch adds a branch to avoid reset log output when the flags are not
set.

[ardb: use braces in multi-line 'if', clarify comment and commit log]
Signed-off-by: Shunyong Yang <shunyong.yang@hxt-semitech.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Joey Zheng <yu.zheng@hxt-semitech.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-17-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/capsule-loader.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index e456f4602df1..96688986da56 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -134,10 +134,16 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info)
 
 	/* Indicate capsule binary uploading is done */
 	cap_info->index = NO_FURTHER_WRITE_ACTION;
-	pr_info("Successfully upload capsule file with reboot type '%s'\n",
-		!cap_info->reset_type ? "RESET_COLD" :
-		cap_info->reset_type == 1 ? "RESET_WARM" :
-		"RESET_SHUTDOWN");
+
+	if (cap_info->header.flags & EFI_CAPSULE_PERSIST_ACROSS_RESET) {
+		pr_info("Successfully uploaded capsule file with reboot type '%s'\n",
+			!cap_info->reset_type ? "RESET_COLD" :
+			cap_info->reset_type == 1 ? "RESET_WARM" :
+			"RESET_SHUTDOWN");
+	} else {
+		pr_info("Successfully processed capsule file\n");
+	}
+
 	return 0;
 }
 

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

* Re: [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs
  2018-05-14  7:50   ` [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs tip-bot for Hans de Goede
@ 2018-05-15  9:18     ` Ard Biesheuvel
  2018-06-21 15:13       ` Ingo Molnar
  0 siblings, 1 reply; 47+ messages in thread
From: Ard Biesheuvel @ 2018-05-15  9:18 UTC (permalink / raw)
  To: Linux Kernel Mailing List, Ard Biesheuvel, H. Peter Anvin,
	Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Hans de Goede,
	Linus Torvalds, Matt Fleming
  Cc: linux-tip-commits

On 14 May 2018 at 09:50, tip-bot for Hans de Goede <tipbot@zytor.com> wrote:
> Commit-ID:  1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> Gitweb:     https://git.kernel.org/tip/1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> Author:     Hans de Goede <hdegoede@redhat.com>
> AuthorDate: Fri, 4 May 2018 08:00:01 +0200
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Mon, 14 May 2018 08:57:49 +0200
>
> efi/x86: Ignore unrealistically large option ROMs
>
> setup_efi_pci() tries to save a copy of each PCI option ROM as this may
> be necessary for the device driver for the PCI device to have access too.
>
> On some systems the efi_pci_io_protocol's romimage and romsize fields
> contain invalid data, which looks a bit like pointers pointing back into
> other EFI code or data. Interpreting these pointers as romsize leads to
> a very large value and if we then try to alloc this amount of memory to
> save a copy the alloc call fails.
>
> This leads to a "Failed to alloc mem for rom" error being printed on the
> EFI console for each PCI device.
>
> This commit avoids the printing of these errors, by checking romsize before
> doing the alloc and if it is larger then the EFI spec limit of 16 MiB
> silently ignore the ROM fields instead of trying to alloc mem and fail.
>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

This looks odd now: I sent this out as

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
Tested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

which clearly conveys that Hans tested the updated version of the patch.

In general, I don't think there is a need to reorder signoffs unless
there is anything wrong with them, no?

> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-efi@vger.kernel.org
> Link: http://lkml.kernel.org/r/20180504060003.19618-16-ard.biesheuvel@linaro.org
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/boot/compressed/eboot.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index dadf32312082..a8a8642d2b0b 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -123,10 +123,17 @@ __setup_efi_pci(efi_pci_io_protocol_t *pci, struct pci_setup_rom **__rom)
>         if (status != EFI_SUCCESS)
>                 return status;
>
> +       /*
> +        * Some firmware images contain EFI function pointers at the place where the
> +        * romimage and romsize fields are supposed to be. Typically the EFI
> +        * code is mapped at high addresses, translating to an unrealistically
> +        * large romsize. The UEFI spec limits the size of option ROMs to 16
> +        * MiB so we reject any ROMs over 16 MiB in size to catch this.
> +        */
>         romimage = (void *)(unsigned long)efi_table_attr(efi_pci_io_protocol,
>                                                          romimage, pci);
>         romsize = efi_table_attr(efi_pci_io_protocol, romsize, pci);
> -       if (!romimage || !romsize)
> +       if (!romimage || !romsize || romsize > SZ_16M)
>                 return EFI_INVALID_PARAMETER;
>
>         size = romsize + sizeof(*rom);

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

* Re: [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs
  2018-05-15  9:18     ` Ard Biesheuvel
@ 2018-06-21 15:13       ` Ingo Molnar
  0 siblings, 0 replies; 47+ messages in thread
From: Ingo Molnar @ 2018-06-21 15:13 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Linux Kernel Mailing List, H. Peter Anvin, Thomas Gleixner,
	Peter Zijlstra, Hans de Goede, Linus Torvalds, Matt Fleming,
	linux-tip-commits


* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 14 May 2018 at 09:50, tip-bot for Hans de Goede <tipbot@zytor.com> wrote:
> > Commit-ID:  1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> > Gitweb:     https://git.kernel.org/tip/1de3a1be8a9345fd0c7d9bb1009b21afe6b6b10f
> > Author:     Hans de Goede <hdegoede@redhat.com>
> > AuthorDate: Fri, 4 May 2018 08:00:01 +0200
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Mon, 14 May 2018 08:57:49 +0200
> >
> > efi/x86: Ignore unrealistically large option ROMs
> >
> > setup_efi_pci() tries to save a copy of each PCI option ROM as this may
> > be necessary for the device driver for the PCI device to have access too.
> >
> > On some systems the efi_pci_io_protocol's romimage and romsize fields
> > contain invalid data, which looks a bit like pointers pointing back into
> > other EFI code or data. Interpreting these pointers as romsize leads to
> > a very large value and if we then try to alloc this amount of memory to
> > save a copy the alloc call fails.
> >
> > This leads to a "Failed to alloc mem for rom" error being printed on the
> > EFI console for each PCI device.
> >
> > This commit avoids the printing of these errors, by checking romsize before
> > doing the alloc and if it is larger then the EFI spec limit of 16 MiB
> > silently ignore the ROM fields instead of trying to alloc mem and fail.
> >
> > Tested-by: Hans de Goede <hdegoede@redhat.com>
> > [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> This looks odd now: I sent this out as
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> [ardb: deduplicate 32/64 bit changes, use SZ_16M symbolic constant]
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> which clearly conveys that Hans tested the updated version of the patch.
> 
> In general, I don't think there is a need to reorder signoffs unless
> there is anything wrong with them, no?

Indeed - this is a script bug that I failed to notice.

Thanks,

	Ingo

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

end of thread, other threads:[~2018-06-21 15:14 UTC | newest]

Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04  5:59 [GIT PULL 00/17] EFI updates for v4.18 Ard Biesheuvel
2018-05-04  5:59 ` [PATCH 01/17] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot Ard Biesheuvel
2018-05-14  7:43   ` [tip:efi/core] " tip-bot for Daniel Kiper
2018-05-04  5:59 ` [PATCH 02/17] efi/cper: Remove the INDENT_SP silliness Ard Biesheuvel
2018-05-14  7:44   ` [tip:efi/core] " tip-bot for Borislav Petkov
2018-05-04  5:59 ` [PATCH 03/17] efi: Fix IA32/X64 Processor Error Record definition Ard Biesheuvel
2018-05-14  7:44   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 04/17] efi: Decode IA32/X64 Processor Error Section Ard Biesheuvel
2018-05-14  7:45   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 05/17] efi: Decode IA32/X64 Processor Error Info Structure Ard Biesheuvel
2018-05-14  7:45   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 06/17] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs Ard Biesheuvel
2018-05-14  7:46   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 07/17] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures Ard Biesheuvel
2018-05-14  7:46   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 08/17] efi: Decode additional IA32/X64 Bus Check fields Ard Biesheuvel
2018-05-14  7:47   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 09/17] efi: Decode IA32/X64 MS Check structure Ard Biesheuvel
2018-05-14  7:47   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 10/17] efi: Decode IA32/X64 Context Info structure Ard Biesheuvel
2018-05-14  7:48   ` [tip:efi/core] " tip-bot for Yazen Ghannam
2018-05-04  5:59 ` [PATCH 11/17] efi/libstub/tpm: Make function efi_retrieve_tpm2_eventlog_1_2() static Ard Biesheuvel
2018-05-14  7:48   ` [tip:efi/core] " tip-bot for Wei Yongjun
2018-05-04  5:59 ` [PATCH 12/17] efi: fix efi_pci_io_protocol32 prototype for mixed mode Ard Biesheuvel
2018-05-14  6:57   ` Ingo Molnar
2018-05-14  7:02     ` Ard Biesheuvel
2018-05-14  7:42   ` [tip:efi/core] efi: Avoid potential crashes, fix the 'struct efi_pci_io_protocol_32' definition " tip-bot for Ard Biesheuvel
2018-05-04  5:59 ` [PATCH 13/17] efi: align efi_pci_io_protocol typedefs to type naming convention Ard Biesheuvel
2018-05-14  7:49   ` [tip:efi/core] efi: Align " tip-bot for Ard Biesheuvel
2018-05-04  6:00 ` [PATCH 14/17] efi/x86: fold __setup_efi_pci32 and __setup_efi_pci64 into one Ard Biesheuvel
2018-05-14  7:49   ` [tip:efi/core] efi/x86: Fold __setup_efi_pci32() and __setup_efi_pci64() into one function tip-bot for Ard Biesheuvel
2018-05-04  6:00 ` [PATCH 15/17] efi/x86: Ignore unrealistically large option roms Ard Biesheuvel
2018-05-14  6:40   ` Ingo Molnar
2018-05-14  6:43   ` [PATCH] efi/x86: Clean up the eboot code a bit Ingo Molnar
2018-05-14  6:47     ` Ard Biesheuvel
2018-05-14  6:58       ` Ingo Molnar
2018-05-14  6:59         ` Ard Biesheuvel
2018-05-14  7:50   ` [tip:efi/core] efi/x86: Ignore unrealistically large option ROMs tip-bot for Hans de Goede
2018-05-15  9:18     ` Ard Biesheuvel
2018-06-21 15:13       ` Ingo Molnar
2018-05-04  6:00 ` [PATCH 16/17] efi/capsule-loader: Don't output reset log when reset flags are not set Ard Biesheuvel
2018-05-14  7:50   ` [tip:efi/core] " tip-bot for Shunyong Yang
2018-05-04  6:00 ` [PATCH 17/17] efi/libstub/arm64: handle randomized TEXT_OFFSET Ard Biesheuvel
2018-05-14  6:47   ` Ingo Molnar
2018-05-14  6:48     ` Ard Biesheuvel
2018-05-14  7:00       ` Ingo Molnar
2018-05-14  7:01         ` Ard Biesheuvel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.