2020-03-17 11:00:02

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 4.19 64/89] efi: Make efi_rts_work accessible to efi page fault handler

From: Sai Praneeth <[email protected]>

commit 9dbbedaa6171247c4c7c40b83f05b200a117c2e0 upstream.

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 <[email protected]>
Suggested-by: Matt Fleming <[email protected]>
Based-on-code-from: Ricardo Neri <[email protected]>
Signed-off-by: Sai Praneeth Prakhya <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Fixes: 3eb420e70d87 (“efi: Use a work queue to invoke EFI Runtime Services”)
Signed-off-by: Wen Yang <[email protected]>
Cc: Caspar Zhang <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
drivers/firmware/efi/runtime-wrappers.c | 53 +++++---------------------------
include/linux/efi.h | 36 +++++++++++++++++++++
2 files changed, 45 insertions(+), 44 deletions(-)

--- 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); \
@@ -191,18 +158,16 @@ extern struct semaphore __efi_uv_runtime
*/
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);
@@ -260,8 +225,8 @@ static void efi_call_rts(struct work_str
*/
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)
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1664,6 +1664,42 @@ 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;
+};
+
+extern struct efi_runtime_work efi_rts_work;
+
/* Workqueue to queue EFI Runtime Services */
extern struct workqueue_struct *efi_rts_wq;




2020-03-19 09:43:51

by Chris Wilson

[permalink] [raw]
Subject: Re: [PATCH 4.19 64/89] efi: Make efi_rts_work accessible to efi page fault handler

Quoting Greg Kroah-Hartman (2020-03-17 10:55:13)
> From: Sai Praneeth <[email protected]>
>
> commit 9dbbedaa6171247c4c7c40b83f05b200a117c2e0 upstream.
>
> 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 <[email protected]>
> Suggested-by: Matt Fleming <[email protected]>
> Based-on-code-from: Ricardo Neri <[email protected]>
> Signed-off-by: Sai Praneeth Prakhya <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Fixes: 3eb420e70d87 (“efi: Use a work queue to invoke EFI Runtime Services”)
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Caspar Zhang <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>

This requires the fix from

commit ef1491e791308317bb9851a0ad380c4a68b58d54
Author: Waiman Long <[email protected]>
Date: Wed Nov 14 09:55:40 2018 -0800

efi: Fix debugobjects warning on 'efi_rts_work'

The following commit:

9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi page fault handler")

converted 'efi_rts_work' from an auto variable to a global variable.
However, when submitting the work, INIT_WORK_ONSTACK() was still used,
causing the following complaint from debugobjects:

ODEBUG: object 00000000ed27b500 is NOT on stack 00000000c7d38760, but annotated.

Change the macro to just INIT_WORK() to eliminate the warning.

Signed-off-by: Waiman Long <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Sai Praneeth Prakhya <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Fixes: 9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi page fault handler")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>

2020-03-19 10:05:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4.19 64/89] efi: Make efi_rts_work accessible to efi page fault handler

On Thu, Mar 19, 2020 at 09:42:10AM +0000, Chris Wilson wrote:
> Quoting Greg Kroah-Hartman (2020-03-17 10:55:13)
> > From: Sai Praneeth <[email protected]>
> >
> > commit 9dbbedaa6171247c4c7c40b83f05b200a117c2e0 upstream.
> >
> > 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 <[email protected]>
> > Suggested-by: Matt Fleming <[email protected]>
> > Based-on-code-from: Ricardo Neri <[email protected]>
> > Signed-off-by: Sai Praneeth Prakhya <[email protected]>
> > Signed-off-by: Ard Biesheuvel <[email protected]>
> > Fixes: 3eb420e70d87 (“efi: Use a work queue to invoke EFI Runtime Services”)
> > Signed-off-by: Wen Yang <[email protected]>
> > Cc: Caspar Zhang <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
> This requires the fix from
>
> commit ef1491e791308317bb9851a0ad380c4a68b58d54
> Author: Waiman Long <[email protected]>
> Date: Wed Nov 14 09:55:40 2018 -0800
>
> efi: Fix debugobjects warning on 'efi_rts_work'
>
> The following commit:
>
> 9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi page fault handler")
>
> converted 'efi_rts_work' from an auto variable to a global variable.
> However, when submitting the work, INIT_WORK_ONSTACK() was still used,
> causing the following complaint from debugobjects:
>
> ODEBUG: object 00000000ed27b500 is NOT on stack 00000000c7d38760, but annotated.
>
> Change the macro to just INIT_WORK() to eliminate the warning.
>
> Signed-off-by: Waiman Long <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Acked-by: Sai Praneeth Prakhya <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> Fixes: 9dbbedaa6171 ("efi: Make efi_rts_work accessible to efi page fault handler")
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>

Thanks for this, now queued up.

greg k-h
>