Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752591AbbLNPIY (ORCPT ); Mon, 14 Dec 2015 10:08:24 -0500 Received: from mail-lb0-f170.google.com ([209.85.217.170]:34058 "EHLO mail-lb0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752291AbbLNPIW (ORCPT ); Mon, 14 Dec 2015 10:08:22 -0500 Date: Mon, 14 Dec 2015 15:08:17 +0000 From: Matt Fleming To: "Prakhya, Sai Praneeth" Cc: Josh Triplett , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Borislav Petkov , "Neri, Ricardo" , "Shankar, Ravi V" Subject: Re: [PATCH] x86/efi-bgrt: Fix kernel panic when mapping BGRT data Message-ID: <20151214150817.GB2571@codeblueprint.co.uk> References: <1449772021-983-1-git-send-email-sai.praneeth.prakhya@intel.com> <20151210214258.GB9584@cloud> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24+41 (02bc14ed1569) (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2176 Lines: 45 On Fri, 11 Dec, at 05:58:09AM, Sai Praneeth Prakhya wrote: > > >The original motivation for efi_lookup_mapped_addr came from > >early_ioremap printing a warning if used on an address range > >already mapped as RAM. Does early_mem* handle that case correctly > >without a warning? > > Thanks a lot Josh for letting me know that. I don't think > early_memremap() does that because early_memremap() and > early_ioremap() both use __early_ioremap() but with different page > protections (and I am not sure how those protections effect warning, > but I will check that). Waiting for comments from Matt and Boris. Right, early_memremap() was introduced for the purpose of mapping RAM, so we shouldn't be seeing any warning here. > >Because not all firmware places the BGRT image in boot services > >memory; some firmware places the BGRT image variously in BIOS > >reserved memory, ACPI reclaim >space, or other strange places. > > > >- Josh Triplett > > I think we should not support buggy firmware implementations because > it's same as encouraging them, instead we could let user know that > he has got a buggy firmware and we skip bgrt code as if bgrt was > disabled and carry on with normal boot process. One way of detecting > buggy implementation without doing page table switch in > efi_bgrt_init() is to enable a chicken bit if we find bgrt header > and image address in EFI_BOOT_SERVICES_DATA regions while we are > switching efi runtime services to virtual mode in > __efi_enter_virtual_mode() and check for the same bit in > efi_bgrt_init(). Trying to inform firmware developers that we noticed some buggy behaviour should definitely be discussed as a separate patch (because it's debatable whether there's any value in that in this scenario). The changes in this patch look OK to me, and I think all of Josh's concerns have been addressed. So unless anyone complains soon, I'm going to apply it and send it to tip. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/