From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936237AbcISLQE (ORCPT ); Mon, 19 Sep 2016 07:16:04 -0400 Received: from mail-wm0-f47.google.com ([74.125.82.47]:36561 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755213AbcISLPe (ORCPT ); Mon, 19 Sep 2016 07:15:34 -0400 Date: Mon, 19 Sep 2016 12:15:31 +0100 From: Matt Fleming To: Mike Galbraith Cc: Ingo Molnar , LKML , linux-efi@vger.kernel.org, Peter Jones , Ard Biesheuvel Subject: Re: [tip regression] efi: Allow drivers to reserve boot services forever == toxic Message-ID: <20160919111531.GC2892@codeblueprint.co.uk> References: <1474005912.3930.10.camel@gmail.com> <20160916093149.GC16797@codeblueprint.co.uk> <1474020059.3881.3.camel@gmail.com> <20160916143007.GF16797@codeblueprint.co.uk> <1474043695.3854.3.camel@gmail.com> <20160917195853.GG16797@codeblueprint.co.uk> <1474178972.3817.2.camel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1474178972.3817.2.camel@gmail.com> 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 On Sun, 18 Sep, at 08:09:32AM, Mike Galbraith wrote: > > +MATT changes + XXX changes (boots/works without #if 0) > [ 0.000000] MIKE md.phys_addr:0xded81000 md.virt_addr:0x0 md.num_pages:520 > [ 0.000000] MIKE mr.range.start:0xdef87000 mr.range.end:0xdef87fff > [ 0.000000] MIKE efi_memmap_install(0x9d640, 51); Brilliant. This is what I've got queued up. Thanks everyone. ----8<---- >>From 7e750e3289a44fe3ad693bde45aea1ad8577dd2a Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 16 Sep 2016 15:12:47 +0100 Subject: [PATCH] x86/efi: Round EFI memmap reservations to EFI_PAGE_SIZE Mike Galbraith reported that his machine started rebooting during boot after, commit 8e80632fb23f ("efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()") The ESRT table on his machine is 56 bytes and at no point in the efi_arch_mem_reserve() call path is that size rounded up to EFI_PAGE_SIZE, nor is the start address on an EFI_PAGE_SIZE boundary. Since the EFI memory map only deals with whole pages, inserting an EFI memory region with 56 bytes results in a new entry covering zero pages, and completely screws up the calculations for the old regions that were trimmed. Round all sizes upwards, and start addresses downwards, to the nearest EFI_PAGE_SIZE boundary. Additionally, efi_memmap_insert() expects the mem::range::end value to be one less than the end address for the region. Reported-by: Mike Galbraith Reported-by: Mike Krinkin Cc: Peter Jones Cc: Ard Biesheuvel Cc: Mark Rutland Cc: Taku Izumi Signed-off-by: Matt Fleming --- arch/x86/platform/efi/quirks.c | 6 +++++- drivers/firmware/efi/memmap.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c index f14b7a9da24b..10aca63a50d7 100644 --- a/arch/x86/platform/efi/quirks.c +++ b/arch/x86/platform/efi/quirks.c @@ -201,8 +201,12 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size) return; } + size += addr % EFI_PAGE_SIZE; + size = round_up(size, EFI_PAGE_SIZE); + addr = round_down(addr, EFI_PAGE_SIZE); + mr.range.start = addr; - mr.range.end = addr + size; + mr.range.end = addr + size - 1; mr.attribute = md.attribute | EFI_MEMORY_RUNTIME; num_entries = efi_memmap_split_count(&md, &mr.range); diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c index cd96086fd851..f03ddecd232b 100644 --- a/drivers/firmware/efi/memmap.c +++ b/drivers/firmware/efi/memmap.c @@ -225,6 +225,17 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, m_end = mem->range.end; m_attr = mem->attribute; + /* + * The EFI memory map deals with regions in EFI_PAGE_SIZE + * units. Ensure that the region described by 'mem' is aligned + * correctly. + */ + if (!IS_ALIGNED(m_start, EFI_PAGE_SIZE) || + !IS_ALIGNED(m_end + 1, EFI_PAGE_SIZE)) { + WARN_ON(1); + return; + } + for (old = old_memmap->map, new = buf; old < old_memmap->map_end; old += old_memmap->desc_size, new += old_memmap->desc_size) { -- 2.9.3 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Fleming Subject: Re: [tip regression] efi: Allow drivers to reserve boot services forever == toxic Date: Mon, 19 Sep 2016 12:15:31 +0100 Message-ID: <20160919111531.GC2892@codeblueprint.co.uk> References: <1474005912.3930.10.camel@gmail.com> <20160916093149.GC16797@codeblueprint.co.uk> <1474020059.3881.3.camel@gmail.com> <20160916143007.GF16797@codeblueprint.co.uk> <1474043695.3854.3.camel@gmail.com> <20160917195853.GG16797@codeblueprint.co.uk> <1474178972.3817.2.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1474178972.3817.2.camel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Mike Galbraith Cc: Ingo Molnar , LKML , linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Peter Jones , Ard Biesheuvel List-Id: linux-efi@vger.kernel.org On Sun, 18 Sep, at 08:09:32AM, Mike Galbraith wrote: > > +MATT changes + XXX changes (boots/works without #if 0) > [ 0.000000] MIKE md.phys_addr:0xded81000 md.virt_addr:0x0 md.num_pages:520 > [ 0.000000] MIKE mr.range.start:0xdef87000 mr.range.end:0xdef87fff > [ 0.000000] MIKE efi_memmap_install(0x9d640, 51); Brilliant. This is what I've got queued up. Thanks everyone. ----8<---- >>From 7e750e3289a44fe3ad693bde45aea1ad8577dd2a Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 16 Sep 2016 15:12:47 +0100 Subject: [PATCH] x86/efi: Round EFI memmap reservations to EFI_PAGE_SIZE Mike Galbraith reported that his machine started rebooting during boot after, commit 8e80632fb23f ("efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()") The ESRT table on his machine is 56 bytes and at no point in the efi_arch_mem_reserve() call path is that size rounded up to EFI_PAGE_SIZE, nor is the start address on an EFI_PAGE_SIZE boundary. Since the EFI memory map only deals with whole pages, inserting an EFI memory region with 56 bytes results in a new entry covering zero pages, and completely screws up the calculations for the old regions that were trimmed. Round all sizes upwards, and start addresses downwards, to the nearest EFI_PAGE_SIZE boundary. Additionally, efi_memmap_insert() expects the mem::range::end value to be one less than the end address for the region. Reported-by: Mike Galbraith Reported-by: Mike Krinkin Cc: Peter Jones Cc: Ard Biesheuvel Cc: Mark Rutland Cc: Taku Izumi Signed-off-by: Matt Fleming --- arch/x86/platform/efi/quirks.c | 6 +++++- drivers/firmware/efi/memmap.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c index f14b7a9da24b..10aca63a50d7 100644 --- a/arch/x86/platform/efi/quirks.c +++ b/arch/x86/platform/efi/quirks.c @@ -201,8 +201,12 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size) return; } + size += addr % EFI_PAGE_SIZE; + size = round_up(size, EFI_PAGE_SIZE); + addr = round_down(addr, EFI_PAGE_SIZE); + mr.range.start = addr; - mr.range.end = addr + size; + mr.range.end = addr + size - 1; mr.attribute = md.attribute | EFI_MEMORY_RUNTIME; num_entries = efi_memmap_split_count(&md, &mr.range); diff --git a/drivers/firmware/efi/memmap.c b/drivers/firmware/efi/memmap.c index cd96086fd851..f03ddecd232b 100644 --- a/drivers/firmware/efi/memmap.c +++ b/drivers/firmware/efi/memmap.c @@ -225,6 +225,17 @@ void __init efi_memmap_insert(struct efi_memory_map *old_memmap, void *buf, m_end = mem->range.end; m_attr = mem->attribute; + /* + * The EFI memory map deals with regions in EFI_PAGE_SIZE + * units. Ensure that the region described by 'mem' is aligned + * correctly. + */ + if (!IS_ALIGNED(m_start, EFI_PAGE_SIZE) || + !IS_ALIGNED(m_end + 1, EFI_PAGE_SIZE)) { + WARN_ON(1); + return; + } + for (old = old_memmap->map, new = buf; old < old_memmap->map_end; old += old_memmap->desc_size, new += old_memmap->desc_size) { -- 2.9.3