linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matt Fleming <matt@console-pimps.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	Matt Fleming <matt.fleming@intel.com>,
	x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Ingo Molnar <mingo@redhat.com>, Nathan Zimmer <nzimmer@sgi.com>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	kernel-janitors@vger.kernel.org, linux-efi@vger.kernel.org,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [patch] x86/efi: use GFP_ATOMIC under spin_lock
Date: Fri, 14 Mar 2014 23:02:01 +0000	[thread overview]
Message-ID: <20140314230201.GF5834@console-pimps.org> (raw)
In-Reply-To: <20140310104328.GG10262@console-pimps.org>

On Mon, 10 Mar, at 10:43:28AM, Matt Fleming wrote:
> 
> I think we're all pretty much agreed that at the very least
> phys_efi_get_time() can go in the bin. Whatever the outcome of this
> thread that'll happen. The question is whether it makes sense to rip out
> all of the time stuff in one go.

OK, I'm chickening out of doing the full delete for v3.15. How about
something like this instead?

---

>From 697c512d44415794396773533715dff45785c585 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming@intel.com>
Date: Thu, 13 Mar 2014 19:41:08 +0000
Subject: [PATCH] x86/efi: Rip out phys_efi_get_time()

Dan reported that phys_efi_get_time() is doing kmalloc(..., GFP_KERNEL)
under a spinlock which is very clearly a bug. Since phys_efi_get_time()
has no users let's just delete it instead of trying to fix it.

Note that since there are no users of phys_efi_get_time(), it is not
possible to actually trigger a GFP_KERNEL alloc under the spinlock.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Nathan Zimmer <nzimmer@sgi.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/platform/efi/efi.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 395decedfa2b..872214435e9d 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -256,21 +256,6 @@ static efi_status_t __init phys_efi_set_virtual_address_map(
 	return status;
 }
 
-static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
-					     efi_time_cap_t *tc)
-{
-	unsigned long flags;
-	efi_status_t status;
-
-	spin_lock_irqsave(&rtc_lock, flags);
-	efi_call_phys_prelog();
-	status = efi_call_phys2(efi_phys.get_time, virt_to_phys(tm),
-				virt_to_phys(tc));
-	efi_call_phys_epilog();
-	spin_unlock_irqrestore(&rtc_lock, flags);
-	return status;
-}
-
 int efi_set_rtc_mmss(const struct timespec *now)
 {
 	unsigned long nowtime = now->tv_sec;
@@ -604,16 +589,9 @@ static int __init efi_runtime_init32(void)
 	 * EFI runtime services before set_virtual_address_map
 	 * is invoked.
 	 */
-	efi_phys.get_time = (efi_get_time_t *)
-			(unsigned long)runtime->get_time;
 	efi_phys.set_virtual_address_map =
 			(efi_set_virtual_address_map_t *)
 			(unsigned long)runtime->set_virtual_address_map;
-	/*
-	 * Make efi_get_time can be called before entering
-	 * virtual mode.
-	 */
-	efi.get_time = phys_efi_get_time;
 	early_iounmap(runtime, sizeof(efi_runtime_services_32_t));
 
 	return 0;
@@ -635,16 +613,9 @@ static int __init efi_runtime_init64(void)
 	 * EFI runtime services before set_virtual_address_map
 	 * is invoked.
 	 */
-	efi_phys.get_time = (efi_get_time_t *)
-			(unsigned long)runtime->get_time;
 	efi_phys.set_virtual_address_map =
 			(efi_set_virtual_address_map_t *)
 			(unsigned long)runtime->set_virtual_address_map;
-	/*
-	 * Make efi_get_time can be called before entering
-	 * virtual mode.
-	 */
-	efi.get_time = phys_efi_get_time;
 	early_iounmap(runtime, sizeof(efi_runtime_services_64_t));
 
 	return 0;
-- 
1.8.5.3

-- 
Matt Fleming, Intel Open Source Technology Center

  parent reply	other threads:[~2014-03-14 23:02 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-07 11:20 [patch] x86/efi: use GFP_ATOMIC under spin_lock Dan Carpenter
     [not found] ` <20140307112055.GE2351-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2014-03-07 12:10   ` Ingo Molnar
     [not found]     ` <20140307121022.GA32575-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-03-07 12:25       ` Dan Carpenter
2014-03-09  6:48         ` Ingo Molnar
2014-03-09  7:14           ` Dan Carpenter
2014-03-09 16:20         ` Matt Fleming
2014-03-09 16:31           ` Matthew Garrett
     [not found]             ` <20140309163141.GA18824-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
2014-03-09 18:50               ` Matt Fleming
2014-03-09 19:00                 ` Matthew Garrett
     [not found]                   ` <20140309190053.GA29555-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
2014-03-10  9:10                     ` Matt Fleming
     [not found]                 ` <20140309185028.GB10262-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2014-03-10  7:27                   ` Jan Beulich
2014-03-10  7:37                     ` Ingo Molnar
2014-03-10  7:45                       ` Jan Beulich
2014-03-10  7:53                         ` Ingo Molnar
2014-03-10  8:22                           ` Jan Beulich
2014-03-10 10:43                             ` Matt Fleming
     [not found]                               ` <20140310104328.GG10262-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2014-03-10 11:05                                 ` Jan Beulich
     [not found]                                   ` <531DAA7602000078001224EF-ce6RLXgGx+vWGUEhTRrCg1aTQe2KTcn/@public.gmane.org>
2014-03-10 16:10                                     ` Matt Fleming
2014-03-14 23:02                               ` Matt Fleming [this message]
2014-03-10  9:12                     ` Matt Fleming
2014-03-10  7:26           ` Jan Beulich
2014-03-10 17:07           ` Dan Carpenter
2014-03-09  7:36   ` Dan Carpenter
2014-03-10 16:47 ` H. Peter Anvin

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=20140314230201.GF5834@console-pimps.org \
    --to=matt@console-pimps.org \
    --cc=JBeulich@suse.com \
    --cc=dan.carpenter@oracle.com \
    --cc=hpa@zytor.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=matt.fleming@intel.com \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=nzimmer@sgi.com \
    --cc=tglx@linutronix.de \
    --cc=x86@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).