linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 0/2] Add efi page fault handler to recover from page
@ 2018-09-10 19:32 Sai Praneeth Prakhya
  2018-09-10 19:32 ` [PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler Sai Praneeth Prakhya
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sai Praneeth Prakhya @ 2018-09-10 19:32 UTC (permalink / raw)
  To: linux-efi, linux-kernel, x86
  Cc: Sai Praneeth, Al Stone, Borislav Petkov, Ingo Molnar,
	Andy Lutomirski, Bhupesh Sharma, Thomas Gleixner, Peter Zijlstra,
	Ard Biesheuvel

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

There may exist some buggy UEFI firmware implementations that access efi
memory regions other than EFI_RUNTIME_SERVICES_<CODE/DATA> even after
the kernel has assumed control of the platform. This violates UEFI
specification. Hence, provide a efi specific page fault handler which
recovers from page faults caused by buggy firmware.

Page faults triggered by firmware happen at ring 0 and if unhandled,
hangs the kernel. So, provide an efi specific page fault handler to:
1. Avoid panics/hangs caused by buggy firmware.
2. Shout loud that the firmware is buggy and hence is not a kernel bug.

The efi page fault handler will check if the access is by
efi_reset_system().
1. If so, then the efi page fault handler will reboot the machine
   through BIOS and not through efi_reset_system().
2. If not, then the efi page fault handler will freeze efi_rts_wq and
   schedules a new process.

This issue was reported by Al Stone when he saw that reboot via EFI hangs
the machine. Upon debugging, I found that it's efi_reset_system() that's
touching memory regions which it shouldn't. To reproduce the same
behavior, I have hacked OVMF and made efi_reset_system() buggy. Along
with efi_reset_system(), I have also modified get_next_high_mono_count()
and set_virtual_address_map(). They illegally access both boot time and
other efi regions.

Testing the patch set:
----------------------
1. Download buggy firmware from here [1].
2. Run a qemu instance with this buggy BIOS and boot mainline kernel.
Add reboot=efi to the kernel command line arguments and after the kernel
is up and running, type "reboot". The kernel should hang while rebooting.
3. With the same setup, boot kernel after applying patches and the
reboot should work fine. Also please notice warning/error messages
printed by kernel.

Changes from RFC to V1:
-----------------------
1. Drop "long jump" technique of dealing with illegal access and instead
   use scheduling away from efi_rts_wq.

Changes from V1 to V2:
----------------------
1. Shortened config name to CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS from
   CONFIG_EFI_WARN_ON_ILLEGAL_ACCESSES.
2. Made the config option available only to expert users.
3. efi_free_boot_services() should be called only when
   CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS is not enabled. Previously, this
   was part of init/main.c file. As it is an architecture agnostic code,
   moved the change to arch/x86/platform/efi/quirks.c file.

Changes from V2 to V3:
----------------------
1. Drop treating illegal access to EFI_BOOT_SERVICES_<CODE/DATA> regions
   separately from illegal accesses to other regions like
   EFI_CONVENTIONAL_MEMORY or EFI_LOADER_<CODE/DATA>.
   In previous versions, illegal access to EFI_BOOT_SERVICES_<CODE/DATA>
   regions were handled by mapping requested region to efi_pgd but from
   V3 they are handled similar to illegal access to other regions i.e by
   freezing efi_rts_wq and scheduling new process.
2. Change __efi_init_fixup attribute to __efi_init.

Changes from V3 to V4:
----------------------
1. Drop saving original memory map passed by kernel. It also means less
   checks in efi page fault handler.
2. Change the config name to EFI_PAGE_FAULT_HANDLER to reflect it's
   functionality more appropriately.

Changes from V4 to V5:
----------------------
1. Drop config option that enables efi page fault handler, instead make
   it default.
2. Call schedule() in an infinite loop to account for spurious wake ups.
3. Introduce "NONE" as an efi runtime service function identifier so that
   it could be used in efi_recover_from_page_fault() to check if the page
   fault was indeed triggered by an efi runtime service.

Note:
-----
Patch set based on "next" branch in efi tree.

[1] https://drive.google.com/drive/folders/1VozKTms92ifyVHAT0ZDQe55ZYL1UE5wt

Sai Praneeth (2):
  efi: Make efi_rts_work accessible to efi page fault handler
  x86/efi: Add efi page fault handler to recover from page faults caused
        by the firmware

 arch/x86/include/asm/efi.h              |  1 +
 arch/x86/mm/fault.c                     |  9 ++++
 arch/x86/platform/efi/quirks.c          | 78 +++++++++++++++++++++++++++++++++
 drivers/firmware/efi/runtime-wrappers.c | 61 +++++++-------------------
 include/linux/efi.h                     | 42 ++++++++++++++++++
 5 files changed, 147 insertions(+), 44 deletions(-)

Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
Suggested-by: Matt Fleming <matt@codeblueprint.co.uk>
Based-on-code-from: Ricardo Neri <ricardo.neri@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Al Stone <astone@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

-- 
2.7.4


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

* [PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler
  2018-09-10 19:32 [PATCH V5 0/2] Add efi page fault handler to recover from page Sai Praneeth Prakhya
@ 2018-09-10 19:32 ` Sai Praneeth Prakhya
  2018-09-10 19:32 ` [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware Sai Praneeth Prakhya
  2018-09-10 19:47 ` [PATCH V5 0/2] Add efi page fault handler to recover from page Prakhya, Sai Praneeth
  2 siblings, 0 replies; 6+ messages in thread
From: Sai Praneeth Prakhya @ 2018-09-10 19:32 UTC (permalink / raw)
  To: linux-efi, linux-kernel, x86
  Cc: Sai Praneeth, Al Stone, Borislav Petkov, Ingo Molnar,
	Andy Lutomirski, Bhupesh Sharma, Thomas Gleixner, Peter Zijlstra,
	Ard Biesheuvel

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

After the kernel has booted, if any accesses by firmware causes a page
fault, the efi page fault handler would freeze efi_rts_wq and schedules
a new process. To do this, the efi page fault handler needs
efi_rts_work. Hence, make it accessible.

There will be no race conditions in accessing this structure, because,
all the calls to efi runtime services are already serialized.

Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
Suggested-by: Matt Fleming <matt@codeblueprint.co.uk>
Based-on-code-from: Ricardo Neri <ricardo.neri@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Al Stone <astone@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 53 ++++++---------------------------
 include/linux/efi.h                     | 36 ++++++++++++++++++++++
 2 files changed, 45 insertions(+), 44 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index aa66cbf23512..b18b2d864c2c 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -45,39 +45,7 @@
 #define __efi_call_virt(f, args...) \
 	__efi_call_virt_pointer(efi.systab->runtime, f, args)
 
-/* efi_runtime_service() function identifiers */
-enum efi_rts_ids {
-	GET_TIME,
-	SET_TIME,
-	GET_WAKEUP_TIME,
-	SET_WAKEUP_TIME,
-	GET_VARIABLE,
-	GET_NEXT_VARIABLE,
-	SET_VARIABLE,
-	QUERY_VARIABLE_INFO,
-	GET_NEXT_HIGH_MONO_COUNT,
-	UPDATE_CAPSULE,
-	QUERY_CAPSULE_CAPS,
-};
-
-/*
- * efi_runtime_work:	Details of EFI Runtime Service work
- * @arg<1-5>:		EFI Runtime Service function arguments
- * @status:		Status of executing EFI Runtime Service
- * @efi_rts_id:		EFI Runtime Service function identifier
- * @efi_rts_comp:	Struct used for handling completions
- */
-struct efi_runtime_work {
-	void *arg1;
-	void *arg2;
-	void *arg3;
-	void *arg4;
-	void *arg5;
-	efi_status_t status;
-	struct work_struct work;
-	enum efi_rts_ids efi_rts_id;
-	struct completion efi_rts_comp;
-};
+struct efi_runtime_work efi_rts_work;
 
 /*
  * efi_queue_work:	Queue efi_runtime_service() and wait until it's done
@@ -91,7 +59,6 @@ struct efi_runtime_work {
  */
 #define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5)		\
 ({									\
-	struct efi_runtime_work efi_rts_work;				\
 	efi_rts_work.status = EFI_ABORTED;				\
 									\
 	init_completion(&efi_rts_work.efi_rts_comp);			\
@@ -184,18 +151,16 @@ static DEFINE_SEMAPHORE(efi_runtime_lock);
  */
 static void efi_call_rts(struct work_struct *work)
 {
-	struct efi_runtime_work *efi_rts_work;
 	void *arg1, *arg2, *arg3, *arg4, *arg5;
 	efi_status_t status = EFI_NOT_FOUND;
 
-	efi_rts_work = container_of(work, struct efi_runtime_work, work);
-	arg1 = efi_rts_work->arg1;
-	arg2 = efi_rts_work->arg2;
-	arg3 = efi_rts_work->arg3;
-	arg4 = efi_rts_work->arg4;
-	arg5 = efi_rts_work->arg5;
+	arg1 = efi_rts_work.arg1;
+	arg2 = efi_rts_work.arg2;
+	arg3 = efi_rts_work.arg3;
+	arg4 = efi_rts_work.arg4;
+	arg5 = efi_rts_work.arg5;
 
-	switch (efi_rts_work->efi_rts_id) {
+	switch (efi_rts_work.efi_rts_id) {
 	case GET_TIME:
 		status = efi_call_virt(get_time, (efi_time_t *)arg1,
 				       (efi_time_cap_t *)arg2);
@@ -253,8 +218,8 @@ static void efi_call_rts(struct work_struct *work)
 		 */
 		pr_err("Requested executing invalid EFI Runtime Service.\n");
 	}
-	efi_rts_work->status = status;
-	complete(&efi_rts_work->efi_rts_comp);
+	efi_rts_work.status = status;
+	complete(&efi_rts_work.efi_rts_comp);
 }
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 401e4b254e30..855992b15269 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1659,7 +1659,43 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
+/* efi_runtime_service() function identifiers */
+enum efi_rts_ids {
+	GET_TIME,
+	SET_TIME,
+	GET_WAKEUP_TIME,
+	SET_WAKEUP_TIME,
+	GET_VARIABLE,
+	GET_NEXT_VARIABLE,
+	SET_VARIABLE,
+	QUERY_VARIABLE_INFO,
+	GET_NEXT_HIGH_MONO_COUNT,
+	UPDATE_CAPSULE,
+	QUERY_CAPSULE_CAPS,
+};
+
+/*
+ * efi_runtime_work:	Details of EFI Runtime Service work
+ * @arg<1-5>:		EFI Runtime Service function arguments
+ * @status:		Status of executing EFI Runtime Service
+ * @efi_rts_id:		EFI Runtime Service function identifier
+ * @efi_rts_comp:	Struct used for handling completions
+ */
+struct efi_runtime_work {
+	void *arg1;
+	void *arg2;
+	void *arg3;
+	void *arg4;
+	void *arg5;
+	efi_status_t status;
+	struct work_struct work;
+	enum efi_rts_ids efi_rts_id;
+	struct completion efi_rts_comp;
+};
+
 /* Workqueue to queue EFI Runtime Services */
 extern struct workqueue_struct *efi_rts_wq;
 
+extern struct efi_runtime_work efi_rts_work;
+
 #endif /* _LINUX_EFI_H */
-- 
2.7.4


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

* [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware
  2018-09-10 19:32 [PATCH V5 0/2] Add efi page fault handler to recover from page Sai Praneeth Prakhya
  2018-09-10 19:32 ` [PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler Sai Praneeth Prakhya
@ 2018-09-10 19:32 ` Sai Praneeth Prakhya
  2018-09-11  8:06   ` kbuild test robot
  2018-09-11  8:19   ` kbuild test robot
  2018-09-10 19:47 ` [PATCH V5 0/2] Add efi page fault handler to recover from page Prakhya, Sai Praneeth
  2 siblings, 2 replies; 6+ messages in thread
From: Sai Praneeth Prakhya @ 2018-09-10 19:32 UTC (permalink / raw)
  To: linux-efi, linux-kernel, x86
  Cc: Sai Praneeth, Al Stone, Borislav Petkov, Ingo Molnar,
	Andy Lutomirski, Bhupesh Sharma, Thomas Gleixner, Peter Zijlstra,
	Ard Biesheuvel

From: Sai Praneeth <sai.praneeth.prakhya@intel.com>

As per the UEFI specification, after the call to ExitBootServices(),
accesses by the firmware to any memory regions except
EFI_RUNTIME_SERVICES_<CODE/DATA> regions is considered illegal. A buggy
firmware could trigger these illegal accesses when an efi runtime
service is invoked and if this happens when the kernel is up and
running, the kernel hangs.

Kernel hangs because the memory region requested by the firmware isn't
mapped in efi_pgd, which causes a page fault in ring 0 and the kernel
fails to handle it, leading to die(). To save kernel from hanging, add
an efi specific page fault handler which recovers from such faults by
1. If the efi runtime service is efi_reset_system(), reboot the machine
   through BIOS.
2. If the efi runtime service is _not_ efi_reset_system(), then, freeze
   efi_rts_wq and schedule a new process.

The efi page fault handler offers us two advantages:
1. Recovers from potential hangs that could be caused by buggy firmware.
2. Shout loud that the firmware is buggy and hence is not a kernel bug.

Tested-by: Bhupesh Sharma <bhsharma@redhat.com>
Suggested-by: Matt Fleming <matt@codeblueprint.co.uk>
Based-on-code-from: Ricardo Neri <ricardo.neri@intel.com>
Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>
Cc: Al Stone <astone@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/x86/include/asm/efi.h              |  1 +
 arch/x86/mm/fault.c                     |  9 ++++
 arch/x86/platform/efi/quirks.c          | 78 +++++++++++++++++++++++++++++++++
 drivers/firmware/efi/runtime-wrappers.c |  8 ++++
 include/linux/efi.h                     |  8 +++-
 5 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index cec5fae23eb3..c1a655f099ef 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -140,6 +140,7 @@ extern void __init efi_apply_memmap_quirks(void);
 extern int __init efi_reuse_config(u64 tables, int nr_tables);
 extern void efi_delete_dummy_variable(void);
 extern void efi_switch_mm(struct mm_struct *mm);
+extern int efi_recover_from_page_fault(unsigned long phys_addr);
 
 struct efi_setup_data {
 	u64 fw_vendor;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 2aafa6ab6103..cc2a2e3a4095 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -16,6 +16,7 @@
 #include <linux/prefetch.h>		/* prefetchw			*/
 #include <linux/context_tracking.h>	/* exception_enter(), ...	*/
 #include <linux/uaccess.h>		/* faulthandler_disabled()	*/
+#include <linux/efi.h>			/* efi_recover_from_page_fault()*/
 
 #include <asm/cpufeature.h>		/* boot_cpu_has, ...		*/
 #include <asm/traps.h>			/* dotraplinkage, ...		*/
@@ -24,6 +25,7 @@
 #include <asm/vsyscall.h>		/* emulate_vsyscall		*/
 #include <asm/vm86.h>			/* struct vm86			*/
 #include <asm/mmu_context.h>		/* vma_pkey()			*/
+#include <asm/efi.h>			/* efi_recover_from_page_fault()*/
 
 #define CREATE_TRACE_POINTS
 #include <asm/trace/exceptions.h>
@@ -790,6 +792,13 @@ no_context(struct pt_regs *regs, unsigned long error_code,
 		return;
 
 	/*
+	 * Buggy firmware could access regions which might page fault, try to
+	 * recover from such faults.
+	 */
+	if (efi_recover_from_page_fault(address))
+		return;
+
+	/*
 	 * Oops. The kernel tried to access some bad page. We'll have to
 	 * terminate things with extreme prejudice:
 	 */
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 844d31cb8a0c..3920ae8cab2a 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -16,6 +16,7 @@
 #include <asm/efi.h>
 #include <asm/uv/uv.h>
 #include <asm/cpu_device_id.h>
+#include <asm/reboot.h>
 
 #define EFI_MIN_RESERVE 5120
 
@@ -654,3 +655,80 @@ int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
 }
 
 #endif
+
+/*
+ * If any access by any efi runtime service causes a page fault, then,
+ * 1. If it's efi_reset_system(), reboot through BIOS.
+ * 2. If any other efi runtime service, then
+ *    a. Return error status to the efi caller process.
+ *    b. Disable EFI Runtime Services forever and
+ *    c. Freeze efi_rts_wq and schedule new process.
+ *
+ * @return: Returns 0, if the page fault is not handled. This function
+ * will never return if the page fault is handled successfully.
+ */
+int efi_recover_from_page_fault(unsigned long phys_addr)
+{
+	if (!IS_ENABLED(CONFIG_X86_64))
+		return 0;
+
+	/*
+	 * Make sure that an efi runtime service caused the page fault.
+	 * "efi_mm" cannot be used to check if the page fault had occurred
+	 * in the firmware context because efi=old_map doesn't use efi_pgd.
+	 */
+	if (efi_rts_work.efi_rts_id == NONE)
+		return 0;
+
+	/*
+	 * Address range 0x0000 - 0x0fff is always mapped in the efi_pgd, so
+	 * page faulting on these addresses isn't expected.
+	 */
+	if (phys_addr >= 0x0000 && phys_addr <= 0x0fff)
+		return 0;
+
+	/*
+	 * Print stack trace as it might be useful to know which EFI Runtime
+	 * Service is buggy.
+	 */
+	WARN(1, FW_BUG "Page fault caused by firmware at PA: 0x%lx\n",
+	     phys_addr);
+
+	/*
+	 * Buggy efi_reset_system() is handled differently from other EFI
+	 * Runtime Services as it doesn't use efi_rts_wq. Although,
+	 * native_machine_emergency_restart() says that machine_real_restart()
+	 * could fail, it's better not to compilcate this fault handler
+	 * because this case occurs *very* rarely and hence could be improved
+	 * on a need by basis.
+	 */
+	if (efi_rts_work.efi_rts_id == RESET_SYSTEM) {
+		pr_info("efi_reset_system() buggy! Reboot through BIOS\n");
+		machine_real_restart(MRR_BIOS);
+		return 0;
+	}
+
+	/*
+	 * Before calling EFI Runtime Service, the kernel has switched the
+	 * calling process to efi_mm. Hence, switch back to task_mm.
+	 */
+	arch_efi_call_virt_teardown();
+
+	/* Signal error status to the efi caller process */
+	efi_rts_work.status = EFI_ABORTED;
+	complete(&efi_rts_work.efi_rts_comp);
+
+	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n");
+
+	/*
+	 * Call schedule() in an infinite loop, so that any spurious wake ups
+	 * will never run efi_rts_wq again.
+	 */
+	for (;;) {
+		set_current_state(TASK_IDLE);
+		schedule();
+	}
+
+	return 0;
+}
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index b18b2d864c2c..7455277a3b65 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -61,6 +61,11 @@ struct efi_runtime_work efi_rts_work;
 ({									\
 	efi_rts_work.status = EFI_ABORTED;				\
 									\
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {			\
+		pr_info("Aborting! EFI Runtime Services disabled\n");	\
+		goto exit;						\
+	}								\
+									\
 	init_completion(&efi_rts_work.efi_rts_comp);			\
 	INIT_WORK_ONSTACK(&efi_rts_work.work, efi_call_rts);		\
 	efi_rts_work.arg1 = _arg1;					\
@@ -79,6 +84,8 @@ struct efi_runtime_work efi_rts_work;
 	else								\
 		pr_err("Failed to queue work to efi_rts_wq.\n");	\
 									\
+exit:									\
+	efi_rts_work.efi_rts_id = NONE;					\
 	efi_rts_work.status;						\
 })
 
@@ -393,6 +400,7 @@ static void virt_efi_reset_system(int reset_type,
 			"could not get exclusive access to the firmware\n");
 		return;
 	}
+	efi_rts_work.efi_rts_id = RESET_SYSTEM;
 	__efi_call_virt(reset_system, reset_type, status, data_size, data);
 	up(&efi_runtime_lock);
 }
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 855992b15269..dea9e7bb5208 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1659,8 +1659,13 @@ struct linux_efi_tpm_eventlog {
 
 extern int efi_tpm_eventlog_init(void);
 
-/* efi_runtime_service() function identifiers */
+/*
+ * efi_runtime_service() function identifiers.
+ * "NONE" is used by efi_recover_from_page_fault() to check if the page
+ * fault happened while executing an efi runtime service.
+ */
 enum efi_rts_ids {
+	NONE,
 	GET_TIME,
 	SET_TIME,
 	GET_WAKEUP_TIME,
@@ -1670,6 +1675,7 @@ enum efi_rts_ids {
 	SET_VARIABLE,
 	QUERY_VARIABLE_INFO,
 	GET_NEXT_HIGH_MONO_COUNT,
+	RESET_SYSTEM,
 	UPDATE_CAPSULE,
 	QUERY_CAPSULE_CAPS,
 };
-- 
2.7.4


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

* RE: [PATCH V5 0/2] Add efi page fault handler to recover from page
  2018-09-10 19:32 [PATCH V5 0/2] Add efi page fault handler to recover from page Sai Praneeth Prakhya
  2018-09-10 19:32 ` [PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler Sai Praneeth Prakhya
  2018-09-10 19:32 ` [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware Sai Praneeth Prakhya
@ 2018-09-10 19:47 ` Prakhya, Sai Praneeth
  2 siblings, 0 replies; 6+ messages in thread
From: Prakhya, Sai Praneeth @ 2018-09-10 19:47 UTC (permalink / raw)
  To: linux-efi, linux-kernel, x86
  Cc: Al Stone, Borislav Petkov, Ingo Molnar, Andy Lutomirski,
	Bhupesh Sharma, Thomas Gleixner, Peter Zijlstra, Ard Biesheuvel

Hi All,

> Changes from V4 to V5:
> ----------------------
> 1. Drop config option that enables efi page fault handler, instead make
>    it default.
> 2. Call schedule() in an infinite loop to account for spurious wake ups.
> 3. Introduce "NONE" as an efi runtime service function identifier so that
>    it could be used in efi_recover_from_page_fault() to check if the page
>    fault was indeed triggered by an efi runtime service.

Please note that apart from dropping the config knob and accounting for 
spurious wake ups, I have added a third change.

I realized that when kernel command line option efi=old_map is passed, the efi 
page fault handler in V4 wouldn't work because, it checks for efi_mm.

In the efi page fault handler, I have also added an explicit check for x86_64 because 
AFAIK, x86_32 bit machines do not suffer from this problem (Boris, please correct me if I am wrong).
Also, I was unable to trigger page faults with buggy OVMF for x86_32.

Just wanted to state the third change explicitly here so that it doesn't escape reviewers eyes.

Regards,
Sai

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

* Re: [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware
  2018-09-10 19:32 ` [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware Sai Praneeth Prakhya
@ 2018-09-11  8:06   ` kbuild test robot
  2018-09-11  8:19   ` kbuild test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-09-11  8:06 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: kbuild-all, linux-efi, linux-kernel, x86, Sai Praneeth, Al Stone,
	Borislav Petkov, Ingo Molnar, Andy Lutomirski, Bhupesh Sharma,
	Thomas Gleixner, Peter Zijlstra, Ard Biesheuvel

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

Hi Sai,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on efi/next]
[cannot apply to v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Make-efi_rts_work-accessible-to-efi-page-fault-handler/20180911-142222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: i386-randconfig-s0-201836 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/mm/fault.o: In function `no_context':
>> arch/x86/mm/fault.c:798: undefined reference to `efi_recover_from_page_fault'

vim +798 arch/x86/mm/fault.c

   776	
   777		/*
   778		 * 32-bit:
   779		 *
   780		 *   Valid to do another page fault here, because if this fault
   781		 *   had been triggered by is_prefetch fixup_exception would have
   782		 *   handled it.
   783		 *
   784		 * 64-bit:
   785		 *
   786		 *   Hall of shame of CPU/BIOS bugs.
   787		 */
   788		if (is_prefetch(regs, error_code, address))
   789			return;
   790	
   791		if (is_errata93(regs, address))
   792			return;
   793	
   794		/*
   795		 * Buggy firmware could access regions which might page fault, try to
   796		 * recover from such faults.
   797		 */
 > 798		if (efi_recover_from_page_fault(address))
   799			return;
   800	
   801		/*
   802		 * Oops. The kernel tried to access some bad page. We'll have to
   803		 * terminate things with extreme prejudice:
   804		 */
   805		flags = oops_begin();
   806	
   807		show_fault_oops(regs, error_code, address);
   808	
   809		if (task_stack_end_corrupted(tsk))
   810			printk(KERN_EMERG "Thread overran stack, or stack corrupted\n");
   811	
   812		tsk->thread.cr2		= address;
   813		tsk->thread.trap_nr	= X86_TRAP_PF;
   814		tsk->thread.error_code	= error_code;
   815	
   816		sig = SIGKILL;
   817		if (__die("Oops", regs, error_code))
   818			sig = 0;
   819	
   820		/* Executive summary in case the body of the oops scrolled away */
   821		printk(KERN_DEFAULT "CR2: %016lx\n", address);
   822	
   823		oops_end(flags, regs, sig);
   824	}
   825	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

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

* Re: [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware
  2018-09-10 19:32 ` [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware Sai Praneeth Prakhya
  2018-09-11  8:06   ` kbuild test robot
@ 2018-09-11  8:19   ` kbuild test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-09-11  8:19 UTC (permalink / raw)
  To: Sai Praneeth Prakhya
  Cc: kbuild-all, linux-efi, linux-kernel, x86, Sai Praneeth, Al Stone,
	Borislav Petkov, Ingo Molnar, Andy Lutomirski, Bhupesh Sharma,
	Thomas Gleixner, Peter Zijlstra, Ard Biesheuvel

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

Hi Sai,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on efi/next]
[cannot apply to v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sai-Praneeth-Prakhya/efi-Make-efi_rts_work-accessible-to-efi-page-fault-handler/20180911-142222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: i386-alldefconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/mm/fault.o: In function `no_context':
>> fault.c:(.text+0x6a1): undefined reference to `efi_recover_from_page_fault'
   fault.c:(.text+0x742): undefined reference to `efi_recover_from_page_fault'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

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

end of thread, other threads:[~2018-09-11  8:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-10 19:32 [PATCH V5 0/2] Add efi page fault handler to recover from page Sai Praneeth Prakhya
2018-09-10 19:32 ` [PATCH V5 1/2] efi: Make efi_rts_work accessible to efi page fault handler Sai Praneeth Prakhya
2018-09-10 19:32 ` [PATCH V5 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware Sai Praneeth Prakhya
2018-09-11  8:06   ` kbuild test robot
2018-09-11  8:19   ` kbuild test robot
2018-09-10 19:47 ` [PATCH V5 0/2] Add efi page fault handler to recover from page Prakhya, Sai Praneeth

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