linux-kernel - Re: [PATCH] x86-64: use EFI to deal with platform wall clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <4FB3B734020000780008415D@nat28.tlf.novell.com>
Date:	Wed, 16 May 2012 13:18:28 +0100
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"Matthew Garrett" <mjg59@...f.ucam.org>
Cc:	<mingo@...e.hu>, <tglx@...utronix.de>,
	<matt.fleming@...ux.intel.com>, <linux-kernel@...r.kernel.org>,
	<hpa@...or.com>
Subject: Re: [PATCH] x86-64: use EFI to deal with platform wall clock

>>> On 15.05.12 at 15:20, Matthew Garrett <mjg59@...f.ucam.org> wrote:
> On Tue, May 15, 2012 at 02:19:07PM +0100, Jan Beulich wrote:
> 
>> I would have expected that things work that way, but they
>> don't. In particular is the function in efi_64.c that's being
>> modified here called from efi_call_phys_{pro,epi}log(), and at
>> that point we can't expect virtual addresses to be uniformly
>> set yet. So it's a physical call that requires the fixup done
>> here, as efi_set_executable() simply expects ->virt_addr to
>> be valid. I suspect that no physical calls other than
>> phys_efi_set_virtual_address_map() were being done so far
>> at all on 64-bit, hiding the problem.
> 
> In that case we need to split the mapping code into two chunks and 
> configure the memory map earlier. You can't depend on __va() doing 
> anything useful on runtime addresses.

Okay, looks like calling efi_ioremap() at this point is possible.
But why is efi_enter_virtual_mode() being called as late as is
the case currently for x86 anyway?

And then again the current logic in efi_enter_virtual_mode()
looks flawed (it assumes two contiguous pieces of direct
mappings, and while on systems with dis-contiguous physical
memory this currently appears to be true, it's not correct - the
holes could have MMIO assignments in them - and hence
shouldn't be relied upon), and I wouldn't want to copy this
elsewhere.

Similarly for efi_ioremap() itself - it neither honors the
cacheability requested by the firmware (calling
set_memory_uc() subsequently isn't sufficient, as the already
established mappings may be used in prefetches already),
nor does its self-recursion look very reliable (why would
init_memory_mapping() do any better on a second call,
and the code doesn't deal with last_map_pfn pointing below
or precisely at phys_addr at all).

Plus the use of set_virtual_address_map is bogus in the first
place, as it makes it impossible for a kexec-ed kernel to also
use EFI services (as it would have to call the function a
second and possibly third time, yet it is not permitted to be
called more than once). Imo all calls have to happen in
physical mode.

So I'm afraid if the patch as I provided it isn't acceptable, and
if the call to efi_enter_virtual_mode() can't be moved ahead
of the one to timekeeping_init(), this winds down to the whole
logic needing a re-write.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/