From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: Ard Biesheuvel <ardb@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
Dave Hansen <dave.hansen@linux.intel.com>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>
Subject: [PATCH v2 04/11] efi/runtime-wrappers: Use type safe encapsulation of call arguments
Date: Fri, 18 Aug 2023 13:37:17 +0200 [thread overview]
Message-ID: <20230818113724.368492-5-ardb@kernel.org> (raw)
In-Reply-To: <20230818113724.368492-1-ardb@kernel.org>
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 | 196 +++++++++++++-------
include/linux/efi.h | 18 +-
2 files changed, 138 insertions(+), 76 deletions(-)
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index a400c4312c829f18..66066e058757c1b5 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -44,20 +44,90 @@
#define __efi_call_virt(f, args...) \
__efi_call_virt_pointer(efi.runtime, f, args)
+union efi_rts_args {
+ 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;
+};
+
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
+ * efi_queue_work: Queue EFI runtime service call and wait for completion
+ * @_rts: EFI runtime service function identifier
+ * @_args: Arguments to pass to the EFI runtime service
*
* 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, _args...) \
({ \
+ efi_rts_work.efi_rts_id = EFI_ ## _rts; \
+ efi_rts_work.args = &(union efi_rts_args){ ._rts = { _args }}; \
efi_rts_work.status = EFI_ABORTED; \
\
if (!efi_enabled(EFI_RUNTIME_SERVICES)) { \
@@ -68,12 +138,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 +234,78 @@ 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;
+ const union efi_rts_args *args = efi_rts_work.args;
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,
+ args->GET_TIME.time,
+ args->GET_TIME.capabilities);
break;
case EFI_SET_TIME:
- status = efi_call_virt(set_time, (efi_time_t *)arg1);
+ status = efi_call_virt(set_time,
+ args->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,
+ args->GET_WAKEUP_TIME.enabled,
+ args->GET_WAKEUP_TIME.pending,
+ args->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,
+ args->SET_WAKEUP_TIME.enable,
+ args->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,
+ args->GET_VARIABLE.name,
+ args->GET_VARIABLE.vendor,
+ args->GET_VARIABLE.attr,
+ args->GET_VARIABLE.data_size,
+ args->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,
+ args->GET_NEXT_VARIABLE.name_size,
+ args->GET_NEXT_VARIABLE.name,
+ args->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,
+ args->SET_VARIABLE.name,
+ args->SET_VARIABLE.vendor,
+ args->SET_VARIABLE.attr,
+ args->SET_VARIABLE.data_size,
+ args->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,
+ args->QUERY_VARIABLE_INFO.attr,
+ args->QUERY_VARIABLE_INFO.storage_space,
+ args->QUERY_VARIABLE_INFO.remaining_space,
+ args->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,
+ args->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);
+ args->UPDATE_CAPSULE.capsules,
+ args->UPDATE_CAPSULE.count,
+ args->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);
+ args->QUERY_CAPSULE_CAPS.capsules,
+ args->QUERY_CAPSULE_CAPS.count,
+ args->QUERY_CAPSULE_CAPS.max_size,
+ args->QUERY_CAPSULE_CAPS.reset_type);
break;
default:
/*
@@ -256,7 +325,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 +336,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 +349,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 +360,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 +375,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 +389,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 +404,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 +439,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 +471,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 +502,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,8 +519,8 @@ 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;
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ab088c662e88d07b..b8c446da608371d2 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1265,23 +1265,21 @@ enum efi_rts_ids {
EFI_QUERY_CAPSULE_CAPS,
};
+union efi_rts_args;
+
/*
* efi_runtime_work: Details of EFI Runtime Service work
- * @arg<1-5>: EFI Runtime Service function arguments
+ * @args: Pointer to union describing the 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;
+ union efi_rts_args *args;
+ efi_status_t status;
+ struct work_struct work;
+ enum efi_rts_ids efi_rts_id;
+ struct completion efi_rts_comp;
};
extern struct efi_runtime_work efi_rts_work;
--
2.39.2
next prev parent reply other threads:[~2023-08-18 11:38 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-18 11:37 [PATCH v2 00/11] efi: Clean up runtime wrapper and wire it up for PRM Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 01/11] efi/x86: Move EFI runtime call setup/teardown helpers out of line Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 02/11] efi/arm64: " Ard Biesheuvel
2023-08-21 10:24 ` Catalin Marinas
2023-08-18 11:37 ` [PATCH v2 03/11] efi/riscv: " Ard Biesheuvel
2023-08-18 11:37 ` Ard Biesheuvel [this message]
2023-08-18 11:37 ` [PATCH v2 05/11] efi/runtime-wrapper: Move workqueue manipulation " Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 06/11] efi/runtime-wrappers: Remove duplicated macro for service returning void Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 07/11] efi/runtime-wrappers: Don't duplicate setup/teardown code Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 08/11] acpi/prmt: Use EFI runtime sandbox to invoke PRM handlers Ard Biesheuvel
2023-08-18 11:37 ` [PATCH v2 09/11] efi/runtime-wrappers: Clean up white space and add __init annotation Ard Biesheuvel
2023-08-18 11:37 ` [RFC PATCH v2 10/11] efi/x86: Realign EFI runtime stack Ard Biesheuvel
2023-08-18 11:37 ` [RFC PATCH v2 11/11] efi/x86: Rely on compiler to emit MS ABI calls Ard Biesheuvel
2023-08-22 2:01 ` [PATCH v2 00/11] efi: Clean up runtime wrapper and wire it up for PRM Nathan Chancellor
2023-08-22 7:53 ` Ard Biesheuvel
2023-08-22 10:00 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230818113724.368492-5-ardb@kernel.org \
--to=ardb@kernel.org \
--cc=aou@eecs.berkeley.edu \
--cc=bp@alien8.de \
--cc=catalin.marinas@arm.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-efi@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=rafael@kernel.org \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.