Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752323Ab3IUPVj (ORCPT ); Sat, 21 Sep 2013 11:21:39 -0400 Received: from mail-ee0-f52.google.com ([74.125.83.52]:60516 "EHLO mail-ee0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751807Ab3IUPVh (ORCPT ); Sat, 21 Sep 2013 11:21:37 -0400 Date: Sat, 21 Sep 2013 17:21:39 +0200 From: Leif Lindholm To: Matt Fleming Cc: Mark Salter , Borislav Petkov , X86 ML , LKML , Borislav Petkov , Matthew Garrett , "H. Peter Anvin" , James Bottomley , Vivek Goyal , Dave Young , linux-efi@vger.kernel.org, Roy Franz Subject: Re: [PATCH 02/11] efi: Remove EFI_PAGE_SHIFT and EFI_PAGE_SIZE Message-ID: <20130921152139.GG2737@rocoto.smurfnet.nu> References: <1379602494-26684-1-git-send-email-bp@alien8.de> <1379602494-26684-3-git-send-email-bp@alien8.de> <20130920104249.GA21381@console-pimps.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130920104249.GA21381@console-pimps.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9878 Lines: 250 On Fri, Sep 20, 2013 at 11:42:49AM +0100, Matt Fleming wrote: > On Thu, 19 Sep, at 04:54:45PM, Borislav Petkov wrote: > > From: Borislav Petkov > > > > ... and use the good old standard defines which we all know. Also, > > simplify math to shift by PAGE_SHIFT instead of multiplying by > > PAGE_SIZE. > > > > Signed-off-by: Borislav Petkov > > --- > > arch/x86/boot/compressed/eboot.c | 12 ++++++------ > > arch/x86/boot/compressed/eboot.h | 1 - > > arch/x86/platform/efi/efi.c | 22 +++++++++++----------- > > include/linux/efi.h | 6 ++---- > > 4 files changed, 19 insertions(+), 22 deletions(-) > > I'm pulling in Leif and Roy just so they're aware of this change, > because while PAGE_SHIFT is always 12 on x86, that's not true for arm64. Thank you. Also adding Mark. > However, I imagine that much work would be needed to allow for page > sizes other than 4K, so I am definitely going to take this patch. This could actually be a problem for us pretty much from day 1. UEFI mandates the use of 4K pages on arm64, but Fedora will be using 64K - so this aspect of this patch will actually break the default usage model of Linux on arm64. It will probably not be a problem on the stub side, and it's not used in many places but it would break efi_lookup_mapped_address(), efi_range_is_wc() and memrange_efi_to_native() for use by arm64. At least the first of these would be a problem. / Leif > > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c > > index b7388a425f09..5c440bf769a8 100644 > > --- a/arch/x86/boot/compressed/eboot.c > > +++ b/arch/x86/boot/compressed/eboot.c > > @@ -96,7 +96,7 @@ static efi_status_t high_alloc(unsigned long size, unsigned long align, > > if (status != EFI_SUCCESS) > > goto fail; > > > > - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; > > + nr_pages = round_up(size, PAGE_SIZE) / PAGE_SIZE; > > again: > > for (i = 0; i < map_size / desc_size; i++) { > > efi_memory_desc_t *desc; > > @@ -111,7 +111,7 @@ again: > > continue; > > > > start = desc->phys_addr; > > - end = start + desc->num_pages * (1UL << EFI_PAGE_SHIFT); > > + end = start + (desc->num_pages << PAGE_SHIFT); > > > > if ((start + size) > end || (start + size) > max) > > continue; > > @@ -173,7 +173,7 @@ static efi_status_t low_alloc(unsigned long size, unsigned long align, > > if (status != EFI_SUCCESS) > > goto fail; > > > > - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; > > + nr_pages = round_up(size, PAGE_SIZE) / PAGE_SIZE; > > for (i = 0; i < map_size / desc_size; i++) { > > efi_memory_desc_t *desc; > > unsigned long m = (unsigned long)map; > > @@ -188,7 +188,7 @@ static efi_status_t low_alloc(unsigned long size, unsigned long align, > > continue; > > > > start = desc->phys_addr; > > - end = start + desc->num_pages * (1UL << EFI_PAGE_SHIFT); > > + end = start + (desc->num_pages << PAGE_SHIFT); > > > > /* > > * Don't allocate at 0x0. It will confuse code that > > @@ -224,7 +224,7 @@ static void low_free(unsigned long size, unsigned long addr) > > { > > unsigned long nr_pages; > > > > - nr_pages = round_up(size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; > > + nr_pages = round_up(size, PAGE_SIZE) / PAGE_SIZE; > > efi_call_phys2(sys_table->boottime->free_pages, addr, nr_pages); > > } > > > > @@ -1128,7 +1128,7 @@ static efi_status_t relocate_kernel(struct setup_header *hdr) > > * possible. > > */ > > start = hdr->pref_address; > > - nr_pages = round_up(hdr->init_size, EFI_PAGE_SIZE) / EFI_PAGE_SIZE; > > + nr_pages = round_up(hdr->init_size, PAGE_SIZE) / PAGE_SIZE; > > > > status = efi_call_phys4(sys_table->boottime->allocate_pages, > > EFI_ALLOCATE_ADDRESS, EFI_LOADER_DATA, > > diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h > > index e5b0a8f91c5f..786398c1bb9a 100644 > > --- a/arch/x86/boot/compressed/eboot.h > > +++ b/arch/x86/boot/compressed/eboot.h > > @@ -11,7 +11,6 @@ > > > > #define DESC_TYPE_CODE_DATA (1 << 0) > > > > -#define EFI_PAGE_SIZE (1UL << EFI_PAGE_SHIFT) > > #define EFI_READ_CHUNK_SIZE (1024 * 1024) > > > > #define EFI_CONSOLE_OUT_DEVICE_GUID \ > > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c > > index 7cec1e9e5494..538c1e6b7b2c 100644 > > --- a/arch/x86/platform/efi/efi.c > > +++ b/arch/x86/platform/efi/efi.c > > @@ -339,7 +339,7 @@ static void __init do_add_efi_memmap(void) > > for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) { > > efi_memory_desc_t *md = p; > > unsigned long long start = md->phys_addr; > > - unsigned long long size = md->num_pages << EFI_PAGE_SHIFT; > > + unsigned long long size = md->num_pages << PAGE_SHIFT; > > int e820_type; > > > > switch (md->type) { > > @@ -416,8 +416,8 @@ static void __init print_efi_memmap(void) > > pr_info("mem%02u: type=%u, attr=0x%llx, " > > "range=[0x%016llx-0x%016llx) (%lluMB)\n", > > i, md->type, md->attribute, md->phys_addr, > > - md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT), > > - (md->num_pages >> (20 - EFI_PAGE_SHIFT))); > > + md->phys_addr + (md->num_pages << PAGE_SHIFT), > > + (md->num_pages >> (20 - PAGE_SHIFT))); > > } > > #endif /* EFI_DEBUG */ > > } > > @@ -429,7 +429,7 @@ void __init efi_reserve_boot_services(void) > > for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) { > > efi_memory_desc_t *md = p; > > u64 start = md->phys_addr; > > - u64 size = md->num_pages << EFI_PAGE_SHIFT; > > + u64 size = md->num_pages << PAGE_SHIFT; > > > > if (md->type != EFI_BOOT_SERVICES_CODE && > > md->type != EFI_BOOT_SERVICES_DATA) > > @@ -473,7 +473,7 @@ void __init efi_free_boot_services(void) > > for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) { > > efi_memory_desc_t *md = p; > > unsigned long long start = md->phys_addr; > > - unsigned long long size = md->num_pages << EFI_PAGE_SHIFT; > > + unsigned long long size = md->num_pages << PAGE_SHIFT; > > > > if (md->type != EFI_BOOT_SERVICES_CODE && > > md->type != EFI_BOOT_SERVICES_DATA) > > @@ -825,7 +825,7 @@ void __iomem *efi_lookup_mapped_addr(u64 phys_addr) > > return NULL; > > for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) { > > efi_memory_desc_t *md = p; > > - u64 size = md->num_pages << EFI_PAGE_SHIFT; > > + u64 size = md->num_pages << PAGE_SHIFT; > > u64 end = md->phys_addr + size; > > if (!(md->attribute & EFI_MEMORY_RUNTIME) && > > md->type != EFI_BOOT_SERVICES_CODE && > > @@ -843,7 +843,7 @@ void __iomem *efi_lookup_mapped_addr(u64 phys_addr) > > > > void efi_memory_uc(u64 addr, unsigned long size) > > { > > - unsigned long page_shift = 1UL << EFI_PAGE_SHIFT; > > + unsigned long page_shift = 1UL << PAGE_SHIFT; > > u64 npages; > > > > npages = round_up(size, page_shift) / page_shift; > > @@ -896,7 +896,7 @@ void __init efi_enter_virtual_mode(void) > > continue; > > } > > > > - prev_size = prev_md->num_pages << EFI_PAGE_SHIFT; > > + prev_size = prev_md->num_pages << PAGE_SHIFT; > > > > if (md->phys_addr == (prev_md->phys_addr + prev_size)) { > > prev_md->num_pages += md->num_pages; > > @@ -914,7 +914,7 @@ void __init efi_enter_virtual_mode(void) > > md->type != EFI_BOOT_SERVICES_DATA) > > continue; > > > > - size = md->num_pages << EFI_PAGE_SHIFT; > > + size = md->num_pages << PAGE_SHIFT; > > end = md->phys_addr + size; > > > > start_pfn = PFN_DOWN(md->phys_addr); > > @@ -1011,7 +1011,7 @@ u32 efi_mem_type(unsigned long phys_addr) > > md = p; > > if ((md->phys_addr <= phys_addr) && > > (phys_addr < (md->phys_addr + > > - (md->num_pages << EFI_PAGE_SHIFT)))) > > + (md->num_pages << PAGE_SHIFT)))) > > return md->type; > > } > > return 0; > > @@ -1026,7 +1026,7 @@ u64 efi_mem_attributes(unsigned long phys_addr) > > md = p; > > if ((md->phys_addr <= phys_addr) && > > (phys_addr < (md->phys_addr + > > - (md->num_pages << EFI_PAGE_SHIFT)))) > > + (md->num_pages << PAGE_SHIFT)))) > > return md->attribute; > > } > > return 0; > > diff --git a/include/linux/efi.h b/include/linux/efi.h > > index 5f8f176154f7..fa47d80ab4b5 100644 > > --- a/include/linux/efi.h > > +++ b/include/linux/efi.h > > @@ -95,8 +95,6 @@ typedef struct { > > #define EFI_MEMORY_RUNTIME ((u64)0x8000000000000000ULL) /* range requires runtime mapping */ > > #define EFI_MEMORY_DESCRIPTOR_VERSION 1 > > > > -#define EFI_PAGE_SHIFT 12 > > - > > typedef struct { > > u32 type; > > u32 pad; > > @@ -611,7 +609,7 @@ static inline int efi_range_is_wc(unsigned long start, unsigned long len) > > { > > unsigned long i; > > > > - for (i = 0; i < len; i += (1UL << EFI_PAGE_SHIFT)) { > > + for (i = 0; i < len; i += PAGE_SIZE) { > > unsigned long paddr = __pa(start + i); > > if (!(efi_mem_attributes(paddr) & EFI_MEMORY_WC)) > > return 0; > > @@ -728,7 +726,7 @@ struct efi_generic_dev_path { > > > > static inline void memrange_efi_to_native(u64 *addr, u64 *npages) > > { > > - *npages = PFN_UP(*addr + (*npages< > + *npages = PFN_UP(*addr + (*npages << PAGE_SHIFT)) - PFN_DOWN(*addr); > > *addr &= PAGE_MASK; > > } > > > > -- > > 1.8.4 > > > > -- > Matt Fleming, Intel Open Source Technology Center > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/