RE: [PATCH V1 2/3] efi: Introduce efi_rts_workqueue and necessary infrastructure to invoke all efi_runtime_services()
> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> > index ac5db5f8dbbf..4714b305ca90 100644
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -76,6 +76,8 @@ static unsigned long *efi_tables[] = {
> > &efi.mem_attr_table,
> > };
> >
> > +struct workqueue_struct *efi_rts_wq;
> > +
> > static bool disable_runtime;
> > static int __init setup_noefi(char *arg) { @@ -329,6 +331,15 @@
> > static int __init efisubsys_init(void)
> > if (!efi_enabled(EFI_BOOT))
> > return 0;
> >
> > + /* Create a work queue to run EFI Runtime Services */
> > + efi_rts_wq = create_workqueue("efi_rts_workqueue");
>
> Please consider alloc_workqueue() instead with the appropriate flags, since
> create_workqueue() and friends are deprecated.
Sure! Will change in V2.
>
> > + if (!efi_rts_wq) {
> > + pr_err("Failed to create efi_rts_workqueue, EFI runtime
> > services "
> > + "disabled.\n");
> > + clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> > + return 0;
> > + }
> > +
> > /*
> > * Clean DUMMY object calls EFI Runtime Service, set_variable(), so
> > * it should be invoked only after efi_rts_workqueue is ready.
> > diff --git a/drivers/firmware/efi/runtime-wrappers.c
> > b/drivers/firmware/efi/runtime-wrappers.c
> > index ae54870b2788..5cdb787da5d3 100644
> > --- a/drivers/firmware/efi/runtime-wrappers.c
> > +++ b/drivers/firmware/efi/runtime-wrappers.c
> > @@ -1,6 +1,14 @@
> > /*
> > * runtime-wrappers.c - Runtime Services function call wrappers
> > *
> > + * Implementation summary:
> > + * ---
> > + * 1. When user/kernel thread requests to execute
> > + efi_runtime_service(),
> > + * enqueue work to efi_rts_workqueue.
> > + * 2. Caller thread waits until the work is finished because it's
> > + * dependent on the return status and execution of efi_runtime_service().
> > + * For instance, get_variable() and get_next_variable().
> > + *
> > * Copyright (C) 2014 Linaro Ltd.
> > *
> > * Split off from arch/x86/platform/efi/efi.c @@ -22,6 +30,8 @@
> > #include #include #include
> >
> > +#include
> > +
> > #include
> >
> > /*
> > @@ -33,6 +43,50 @@
> > #define __efi_call_virt(f, args...) \
> > __efi_call_virt_pointer(efi.systab->runtime, f, args)
> >
> > +/* Each EFI Runtime Service is represented with a unique number */
> > +#define GET_TIME 0
> > +#define SET_TIME 1
> > +#define GET_WAKEUP_TIME2
> > +#define SET_WAKEUP_TIME3
> > +#define GET_VARIABLE 4
> > +#define GET_NEXT_VARIABLE 5
> > +#define SET_VARIABLE 6
> > +#define SET_VARIABLE_NONBLOCKING 7
> > +#define QUERY_VARIABLE_INFO8
> > +#define QUERY_VARIABLE_INFO_NONBLOCKING9
> > +#define GET_NEXT_HIGH_MONO_COUNT 10
> > +#define RESET_SYSTEM 11
> > +#define UPDATE_CAPSULE 12
> > +#define QUERY_CAPSULE_CAPS 13
>
> An enum would be better, given these are just internal, contiguous IDs.
>
Makes sense.
> > +
> > +/*
> > + * 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. So,
> > +queue_work()
> > + * should never fail.
> > + */
> > +#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5)
> > \
> > +({ \
> > + struct efi_runtime_work efi_rts_work; \
> > + efi_rts_work.status = EFI_ABORTED; \
> > + \
> > + INIT_WORK_ONSTACK(&efi_rts_work.work, efi_call_rts);\
> > + efi_rts_work.func = _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; \
> > + if (queue_work(efi_rts_wq, &efi_rts_work.work))
Re: [PATCH V1 2/3] efi: Introduce efi_rts_workqueue and necessary infrastructure to invoke all efi_runtime_services()
On Sat, Feb 24, 2018 at 11:10 PM, Sai Praneeth Prakhya
wrote:
> From: Sai Praneeth
>
> When a process requests the kernel to execute any efi_runtime_service(),
> the requested efi_runtime_service (represented as an identifier) and its
> arguments are packed into a struct named efi_runtime_work and queued
> onto work queue named efi_rts_wq. The caller then waits until the work
> is completed.
>
> Introduce necessary infrastructure:
> 1. Creating workqueue named efi_rts_wq
> 2. A macro (efi_queue_work()) that
> a. populates efi_runtime_work
> b. queues work onto efi_rts_wq and
> c. waits until worker thread returns
> 3. A handler function that
> a. understands efi_runtime_work and
> b. invokes the appropriate efi_runtime_service() with the
> appropriate arguments
>
> The caller thread has to wait until the worker thread returns, because
> it's dependent on the return status of efi_runtime_service() and, in
> specific cases, the arguments populated by efi_runtime_service(). Some
> efi_runtime_services() takes a pointer to buffer as an argument and
> fills up the buffer with requested data. For instance,
> efi_get_variable() and efi_get_next_variable(). Hence, caller process
> cannot just post the work and get going.
>
> Some facts about efi_runtime_services():
> 1. A quick look at all the efi_runtime_services() shows that any
> efi_runtime_service() has five or less arguments.
> 2. An argument of efi_runtime_service() can be a value (of any type)
> or a pointer (of any type).
> Hence, efi_runtime_work has five void pointers to store these 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.
>
> Signed-off-by: Sai Praneeth Prakhya
> Suggested-by: Andy Lutomirski
> Cc: Lee, Chun-Yi
> Cc: Borislav Petkov
> Cc: Tony Luck
> Cc: Will Deacon
> Cc: Dave Hansen
> Cc: Mark Rutland
> Cc: Bhupesh Sharma
> Cc: Ricardo Neri
> Cc: Ravi Shankar
> Cc: Matt Fleming
> Cc: Peter Zijlstra
> Cc: Ard Biesheuvel
> Cc: Dan Williams
> ---
> drivers/firmware/efi/efi.c | 11 +++
> drivers/firmware/efi/runtime-wrappers.c | 143
>
> include/linux/efi.h | 23 +
> 3 files changed, 177 insertions(+)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index ac5db5f8dbbf..4714b305ca90 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -76,6 +76,8 @@ static unsigned long *efi_tables[] = {
> &efi.mem_attr_table,
> };
>
> +struct workqueue_struct *efi_rts_wq;
> +
> static bool disable_runtime;
> static int __init setup_noefi(char *arg)
> {
> @@ -329,6 +331,15 @@ static int __init efisubsys_init(void)
> if (!efi_enabled(EFI_BOOT))
> return 0;
>
> + /* Create a work queue to run EFI Runtime Services */
> + efi_rts_wq = create_workqueue("efi_rts_workqueue");
Please consider alloc_workqueue() instead with the appropriate flags,
since create_workqueue() and friends are deprecated.
> + if (!efi_rts_wq) {
> + pr_err("Failed to create efi_rts_workqueue, EFI runtime
> services "
> + "disabled.\n");
> + clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> + return 0;
> + }
> +
> /*
> * Clean DUMMY object calls EFI Runtime Service, set_variable(), so
> * it should be invoked only after efi_rts_workqueue is ready.
> diff --git a/drivers/firmware/efi/runtime-wrappers.c
> b/drivers/firmware/efi/runtime-wrappers.c
> index ae54870b2788..5cdb787da5d3 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -1,6 +1,14 @@
> /*
> * runtime-wrappers.c - Runtime Services function call wrappers
> *
> + * Implementation summary:
> + * ---
> + * 1. When user/kernel thread requests to execute efi_runtime_service(),
> + * enqueue work to efi_rts_workqueue.
> + * 2. Caller thread waits until the work is finished because it's
> + * dependent on the return status and execution of efi_runtime_service().
> + * For instance, get_variable() and get_next_variable().
> + *
> * Copyright (C) 2014 Linaro Ltd.
> *
> * Split off from arch/x86/platform/efi/efi.c
> @@ -22,6 +30,8 @@
> #include
> #include
> #include
> +#include
> +
> #include
>
> /*
> @@ -33,6 +43,50 @@
> #define __efi_call_virt(f, args...) \
> __efi_call_virt_pointer(efi.systab->runtime, f, args)
>
> +/* Each EFI Runtime Service is represented with a unique number */
> +#define GET_TIME 0
> +#define SET_TIME 1
> +#define GET_WAKEUP_TIME2
> +#define SET_W
[PATCH V1 2/3] efi: Introduce efi_rts_workqueue and necessary infrastructure to invoke all efi_runtime_services()
From: Sai Praneeth
When a process requests the kernel to execute any efi_runtime_service(),
the requested efi_runtime_service (represented as an identifier) and its
arguments are packed into a struct named efi_runtime_work and queued
onto work queue named efi_rts_wq. The caller then waits until the work
is completed.
Introduce necessary infrastructure:
1. Creating workqueue named efi_rts_wq
2. A macro (efi_queue_work()) that
a. populates efi_runtime_work
b. queues work onto efi_rts_wq and
c. waits until worker thread returns
3. A handler function that
a. understands efi_runtime_work and
b. invokes the appropriate efi_runtime_service() with the
appropriate arguments
The caller thread has to wait until the worker thread returns, because
it's dependent on the return status of efi_runtime_service() and, in
specific cases, the arguments populated by efi_runtime_service(). Some
efi_runtime_services() takes a pointer to buffer as an argument and
fills up the buffer with requested data. For instance,
efi_get_variable() and efi_get_next_variable(). Hence, caller process
cannot just post the work and get going.
Some facts about efi_runtime_services():
1. A quick look at all the efi_runtime_services() shows that any
efi_runtime_service() has five or less arguments.
2. An argument of efi_runtime_service() can be a value (of any type)
or a pointer (of any type).
Hence, efi_runtime_work has five void pointers to store these 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.
Signed-off-by: Sai Praneeth Prakhya
Suggested-by: Andy Lutomirski
Cc: Lee, Chun-Yi
Cc: Borislav Petkov
Cc: Tony Luck
Cc: Will Deacon
Cc: Dave Hansen
Cc: Mark Rutland
Cc: Bhupesh Sharma
Cc: Ricardo Neri
Cc: Ravi Shankar
Cc: Matt Fleming
Cc: Peter Zijlstra
Cc: Ard Biesheuvel
Cc: Dan Williams
---
drivers/firmware/efi/efi.c | 11 +++
drivers/firmware/efi/runtime-wrappers.c | 143
include/linux/efi.h | 23 +
3 files changed, 177 insertions(+)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index ac5db5f8dbbf..4714b305ca90 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -76,6 +76,8 @@ static unsigned long *efi_tables[] = {
&efi.mem_attr_table,
};
+struct workqueue_struct *efi_rts_wq;
+
static bool disable_runtime;
static int __init setup_noefi(char *arg)
{
@@ -329,6 +331,15 @@ static int __init efisubsys_init(void)
if (!efi_enabled(EFI_BOOT))
return 0;
+ /* Create a work queue to run EFI Runtime Services */
+ efi_rts_wq = create_workqueue("efi_rts_workqueue");
+ if (!efi_rts_wq) {
+ pr_err("Failed to create efi_rts_workqueue, EFI runtime
services "
+ "disabled.\n");
+ clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+ return 0;
+ }
+
/*
* Clean DUMMY object calls EFI Runtime Service, set_variable(), so
* it should be invoked only after efi_rts_workqueue is ready.
diff --git a/drivers/firmware/efi/runtime-wrappers.c
b/drivers/firmware/efi/runtime-wrappers.c
index ae54870b2788..5cdb787da5d3 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -1,6 +1,14 @@
/*
* runtime-wrappers.c - Runtime Services function call wrappers
*
+ * Implementation summary:
+ * ---
+ * 1. When user/kernel thread requests to execute efi_runtime_service(),
+ * enqueue work to efi_rts_workqueue.
+ * 2. Caller thread waits until the work is finished because it's
+ * dependent on the return status and execution of efi_runtime_service().
+ * For instance, get_variable() and get_next_variable().
+ *
* Copyright (C) 2014 Linaro Ltd.
*
* Split off from arch/x86/platform/efi/efi.c
@@ -22,6 +30,8 @@
#include
#include
#include
+#include
+
#include
/*
@@ -33,6 +43,50 @@
#define __efi_call_virt(f, args...) \
__efi_call_virt_pointer(efi.systab->runtime, f, args)
+/* Each EFI Runtime Service is represented with a unique number */
+#define GET_TIME 0
+#define SET_TIME 1
+#define GET_WAKEUP_TIME2
+#define SET_WAKEUP_TIME3
+#define GET_VARIABLE 4
+#define GET_NEXT_VARIABLE 5
+#define SET_VARIABLE 6
+#define SET_VARIABLE_NONBLOCKING 7
+#define QUERY_VARIABLE_INFO8
+#define QUERY_VARIABLE_INFO_NONBLOCKING9
+#define GET_NEXT_HIGH_MON
