* [PATCH 0/3] EFI memmap fix
@ 2013-12-16 23:36 Borislav Petkov
2013-12-16 23:36 ` [PATCH 1/3] x86, ptdump: Add the functionality to dump an arbitrary pagetable Borislav Petkov
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-16 23:36 UTC (permalink / raw)
To: Linux EFI, X86 ML, LKML
Cc: Borislav Petkov, Matt Fleming, Matthew Garrett, H. Peter Anvin,
Dave Young, James Bottomley, Vivek Goyal, Toshi Kani,
Arjan van de Ven
From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
Hi guys,
this is the result of Toshi and me debugging a #GP on one of his big HP
boxes sporting UEFI. Each commit message should be self-explanatory so
please look there.
This has more or less an RFC nature thus I'm sending it now to collect
feedback. It is going to wait in the EFI queue anyway after the kexec
stuff gets sorted out first.
Comments and suggestions as always are very much appreciated.
Thanks.
Borislav Petkov (3):
x86, ptdump: Add the functionality to dump an arbitrary pagetable
efi: Dump the EFI page table
efi: Make efi virtual runtime map passing more robust
arch/x86/include/asm/pgtable.h | 3 +-
arch/x86/mm/dump_pagetables.c | 77 ++++++++++++++++++++++++++++--------------
arch/x86/platform/efi/efi.c | 68 ++++++++++++++++++++++++++++++++-----
3 files changed, 112 insertions(+), 36 deletions(-)
--
1.8.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] x86, ptdump: Add the functionality to dump an arbitrary pagetable
2013-12-16 23:36 [PATCH 0/3] EFI memmap fix Borislav Petkov
@ 2013-12-16 23:36 ` Borislav Petkov
2013-12-16 23:36 ` [PATCH 3/3] efi: Make efi virtual runtime map passing more robust Borislav Petkov
[not found] ` <1387236997-26975-1-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-16 23:36 UTC (permalink / raw)
To: Linux EFI, X86 ML, LKML
Cc: Borislav Petkov, Arjan van de Ven, Matt Fleming, Matthew Garrett,
H. Peter Anvin, Dave Young, James Bottomley, Vivek Goyal,
Toshi Kani
From: Borislav Petkov <bp@suse.de>
With reusing the ->trampoline_pgd page table for mapping EFI regions in
order to use them after having switched to EFI virtual mode, it is very
useful to be able to dump aforementioned page table in dmesg. This adds
that functionality through the walk_pgd_level() interface which can be
called from somewhere else.
The original functionality of dumping to debugfs remains untouched.
Cc: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/include/asm/pgtable.h | 3 +-
arch/x86/mm/dump_pagetables.c | 77 ++++++++++++++++++++++++++++--------------
2 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 3d1999458709..595305f59da0 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -15,9 +15,10 @@
: (prot))
#ifndef __ASSEMBLY__
-
#include <asm/x86_init.h>
+void walk_pgd_level(struct seq_file *m, pgd_t *pgd);
+
/*
* ZERO_PAGE is a global shared page that is always zero: used
* for zero-mapped memory areas etc..
diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
index 0002a3a33081..f987ecff9226 100644
--- a/arch/x86/mm/dump_pagetables.c
+++ b/arch/x86/mm/dump_pagetables.c
@@ -19,6 +19,8 @@
#include <asm/pgtable.h>
+static bool dump_to_dmesg;
+
/*
* The dumper groups pagetable entries of the same type into one, and for
* that it needs to keep some state when walking, and flush this state
@@ -88,6 +90,24 @@ static struct addr_marker address_markers[] = {
#define PUD_LEVEL_MULT (PTRS_PER_PMD * PMD_LEVEL_MULT)
#define PGD_LEVEL_MULT (PTRS_PER_PUD * PUD_LEVEL_MULT)
+#define pt_dump_seq_printf(m, fmt, args...) \
+({ \
+ if (dump_to_dmesg) \
+ printk(KERN_INFO fmt, ##args); \
+ else \
+ if (m) \
+ seq_printf(m, fmt, ##args); \
+})
+
+#define pt_dump_cont_printf(m, fmt, args...) \
+({ \
+ if (dump_to_dmesg) \
+ printk(KERN_CONT fmt, ##args); \
+ else \
+ if (m) \
+ seq_printf(m, fmt, ##args); \
+})
+
/*
* Print a readable form of a pgprot_t to the seq_file
*/
@@ -99,47 +119,47 @@ static void printk_prot(struct seq_file *m, pgprot_t prot, int level)
if (!pgprot_val(prot)) {
/* Not present */
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
} else {
if (pr & _PAGE_USER)
- seq_printf(m, "USR ");
+ pt_dump_cont_printf(m, "USR ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
if (pr & _PAGE_RW)
- seq_printf(m, "RW ");
+ pt_dump_cont_printf(m, "RW ");
else
- seq_printf(m, "ro ");
+ pt_dump_cont_printf(m, "ro ");
if (pr & _PAGE_PWT)
- seq_printf(m, "PWT ");
+ pt_dump_cont_printf(m, "PWT ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
if (pr & _PAGE_PCD)
- seq_printf(m, "PCD ");
+ pt_dump_cont_printf(m, "PCD ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
/* Bit 9 has a different meaning on level 3 vs 4 */
if (level <= 3) {
if (pr & _PAGE_PSE)
- seq_printf(m, "PSE ");
+ pt_dump_cont_printf(m, "PSE ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
} else {
if (pr & _PAGE_PAT)
- seq_printf(m, "pat ");
+ pt_dump_cont_printf(m, "pat ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
}
if (pr & _PAGE_GLOBAL)
- seq_printf(m, "GLB ");
+ pt_dump_cont_printf(m, "GLB ");
else
- seq_printf(m, " ");
+ pt_dump_cont_printf(m, " ");
if (pr & _PAGE_NX)
- seq_printf(m, "NX ");
+ pt_dump_cont_printf(m, "NX ");
else
- seq_printf(m, "x ");
+ pt_dump_cont_printf(m, "x ");
}
- seq_printf(m, "%s\n", level_name[level]);
+ pt_dump_cont_printf(m, "%s\n", level_name[level]);
}
/*
@@ -178,7 +198,7 @@ static void note_page(struct seq_file *m, struct pg_state *st,
st->current_prot = new_prot;
st->level = level;
st->marker = address_markers;
- seq_printf(m, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(m, "---[ %s ]---\n", st->marker->name);
} else if (prot != cur || level != st->level ||
st->current_address >= st->marker[1].start_address) {
const char *unit = units;
@@ -188,16 +208,16 @@ static void note_page(struct seq_file *m, struct pg_state *st,
/*
* Now print the actual finished series
*/
- seq_printf(m, "0x%0*lx-0x%0*lx ",
- width, st->start_address,
- width, st->current_address);
+ pt_dump_seq_printf(m, "0x%0*lx-0x%0*lx ",
+ width, st->start_address,
+ width, st->current_address);
delta = (st->current_address - st->start_address) >> 10;
while (!(delta & 1023) && unit[1]) {
delta >>= 10;
unit++;
}
- seq_printf(m, "%9lu%c ", delta, *unit);
+ pt_dump_cont_printf(m, "%9lu%c ", delta, *unit);
printk_prot(m, st->current_prot, st->level);
/*
@@ -207,7 +227,7 @@ static void note_page(struct seq_file *m, struct pg_state *st,
*/
if (st->current_address >= st->marker[1].start_address) {
st->marker++;
- seq_printf(m, "---[ %s ]---\n", st->marker->name);
+ pt_dump_seq_printf(m, "---[ %s ]---\n", st->marker->name);
}
st->start_address = st->current_address;
@@ -296,7 +316,7 @@ static void walk_pud_level(struct seq_file *m, struct pg_state *st, pgd_t addr,
#define pgd_none(a) pud_none(__pud(pgd_val(a)))
#endif
-static void walk_pgd_level(struct seq_file *m)
+void walk_pgd_level(struct seq_file *m, pgd_t *pgd)
{
#ifdef CONFIG_X86_64
pgd_t *start = (pgd_t *) &init_level4_pgt;
@@ -306,6 +326,11 @@ static void walk_pgd_level(struct seq_file *m)
int i;
struct pg_state st;
+ if (pgd) {
+ start = pgd;
+ dump_to_dmesg = true;
+ }
+
memset(&st, 0, sizeof(st));
for (i = 0; i < PTRS_PER_PGD; i++) {
@@ -331,7 +356,7 @@ static void walk_pgd_level(struct seq_file *m)
static int ptdump_show(struct seq_file *m, void *v)
{
- walk_pgd_level(m);
+ walk_pgd_level(m, NULL);
return 0;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] efi: Dump the EFI page table
[not found] ` <1387236997-26975-1-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
@ 2013-12-16 23:36 ` Borislav Petkov
2013-12-17 1:11 ` [PATCH 0/3] EFI memmap fix Toshi Kani
1 sibling, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-16 23:36 UTC (permalink / raw)
To: Linux EFI, X86 ML, LKML
Cc: Borislav Petkov, Matt Fleming, Matthew Garrett, H. Peter Anvin,
Dave Young, James Bottomley, Vivek Goyal, Toshi Kani,
Arjan van de Ven
From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
This is very useful for debugging issues with the recently added
pagetable switching code for EFI virtual mode.
Signed-off-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
---
arch/x86/platform/efi/efi.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 7d8caccbf2f3..51d6285701e9 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -740,6 +740,15 @@ void efi_memory_uc(u64 addr, unsigned long size)
set_memory_uc(addr, npages);
}
+static void efi_dump_pagetable(void)
+{
+#if defined(EFI_DEBUG) && defined(CONFIG_X86_PTDUMP)
+ pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
+
+ walk_pgd_level(NULL, pgd);
+#endif
+}
+
void __init old_map_region(efi_memory_desc_t *md)
{
u64 start_pfn, end_pfn, end;
@@ -869,6 +878,8 @@ void __init efi_enter_virtual_mode(void)
efi_setup_page_tables();
efi_sync_low_kernel_mappings();
+ efi_dump_pagetable();
+
status = phys_efi_set_virtual_address_map(
memmap.desc_size * count,
memmap.desc_size,
--
1.8.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
2013-12-16 23:36 [PATCH 0/3] EFI memmap fix Borislav Petkov
2013-12-16 23:36 ` [PATCH 1/3] x86, ptdump: Add the functionality to dump an arbitrary pagetable Borislav Petkov
@ 2013-12-16 23:36 ` Borislav Petkov
[not found] ` <1387236997-26975-4-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2013-12-18 9:07 ` Dave Young
[not found] ` <1387236997-26975-1-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2 siblings, 2 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-16 23:36 UTC (permalink / raw)
To: Linux EFI, X86 ML, LKML
Cc: Borislav Petkov, Matt Fleming, Matthew Garrett, H. Peter Anvin,
Dave Young, James Bottomley, Vivek Goyal, Toshi Kani,
Arjan van de Ven
From: Borislav Petkov <bp@suse.de>
Currently, running SetVirtualAddressMap() and passing the physical
address of the virtual map array was working only by a lucky coincidence
because the memory was present in the EFI page table too. Until Toshi
went and booted this on a big HP box - the krealloc() manner of resizing
the memmap we're doing did allocate from such physical addresses which
were not mapped anymore and boom:
http://lkml.kernel.org/r/1386806463.1791.295.camel@misato.fc.hp.com
One way to take care of that issue is to reimplement the krealloc thing
but with pages. We start with contiguous pages of order 1, i.e. 2 pages,
and when we deplete that memory (shouldn't happen all that often but you
know firmware) we realloc the next power-of-two pages.
Having the pages, it is much more handy and easy to map them into the
EFI page table with the already existing mapping code which we're using
for building the virtual mappings.
And, it doesn't matter all that much how much pages we've used as we're
freeing them right after they've fulfilled their purpose at the end of
the function anyway.
Reported-by: Toshi Kani <toshi.kani@hp.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/platform/efi/efi.c | 57 ++++++++++++++++++++++++++++++++++++++-------
1 file changed, 48 insertions(+), 9 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 51d6285701e9..39c52cc9b63a 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -112,7 +112,6 @@ static int __init setup_storage_paranoia(char *arg)
}
early_param("efi_no_storage_paranoia", setup_storage_paranoia);
-
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
unsigned long flags;
@@ -775,6 +774,27 @@ void __init old_map_region(efi_memory_desc_t *md)
(unsigned long long)md->phys_addr);
}
+static void *realloc_pages(void *old_memmap, int old_shift)
+{
+ void *ret;
+
+ ret = (void *)__get_free_pages(GFP_KERNEL, old_shift + 1);
+ if (!ret)
+ goto out;
+
+ /*
+ * A first-time allocation doesn't have anything to copy.
+ */
+ if (!old_memmap)
+ return ret;
+
+ memcpy(ret, old_memmap, PAGE_SIZE << old_shift);
+
+out:
+ __free_pages(old_memmap, old_shift);
+ return ret;
+}
+
/*
* This function will switch the EFI runtime services to virtual mode.
* Essentially, we look through the EFI memmap and map every region that
@@ -794,12 +814,13 @@ void __init old_map_region(efi_memory_desc_t *md)
*/
void __init efi_enter_virtual_mode(void)
{
+ pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
+ unsigned long size, new_memmap_left = 0;
efi_memory_desc_t *md, *prev_md = NULL;
+ int count = 0, new_memmap_shift = 0;
void *p, *new_memmap = NULL;
- unsigned long size;
efi_status_t status;
u64 end, systab;
- int count = 0;
efi.systab = NULL;
@@ -862,14 +883,19 @@ void __init efi_enter_virtual_mode(void)
efi.systab = (efi_system_table_t *) (unsigned long) systab;
}
- new_memmap = krealloc(new_memmap,
- (count + 1) * memmap.desc_size,
- GFP_KERNEL);
- if (!new_memmap)
- goto err_out;
+ if (new_memmap_left < memmap.desc_size) {
+ new_memmap = realloc_pages(new_memmap, new_memmap_shift);
+ if (!new_memmap)
+ goto err_out;
+
+ new_memmap_shift++;
+ new_memmap_left += PAGE_SIZE << new_memmap_shift;
+ }
memcpy(new_memmap + (count * memmap.desc_size), md,
memmap.desc_size);
+
+ new_memmap_left -= memmap.desc_size;
count++;
}
@@ -880,6 +906,19 @@ void __init efi_enter_virtual_mode(void)
efi_dump_pagetable();
+ /*
+ * It can happen that the physical address of new_memmap lands in memory
+ * which is not mapped in the EFI page table. Therefore we need to go
+ * and ident-map those pages containing the map before calling
+ * phys_efi_set_virtual_address_map().
+ */
+ if (kernel_map_pages_in_pgd(pgd, __pa(new_memmap), __pa(new_memmap),
+ 1 << new_memmap_shift, _PAGE_NX)) {
+ pr_err("Error ident-mapping new memmap (0x%lx)!\n",
+ __pa(new_memmap));
+ goto err_out;
+ }
+
status = phys_efi_set_virtual_address_map(
memmap.desc_size * count,
memmap.desc_size,
@@ -916,7 +955,7 @@ void __init efi_enter_virtual_mode(void)
if (efi_enabled(EFI_OLD_MEMMAP) && (__supported_pte_mask & _PAGE_NX))
runtime_code_page_mkexec();
- kfree(new_memmap);
+ __free_pages(new_memmap, new_memmap_shift);
/* clean DUMMY object */
efi.set_variable(efi_dummy_name, &EFI_DUMMY_GUID,
--
1.8.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/3] EFI memmap fix
[not found] ` <1387236997-26975-1-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2013-12-16 23:36 ` [PATCH 2/3] efi: Dump the EFI page table Borislav Petkov
@ 2013-12-17 1:11 ` Toshi Kani
1 sibling, 0 replies; 12+ messages in thread
From: Toshi Kani @ 2013-12-17 1:11 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matt Fleming,
Matthew Garrett, H. Peter Anvin, Dave Young, James Bottomley,
Vivek Goyal, Arjan van de Ven
On Tue, 2013-12-17 at 00:36 +0100, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
>
> Hi guys,
>
> this is the result of Toshi and me debugging a #GP on one of his big HP
> boxes sporting UEFI. Each commit message should be self-explanatory so
> please look there.
>
> This has more or less an RFC nature thus I'm sending it now to collect
> feedback. It is going to wait in the EFI queue anyway after the kexec
> stuff gets sorted out first.
>
> Comments and suggestions as always are very much appreciated.
>
> Thanks.
>
> Borislav Petkov (3):
> x86, ptdump: Add the functionality to dump an arbitrary pagetable
> efi: Dump the EFI page table
> efi: Make efi virtual runtime map passing more robust
>
> arch/x86/include/asm/pgtable.h | 3 +-
> arch/x86/mm/dump_pagetables.c | 77 ++++++++++++++++++++++++++++--------------
> arch/x86/platform/efi/efi.c | 68 ++++++++++++++++++++++++++++++++-----
> 3 files changed, 112 insertions(+), 36 deletions(-)
>
Thanks for the quick turnaround! For the series:
Tested-by: Toshi Kani <toshi.kani-VXdhtT5mjnY@public.gmane.org>
Toshi
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
[not found] ` <1387236997-26975-4-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
@ 2013-12-17 11:23 ` Borislav Petkov
2013-12-17 12:10 ` Matt Fleming
1 sibling, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-17 11:23 UTC (permalink / raw)
To: Linux EFI, X86 ML, LKML
Cc: Borislav Petkov, Matt Fleming, Matthew Garrett, H. Peter Anvin,
Dave Young, James Bottomley, Vivek Goyal, Toshi Kani,
Arjan van de Ven
On Tue, Dec 17, 2013 at 12:36:37AM +0100, Borislav Petkov wrote:
> @@ -880,6 +906,19 @@ void __init efi_enter_virtual_mode(void)
>
> efi_dump_pagetable();
>
> + /*
> + * It can happen that the physical address of new_memmap lands in memory
> + * which is not mapped in the EFI page table. Therefore we need to go
> + * and ident-map those pages containing the map before calling
> + * phys_efi_set_virtual_address_map().
> + */
> + if (kernel_map_pages_in_pgd(pgd, __pa(new_memmap), __pa(new_memmap),
> + 1 << new_memmap_shift, _PAGE_NX)) {
> + pr_err("Error ident-mapping new memmap (0x%lx)!\n",
> + __pa(new_memmap));
> + goto err_out;
> + }
> +
> status = phys_efi_set_virtual_address_map(
> memmap.desc_size * count,
> memmap.desc_size,
> @@ -916,7 +955,7 @@ void __init efi_enter_virtual_mode(void)
> if (efi_enabled(EFI_OLD_MEMMAP) && (__supported_pte_mask & _PAGE_NX))
> runtime_code_page_mkexec();
>
> - kfree(new_memmap);
> + __free_pages(new_memmap, new_memmap_shift);
Note to self: carve out the error path of populate_pgd() into a separate
function and call it here to unmap the pages previously mapped above as
we're freeing them here.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
[not found] ` <1387236997-26975-4-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2013-12-17 11:23 ` Borislav Petkov
@ 2013-12-17 12:10 ` Matt Fleming
2013-12-17 13:40 ` Borislav Petkov
1 sibling, 1 reply; 12+ messages in thread
From: Matt Fleming @ 2013-12-17 12:10 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matthew Garrett,
H. Peter Anvin, Dave Young, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On Tue, 17 Dec, at 12:36:37AM, Borislav Petkov wrote:
> From: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
>
> Currently, running SetVirtualAddressMap() and passing the physical
> address of the virtual map array was working only by a lucky coincidence
> because the memory was present in the EFI page table too. Until Toshi
> went and booted this on a big HP box - the krealloc() manner of resizing
> the memmap we're doing did allocate from such physical addresses which
> were not mapped anymore and boom:
>
> http://lkml.kernel.org/r/1386806463.1791.295.camel-RbGIw1UOYPVo/CpIj0byZw@public.gmane.org
>
> One way to take care of that issue is to reimplement the krealloc thing
> but with pages. We start with contiguous pages of order 1, i.e. 2 pages,
> and when we deplete that memory (shouldn't happen all that often but you
> know firmware) we realloc the next power-of-two pages.
>
> Having the pages, it is much more handy and easy to map them into the
> EFI page table with the already existing mapping code which we're using
> for building the virtual mappings.
>
> And, it doesn't matter all that much how much pages we've used as we're
> freeing them right after they've fulfilled their purpose at the end of
> the function anyway.
>
> Reported-by: Toshi Kani <toshi.kani-VXdhtT5mjnY@public.gmane.org>
> Signed-off-by: Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
> ---
> arch/x86/platform/efi/efi.c | 57 ++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 48 insertions(+), 9 deletions(-)
[...]
> @@ -794,12 +814,13 @@ void __init old_map_region(efi_memory_desc_t *md)
> */
> void __init efi_enter_virtual_mode(void)
> {
> + pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
> + unsigned long size, new_memmap_left = 0;
> efi_memory_desc_t *md, *prev_md = NULL;
> + int count = 0, new_memmap_shift = 0;
> void *p, *new_memmap = NULL;
> - unsigned long size;
> efi_status_t status;
> u64 end, systab;
> - int count = 0;
>
> efi.systab = NULL;
You sunk my i386 battleship,
/home/build/git/efi/arch/x86/platform/efi/efi.c:824:24: error: ‘struct real_mode_header’ has no member named ‘trampoline_pgd’
make[4]: *** [arch/x86/platform/efi/efi.o] Error 1
make[3]: *** [arch/x86/platform/efi] Error 2
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
2013-12-17 12:10 ` Matt Fleming
@ 2013-12-17 13:40 ` Borislav Petkov
2013-12-17 13:51 ` Matt Fleming
0 siblings, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2013-12-17 13:40 UTC (permalink / raw)
To: Matt Fleming
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matthew Garrett,
H. Peter Anvin, Dave Young, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On Tue, Dec 17, 2013 at 12:10:24PM +0000, Matt Fleming wrote:
> You sunk my i386 battleship,
>
> /home/build/git/efi/arch/x86/platform/efi/efi.c:824:24: error: ‘struct real_mode_header’ has no member named ‘trampoline_pgd’
> make[4]: *** [arch/x86/platform/efi/efi.o] Error 1
> make[3]: *** [arch/x86/platform/efi] Error 2
Right, this is a recurring issue with the trampoline_pgd. What do you
prefer: an #ifdef CONFIG_X86_64 or a separate function in efi_$(BITS).c?
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
2013-12-17 13:40 ` Borislav Petkov
@ 2013-12-17 13:51 ` Matt Fleming
[not found] ` <20131217135156.GC3145-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Matt Fleming @ 2013-12-17 13:51 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matthew Garrett,
H. Peter Anvin, Dave Young, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On Tue, 17 Dec, at 02:40:06PM, Borislav Petkov wrote:
> On Tue, Dec 17, 2013 at 12:10:24PM +0000, Matt Fleming wrote:
> > You sunk my i386 battleship,
> >
> > /home/build/git/efi/arch/x86/platform/efi/efi.c:824:24: error: ‘struct real_mode_header’ has no member named ‘trampoline_pgd’
> > make[4]: *** [arch/x86/platform/efi/efi.o] Error 1
> > make[3]: *** [arch/x86/platform/efi] Error 2
>
> Right, this is a recurring issue with the trampoline_pgd. What do you
> prefer: an #ifdef CONFIG_X86_64 or a separate function in efi_$(BITS).c?
Couldn't we do the mapping in efi_setup_page_tables() if we pass
__pa(new_memmap) and 1 << new_memmap_shift as arguments?
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
[not found] ` <20131217135156.GC3145-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
@ 2013-12-17 14:28 ` Borislav Petkov
0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-17 14:28 UTC (permalink / raw)
To: Matt Fleming
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matthew Garrett,
H. Peter Anvin, Dave Young, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On Tue, Dec 17, 2013 at 01:51:56PM +0000, Matt Fleming wrote:
> Couldn't we do the mapping in efi_setup_page_tables() if we pass
> __pa(new_memmap) and 1 << new_memmap_shift as arguments?
Yep, not a bad idea. I'll give it a try.
Thanks.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
2013-12-16 23:36 ` [PATCH 3/3] efi: Make efi virtual runtime map passing more robust Borislav Petkov
[not found] ` <1387236997-26975-4-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
@ 2013-12-18 9:07 ` Dave Young
[not found] ` <20131218090757.GA15594-je1gSBvt1TcFLmT5oZ11vB/sF2h8X+2i0E9HWUfgJXw@public.gmane.org>
1 sibling, 1 reply; 12+ messages in thread
From: Dave Young @ 2013-12-18 9:07 UTC (permalink / raw)
To: Borislav Petkov
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matt Fleming,
Matthew Garrett, H. Peter Anvin, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On 12/17/13 at 12:36am, Borislav Petkov wrote:
> From: Borislav Petkov <bp@suse.de>
>
> Currently, running SetVirtualAddressMap() and passing the physical
> address of the virtual map array was working only by a lucky coincidence
> because the memory was present in the EFI page table too. Until Toshi
> went and booted this on a big HP box - the krealloc() manner of resizing
> the memmap we're doing did allocate from such physical addresses which
> were not mapped anymore and boom:
>
> http://lkml.kernel.org/r/1386806463.1791.295.camel@misato.fc.hp.com
>
> One way to take care of that issue is to reimplement the krealloc thing
> but with pages. We start with contiguous pages of order 1, i.e. 2 pages,
> and when we deplete that memory (shouldn't happen all that often but you
> know firmware) we realloc the next power-of-two pages.
How about firstly count the md numbers in the 1st loop, then get/roundup
the total size, alloc the pages, map the mds one by one in another loop.
Thanks
Dave
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] efi: Make efi virtual runtime map passing more robust
[not found] ` <20131218090757.GA15594-je1gSBvt1TcFLmT5oZ11vB/sF2h8X+2i0E9HWUfgJXw@public.gmane.org>
@ 2013-12-18 11:42 ` Borislav Petkov
0 siblings, 0 replies; 12+ messages in thread
From: Borislav Petkov @ 2013-12-18 11:42 UTC (permalink / raw)
To: Dave Young
Cc: Linux EFI, X86 ML, LKML, Borislav Petkov, Matt Fleming,
Matthew Garrett, H. Peter Anvin, James Bottomley, Vivek Goyal,
Toshi Kani, Arjan van de Ven
On Wed, Dec 18, 2013 at 05:07:57PM +0800, Dave Young wrote:
> How about firstly count the md numbers in the 1st loop, then
> get/roundup the total size, alloc the pages, map the mds one by one in
> another loop.
No need as most systems would never need to reallocate. Even on Toshi's
big system, the mds fit in less than one page - having a second one is
an additional safeguard, i.e. 2 pages should be more than enough in the
majority of the cases.
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-12-18 11:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-16 23:36 [PATCH 0/3] EFI memmap fix Borislav Petkov
2013-12-16 23:36 ` [PATCH 1/3] x86, ptdump: Add the functionality to dump an arbitrary pagetable Borislav Petkov
2013-12-16 23:36 ` [PATCH 3/3] efi: Make efi virtual runtime map passing more robust Borislav Petkov
[not found] ` <1387236997-26975-4-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2013-12-17 11:23 ` Borislav Petkov
2013-12-17 12:10 ` Matt Fleming
2013-12-17 13:40 ` Borislav Petkov
2013-12-17 13:51 ` Matt Fleming
[not found] ` <20131217135156.GC3145-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
2013-12-17 14:28 ` Borislav Petkov
2013-12-18 9:07 ` Dave Young
[not found] ` <20131218090757.GA15594-je1gSBvt1TcFLmT5oZ11vB/sF2h8X+2i0E9HWUfgJXw@public.gmane.org>
2013-12-18 11:42 ` Borislav Petkov
[not found] ` <1387236997-26975-1-git-send-email-bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
2013-12-16 23:36 ` [PATCH 2/3] efi: Dump the EFI page table Borislav Petkov
2013-12-17 1:11 ` [PATCH 0/3] EFI memmap fix Toshi Kani
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).