[PATCH] UEFI: Don't use UEFI time services on 32-bit

[PATCH] UEFI: Don't use UEFI time services on 32-bit

[Matthew Garrett]

UEFI time services are often broken once we're in virtual mode. We were
already refusing to use them on 64-bit systems, but it turns out that
they're also broken on some 32-bit firmware, including the Dell Venue.
Disable them for now, we can revisit once we have the 1:1 mappings code
incorporated.

Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
---
 arch/x86/platform/efi/efi.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 92c0234..cceb813 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -690,13 +690,6 @@ void __init efi_init(void)
 
 	set_bit(EFI_MEMMAP, &x86_efi_facility);
 
-#ifdef CONFIG_X86_32
-	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
-- 
1.8.4.2

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[joeyli]

Hi Matthew, 

於 五，2013-11-29 於 14:44 -0500，Matthew Garrett 提到：
> 
> UEFI time services are often broken once we're in virtual mode. We
> were
> already refusing to use them on 64-bit systems, but it turns out that
> they're also broken on some 32-bit firmware, including the Dell Venue.
> Disable them for now, we can revisit once we have the 1:1 mappings
> code
> incorporated.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org 

Does there have chance to test Borislav's EFI runtime mapping work on
Dell Venue?

Borislav's patches here:
git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#efi

I am sure his patchset can walkaround physical address accessing issue
on a broken 64-bit machine. But, I didn't test it on 32-bit issue
machine.


Thanks a lot!
Joey Lee

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[Borislav Petkov]

On Sat, Nov 30, 2013 at 06:04:07AM +0800, joeyli wrote:
> I am sure his patchset can walkaround physical address accessing issue
> on a broken 64-bit machine. But, I didn't test it on 32-bit issue
> machine.

Yeah, my patches don't change 32-bit so I wouldn't expect any
difference, unfortunately.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[Matt Fleming]

On Sat, 30 Nov, at 06:04:07AM, joeyli wrote:
> Does there have chance to test Borislav's EFI runtime mapping work on
> Dell Venue?
> 
> Borislav's patches here:
> git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git#efi
> 
> I am sure his patchset can walkaround physical address accessing issue
> on a broken 64-bit machine. But, I didn't test it on 32-bit issue
> machine.

Unfortunately there's no guarantee that once you fix the physical
accesses the runtime services will work. I've got a 64-bit box that
returns vendor-specific error codes once physical address accesses are
handled.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[H. Peter Anvin]

On 11/29/2013 11:44 AM, Matthew Garrett wrote:
> UEFI time services are often broken once we're in virtual mode. We were
> already refusing to use them on 64-bit systems, but it turns out that
> they're also broken on some 32-bit firmware, including the Dell Venue.
> Disable them for now, we can revisit once we have the 1:1 mappings code
> incorporated.
> 
> Signed-off-by: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
> Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> ---
>  arch/x86/platform/efi/efi.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 92c0234..cceb813 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -690,13 +690,6 @@ void __init efi_init(void)
>  
>  	set_bit(EFI_MEMMAP, &x86_efi_facility);
>  
> -#ifdef CONFIG_X86_32
> -	if (efi_is_native()) {
> -		x86_platform.get_wallclock = efi_get_time;
> -		x86_platform.set_wallclock = efi_set_rtc_mmss;
> -	}
> -#endif
> -

Does the Dell Venue have either an ACPI TAD or one of the PNP0B0x
devices exposed?

	-hpa

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[H. Peter Anvin]

On 11/29/2013 05:45 PM, H. Peter Anvin wrote:
> 
> Does the Dell Venue have either an ACPI TAD or one of the PNP0B0x
> devices exposed?
> 

Ping on this?

Either way, I maintain what I have said in the past:

Unless we find evidence to the contrary, we should probably do:

ACPI TAD > PNP0B0x > EFI > hard probing

It might even be that the EFI can't be trusted enough to be in the list
at all.

	-hpa

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[Matthew Garrett]

On Tue, 2013-12-10 at 14:54 -0800, H. Peter Anvin wrote:

> Ping on this?

No TAD, yes ACPI declaration for RTC.

> Unless we find evidence to the contrary, we should probably do:
> 
> ACPI TAD > PNP0B0x > EFI > hard probing

EFI is preferable to PNP in that it gives us a timezone, and Windows
certainly calls the EFI time functions. *However*, it doesn't appear to
do so once the system is booted. So we probably want to call it in the
boot stub and find some way to pass the timezone information up to the
kernel, and then spend some more time instrumenting Windows to figure
out how it makes time calls.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[H. Peter Anvin]

On 12/10/2013 02:58 PM, Matthew Garrett wrote:
> On Tue, 2013-12-10 at 14:54 -0800, H. Peter Anvin wrote:
> 
>> Ping on this?
> 
> No TAD, yes ACPI declaration for RTC.

OK, that is unfortunate.

>> Unless we find evidence to the contrary, we should probably do:
>>
>> ACPI TAD > PNP0B0x > EFI > hard probing
> 
> EFI is preferable to PNP in that it gives us a timezone, and Windows
> certainly calls the EFI time functions. *However*, it doesn't appear to
> do so once the system is booted. So we probably want to call it in the
> boot stub and find some way to pass the timezone information up to the
> kernel, and then spend some more time instrumenting Windows to figure
> out how it makes time calls.

TAD would also give us the timezone.  I'm not sure how you can
realistically only use the time function during boot, however, unless
you inherently assume it is coherent with the hardware RTC, since you
wouldn't be able to set it.

I suspect that means that Windows doesn't set the timezone information,
which is highly unfortunate since Windows is the main offender in not
using UTC time.

	-hpa

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[Matthew Garrett]

On Tue, 2013-12-10 at 15:20 -0800, H. Peter Anvin wrote:
> On 12/10/2013 02:58 PM, Matthew Garrett wrote:
> > EFI is preferable to PNP in that it gives us a timezone, and Windows
> > certainly calls the EFI time functions. *However*, it doesn't appear to
> > do so once the system is booted. So we probably want to call it in the
> > boot stub and find some way to pass the timezone information up to the
> > kernel, and then spend some more time instrumenting Windows to figure
> > out how it makes time calls.
> 
> TAD would also give us the timezone.  I'm not sure how you can
> realistically only use the time function during boot, however, unless
> you inherently assume it is coherent with the hardware RTC, since you
> wouldn't be able to set it.

If we can verify that Windows actually uses TAD then I'd agree it's
preferable to EFI, but if we can't then I wouldn't be so sure. I'll redo
my Windows instrumentation and figure out under which circumstances it's
calling the time functions.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[H. Peter Anvin]

On 12/10/2013 03:24 PM, Matthew Garrett wrote:
>>
>> TAD would also give us the timezone.  I'm not sure how you can
>> realistically only use the time function during boot, however, unless
>> you inherently assume it is coherent with the hardware RTC, since you
>> wouldn't be able to set it.
> 
> If we can verify that Windows actually uses TAD then I'd agree it's
> preferable to EFI, but if we can't then I wouldn't be so sure. I'll redo
> my Windows instrumentation and figure out under which circumstances it's
> calling the time functions.
> 

Well, *which version* of Windows is a big question here.  Keep in mind
that the TAD was only introduced with ACPI 5 so it is quite a new
feature.  I would really like to see it implemented in SeaBIOS and/or
OVMF so we can test it out in Qemu.

	-hpa

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[joeyli]

於 二，2013-12-10 於 15:51 -0800，H. Peter Anvin 提到：
> On 12/10/2013 03:24 PM, Matthew Garrett wrote:
> >>
> >> TAD would also give us the timezone.  I'm not sure how you can
> >> realistically only use the time function during boot, however, unless
> >> you inherently assume it is coherent with the hardware RTC, since you
> >> wouldn't be able to set it.
> > 
> > If we can verify that Windows actually uses TAD then I'd agree it's
> > preferable to EFI, but if we can't then I wouldn't be so sure. I'll redo
> > my Windows instrumentation and figure out under which circumstances it's
> > calling the time functions.
> > 
> 
> Well, *which version* of Windows is a big question here.  Keep in mind
> that the TAD was only introduced with ACPI 5 so it is quite a new
> feature.  I would really like to see it implemented in SeaBIOS and/or
> OVMF so we can test it out in Qemu.
> 
> 	-hpa
> 

Actually, I am working on the timezone support of ACPI TIME and EFI
TIME. My current implementation is using EFI time services to deal with
clock in RTC.

The ACPI TAD is a generic device that need parse DSDT for access it. The
DSDT parser is in subsystem initial stage. But in start_kernel, the
timekeeping_init() call wallclock functions to deal system clock with
RTC, it's too early for access ACPI TAD, so it still access CMOS
interface unless switch it to EFI services.

If we adapt to ACPI TAD, then how does system clock deal with RTC
through it when system boot?


Thanks
Joey Lee

Re: [PATCH] UEFI: Don't use UEFI time services on 32-bit

[H. Peter Anvin]

On 12/10/2013 04:20 PM, joeyli wrote:
> 
> Actually, I am working on the timezone support of ACPI TIME and EFI
> TIME. My current implementation is using EFI time services to deal with
> clock in RTC.
> 
> The ACPI TAD is a generic device that need parse DSDT for access it. The
> DSDT parser is in subsystem initial stage. But in start_kernel, the
> timekeeping_init() call wallclock functions to deal system clock with
> RTC, it's too early for access ACPI TAD, so it still access CMOS
> interface unless switch it to EFI services.
> 
> If we adapt to ACPI TAD, then how does system clock deal with RTC
> through it when system boot?
> 

I don't really have a good answer for that.  We could call the EFI
services from the boot stub, of course, and pass in an approximate time
(and perhaps the TSC value to go with it), but I'm not sure if that will
be adequate.

The other alternative would be to either parse ACPI earlier or
initialize wall time later.

	-hpa

[tip:x86/urgent] x86, efi: Don't use (U) EFI time services on 32 bit

[tip-bot for Matthew Garrett]

Commit-ID:  04bf9ba720fcc4fa313fa122b799ae0989b6cd50
Gitweb:     http://git.kernel.org/tip/04bf9ba720fcc4fa313fa122b799ae0989b6cd50
Author:     Matthew Garrett <matthew.garrett@nebula.com>
AuthorDate: Fri, 29 Nov 2013 14:44:43 -0500
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 10 Dec 2013 15:02:34 -0800

x86, efi: Don't use (U)EFI time services on 32 bit

UEFI time services are often broken once we're in virtual mode. We were
already refusing to use them on 64-bit systems, but it turns out that
they're also broken on some 32-bit firmware, including the Dell Venue.
Disable them for now, we can revisit once we have the 1:1 mappings code
incorporated.

Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
Link: http://lkml.kernel.org/r/1385754283-2464-1-git-send-email-matthew.garrett@nebula.com
Cc: <stable@vger.kernel.org>
Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/platform/efi/efi.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 92c0234..cceb813 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -690,13 +690,6 @@ void __init efi_init(void)
 
 	set_bit(EFI_MEMMAP, &x86_efi_facility);
 
-#ifdef CONFIG_X86_32
-	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