[PATCH 0/4] efi: Clean up runtime wrapper and wire it up for PRM

[PATCH 0/4] efi: Clean up runtime wrapper and wire it up for PRM

[Ard Biesheuvel]

ACPI PRM uses the EFI runtime services infrastructure, but currently, it
issues the firmware calls directly, instead of going through the
wrappers and handing off the call to the EFI workqueue.

Given that ACPI PRM is used for vendor code rather than core EFI runtime
services, it would be nice if these calls get sandboxed in the same way
as EFI runtime services calls are. This ensures that any faults
occurring in the firmware are handled gracefully and don't bring down
the kernel.

Another reason for using the work queue is the fact that on some
platforms, the EFI memory regions are remapping into the lower address
space, and this means that sampling the instruction pointer in a perf
interrupt may cause confusion about whether the task is running in user
space or in the firmware.

So let's move the ACPI PRM calls into the EFI runtime wrapper
infrastructure. Before that, let's clean it up a bit.

Ard Biesheuvel (4):
  efi/runtime-wrappers: Use type safe encapsulation of call arguments
  efi/runtime-wrapper: Move workqueue manipulation out of line
  efi/runtime-wrappers: Remove duplicated macro for service returning
    void
  acpi/prmt: Use EFI runtime sandbox to invoke PRM handlers

 drivers/acpi/prmt.c                     |   8 +-
 drivers/firmware/efi/runtime-wrappers.c | 201 +++++++++++---------
 include/linux/efi.h                     | 110 ++++++++---
 3 files changed, 198 insertions(+), 121 deletions(-)

-- 
2.39.2

[PATCH 1/4] efi/runtime-wrappers: Use type safe encapsulation of call arguments

[Ard Biesheuvel]

The current code that marshalls the EFI runtime call arguments to hand
them off to a async helper does so in a type unsafe and slightly messy
manner - everything is cast to void* except for some integral types that
are passed by reference and dereferenced on the receiver end.

Let's clean this up a bit, and record the arguments of each runtime
service invocation exactly as they are issued, in a manner that permits
the compiler to check the types of the arguments at both ends.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 124 ++++++++++----------
 include/linux/efi.h                     |  73 +++++++++++-
 2 files changed, 126 insertions(+), 71 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index a400c4312c829f18..5d48d493f57e7c38 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -49,15 +49,16 @@ struct efi_runtime_work efi_rts_work;
 /*
  * efi_queue_work:	Queue efi_runtime_service() and wait until it's done
  * @rts:		efi_runtime_service() function identifier
- * @rts_arg<1-5>:	efi_runtime_service() function arguments
  *
  * Accesses to efi_runtime_services() are serialized by a binary
  * semaphore (efi_runtime_lock) and caller waits until the work is
  * finished, hence _only_ one work is queued at a time and the caller
  * thread waits for completion.
  */
-#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5)		\
+#define efi_queue_work(_rts, ...)					\
 ({									\
+	efi_rts_work._rts = (typeof(efi_rts_work._rts)){ __VA_ARGS__ };	\
+	efi_rts_work.efi_rts_id = EFI_ ## _rts;				\
 	efi_rts_work.status = EFI_ABORTED;				\
 									\
 	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {			\
@@ -68,12 +69,6 @@ struct efi_runtime_work efi_rts_work;
 									\
 	init_completion(&efi_rts_work.efi_rts_comp);			\
 	INIT_WORK(&efi_rts_work.work, efi_call_rts);			\
-	efi_rts_work.arg1 = _arg1;					\
-	efi_rts_work.arg2 = _arg2;					\
-	efi_rts_work.arg3 = _arg3;					\
-	efi_rts_work.arg4 = _arg4;					\
-	efi_rts_work.arg5 = _arg5;					\
-	efi_rts_work.efi_rts_id = _rts;					\
 									\
 	/*								\
 	 * queue_work() returns 0 if work was already on queue,         \
@@ -170,73 +165,77 @@ extern struct semaphore __efi_uv_runtime_lock __alias(efi_runtime_lock);
 /*
  * Calls the appropriate efi_runtime_service() with the appropriate
  * arguments.
- *
- * Semantics followed by efi_call_rts() to understand efi_runtime_work:
- * 1. If argument was a pointer, recast it from void pointer to original
- * pointer type.
- * 2. If argument was a value, recast it from void pointer to original
- * pointer type and dereference it.
  */
 static void efi_call_rts(struct work_struct *work)
 {
-	void *arg1, *arg2, *arg3, *arg4, *arg5;
 	efi_status_t status = EFI_NOT_FOUND;
 
-	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) {
 	case EFI_GET_TIME:
-		status = efi_call_virt(get_time, (efi_time_t *)arg1,
-				       (efi_time_cap_t *)arg2);
+		status = efi_call_virt(get_time,
+				       efi_rts_work.GET_TIME.time,
+				       efi_rts_work.GET_TIME.capabilities);
 		break;
 	case EFI_SET_TIME:
-		status = efi_call_virt(set_time, (efi_time_t *)arg1);
+		status = efi_call_virt(set_time,
+				       efi_rts_work.SET_TIME.time);
 		break;
 	case EFI_GET_WAKEUP_TIME:
-		status = efi_call_virt(get_wakeup_time, (efi_bool_t *)arg1,
-				       (efi_bool_t *)arg2, (efi_time_t *)arg3);
+		status = efi_call_virt(get_wakeup_time,
+				       efi_rts_work.GET_WAKEUP_TIME.enabled,
+				       efi_rts_work.GET_WAKEUP_TIME.pending,
+				       efi_rts_work.GET_WAKEUP_TIME.time);
 		break;
 	case EFI_SET_WAKEUP_TIME:
-		status = efi_call_virt(set_wakeup_time, *(efi_bool_t *)arg1,
-				       (efi_time_t *)arg2);
+		status = efi_call_virt(set_wakeup_time,
+				       efi_rts_work.SET_WAKEUP_TIME.enable,
+				       efi_rts_work.SET_WAKEUP_TIME.time);
 		break;
 	case EFI_GET_VARIABLE:
-		status = efi_call_virt(get_variable, (efi_char16_t *)arg1,
-				       (efi_guid_t *)arg2, (u32 *)arg3,
-				       (unsigned long *)arg4, (void *)arg5);
+		status = efi_call_virt(get_variable,
+				       efi_rts_work.GET_VARIABLE.name,
+				       efi_rts_work.GET_VARIABLE.vendor,
+				       efi_rts_work.GET_VARIABLE.attr,
+				       efi_rts_work.GET_VARIABLE.data_size,
+				       efi_rts_work.GET_VARIABLE.data);
 		break;
 	case EFI_GET_NEXT_VARIABLE:
-		status = efi_call_virt(get_next_variable, (unsigned long *)arg1,
-				       (efi_char16_t *)arg2,
-				       (efi_guid_t *)arg3);
+		status = efi_call_virt(get_next_variable,
+				       efi_rts_work.GET_NEXT_VARIABLE.name_size,
+				       efi_rts_work.GET_NEXT_VARIABLE.name,
+				       efi_rts_work.GET_NEXT_VARIABLE.vendor);
 		break;
 	case EFI_SET_VARIABLE:
-		status = efi_call_virt(set_variable, (efi_char16_t *)arg1,
-				       (efi_guid_t *)arg2, *(u32 *)arg3,
-				       *(unsigned long *)arg4, (void *)arg5);
+		status = efi_call_virt(set_variable,
+				       efi_rts_work.SET_VARIABLE.name,
+				       efi_rts_work.SET_VARIABLE.vendor,
+				       efi_rts_work.SET_VARIABLE.attr,
+				       efi_rts_work.SET_VARIABLE.data_size,
+				       efi_rts_work.SET_VARIABLE.data);
 		break;
 	case EFI_QUERY_VARIABLE_INFO:
-		status = efi_call_virt(query_variable_info, *(u32 *)arg1,
-				       (u64 *)arg2, (u64 *)arg3, (u64 *)arg4);
+		status = efi_call_virt(query_variable_info,
+				       efi_rts_work.QUERY_VARIABLE_INFO.attr,
+				       efi_rts_work.QUERY_VARIABLE_INFO.storage_space,
+				       efi_rts_work.QUERY_VARIABLE_INFO.remaining_space,
+				       efi_rts_work.QUERY_VARIABLE_INFO.max_variable_size);
 		break;
 	case EFI_GET_NEXT_HIGH_MONO_COUNT:
-		status = efi_call_virt(get_next_high_mono_count, (u32 *)arg1);
+		status = efi_call_virt(get_next_high_mono_count,
+				       efi_rts_work.GET_NEXT_HIGH_MONO_COUNT.high_count);
 		break;
 	case EFI_UPDATE_CAPSULE:
 		status = efi_call_virt(update_capsule,
-				       (efi_capsule_header_t **)arg1,
-				       *(unsigned long *)arg2,
-				       *(unsigned long *)arg3);
+				       efi_rts_work.UPDATE_CAPSULE.capsules,
+				       efi_rts_work.UPDATE_CAPSULE.count,
+				       efi_rts_work.UPDATE_CAPSULE.sg_list);
 		break;
 	case EFI_QUERY_CAPSULE_CAPS:
 		status = efi_call_virt(query_capsule_caps,
-				       (efi_capsule_header_t **)arg1,
-				       *(unsigned long *)arg2, (u64 *)arg3,
-				       (int *)arg4);
+				       efi_rts_work.QUERY_CAPSULE_CAPS.capsules,
+				       efi_rts_work.QUERY_CAPSULE_CAPS.count,
+				       efi_rts_work.QUERY_CAPSULE_CAPS.max_size,
+				       efi_rts_work.QUERY_CAPSULE_CAPS.reset_type);
 		break;
 	default:
 		/*
@@ -256,7 +255,7 @@ static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_GET_TIME, tm, tc, NULL, NULL, NULL);
+	status = efi_queue_work(GET_TIME, tm, tc);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -267,7 +266,7 @@ static efi_status_t virt_efi_set_time(efi_time_t *tm)
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_SET_TIME, tm, NULL, NULL, NULL, NULL);
+	status = efi_queue_work(SET_TIME, tm);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -280,8 +279,7 @@ static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_GET_WAKEUP_TIME, enabled, pending, tm, NULL,
-				NULL);
+	status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -292,8 +290,7 @@ static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_SET_WAKEUP_TIME, &enabled, tm, NULL, NULL,
-				NULL);
+	status = efi_queue_work(SET_WAKEUP_TIME, enabled, tm);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -308,7 +305,7 @@ static efi_status_t virt_efi_get_variable(efi_char16_t *name,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_GET_VARIABLE, name, vendor, attr, data_size,
+	status = efi_queue_work(GET_VARIABLE, name, vendor, attr, data_size,
 				data);
 	up(&efi_runtime_lock);
 	return status;
@@ -322,8 +319,7 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_GET_NEXT_VARIABLE, name_size, name, vendor,
-				NULL, NULL);
+	status = efi_queue_work(GET_NEXT_VARIABLE, name_size, name, vendor);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -338,7 +334,7 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_SET_VARIABLE, name, vendor, &attr, &data_size,
+	status = efi_queue_work(SET_VARIABLE, name, vendor, attr, data_size,
 				data);
 	up(&efi_runtime_lock);
 	return status;
@@ -373,8 +369,8 @@ static efi_status_t virt_efi_query_variable_info(u32 attr,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_QUERY_VARIABLE_INFO, &attr, storage_space,
-				remaining_space, max_variable_size, NULL);
+	status = efi_queue_work(QUERY_VARIABLE_INFO, attr, storage_space,
+				remaining_space, max_variable_size);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -405,8 +401,7 @@ static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_GET_NEXT_HIGH_MONO_COUNT, count, NULL, NULL,
-				NULL, NULL);
+	status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -437,8 +432,7 @@ static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_UPDATE_CAPSULE, capsules, &count, &sg_list,
-				NULL, NULL);
+	status = efi_queue_work(UPDATE_CAPSULE, capsules, count, sg_list);
 	up(&efi_runtime_lock);
 	return status;
 }
@@ -455,13 +449,13 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
 
 	if (down_interruptible(&efi_runtime_lock))
 		return EFI_ABORTED;
-	status = efi_queue_work(EFI_QUERY_CAPSULE_CAPS, capsules, &count,
-				max_size, reset_type, NULL);
+	status = efi_queue_work(QUERY_CAPSULE_CAPS, capsules, count,
+				max_size, reset_type);
 	up(&efi_runtime_lock);
 	return status;
 }
 
-void efi_native_runtime_setup(void)
+void __init efi_native_runtime_setup(void)
 {
 	efi.get_time = virt_efi_get_time;
 	efi.set_time = virt_efi_set_time;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ab088c662e88d07b..b34e11a5e969282c 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1267,17 +1267,78 @@ enum efi_rts_ids {
 
 /*
  * 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;
+	union {
+		struct {
+			efi_time_t 	*time;
+			efi_time_cap_t	*capabilities;
+		} GET_TIME;
+
+		struct {
+			efi_time_t	*time;
+		} SET_TIME;
+
+		struct {
+			efi_bool_t	*enabled;
+			efi_bool_t	*pending;
+			efi_time_t	*time;
+		} GET_WAKEUP_TIME;
+
+		struct {
+			efi_bool_t	enable;
+			efi_time_t	*time;
+		} SET_WAKEUP_TIME;
+
+		struct {
+			efi_char16_t	*name;
+			efi_guid_t	*vendor;
+			u32		*attr;
+			unsigned long	*data_size;
+			void		*data;
+		} GET_VARIABLE;
+
+		struct {
+			unsigned long	*name_size;
+			efi_char16_t	*name;
+			efi_guid_t 	*vendor;
+		} GET_NEXT_VARIABLE;
+
+		struct {
+			efi_char16_t	*name;
+			efi_guid_t	*vendor;
+			u32		attr;
+			unsigned long	data_size;
+			void		*data;
+		} SET_VARIABLE;
+
+		struct {
+			u32		attr;
+			u64		*storage_space;
+			u64		*remaining_space;
+			u64		*max_variable_size;
+		} QUERY_VARIABLE_INFO;
+
+		struct {
+			u32		*high_count;
+		} GET_NEXT_HIGH_MONO_COUNT;
+
+		struct {
+			efi_capsule_header_t **capsules;
+			unsigned long	count;
+			unsigned long	sg_list;
+		} UPDATE_CAPSULE;
+
+		struct {
+			efi_capsule_header_t **capsules;
+			unsigned long	count;
+			u64		*max_size;
+			int		*reset_type;
+		} QUERY_CAPSULE_CAPS;
+	};
 	efi_status_t status;
 	struct work_struct work;
 	enum efi_rts_ids efi_rts_id;
-- 
2.39.2

[PATCH 2/4] efi/runtime-wrapper: Move workqueue manipulation out of line

[Ard Biesheuvel]

efi_queue_work() is a macro that implements the non-trivial manipulation
of the EFI runtime workqueue and completion data structure, most of
which is generic, and could be shared between all the users of the
macro. So move it out of the macro and into a new helper function.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/runtime-wrappers.c | 53 +++++++++++---------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 5d48d493f57e7c38..158685e26f430ac9 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -59,30 +59,7 @@ struct efi_runtime_work efi_rts_work;
 ({									\
 	efi_rts_work._rts = (typeof(efi_rts_work._rts)){ __VA_ARGS__ };	\
 	efi_rts_work.efi_rts_id = EFI_ ## _rts;				\
-	efi_rts_work.status = EFI_ABORTED;				\
-									\
-	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {			\
-		pr_warn_once("EFI Runtime Services are disabled!\n");	\
-		efi_rts_work.status = EFI_DEVICE_ERROR;			\
-		goto exit;						\
-	}								\
-									\
-	init_completion(&efi_rts_work.efi_rts_comp);			\
-	INIT_WORK(&efi_rts_work.work, efi_call_rts);			\
-									\
-	/*								\
-	 * queue_work() returns 0 if work was already on queue,         \
-	 * _ideally_ this should never happen.                          \
-	 */								\
-	if (queue_work(efi_rts_wq, &efi_rts_work.work))			\
-		wait_for_completion(&efi_rts_work.efi_rts_comp);	\
-	else								\
-		pr_err("Failed to queue work to efi_rts_wq.\n");	\
-									\
-	WARN_ON_ONCE(efi_rts_work.status == EFI_ABORTED);		\
-exit:									\
-	efi_rts_work.efi_rts_id = EFI_NONE;				\
-	efi_rts_work.status;						\
+	__efi_queue_work();						\
 })
 
 #ifndef arch_efi_save_flags
@@ -249,6 +226,34 @@ static void efi_call_rts(struct work_struct *work)
 	complete(&efi_rts_work.efi_rts_comp);
 }
 
+static efi_status_t __efi_queue_work(void)
+{
+	efi_rts_work.status = EFI_ABORTED;
+
+	if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
+		pr_warn_once("EFI Runtime Services are disabled!\n");
+		efi_rts_work.status = EFI_DEVICE_ERROR;
+		goto exit;
+	}
+
+	init_completion(&efi_rts_work.efi_rts_comp);
+	INIT_WORK(&efi_rts_work.work, efi_call_rts);
+
+	/*
+	 * queue_work() returns 0 if work was already on queue,
+	 * _ideally_ this should never happen.
+	 */
+	if (queue_work(efi_rts_wq, &efi_rts_work.work))
+		wait_for_completion(&efi_rts_work.efi_rts_comp);
+	else
+		pr_err("Failed to queue work to efi_rts_wq.\n");
+
+	WARN_ON_ONCE(efi_rts_work.status == EFI_ABORTED);
+exit:
+	efi_rts_work.efi_rts_id = EFI_NONE;
+	return efi_rts_work.status;
+}
+
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {
 	efi_status_t status;
-- 
2.39.2

[PATCH 3/4] efi/runtime-wrappers: Remove duplicated macro for service returning void

[Ard Biesheuvel]

__efi_call_virt() exists as an alternative for efi_call_virt() for the
sole reason that ResetSystem() returns void, and so we cannot use a call
to it in the RHS of an assignment.

Now that we support the use of _Generic, this is no longer needed, and
we can handle this by emitting a comma expression inside the default
branch of a _Generic() switch.

As a bonus, this ensures that the runtime service call is always
constructed and type checked by the compiler, as it is passed to
_Generic() to infer the return type. (both x86 and arm64 override
arch_efi_call_virt() to invoke a type unsafe variadic wrapper function
implemented in assembler)

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/runtime-wrappers.c |  4 +---
 include/linux/efi.h                     | 24 ++++----------------
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 158685e26f430ac9..b3ef208299ae591e 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -41,8 +41,6 @@
  */
 #define efi_call_virt(f, args...)   \
 	efi_call_virt_pointer(efi.runtime, f, args)
-#define __efi_call_virt(f, args...) \
-	__efi_call_virt_pointer(efi.runtime, f, args)
 
 struct efi_runtime_work efi_rts_work;
 
@@ -422,7 +420,7 @@ static void virt_efi_reset_system(int reset_type,
 		return;
 	}
 	efi_rts_work.efi_rts_id = EFI_RESET_SYSTEM;
-	__efi_call_virt(reset_system, reset_type, status, data_size, data);
+	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 b34e11a5e969282c..c72715821016851b 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1171,8 +1171,7 @@ static inline void efi_check_for_embedded_firmwares(void) { }
 #define arch_efi_call_virt(p, f, args...)	((p)->f(args))
 
 /*
- * Arch code can implement the following three template macros, avoiding
- * reptition for the void/non-void return cases of {__,}efi_call_virt():
+ * Arch code must implement the following three template macros:
  *
  *  * arch_efi_call_virt_setup()
  *
@@ -1181,9 +1180,7 @@ static inline void efi_check_for_embedded_firmwares(void) { }
  *
  *  * arch_efi_call_virt()
  *
- *    Performs the call. The last expression in the macro must be the call
- *    itself, allowing the logic to be shared by the void and non-void
- *    cases.
+ *    Performs the call.
  *
  *  * arch_efi_call_virt_teardown()
  *
@@ -1198,7 +1195,9 @@ static inline void efi_check_for_embedded_firmwares(void) { }
 	arch_efi_call_virt_setup();					\
 									\
 	__flags = efi_call_virt_save_flags();				\
-	__s = arch_efi_call_virt(p, f, args);				\
+	__s = _Generic((p)->f(args),					\
+	       efi_status_t: arch_efi_call_virt((p), f, args),		\
+	       default: (arch_efi_call_virt((p), f, args), EFI_ABORTED));\
 	efi_call_virt_check_flags(__flags, __stringify(f));		\
 									\
 	arch_efi_call_virt_teardown();					\
@@ -1206,19 +1205,6 @@ static inline void efi_check_for_embedded_firmwares(void) { }
 	__s;								\
 })
 
-#define __efi_call_virt_pointer(p, f, args...)				\
-({									\
-	unsigned long __flags;						\
-									\
-	arch_efi_call_virt_setup();					\
-									\
-	__flags = efi_call_virt_save_flags();				\
-	arch_efi_call_virt(p, f, args);					\
-	efi_call_virt_check_flags(__flags, __stringify(f));		\
-									\
-	arch_efi_call_virt_teardown();					\
-})
-
 #define EFI_RANDOM_SEED_SIZE		32U // BLAKE2S_HASH_SIZE
 
 struct linux_efi_random_seed {
-- 
2.39.2

[PATCH 4/4] acpi/prmt: Use EFI runtime sandbox to invoke PRM handlers

[Ard Biesheuvel]

Instead of bypassing the kernel's adaptation layer for performing EFI
runtime calls, wire up ACPI PRM handling into it. This means these calls
can no longer occur concurrently with EFI runtime calls, and will be
made from the EFI runtime workqueue. It also means any page faults
occurring during PRM handling will be identified correctly as
originating in firmware code.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/acpi/prmt.c                     |  8 ++++----
 drivers/firmware/efi/runtime-wrappers.c | 20 ++++++++++++++++++++
 include/linux/efi.h                     | 13 +++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
index 3d4c4620f9f95309..95be1c80db387faa 100644
--- a/drivers/acpi/prmt.c
+++ b/drivers/acpi/prmt.c
@@ -53,7 +53,7 @@ static LIST_HEAD(prm_module_list);
 
 struct prm_handler_info {
 	guid_t guid;
-	void *handler_addr;
+	efi_acpi_prm_handler_t *handler_addr;
 	u64 static_data_buffer_addr;
 	u64 acpi_param_buffer_addr;
 
@@ -260,9 +260,9 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
 		context.static_data_buffer = handler->static_data_buffer_addr;
 		context.mmio_ranges = module->mmio_info;
 
-		status = efi_call_virt_pointer(handler, handler_addr,
-					       handler->acpi_param_buffer_addr,
-					       &context);
+		status = efi_call_acpi_prm_handler(handler->handler_addr,
+						   handler->acpi_param_buffer_addr,
+						   &context);
 		if (status == EFI_SUCCESS) {
 			buffer->prm_status = PRM_HANDLER_SUCCESS;
 		} else {
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index b3ef208299ae591e..ce306cd1efdfda21 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -212,6 +212,12 @@ static void efi_call_rts(struct work_struct *work)
 				       efi_rts_work.QUERY_CAPSULE_CAPS.max_size,
 				       efi_rts_work.QUERY_CAPSULE_CAPS.reset_type);
 		break;
+	case EFI_ACPI_PRM_HANDLER:
+		status = efi_call_virt_pointer(&efi_rts_work.ACPI_PRM_HANDLER,
+					       handler_addr,
+					       efi_rts_work.ACPI_PRM_HANDLER.param_buffer_addr,
+					       efi_rts_work.ACPI_PRM_HANDLER.context);
+		break;
 	default:
 		/*
 		 * Ideally, we should never reach here because a caller of this
@@ -475,3 +481,17 @@ void __init efi_native_runtime_setup(void)
 	efi.update_capsule = virt_efi_update_capsule;
 	efi.query_capsule_caps = virt_efi_query_capsule_caps;
 }
+
+efi_status_t efi_call_acpi_prm_handler(efi_acpi_prm_handler_t *handler_addr,
+				       efi_physical_addr_t param_buffer_addr,
+				       void *context)
+{
+	efi_status_t status;
+
+	if (down_interruptible(&efi_runtime_lock))
+		return EFI_ABORTED;
+	status = efi_queue_work(ACPI_PRM_HANDLER, handler_addr,
+				param_buffer_addr, context);
+	up(&efi_runtime_lock);
+	return status;
+}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index c72715821016851b..065af735d90a411c 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1230,6 +1230,12 @@ extern int efi_tpm_final_log_size;
 
 extern unsigned long rci2_table_phys;
 
+typedef efi_status_t (__efiapi efi_acpi_prm_handler_t)(efi_physical_addr_t, void *);
+
+efi_status_t efi_call_acpi_prm_handler(efi_acpi_prm_handler_t *handler_addr,
+				       efi_physical_addr_t param_buffer_addr,
+				       void *context);
+
 /*
  * efi_runtime_service() function identifiers.
  * "NONE" is used by efi_recover_from_page_fault() to check if the page
@@ -1249,6 +1255,7 @@ enum efi_rts_ids {
 	EFI_RESET_SYSTEM,
 	EFI_UPDATE_CAPSULE,
 	EFI_QUERY_CAPSULE_CAPS,
+	EFI_ACPI_PRM_HANDLER,
 };
 
 /*
@@ -1324,6 +1331,12 @@ struct efi_runtime_work {
 			u64		*max_size;
 			int		*reset_type;
 		} QUERY_CAPSULE_CAPS;
+
+		struct {
+			efi_acpi_prm_handler_t	*handler_addr;
+			efi_physical_addr_t	param_buffer_addr;
+			void			*context;
+		} ACPI_PRM_HANDLER;
 	};
 	efi_status_t status;
 	struct work_struct work;
-- 
2.39.2

Re: [PATCH 4/4] acpi/prmt: Use EFI runtime sandbox to invoke PRM handlers

[Rafael J. Wysocki]

On Fri, Aug 4, 2023 at 6:04 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Instead of bypassing the kernel's adaptation layer for performing EFI
> runtime calls, wire up ACPI PRM handling into it. This means these calls
> can no longer occur concurrently with EFI runtime calls, and will be
> made from the EFI runtime workqueue. It also means any page faults
> occurring during PRM handling will be identified correctly as
> originating in firmware code.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks!

> ---
>  drivers/acpi/prmt.c                     |  8 ++++----
>  drivers/firmware/efi/runtime-wrappers.c | 20 ++++++++++++++++++++
>  include/linux/efi.h                     | 13 +++++++++++++
>  3 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> index 3d4c4620f9f95309..95be1c80db387faa 100644
> --- a/drivers/acpi/prmt.c
> +++ b/drivers/acpi/prmt.c
> @@ -53,7 +53,7 @@ static LIST_HEAD(prm_module_list);
>
>  struct prm_handler_info {
>         guid_t guid;
> -       void *handler_addr;
> +       efi_acpi_prm_handler_t *handler_addr;
>         u64 static_data_buffer_addr;
>         u64 acpi_param_buffer_addr;
>
> @@ -260,9 +260,9 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
>                 context.static_data_buffer = handler->static_data_buffer_addr;
>                 context.mmio_ranges = module->mmio_info;
>
> -               status = efi_call_virt_pointer(handler, handler_addr,
> -                                              handler->acpi_param_buffer_addr,
> -                                              &context);
> +               status = efi_call_acpi_prm_handler(handler->handler_addr,
> +                                                  handler->acpi_param_buffer_addr,
> +                                                  &context);
>                 if (status == EFI_SUCCESS) {
>                         buffer->prm_status = PRM_HANDLER_SUCCESS;
>                 } else {
> diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
> index b3ef208299ae591e..ce306cd1efdfda21 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -212,6 +212,12 @@ static void efi_call_rts(struct work_struct *work)
>                                        efi_rts_work.QUERY_CAPSULE_CAPS.max_size,
>                                        efi_rts_work.QUERY_CAPSULE_CAPS.reset_type);
>                 break;
> +       case EFI_ACPI_PRM_HANDLER:
> +               status = efi_call_virt_pointer(&efi_rts_work.ACPI_PRM_HANDLER,
> +                                              handler_addr,
> +                                              efi_rts_work.ACPI_PRM_HANDLER.param_buffer_addr,
> +                                              efi_rts_work.ACPI_PRM_HANDLER.context);
> +               break;
>         default:
>                 /*
>                  * Ideally, we should never reach here because a caller of this
> @@ -475,3 +481,17 @@ void __init efi_native_runtime_setup(void)
>         efi.update_capsule = virt_efi_update_capsule;
>         efi.query_capsule_caps = virt_efi_query_capsule_caps;
>  }
> +
> +efi_status_t efi_call_acpi_prm_handler(efi_acpi_prm_handler_t *handler_addr,
> +                                      efi_physical_addr_t param_buffer_addr,
> +                                      void *context)
> +{
> +       efi_status_t status;
> +
> +       if (down_interruptible(&efi_runtime_lock))
> +               return EFI_ABORTED;
> +       status = efi_queue_work(ACPI_PRM_HANDLER, handler_addr,
> +                               param_buffer_addr, context);
> +       up(&efi_runtime_lock);
> +       return status;
> +}
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index c72715821016851b..065af735d90a411c 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1230,6 +1230,12 @@ extern int efi_tpm_final_log_size;
>
>  extern unsigned long rci2_table_phys;
>
> +typedef efi_status_t (__efiapi efi_acpi_prm_handler_t)(efi_physical_addr_t, void *);
> +
> +efi_status_t efi_call_acpi_prm_handler(efi_acpi_prm_handler_t *handler_addr,
> +                                      efi_physical_addr_t param_buffer_addr,
> +                                      void *context);
> +
>  /*
>   * efi_runtime_service() function identifiers.
>   * "NONE" is used by efi_recover_from_page_fault() to check if the page
> @@ -1249,6 +1255,7 @@ enum efi_rts_ids {
>         EFI_RESET_SYSTEM,
>         EFI_UPDATE_CAPSULE,
>         EFI_QUERY_CAPSULE_CAPS,
> +       EFI_ACPI_PRM_HANDLER,
>  };
>
>  /*
> @@ -1324,6 +1331,12 @@ struct efi_runtime_work {
>                         u64             *max_size;
>                         int             *reset_type;
>                 } QUERY_CAPSULE_CAPS;
> +
> +               struct {
> +                       efi_acpi_prm_handler_t  *handler_addr;
> +                       efi_physical_addr_t     param_buffer_addr;
> +                       void                    *context;
> +               } ACPI_PRM_HANDLER;
>         };
>         efi_status_t status;
>         struct work_struct work;
> --
> 2.39.2
>