From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754959AbbI0HEE (ORCPT ); Sun, 27 Sep 2015 03:04:04 -0400 Received: from mail-wi0-f175.google.com ([209.85.212.175]:37202 "EHLO mail-wi0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753639AbbI0HEA (ORCPT ); Sun, 27 Sep 2015 03:04:00 -0400 Date: Sun, 27 Sep 2015 09:03:55 +0200 From: Ingo Molnar To: Matt Fleming Cc: Thomas Gleixner , "H. Peter Anvin" , Matt Fleming , linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org, "Lee, Chun-Yi" , Borislav Petkov , Leif Lindholm , Peter Jones , James Bottomley , Matthew Garrett , Dave Young , stable@vger.kernel.org, Ard Biesheuvel , Linus Torvalds , Borislav Petkov , Andy Lutomirski , Denys Vlasenko , Brian Gerst , Andrew Morton Subject: Re: [PATCH 1/2] x86/efi: Map EFI memmap entries in-order at runtime Message-ID: <20150927070355.GB26125@gmail.com> References: <1443218539-7610-1-git-send-email-matt@codeblueprint.co.uk> <1443218539-7610-2-git-send-email-matt@codeblueprint.co.uk> <20150926055643.GA25877@gmail.com> <20150926134329.GA3144@codeblueprint.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150926134329.GA3144@codeblueprint.co.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Matt Fleming wrote: > > So could we make the whole code obviously bottom-up? Such as first calculating > > the size of virtual memory needed, then allocating a _single_, obviously > > continuous mapping, and then doing a very clear in-order mapping within that > > window? That would remove any bitness and legacy dependencies. > > So, we could, and in fact the first version of this patch did just that. You can > find it here, > > https://lkml.kernel.org/r/1441372447-23439-1-git-send-email-matt@codeblueprint.co.uk > > But Ard suggested re-using the existing code and simply changing the order we > map the memmap entries in. Such implementational arguments (which are an internal cost) never trump compatibility and robustness concerns (which are an external constraint). > [...] And given the constraint for a small patch for backporting, I think it's a > better solution. [...] Ugh, backporting size is _even less_ of a valid argument when it comes to firmware support correctness! We can (perhaps) use these already existing patches as a simpler backport, but there's absolutely no reason to keep that code as a solution: > [...] The actual virtual addresses we pick are exactly the same with the two > patches. So I'm NAK-ing this for now: - The code is it reads today pretends to be an 'allocator'. It is _NOT_ an allocator, because all the sections have already been determined by the firmware, and, as we just learned the hard way, we do not want to deviate from that! There's nothing to 'allocate'! What these patches seem to implement is an elaborate 'allocator' that ends up doing nothing on 'new 64-bit' ... - The 32-bit and 64-bit and 'old_mmap' asymmetries: if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) { seem fragile and nonsensical. The question is: is it possible for the whole EFI image to be larger than a couple of megabytes? If not then 32-bit should just mirror the firmware layout as well, and if EFI_OLD_MEMMAP does anything differently from this _obvious_ 1:1 mapping of the EFI memory offsets then it's not worth keeping as a legacy, because there's just nothing better than mirroring the firmware layout. My suggestion would be to just 1:1 map what the EFI tables describe, modulo the single absolute offset by which we shift the whole thing to a single base. Is there any technical reason why we'd want to deviate from that? Gigabytes of tables or gigabytes of holes that 32-bit cannot handle? Firmware that wants an OS layout that differs from the firmware layout? Also, nobody seems to be asking the obvious hardware compatibility question when trying to implement a standard influenced in great part by an entity that is partly ignorant of and partly hostile to Linux: how does Windows map the EFI sections, under what OSs are these firmware versions tested? I suspect no firmware is released that crashes on bootup on all OSs that can run on that hardware, right? Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ingo Molnar Subject: Re: [PATCH 1/2] x86/efi: Map EFI memmap entries in-order at runtime Date: Sun, 27 Sep 2015 09:03:55 +0200 Message-ID: <20150927070355.GB26125@gmail.com> References: <1443218539-7610-1-git-send-email-matt@codeblueprint.co.uk> <1443218539-7610-2-git-send-email-matt@codeblueprint.co.uk> <20150926055643.GA25877@gmail.com> <20150926134329.GA3144@codeblueprint.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20150926134329.GA3144-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Matt Fleming Cc: Thomas Gleixner , "H. Peter Anvin" , Matt Fleming , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Lee, Chun-Yi" , Borislav Petkov , Leif Lindholm , Peter Jones , James Bottomley , Matthew Garrett , Dave Young , stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ard Biesheuvel , Linus Torvalds , Borislav Petkov , Andy Lutomirski , Denys Vlasenko , Brian Gerst , Andrew Morton List-Id: linux-efi@vger.kernel.org * Matt Fleming wrote: > > So could we make the whole code obviously bottom-up? Such as first calculating > > the size of virtual memory needed, then allocating a _single_, obviously > > continuous mapping, and then doing a very clear in-order mapping within that > > window? That would remove any bitness and legacy dependencies. > > So, we could, and in fact the first version of this patch did just that. You can > find it here, > > https://lkml.kernel.org/r/1441372447-23439-1-git-send-email-matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org > > But Ard suggested re-using the existing code and simply changing the order we > map the memmap entries in. Such implementational arguments (which are an internal cost) never trump compatibility and robustness concerns (which are an external constraint). > [...] And given the constraint for a small patch for backporting, I think it's a > better solution. [...] Ugh, backporting size is _even less_ of a valid argument when it comes to firmware support correctness! We can (perhaps) use these already existing patches as a simpler backport, but there's absolutely no reason to keep that code as a solution: > [...] The actual virtual addresses we pick are exactly the same with the two > patches. So I'm NAK-ing this for now: - The code is it reads today pretends to be an 'allocator'. It is _NOT_ an allocator, because all the sections have already been determined by the firmware, and, as we just learned the hard way, we do not want to deviate from that! There's nothing to 'allocate'! What these patches seem to implement is an elaborate 'allocator' that ends up doing nothing on 'new 64-bit' ... - The 32-bit and 64-bit and 'old_mmap' asymmetries: if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) { seem fragile and nonsensical. The question is: is it possible for the whole EFI image to be larger than a couple of megabytes? If not then 32-bit should just mirror the firmware layout as well, and if EFI_OLD_MEMMAP does anything differently from this _obvious_ 1:1 mapping of the EFI memory offsets then it's not worth keeping as a legacy, because there's just nothing better than mirroring the firmware layout. My suggestion would be to just 1:1 map what the EFI tables describe, modulo the single absolute offset by which we shift the whole thing to a single base. Is there any technical reason why we'd want to deviate from that? Gigabytes of tables or gigabytes of holes that 32-bit cannot handle? Firmware that wants an OS layout that differs from the firmware layout? Also, nobody seems to be asking the obvious hardware compatibility question when trying to implement a standard influenced in great part by an entity that is partly ignorant of and partly hostile to Linux: how does Windows map the EFI sections, under what OSs are these firmware versions tested? I suspect no firmware is released that crashes on bootup on all OSs that can run on that hardware, right? Thanks, Ingo