diff mbox

[02/14] x86-64/efi: Use EFI to deal with platform wall clock (again)

Message ID 1387448416-11672-1-git-send-email-jlee@suse.com
State Accepted
Headers show

Commit Message

Lee, Chun-Yi Dec. 19, 2013, 10:20 a.m. UTC
From: Jan Beulich <JBeulich@suse.com>

Other than ix86, x86-64 on EFI so far didn't set the
{g,s}et_wallclock accessors to the EFI routines, thus
incorrectly using raw RTC accesses instead.

Simply removing the #ifdef around the respective code isn't
enough, however: While so far early get-time calls were done in
physical mode, this doesn't work properly for x86-64, as virtual
addresses would still need to be set up for all runtime regions
(which wasn't the case on the system I have access to), so
instead the patch moves the call to efi_enter_virtual_mode()
ahead (which in turn allows to drop all code related to calling
efi-get-time in physical mode).

Additionally the earlier calling of efi_set_executable()
requires the CPA code to cope, i.e. during early boot it must be
avoided to call cpa_flush_array(), as the first thing this
function does is a BUG_ON(irqs_disabled()).

Also make the two EFI functions in question here static -
they're not being referenced elsewhere.

History:

    This commit was originally merged as bacef661acdb ("x86-64/efi:
    Use EFI to deal with platform wall clock") but it resulted in some
    ASUS machines no longer booting due to a firmware bug, and so was
    reverted in f026cfa82f62.

    Then a pre-emptive fix for the buggy ASUS firmware was merged in
    03a1c254975e ("x86, efi: 1:1 pagetable mapping for virtual EFI
    calls") but it causes odd bootup problems on x86-64. So this patch
    revoked again by 11520e5e7c1.

    Now Borislav Petkov's "EFI runtime services virtual mapping" is
    merged to EFI 'next' branch. So this patch can be reapplied again.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Matt Fleming <matt.fleming@intel.com>
Acked-by: Matthew Garrett <mjg@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com> [added commit history]
Acked-by: Lee, Chun-Yi <jlee@suse.com>
---
 arch/x86/mm/pageattr.c      |   10 ++++++----
 arch/x86/platform/efi/efi.c |   35 +++++++++++------------------------
 include/linux/efi.h         |    2 --
 init/main.c                 |    8 ++++----
 4 files changed, 21 insertions(+), 34 deletions(-)

Comments

Matt Fleming Dec. 19, 2013, 10:49 a.m. UTC | #1
On Thu, 19 Dec, at 06:20:16PM, Lee, Chun-Yi wrote:
> From: Jan Beulich <JBeulich@suse.com>
> 
> Other than ix86, x86-64 on EFI so far didn't set the
> {g,s}et_wallclock accessors to the EFI routines, thus
> incorrectly using raw RTC accesses instead.
> 
> Simply removing the #ifdef around the respective code isn't
> enough, however: While so far early get-time calls were done in
> physical mode, this doesn't work properly for x86-64, as virtual
> addresses would still need to be set up for all runtime regions
> (which wasn't the case on the system I have access to), so
> instead the patch moves the call to efi_enter_virtual_mode()
> ahead (which in turn allows to drop all code related to calling
> efi-get-time in physical mode).
> 
> Additionally the earlier calling of efi_set_executable()
> requires the CPA code to cope, i.e. during early boot it must be
> avoided to call cpa_flush_array(), as the first thing this
> function does is a BUG_ON(irqs_disabled()).
> 
> Also make the two EFI functions in question here static -
> they're not being referenced elsewhere.
> 
> History:
> 
>     This commit was originally merged as bacef661acdb ("x86-64/efi:
>     Use EFI to deal with platform wall clock") but it resulted in some
>     ASUS machines no longer booting due to a firmware bug, and so was
>     reverted in f026cfa82f62.
> 
>     Then a pre-emptive fix for the buggy ASUS firmware was merged in
>     03a1c254975e ("x86, efi: 1:1 pagetable mapping for virtual EFI
>     calls") but it causes odd bootup problems on x86-64. So this patch
>     revoked again by 11520e5e7c1.
> 
>     Now Borislav Petkov's "EFI runtime services virtual mapping" is
>     merged to EFI 'next' branch. So this patch can be reapplied again.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Tested-by: Matt Fleming <matt.fleming@intel.com>
> Acked-by: Matthew Garrett <mjg@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com> [added commit history]
> Acked-by: Lee, Chun-Yi <jlee@suse.com>
> ---
>  arch/x86/mm/pageattr.c      |   10 ++++++----
>  arch/x86/platform/efi/efi.c |   35 +++++++++++------------------------
>  include/linux/efi.h         |    2 --
>  init/main.c                 |    8 ++++----
>  4 files changed, 21 insertions(+), 34 deletions(-)

Lee, you can't just simply resend this patch with all the tags - I
haven't tested this version with any recent changes and I'm pretty sure
Matthew isn't going to Ack it.

Do you know if anyone has tested this patch with Borislav's recent
changes?
joeyli Dec. 19, 2013, 1:32 p.m. UTC | #2
於 四,2013-12-19 於 10:49 +0000,Matt Fleming 提到:
> On Thu, 19 Dec, at 06:20:16PM, Lee, Chun-Yi wrote:
> > From: Jan Beulich <JBeulich@suse.com>
> > 
> > Other than ix86, x86-64 on EFI so far didn't set the
> > {g,s}et_wallclock accessors to the EFI routines, thus
> > incorrectly using raw RTC accesses instead.
> > 
> > Simply removing the #ifdef around the respective code isn't
> > enough, however: While so far early get-time calls were done in
> > physical mode, this doesn't work properly for x86-64, as virtual
> > addresses would still need to be set up for all runtime regions
> > (which wasn't the case on the system I have access to), so
> > instead the patch moves the call to efi_enter_virtual_mode()
> > ahead (which in turn allows to drop all code related to calling
> > efi-get-time in physical mode).
> > 
> > Additionally the earlier calling of efi_set_executable()
> > requires the CPA code to cope, i.e. during early boot it must be
> > avoided to call cpa_flush_array(), as the first thing this
> > function does is a BUG_ON(irqs_disabled()).
> > 
> > Also make the two EFI functions in question here static -
> > they're not being referenced elsewhere.
> > 
> > History:
> > 
> >     This commit was originally merged as bacef661acdb ("x86-64/efi:
> >     Use EFI to deal with platform wall clock") but it resulted in some
> >     ASUS machines no longer booting due to a firmware bug, and so was
> >     reverted in f026cfa82f62.
> > 
> >     Then a pre-emptive fix for the buggy ASUS firmware was merged in
> >     03a1c254975e ("x86, efi: 1:1 pagetable mapping for virtual EFI
> >     calls") but it causes odd bootup problems on x86-64. So this patch
> >     revoked again by 11520e5e7c1.
> > 
> >     Now Borislav Petkov's "EFI runtime services virtual mapping" is
> >     merged to EFI 'next' branch. So this patch can be reapplied again.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > Tested-by: Matt Fleming <matt.fleming@intel.com>
> > Acked-by: Matthew Garrett <mjg@redhat.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Signed-off-by: Matt Fleming <matt.fleming@intel.com> [added commit history]
> > Acked-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  arch/x86/mm/pageattr.c      |   10 ++++++----
> >  arch/x86/platform/efi/efi.c |   35 +++++++++++------------------------
> >  include/linux/efi.h         |    2 --
> >  init/main.c                 |    8 ++++----
> >  4 files changed, 21 insertions(+), 34 deletions(-)
> 
> Lee, you can't just simply resend this patch with all the tags - I
> haven't tested this version with any recent changes and I'm pretty sure
> Matthew isn't going to Ack it.

I am very sorry for I didn't remove those tags before send it. I will
remove it when send second version.

> 
> Do you know if anyone has tested this patch with Borislav's recent
> changes?
> 

I tested Borislav's patch set on a issue BIOS and make sure it works,
but I have no chance to test this patch with it. I will find a time to
test it.


Thanks a lot!
Joey Lee
Alan Cox Dec. 20, 2013, 11:29 a.m. UTC | #3
On Thu, 19 Dec 2013 21:32:19 +0800
joeyli <jlee@suse.com> wrote:

> 於 四,2013-12-19 於 10:49 +0000,Matt Fleming 提到:
> > On Thu, 19 Dec, at 06:20:16PM, Lee, Chun-Yi wrote:
> > > From: Jan Beulich <JBeulich@suse.com>
> > > 
> > > Other than ix86, x86-64 on EFI so far didn't set the
> > > {g,s}et_wallclock accessors to the EFI routines, thus
> > > incorrectly using raw RTC accesses instead.

Probably wise.

Before anyone reverts that please make sure it also works on EFI32,
because right now we crash and burn on multiple EFI32 tablets querying
the time.

Alan
H. Peter Anvin Dec. 23, 2013, 11:25 p.m. UTC | #4
On 12/20/2013 03:29 AM, One Thousand Gnomes wrote:
> On Thu, 19 Dec 2013 21:32:19 +0800
> joeyli <jlee@suse.com> wrote:
> 
>> 於 四,2013-12-19 於 10:49 +0000,Matt Fleming 提到:
>>> On Thu, 19 Dec, at 06:20:16PM, Lee, Chun-Yi wrote:
>>>> From: Jan Beulich <JBeulich@suse.com>
>>>>
>>>> Other than ix86, x86-64 on EFI so far didn't set the
>>>> {g,s}et_wallclock accessors to the EFI routines, thus
>>>> incorrectly using raw RTC accesses instead.
> 
> Probably wise.
> 
> Before anyone reverts that please make sure it also works on EFI32,
> because right now we crash and burn on multiple EFI32 tablets querying
> the time.
> 

Quite -- we in fact did just the opposite (checkin 04bf9ba720fc).

	-hpa
diff mbox

Patch

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index bb32480..fd5545e 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -983,11 +983,13 @@  static int change_page_attr_set_clr(unsigned long *addr, int numpages,
 
 	/*
 	 * On success we use clflush, when the CPU supports it to
-	 * avoid the wbindv. If the CPU does not support it and in the
-	 * error case we fall back to cpa_flush_all (which uses
-	 * wbindv):
+	 * avoid the wbindv. If the CPU does not support it, in the
+	 * error case, and during early boot (for EFI) we fall back
+	 * to cpa_flush_all (which uses wbinvd):
 	 */
-	if (!ret && cpu_has_clflush) {
+	if (unlikely(early_boot_irqs_disabled))
+		__cpa_flush_all((void *)(long)cache);
+	else if (!ret && cpu_has_clflush) {
 		if (cpa.flags & (CPA_PAGES_ARRAY | CPA_ARRAY)) {
 			cpa_flush_array(addr, numpages, cache,
 					cpa.flags, pages);
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index cceb813..7a7a692 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -253,22 +253,7 @@  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)
+static int efi_set_rtc_mmss(const struct timespec *now)
 {
 	unsigned long nowtime = now->tv_sec;
 	efi_status_t 	status;
@@ -305,7 +290,7 @@  int efi_set_rtc_mmss(const struct timespec *now)
 	return 0;
 }
 
-void efi_get_time(struct timespec *now)
+static void efi_get_time(struct timespec *now)
 {
 	efi_status_t status;
 	efi_time_t eft;
@@ -592,18 +577,13 @@  static int __init efi_runtime_init(void)
 	}
 	/*
 	 * We will only need *early* access to the following
-	 * two EFI runtime services before set_virtual_address_map
+	 * EFI runtime service before set_virtual_address_map
 	 * is invoked.
 	 */
-	efi_phys.get_time = (efi_get_time_t *)runtime->get_time;
 	efi_phys.set_virtual_address_map =
 		(efi_set_virtual_address_map_t *)
 		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_t));
 
 	return 0;
@@ -690,6 +670,13 @@  void __init efi_init(void)
 
 	set_bit(EFI_MEMMAP, &x86_efi_facility);
 
+#ifdef CONFIG_X86_64
+	if (efi_is_native()) {
+		x86_platform.get_wallclock = efi_get_time;
+		x86_platform.set_wallclock = efi_set_rtc_mmss;
+	}
+#endif
+
 #if EFI_DEBUG
 	print_efi_memmap();
 #endif
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 11ce678..fc8fa0e 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -613,8 +613,6 @@  extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size);
 extern int __init efi_uart_console_only (void);
 extern void efi_initialize_iomem_resources(struct resource *code_resource,
 		struct resource *data_resource, struct resource *bss_resource);
-extern void efi_get_time(struct timespec *now);
-extern int efi_set_rtc_mmss(const struct timespec *now);
 extern void efi_reserve_boot_services(void);
 extern struct efi_memory_map memmap;
 
diff --git a/init/main.c b/init/main.c
index febc511..61164ce 100644
--- a/init/main.c
+++ b/init/main.c
@@ -478,6 +478,10 @@  static void __init mm_init(void)
 	percpu_init_late();
 	pgtable_cache_init();
 	vmalloc_init();
+#ifdef CONFIG_X86
+	if (efi_enabled(EFI_RUNTIME_SERVICES))
+		efi_enter_virtual_mode();
+#endif
 }
 
 asmlinkage void __init start_kernel(void)
@@ -615,10 +619,6 @@  asmlinkage void __init start_kernel(void)
 	calibrate_delay();
 	pidmap_init();
 	anon_vma_init();
-#ifdef CONFIG_X86
-	if (efi_enabled(EFI_RUNTIME_SERVICES))
-		efi_enter_virtual_mode();
-#endif
 	thread_info_cache_init();
 	cred_init();
 	fork_init(totalram_pages);