Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Sasha Levin <sashal@kernel.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable <stable@vger.kernel.org>,
	Sai Praneeth <sai.praneeth.prakhya@intel.com>,
	Bhupesh Sharma <bhsharma@redhat.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	linux-efi <linux-efi@vger.kernel.org>
Subject: Re: [PATCH AUTOSEL 4.19 134/191] efi: Make efi_rts_work accessible to efi page fault handler
Date: Sun, 10 Nov 2019 08:35:40 +0100
Message-ID: <CAKv+Gu-Ej9sjNpr4Xwhfy1d__mWbGXn6e7Q0YMQ5JT2SeDLPAQ@mail.gmail.com> (raw)
In-Reply-To: <20191110024013.29782-134-sashal@kernel.org>

On Sun, 10 Nov 2019 at 03:44, Sasha Levin <sashal@kernel.org> wrote:
>
> From: Sai Praneeth <sai.praneeth.prakhya@intel.com>
>
> [ Upstream commit 9dbbedaa6171247c4c7c40b83f05b200a117c2e0 ]
>
> 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>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

NAK

Would it be possible to disregard all EFI patches for -stable unless
they have a fixes tag? EFI subtly depends on lots of firmware quirks
across various architectures, and so grabbing random patches and
backporting them is really not a good idea in general.

> ---
>  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 1606abead22cc..b31e3d3729a6d 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);                    \
> @@ -191,18 +158,16 @@ extern struct semaphore __efi_uv_runtime_lock __alias(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);
> @@ -260,8 +225,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 f43fc61fbe2c9..9d4c25090fd04 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1666,6 +1666,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;
>
> --
> 2.20.1
>

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20191110024013.29782-1-sashal@kernel.org>
2019-11-10  2:39 ` [PATCH AUTOSEL 4.19 133/191] efi: honour memory reservations passed via a linux specific config table Sasha Levin
2019-11-10  7:33   ` Ard Biesheuvel
2019-11-10 13:27     ` Sasha Levin
2019-11-10 14:16       ` Ard Biesheuvel
2019-11-10 15:56         ` Sasha Levin
2019-11-10 16:26           ` Ard Biesheuvel
2019-11-10 18:08             ` Marc Zyngier
2019-11-10  2:39 ` [PATCH AUTOSEL 4.19 134/191] efi: Make efi_rts_work accessible to efi page fault handler Sasha Levin
2019-11-10  7:35   ` Ard Biesheuvel [this message]
2019-11-10  2:39 ` [PATCH AUTOSEL 4.19 135/191] efi/x86: Handle page faults occurring while running EFI runtime services Sasha Levin
2019-11-10  7:40   ` Ard Biesheuvel
2019-11-10  2:40 ` [PATCH AUTOSEL 4.19 191/191] x86/efi: fix a -Wtype-limits compilation warning Sasha Levin

Reply instructions:

You may reply publically 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=CAKv+Gu-Ej9sjNpr4Xwhfy1d__mWbGXn6e7Q0YMQ5JT2SeDLPAQ@mail.gmail.com \
    --to=ard.biesheuvel@linaro.org \
    --cc=bhsharma@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=sai.praneeth.prakhya@intel.com \
    --cc=sashal@kernel.org \
    --cc=stable@vger.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

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git