From: Leif Lindholm <leif.lindholm@linaro.org>
To: Matt Fleming <matt@console-pimps.org>
Cc: Mark Salter <msalter@redhat.com>, Borislav Petkov <bp@alien8.de>,
X86 ML <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@suse.de>,
Matthew Garrett <mjg59@srcf.ucam.org>,
"H. Peter Anvin" <hpa@zytor.com>,
James Bottomley <James.Bottomley@HansenPartnership.com>,
Vivek Goyal <vgoyal@redhat.com>, Dave Young <dyoung@redhat.com>,
linux-efi@vger.kernel.org, Roy Franz <roy.franz@linaro.org>
Subject: Re: [PATCH 02/11] efi: Remove EFI_PAGE_SHIFT and EFI_PAGE_SIZE
Date: Sat, 21 Sep 2013 17:21:39 +0200 [thread overview]
Message-ID: <20130921152139.GG2737@rocoto.smurfnet.nu> (raw)
In-Reply-To: <20130920104249.GA21381@console-pimps.org>
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 <bp@suse.de>
> >
> > ... 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 <bp@suse.de>
> > ---
> > 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<<EFI_PAGE_SHIFT)) - PFN_DOWN(*addr);
> > + *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
next prev parent reply other threads:[~2013-09-21 15:21 UTC|newest]
Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-19 14:54 [PATCH 00/11] EFI runtime services virtual mapping Borislav Petkov
2013-09-19 14:54 ` [PATCH 01/11] efi: Simplify EFI_DEBUG Borislav Petkov
2013-09-19 14:54 ` [PATCH 02/11] efi: Remove EFI_PAGE_SHIFT and EFI_PAGE_SIZE Borislav Petkov
2013-09-20 10:42 ` Matt Fleming
2013-09-21 15:21 ` Leif Lindholm [this message]
2013-09-21 15:41 ` Borislav Petkov
2013-09-21 15:50 ` Borislav Petkov
2013-09-21 16:01 ` Leif Lindholm
2013-09-21 16:03 ` Borislav Petkov
2013-09-21 15:59 ` Leif Lindholm
2013-09-19 14:54 ` [PATCH 03/11] x86, pageattr: Lookup address in an arbitrary PGD Borislav Petkov
2013-09-19 14:54 ` [PATCH 04/11] x86, pageattr: Add a PGD pagetable populating function Borislav Petkov
2013-09-19 14:54 ` [PATCH 05/11] x86, pageattr: Add a PUD " Borislav Petkov
2013-09-19 14:54 ` [PATCH 06/11] x86, pageattr: Add a PMD " Borislav Petkov
2013-09-19 14:54 ` [PATCH 07/11] x86, pageattr: Add a PTE " Borislav Petkov
2013-09-19 14:54 ` [PATCH 08/11] x86, pageattr: Add a PUD error unwinding path Borislav Petkov
2013-09-19 14:54 ` [PATCH 09/11] x86, pageattr: Add last levels of error path Borislav Petkov
2013-09-19 14:54 ` [PATCH 10/11] x86, cpa: Map in an arbitrary pgd Borislav Petkov
2013-09-19 14:54 ` [PATCH 11/11] EFI: Runtime services virtual mapping Borislav Petkov
2013-09-21 11:39 ` [PATCH -v2] " Borislav Petkov
2013-09-22 12:35 ` Dave Young
2013-09-22 13:37 ` Borislav Petkov
2013-09-22 14:00 ` Dave Young
2013-09-22 14:31 ` Dave Young
2013-09-22 15:27 ` H. Peter Anvin
2013-09-22 16:38 ` Borislav Petkov
2013-09-23 5:45 ` Dave Young
2013-09-24 2:52 ` Dave Young
2013-09-24 3:06 ` H. Peter Anvin
2013-09-24 4:57 ` Dave Young
2013-09-24 4:58 ` Dave Young
2013-09-24 5:23 ` Dave Young
2013-09-24 8:57 ` Dave Young
2013-09-24 9:43 ` Borislav Petkov
2013-09-24 10:01 ` Dave Young
2013-09-24 12:45 ` Dave Young
2013-10-02 10:04 ` Borislav Petkov
2013-10-02 15:43 ` H. Peter Anvin
2013-10-02 17:05 ` Borislav Petkov
2013-10-02 17:32 ` H. Peter Anvin
2013-10-02 18:42 ` Borislav Petkov
2013-10-02 18:46 ` H. Peter Anvin
2013-10-04 9:42 ` Borislav Petkov
2013-10-04 14:43 ` H. Peter Anvin
2013-10-04 14:50 ` Borislav Petkov
2013-09-23 5:47 ` Dave Young
2013-09-23 6:29 ` Borislav Petkov
2013-09-23 7:08 ` Dave Young
2013-09-23 8:45 ` Borislav Petkov
2013-09-25 9:24 ` Borislav Petkov
2013-09-20 7:29 ` [PATCH 00/11] EFI runtime " Dave Young
2013-09-20 8:19 ` Dave Young
2013-09-20 9:33 ` Borislav Petkov
2013-09-20 10:07 ` Dave Young
2013-09-20 9:05 ` Borislav Petkov
2013-09-20 9:44 ` Matt Fleming
2013-09-20 9:49 ` Matt Fleming
2013-09-20 10:02 ` Borislav Petkov
2013-09-20 11:51 ` Dave Young
2013-09-20 12:29 ` Matt Fleming
2013-09-20 14:04 ` Dave Young
2013-10-08 16:45 ` Borislav Petkov
2013-10-08 16:47 ` [PATCH 11/12] efi: Add an efi= kernel command line parameter Borislav Petkov
2013-10-28 11:02 ` Matt Fleming
2013-10-28 11:10 ` Borislav Petkov
2013-10-08 16:48 ` [PATCH 12/12] EFI: Runtime services virtual mapping Borislav Petkov
2013-10-10 8:06 ` Dave Young
2013-10-10 8:14 ` Dave Young
2013-10-10 8:58 ` Borislav Petkov
2013-10-10 12:34 ` Matt Fleming
2013-10-11 6:24 ` Dave Young
2013-10-11 7:41 ` Borislav Petkov
2013-10-12 7:54 ` Dave Young
2013-10-12 10:13 ` Matt Fleming
2013-10-12 10:30 ` Borislav Petkov
2013-10-13 3:11 ` Dave Young
2013-10-13 9:25 ` Borislav Petkov
2013-10-14 15:58 ` Borislav Petkov
2013-10-21 12:47 ` Dave Young
2013-10-21 13:37 ` Borislav Petkov
2013-10-21 15:04 ` Dave Young
2013-10-22 11:18 ` Borislav Petkov
2013-10-23 2:17 ` Dave Young
2013-10-23 12:25 ` Borislav Petkov
2013-10-23 12:37 ` Matthew Garrett
2013-10-23 12:51 ` Dave Young
2013-10-23 13:11 ` Borislav Petkov
2013-10-26 15:50 ` Matt Fleming
2013-10-13 3:06 ` Dave Young
2013-10-11 10:27 ` Matt Fleming
2013-10-11 13:42 ` Dave Young
2013-10-12 2:14 ` Dave Young
2013-10-14 15:57 ` Peter Jones
2013-10-16 6:27 ` Dave Young
2013-10-28 11:22 ` Matt Fleming
2013-10-28 16:00 ` Borislav Petkov
2013-10-29 6:47 ` Dave Young
2013-10-29 9:40 ` Borislav Petkov
2013-10-30 9:32 ` Dave Young
2013-10-30 10:45 ` Borislav Petkov
2013-10-31 7:07 ` Dave Young
2013-10-14 13:04 ` [PATCH 00/11] EFI runtime " Matt Fleming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130921152139.GG2737@rocoto.smurfnet.nu \
--to=leif.lindholm@linaro.org \
--cc=James.Bottomley@HansenPartnership.com \
--cc=bp@alien8.de \
--cc=bp@suse.de \
--cc=dyoung@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matt@console-pimps.org \
--cc=mjg59@srcf.ucam.org \
--cc=msalter@redhat.com \
--cc=roy.franz@linaro.org \
--cc=vgoyal@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®