[PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Lee, Chun-Yi]

When testing hibernate, I found the EFI runtime services was broken
on some old EFI machines on my hand, Intel DQ57TM development board
and Acer Gateway Z5WT2 notebook.

After printing the EFI memmap and virtual address mapping on -4G area,
found those issue machines keep the physical address of Runtime
Data/Code regions unchanged but not Boot Data/Code. The logs were
attached on openSUSE bug:
    https://bugzilla.suse.com/show_bug.cgi?id=939979

Due to Boot Data/Code can be used by OS as available memory regions,
so those old EFI BIOS do not keep the physical address of Boot regions
unchanged. But, address of Runtime regions are the same.

On Intel DQ57TM, sometimes the order of EFI Boot regions changed. On
Acer Gateway Z5WT2, the amount of EFI Boot regions changed.

The above changing of EFI Boot regions causes the EFI Runtime Data/Code
may not mapping to constant virtual address, that's because the EFI Boot
and Runtime regions are interleaved and EFI va mapping applied PMD
2M-aligned logic.

A workaround of this situation is mapping Boot and Runtime regions to
different starting virtual address. Then the changing of Boot Data/Code
regions will not affect to the virtual address mapping to Runtime
Data/Code.

This patch adds codes for mapping Boot Data/Code regions start from
0xffff_ffff_0000_0000, has 1G space. And mapping Runtime Data/Code
regions start from 0xffff_fffe_c000_0000 that has 63G space.

Link: https://bugzilla.suse.com/show_bug.cgi?id=939979
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 arch/x86/platform/efi/efi_64.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index a0ac0f9..fde7f8f 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -42,10 +42,14 @@
 #include <asm/time.h>
 
 /*
- * We allocate runtime services regions bottom-up, starting from -4G, i.e.
- * 0xffff_ffff_0000_0000 and limit EFI VA mapping space to 64G.
+ * We allocate boot and runtime services regions bottom-up, starting from -4G,
+ * i.e. 0xffff_ffff_0000_0000 and limit EFI VA mapping space to 64G.
+ *
+ * Boot Data/Code are starting from 0xffff_ffff_0000_0000 (1G space)
+ * Runtime Data/Code are starting from 0xffff_fffe_c000_0000 (63G space)
  */
-static u64 efi_va = EFI_VA_START;
+static u64 efi_boot_va = EFI_VA_START;
+static u64 efi_runtime_va = EFI_VA_START - 0x40000000;
 
 /*
  * Scratch space used for switching the pagetable in the EFI stub
@@ -218,6 +222,7 @@ void __init efi_map_region(efi_memory_desc_t *md)
 {
 	unsigned long size = md->num_pages << PAGE_SHIFT;
 	u64 pa = md->phys_addr;
+	u64 *efi_va = &efi_boot_va;
 
 	if (efi_enabled(EFI_OLD_MEMMAP))
 		return old_map_region(md);
@@ -239,30 +244,33 @@ void __init efi_map_region(efi_memory_desc_t *md)
 		return;
 	}
 
-	efi_va -= size;
+	if (md->attribute & EFI_MEMORY_RUNTIME)
+		efi_va = &efi_runtime_va;
+
+	*efi_va -= size;
 
 	/* Is PA 2M-aligned? */
 	if (!(pa & (PMD_SIZE - 1))) {
-		efi_va &= PMD_MASK;
+		*efi_va &= PMD_MASK;
 	} else {
 		u64 pa_offset = pa & (PMD_SIZE - 1);
-		u64 prev_va = efi_va;
+		u64 prev_va = *efi_va;
 
 		/* get us the same offset within this 2M page */
-		efi_va = (efi_va & PMD_MASK) + pa_offset;
+		*efi_va = (*efi_va & PMD_MASK) + pa_offset;
 
-		if (efi_va > prev_va)
-			efi_va -= PMD_SIZE;
+		if (*efi_va > prev_va)
+			*efi_va -= PMD_SIZE;
 	}
 
-	if (efi_va < EFI_VA_END) {
+	if (*efi_va < EFI_VA_END) {
 		pr_warn(FW_WARN "VA address range overflow!\n");
 		return;
 	}
 
 	/* Do the VA map */
-	__map_region(md, efi_va);
-	md->virt_addr = efi_va;
+	__map_region(md, *efi_va);
+	md->virt_addr = *efi_va;
 }
 
 /*
-- 
1.8.4.5

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[H. Peter Anvin]

On 07/29/2015 09:32 PM, Lee, Chun-Yi wrote:
> When testing hibernate, I found the EFI runtime services was broken
> on some old EFI machines on my hand, Intel DQ57TM development board
> and Acer Gateway Z5WT2 notebook.
> 
> After printing the EFI memmap and virtual address mapping on -4G area,
> found those issue machines keep the physical address of Runtime
> Data/Code regions unchanged but not Boot Data/Code. The logs were
> attached on openSUSE bug:
>     https://bugzilla.suse.com/show_bug.cgi?id=939979
> 
> Due to Boot Data/Code can be used by OS as available memory regions,
> so those old EFI BIOS do not keep the physical address of Boot regions
> unchanged. But, address of Runtime regions are the same.
> 
> On Intel DQ57TM, sometimes the order of EFI Boot regions changed. On
> Acer Gateway Z5WT2, the amount of EFI Boot regions changed.
> 
> The above changing of EFI Boot regions causes the EFI Runtime Data/Code
> may not mapping to constant virtual address, that's because the EFI Boot
> and Runtime regions are interleaved and EFI va mapping applied PMD
> 2M-aligned logic.
> 
> A workaround of this situation is mapping Boot and Runtime regions to
> different starting virtual address. Then the changing of Boot Data/Code
> regions will not affect to the virtual address mapping to Runtime
> Data/Code.
> 
> This patch adds codes for mapping Boot Data/Code regions start from
> 0xffff_ffff_0000_0000, has 1G space. And mapping Runtime Data/Code
> regions start from 0xffff_fffe_c000_0000 that has 63G space.
> 

This changelog is at least partially incomprehensive.  It also seems
more than a bit aggressive to expect that 1 GB will be sufficient forever.

	-hpa

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Borislav Petkov]

On Thu, Jul 30, 2015 at 12:53:42AM -0700, H. Peter Anvin wrote:
> This changelog is at least partially incomprehensive. It also seems
> more than a bit aggressive to expect that 1 GB will be sufficient
> forever.

Right, before we do anything, I'd like for us to figure out first why
this is a problem all of a sudden. And why should we even keep boot
code/data, if it is fair game, once runtime services get enabled.

Matt, can you please chime in first before we even talk about a
solution...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Matt Fleming]

On Thu, 30 Jul, at 10:03:23AM, Borislav Petkov wrote:
> On Thu, Jul 30, 2015 at 12:53:42AM -0700, H. Peter Anvin wrote:
> > This changelog is at least partially incomprehensive. It also seems
> > more than a bit aggressive to expect that 1 GB will be sufficient
> > forever.
> 
> Right, before we do anything, I'd like for us to figure out first why
> this is a problem all of a sudden. And why should we even keep boot
> code/data, if it is fair game, once runtime services get enabled.
> 
> Matt, can you please chime in first before we even talk about a
> solution...

Yeah, I do not understand the issue properly.

Why do we care about EfiBoot* regions after hibernate? Surely we've
already freed those regions in efi_free_boot_services() during boot and
nobody should be touching them again? If the firmware does, that's a
whole new bug we've never encountered before.

And we obviously can't allow the runtime regions to move around during
hibernate/resume because we've already informed the firmware where those
regions live during SetVirtualAddressMap() at boot.

I admit that I haven't looked at the hibernate code paths. Let me go do
that now.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Thu, Jul 30, 2015 at 11:11:31AM +0100, Matt Fleming wrote:
> On Thu, 30 Jul, at 10:03:23AM, Borislav Petkov wrote:
> > On Thu, Jul 30, 2015 at 12:53:42AM -0700, H. Peter Anvin wrote:
> > > This changelog is at least partially incomprehensive. It also seems
> > > more than a bit aggressive to expect that 1 GB will be sufficient
> > > forever.
> > 
> > Right, before we do anything, I'd like for us to figure out first why
> > this is a problem all of a sudden. And why should we even keep boot
> > code/data, if it is fair game, once runtime services get enabled.
> > 
> > Matt, can you please chime in first before we even talk about a
> > solution...
> 
> Yeah, I do not understand the issue properly.
> 
> Why do we care about EfiBoot* regions after hibernate? Surely we've
> already freed those regions in efi_free_boot_services() during boot and
> nobody should be touching them again? If the firmware does, that's a
> whole new bug we've never encountered before.
> 
> And we obviously can't allow the runtime regions to move around during
> hibernate/resume because we've already informed the firmware where those
> regions live during SetVirtualAddressMap() at boot.
> 
> I admit that I haven't looked at the hibernate code paths. Let me go do
> that now.
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center

In efi_map_regions(), kernel mapping EFI_MEMORY_RUNTIME and
EFI_BOOT_SERVICES_CODE/DATA regions to trampoline_pgd.

UEFI keeps the physical address of Runtime Code/Data were not changed in
hibernate cycle. But the changes of Boot regions causes Runtime Code/Data
mapping to different virtual address in trampoline_pgd.

The following is a case from Intel DQ57TM.

Boot:
[    0.036509] efi: mem11: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b7f7e000-0x00000000b7f9e000) va=[0xfffffffefff7e000-0xfffffffefff9e000) (0MB)
[    0.051393] efi: mem17: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8b4f000-0x00000000b8b50000) va=[0xfffffffefff4f000-0xfffffffefff50000) (0MB)
[    0.066271] efi: mem19: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8c2f000-0x00000000b9135000) va=[0xfffffffeffa2f000-0xfffffffefff35000) (5MB)
[    0.081152] efi: mem21: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9138000-0x00000000baf7d000) va=[0xfffffffefdb38000-0xfffffffeff97d000) (30MB)
[    0.096120] efi: mem23: [Boot Code          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb04b000-0x00000000bb385000) va=[0xfffffffefd64b000-0xfffffffefd985000) (3MB)
[    0.111002] efi: mem24: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd585000-0xfffffffefd5e5000) (0MB)
[    0.125883] efi: mem25: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd3e5000-0xfffffffefd445000) (0MB)
[    0.140764] efi: mem29: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefd1ff000-0xfffffffefd200000) (0MB)

Hibernate resumed:
[    0.036486] efi: mem11: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b7f7e000-0x00000000b7f9e000) va=[0xfffffffefff7e000-0xfffffffefff9e000) (0MB)
[    0.051375] efi: mem17: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8c0d000-0x00000000b9113000) va=[0xfffffffeffa0d000-0xfffffffefff13000) (5MB)	<=== order of Boot Data changed
[    0.066253] efi: mem20: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9132000-0x00000000b9133000) va=[0xfffffffeff932000-0xfffffffeff933000) (0MB)
[    0.081135] efi: mem22: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9138000-0x00000000baf7d000) va=[0xfffffffefd938000-0xfffffffeff77d000) (30MB)
[    0.096103] efi: mem24: [Boot Code          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb04b000-0x00000000bb385000) va=[0xfffffffefd44b000-0xfffffffefd785000) (3MB)
[    0.110984] efi: mem25: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd385000-0xfffffffefd3e5000) (0MB)	<=== va changed
[    0.125865] efi: mem26: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd1e5000-0xfffffffefd245000) (0MB)
[    0.140747] efi: mem30: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefcfff000-0xfffffffefd000000) (0MB)

Look at the origin va of Runtime Code is 0xfffffffefd585000, but after hibernate
result boot, the new va is 0xfffffffefd385000.

Issue machine's UEFI keeps the PA of Runtime regions unchanged, but Boot regions
moved. That causes Runtime regions mapping to different VA.



Regards
Joey Lee

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Thu, Jul 30, 2015 at 07:09:59PM +0800, joeyli wrote:
> On Thu, Jul 30, 2015 at 11:11:31AM +0100, Matt Fleming wrote:
> > On Thu, 30 Jul, at 10:03:23AM, Borislav Petkov wrote:
> > > On Thu, Jul 30, 2015 at 12:53:42AM -0700, H. Peter Anvin wrote:
> > > > This changelog is at least partially incomprehensive. It also seems
> > > > more than a bit aggressive to expect that 1 GB will be sufficient
> > > > forever.
> > > 
> > > Right, before we do anything, I'd like for us to figure out first why
> > > this is a problem all of a sudden. And why should we even keep boot
> > > code/data, if it is fair game, once runtime services get enabled.
> > > 
> > > Matt, can you please chime in first before we even talk about a
> > > solution...
> > 
> > Yeah, I do not understand the issue properly.
> > 
> > Why do we care about EfiBoot* regions after hibernate? Surely we've
> > already freed those regions in efi_free_boot_services() during boot and
> > nobody should be touching them again? If the firmware does, that's a
> > whole new bug we've never encountered before.
> > 
> > And we obviously can't allow the runtime regions to move around during
> > hibernate/resume because we've already informed the firmware where those
> > regions live during SetVirtualAddressMap() at boot.
> > 
> > I admit that I haven't looked at the hibernate code paths. Let me go do
> > that now.
> > 
> > -- 
> > Matt Fleming, Intel Open Source Technology Center
> 
> In efi_map_regions(), kernel mapping EFI_MEMORY_RUNTIME and
> EFI_BOOT_SERVICES_CODE/DATA regions to trampoline_pgd.
> 
> UEFI keeps the physical address of Runtime Code/Data were not changed in
> hibernate cycle. But the changes of Boot regions causes Runtime Code/Data
> mapping to different virtual address in trampoline_pgd.
> 
> The following is a case from Intel DQ57TM.
> 
> Boot:
> [    0.036509] efi: mem11: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b7f7e000-0x00000000b7f9e000) va=[0xfffffffefff7e000-0xfffffffefff9e000) (0MB)
> [    0.051393] efi: mem17: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8b4f000-0x00000000b8b50000) va=[0xfffffffefff4f000-0xfffffffefff50000) (0MB)
> [    0.066271] efi: mem19: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8c2f000-0x00000000b9135000) va=[0xfffffffeffa2f000-0xfffffffefff35000) (5MB)
> [    0.081152] efi: mem21: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9138000-0x00000000baf7d000) va=[0xfffffffefdb38000-0xfffffffeff97d000) (30MB)
> [    0.096120] efi: mem23: [Boot Code          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb04b000-0x00000000bb385000) va=[0xfffffffefd64b000-0xfffffffefd985000) (3MB)
> [    0.111002] efi: mem24: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd585000-0xfffffffefd5e5000) (0MB)
> [    0.125883] efi: mem25: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd3e5000-0xfffffffefd445000) (0MB)
> [    0.140764] efi: mem29: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefd1ff000-0xfffffffefd200000) (0MB)
> 
> Hibernate resumed:
> [    0.036486] efi: mem11: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b7f7e000-0x00000000b7f9e000) va=[0xfffffffefff7e000-0xfffffffefff9e000) (0MB)
> [    0.051375] efi: mem17: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b8c0d000-0x00000000b9113000) va=[0xfffffffeffa0d000-0xfffffffefff13000) (5MB)	<=== order of Boot Data changed
> [    0.066253] efi: mem20: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9132000-0x00000000b9133000) va=[0xfffffffeff932000-0xfffffffeff933000) (0MB)
> [    0.081135] efi: mem22: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000b9138000-0x00000000baf7d000) va=[0xfffffffefd938000-0xfffffffeff77d000) (30MB)
> [    0.096103] efi: mem24: [Boot Code          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb04b000-0x00000000bb385000) va=[0xfffffffefd44b000-0xfffffffefd785000) (3MB)
> [    0.110984] efi: mem25: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd385000-0xfffffffefd3e5000) (0MB)	<=== va changed
> [    0.125865] efi: mem26: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd1e5000-0xfffffffefd245000) (0MB)
> [    0.140747] efi: mem30: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefcfff000-0xfffffffefd000000) (0MB)
> 
> Look at the origin va of Runtime Code is 0xfffffffefd585000, but after hibernate
> result boot, the new va is 0xfffffffefd385000.
> 
> Issue machine's UEFI keeps the PA of Runtime regions unchanged, but Boot regions
> moved. That causes Runtime regions mapping to different VA.
> 
> 
> 
> Regards
> Joey Lee

In the above case, just simply accessing EFI variable through efivars to
reproduce issue:

linux-aiip:~ # hexdump -C /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
Killed

[  257.856185] BUG: unable to handle kernel paging request at fffffffefd244e60		<=== address is in Runtime Data region
[  257.863200] IP: [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
[  257.869586] PGD 1a0c067 PUD 13b0ae063 PMD 13b0bc063 PTE 0
[  257.875056] Oops: 0000 [#1] SMP 
[...snip]
[  258.149804] RIP  [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
[  258.156273]  RSP <ffff8800ba07fd78>
[  258.159769] CR2: fffffffefd244e60
[  258.163090] ---[ end trace 9edb589760c07d3e ]---

0xfffffffefd244e60 is covered by Runtime Data region after hibernate resumed:  

[    0.125865] efi: mem26: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd1e5000-0xfffffffefd245000) (0MB)
										  					     			0xfffffffefd244e60

But this va address didn't mapped to any pa in hibernating kernel (image kernel):

[    0.111002] efi: mem24: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd585000-0xfffffffefd5e5000) (0MB)
[    0.125883] efi: mem25: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd3e5000-0xfffffffefd445000) (0MB)
																                0xfffffffefd244e60	<=== no mapping
[    0.140764] efi: mem29: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefd1ff000-0xfffffffefd200000) (0MB)


So, when call EFI runtime services, got problem.


Regards
Joey Lee

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Matt Fleming]

On Thu, 30 Jul, at 07:18:19PM, joeyli wrote:
> 
> In the above case, just simply accessing EFI variable through efivars to
> reproduce issue:
> 
> linux-aiip:~ # hexdump -C /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
> Killed
> 
> [  257.856185] BUG: unable to handle kernel paging request at fffffffefd244e60		<=== address is in Runtime Data region
> [  257.863200] IP: [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
> [  257.869586] PGD 1a0c067 PUD 13b0ae063 PMD 13b0bc063 PTE 0
> [  257.875056] Oops: 0000 [#1] SMP 
> [...snip]
> [  258.149804] RIP  [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
> [  258.156273]  RSP <ffff8800ba07fd78>
> [  258.159769] CR2: fffffffefd244e60
> [  258.163090] ---[ end trace 9edb589760c07d3e ]---
> 
> 0xfffffffefd244e60 is covered by Runtime Data region after hibernate resumed:  
> 
> [    0.125865] efi: mem26: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd1e5000-0xfffffffefd245000) (0MB)
> 										  					     			0xfffffffefd244e60
> 
> But this va address didn't mapped to any pa in hibernating kernel (image kernel):
> 
> [    0.111002] efi: mem24: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd585000-0xfffffffefd5e5000) (0MB)
> [    0.125883] efi: mem25: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd3e5000-0xfffffffefd445000) (0MB)
> 																                0xfffffffefd244e60	<=== no mapping
> [    0.140764] efi: mem29: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefd1ff000-0xfffffffefd200000) (0MB)
> 
> 
> So, when call EFI runtime services, got problem.

Why is the new kernel calling the EFI runtime services via the old
mappings?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Thu, Jul 30, 2015 at 01:09:16PM +0100, Matt Fleming wrote:
> On Thu, 30 Jul, at 07:18:19PM, joeyli wrote:
> > 
> > In the above case, just simply accessing EFI variable through efivars to
> > reproduce issue:
> > 
> > linux-aiip:~ # hexdump -C /sys/firmware/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
> > Killed
> > 
> > [  257.856185] BUG: unable to handle kernel paging request at fffffffefd244e60		<=== address is in Runtime Data region
> > [  257.863200] IP: [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
> > [  257.869586] PGD 1a0c067 PUD 13b0ae063 PMD 13b0bc063 PTE 0
> > [  257.875056] Oops: 0000 [#1] SMP 
> > [...snip]
> > [  258.149804] RIP  [<ffffffff81457cdd>] virt_efi_get_variable+0x5d/0xa0
> > [  258.156273]  RSP <ffff8800ba07fd78>
> > [  258.159769] CR2: fffffffefd244e60
> > [  258.163090] ---[ end trace 9edb589760c07d3e ]---
> > 
> > 0xfffffffefd244e60 is covered by Runtime Data region after hibernate resumed:  
> > 
> > [    0.125865] efi: mem26: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd1e5000-0xfffffffefd245000) (0MB)
> > 										  					     			0xfffffffefd244e60
> > 
> > But this va address didn't mapped to any pa in hibernating kernel (image kernel):
> > 
> > [    0.111002] efi: mem24: [Runtime Code       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb385000-0x00000000bb3e5000) va=[0xfffffffefd585000-0xfffffffefd5e5000) (0MB)
> > [    0.125883] efi: mem25: [Runtime Data       |RUN|  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb3e5000-0x00000000bb445000) va=[0xfffffffefd3e5000-0xfffffffefd445000) (0MB)
> > 																                0xfffffffefd244e60	<=== no mapping
> > [    0.140764] efi: mem29: [Boot Data          |   |  |  |  |   |WB|WT|WC|UC] pa=[0x00000000bb7ff000-0x00000000bb800000) va=[0xfffffffefd1ff000-0xfffffffefd200000) (0MB)
> > 
> > 
> > So, when call EFI runtime services, got problem.
> 
> Why is the new kernel calling the EFI runtime services via the old
> mappings?
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center

I think hibernate overwrite it.

Joey Lee

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Matt Fleming]

On Thu, 30 Jul, at 08:31:16PM, joeyli wrote:
> 
> I think hibernate overwrite it.

We absolutely must get a more detailed answer before going any further.

Simply put, if we're remapping the EFI regions into the virtual address
space and calling SetVirtualAddressMap() on hibernate resume there is no
reason that anyone should be using the old mappings.

And since you've demonstrated that we *are* using the old mappings,
we've likely got a bug somewhere that we need to get a handle on before
we paper over the issue.

Where exactly is the old mapping address being used? Is it that
efi.systab->runtime->get_variable is incorrect? If you could paste the
disassembled output where the page fault occurs, that would be helpful.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Thu, Jul 30, 2015 at 02:17:23PM +0100, Matt Fleming wrote:
> On Thu, 30 Jul, at 08:31:16PM, joeyli wrote:
> > 
> > I think hibernate overwrite it.
> 
> We absolutely must get a more detailed answer before going any further.
> 
> Simply put, if we're remapping the EFI regions into the virtual address
> space and calling SetVirtualAddressMap() on hibernate resume there is no
> reason that anyone should be using the old mappings.
> 
> And since you've demonstrated that we *are* using the old mappings,
> we've likely got a bug somewhere that we need to get a handle on before
> we paper over the issue.
> 
> Where exactly is the old mapping address being used? Is it that
> efi.systab->runtime->get_variable is incorrect? If you could paste the
> disassembled output where the page fault occurs, that would be helpful.
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center

OK, understood! Thanks for your suggestion!

But, I have a question about mapping Boot Code/Data to -4G area. I understand
we need Runtime regions, and 1:1 mapping is the workaround of some buggy BIOS.
But why should kernel mapping Boot regions to -4G area?


Thanks a lot!
Joey Lee

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Matt Fleming]

On Thu, 30 Jul, at 09:39:47PM, joeyli wrote:
> 
> OK, understood! Thanks for your suggestion!
> 
> But, I have a question about mapping Boot Code/Data to -4G area. I understand
> we need Runtime regions, and 1:1 mapping is the workaround of some buggy BIOS.
> But why should kernel mapping Boot regions to -4G area?

We map the Boot Service regions in the -4G area for simplicity, i.e.
it's easier to iterate through the EFI memory map in order, and map
things starting at -4G and working down to lower addresses, than it
would be to make different mapping decisions based on the region type.

As for why we map the Boot Service regions *at all*, that's also a bug
workaround, see commit 916f676f8dc0 ("x86, efi: Retain boot service code
until after switching to virtual mode"). We setup the mappings before we
call efi_free_boot_services().

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Thu, Jul 30, 2015 at 03:05:42PM +0100, Matt Fleming wrote:
> On Thu, 30 Jul, at 09:39:47PM, joeyli wrote:
> > 
> > OK, understood! Thanks for your suggestion!
> > 
> > But, I have a question about mapping Boot Code/Data to -4G area. I understand
> > we need Runtime regions, and 1:1 mapping is the workaround of some buggy BIOS.
> > But why should kernel mapping Boot regions to -4G area?
> 
> We map the Boot Service regions in the -4G area for simplicity, i.e.
> it's easier to iterate through the EFI memory map in order, and map
> things starting at -4G and working down to lower addresses, than it
> would be to make different mapping decisions based on the region type.
> 
> As for why we map the Boot Service regions *at all*, that's also a bug
> workaround, see commit 916f676f8dc0 ("x86, efi: Retain boot service code
> until after switching to virtual mode"). We setup the mappings before we
> call efi_free_boot_services().
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center

Thanks for your explanation.

For my issue, I will check if rewriting the VA of runtime services can fix issue.
If not, then I think need find a way to sync the mapping in EFI page table between
boot kernel and image kernel.


Thanks a lot!
Joey Lee

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[Matt Fleming]

On Thu, 30 Jul, at 10:16:01PM, joeyli wrote:
> 
> Thanks for your explanation.
> 
> For my issue, I will check if rewriting the VA of runtime services can fix issue.
> If not, then I think need find a way to sync the mapping in EFI page table between
> boot kernel and image kernel.

Hey Joey, did you make any progress with this issue?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86_64/efi: Mapping Boot and Runtime EFI memory regions to different starting virtual address

[joeyli]

On Wed, Aug 19, 2015 at 05:31:45PM +0100, Matt Fleming wrote:
> On Thu, 30 Jul, at 10:16:01PM, joeyli wrote:
> > 
> > Thanks for your explanation.
> > 
> > For my issue, I will check if rewriting the VA of runtime services can fix issue.
> > If not, then I think need find a way to sync the mapping in EFI page table between
> > boot kernel and image kernel.
> 
> Hey Joey, did you make any progress with this issue?
> 
> -- 
> Matt Fleming, Intel Open Source Technology Center

I am sticky on other issue and hope back to trace this problem in this week.
Sorry for my delay!


Joey Lee