diff mbox series

[U-Boot,v3,2/2] x86: efi_loader: Use efi_add_conventional_memory_map()

Message ID A1484485FD99714DB2AB2C5EF81E7AC2AA7B9CF0@ORSMSX116.amr.corp.intel.com
State Accepted
Commit 5793553fa24077e3b91028f8097fb1fdbede1480
Delegated to: Bin Meng
Headers show
Series x86: efi_loader: Fix invalid address return from efi_alloc() | expand

Commit Message

Park, Aiden Sept. 3, 2019, 5:43 p.m. UTC 
Use efi_add_conventional_memory_map() to configure EFI conventional memory
properly with ram_top value. This will give 32bit mode U-Boot proper
conventional memory regions even if e820 has a entry which is greater than
32bit address space.

Signed-off-by: Aiden Park <aiden.park@intel.com>
---
Changes in v3:
  * Split a single commit to two relevant commits

Changes in v2:
  * Add efi_add_conventional_memory_map() for common code re-use

 arch/x86/lib/e820.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Heinrich Schuchardt Sept. 3, 2019, 7:21 p.m. UTC | #1 
On 9/3/19 7:43 PM, Park, Aiden wrote:
> Use efi_add_conventional_memory_map() to configure EFI conventional memory
> properly with ram_top value. This will give 32bit mode U-Boot proper
> conventional memory regions even if e820 has a entry which is greater than
> 32bit address space.
>
> Signed-off-by: Aiden Park <aiden.park@intel.com>

Together with Bin's patch series for supporting >3GB
(https://lists.denx.de/pipermail/u-boot/2019-August/382332.html) I see
the following memory map on an 8GB qemu-x86_defconfig
(CONFIG_CMD_EFIDEBUG=y):

==> efidebug memmap
Type             Start            End              Attributes
================ ================ ================ ==========
CONVENTIONAL     0000000000000000-00000000000a0000 WB
RESERVED         00000000000a0000-0000000000100000 WB
CONVENTIONAL     0000000000100000-00000000becf4000 WB
LOADER DATA      00000000becf4000-00000000becf5000 WB
BOOT DATA        00000000becf5000-00000000becf6000 WB
RUNTIME DATA     00000000becf6000-00000000bed07000 WB|RT
BOOT DATA        00000000bed07000-00000000bed09000 WB
RESERVED         00000000bed09000-00000000bed0a000 WB
BOOT DATA        00000000bed0a000-00000000bed0c000 WB
RUNTIME DATA     00000000bed0c000-00000000bed0d000 WB|RT
LOADER DATA      00000000bed0d000-00000000bff4f000 WB
RUNTIME CODE     00000000bff4f000-00000000bff50000 WB|RT
LOADER DATA      00000000bff50000-00000000c0000000 WB
RESERVED         00000000e0000000-00000000f0000000 WB
BOOT DATA        0000000100000000-0000000240000000 WB

Close to 3GB in low memory and 5 GB above the 4GB boundary.

Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

@Bin:
If you plan to create a pull request for RC4, you can take both patches
to avoid unnecessary dependencies.

Otherwise I will try to get patch 1/2 into RC4.

Best regards

Heinrich
Park, Aiden Sept. 10, 2019, 5:40 a.m. UTC | #2 
Hi Bin,

> -----Original Message-----
> From: Heinrich Schuchardt [mailto:xypron.glpk@gmx.de]
> Sent: Tuesday, September 3, 2019 12:21 PM
> To: Park, Aiden <aiden.park@intel.com>; Bin Meng <bmeng.cn@gmail.com>;
> Alexander Graf <agraf@csgraf.de>; u-boot@lists.denx.de
> Subject: Re: [PATCH v3 2/2] x86: efi_loader: Use
> efi_add_conventional_memory_map()
> 
> On 9/3/19 7:43 PM, Park, Aiden wrote:
> > Use efi_add_conventional_memory_map() to configure EFI conventional
> > memory properly with ram_top value. This will give 32bit mode U-Boot
> > proper conventional memory regions even if e820 has a entry which is
> > greater than 32bit address space.
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> 
> Together with Bin's patch series for supporting >3GB
> (https://lists.denx.de/pipermail/u-boot/2019-August/382332.html) I see the
> following memory map on an 8GB qemu-x86_defconfig
> (CONFIG_CMD_EFIDEBUG=y):
> 
> ==> efidebug memmap
> Type             Start            End              Attributes
> ================ ================ ================ ==========
> CONVENTIONAL     0000000000000000-00000000000a0000 WB
> RESERVED         00000000000a0000-0000000000100000 WB
> CONVENTIONAL     0000000000100000-00000000becf4000 WB
> LOADER DATA      00000000becf4000-00000000becf5000 WB
> BOOT DATA        00000000becf5000-00000000becf6000 WB
> RUNTIME DATA     00000000becf6000-00000000bed07000 WB|RT
> BOOT DATA        00000000bed07000-00000000bed09000 WB
> RESERVED         00000000bed09000-00000000bed0a000 WB
> BOOT DATA        00000000bed0a000-00000000bed0c000 WB
> RUNTIME DATA     00000000bed0c000-00000000bed0d000 WB|RT
> LOADER DATA      00000000bed0d000-00000000bff4f000 WB
> RUNTIME CODE     00000000bff4f000-00000000bff50000 WB|RT
> LOADER DATA      00000000bff50000-00000000c0000000 WB
> RESERVED         00000000e0000000-00000000f0000000 WB
> BOOT DATA        0000000100000000-0000000240000000 WB
> 
> Close to 3GB in low memory and 5 GB above the 4GB boundary.
> 
> Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> @Bin:
> If you plan to create a pull request for RC4, you can take both patches to avoid
> unnecessary dependencies.
> 
> Otherwise I will try to get patch 1/2 into RC4.
> 
In U-Boot master branch, it looks Heinrich did pull request with patch 1/2 and
it was already merged. Can you merge patch 2/2 as well? This blocks OS booting
on real hardware. Appreciate your help.

> Best regards
> 
> Heinrich

Best Regards,
Aiden
Bin Meng Sept. 10, 2019, 6:10 a.m. UTC | #3 
Hi Aiden,

On Tue, Sep 10, 2019 at 1:40 PM Park, Aiden <aiden.park@intel.com> wrote:
>
> Hi Bin,
>
> > -----Original Message-----
> > From: Heinrich Schuchardt [mailto:xypron.glpk@gmx.de]
> > Sent: Tuesday, September 3, 2019 12:21 PM
> > To: Park, Aiden <aiden.park@intel.com>; Bin Meng <bmeng.cn@gmail.com>;
> > Alexander Graf <agraf@csgraf.de>; u-boot@lists.denx.de
> > Subject: Re: [PATCH v3 2/2] x86: efi_loader: Use
> > efi_add_conventional_memory_map()
> >
> > On 9/3/19 7:43 PM, Park, Aiden wrote:
> > > Use efi_add_conventional_memory_map() to configure EFI conventional
> > > memory properly with ram_top value. This will give 32bit mode U-Boot
> > > proper conventional memory regions even if e820 has a entry which is
> > > greater than 32bit address space.
> > >
> > > Signed-off-by: Aiden Park <aiden.park@intel.com>
> >
> > Together with Bin's patch series for supporting >3GB
> > (https://lists.denx.de/pipermail/u-boot/2019-August/382332.html) I see the
> > following memory map on an 8GB qemu-x86_defconfig
> > (CONFIG_CMD_EFIDEBUG=y):
> >
> > ==> efidebug memmap
> > Type             Start            End              Attributes
> > ================ ================ ================ ==========
> > CONVENTIONAL     0000000000000000-00000000000a0000 WB
> > RESERVED         00000000000a0000-0000000000100000 WB
> > CONVENTIONAL     0000000000100000-00000000becf4000 WB
> > LOADER DATA      00000000becf4000-00000000becf5000 WB
> > BOOT DATA        00000000becf5000-00000000becf6000 WB
> > RUNTIME DATA     00000000becf6000-00000000bed07000 WB|RT
> > BOOT DATA        00000000bed07000-00000000bed09000 WB
> > RESERVED         00000000bed09000-00000000bed0a000 WB
> > BOOT DATA        00000000bed0a000-00000000bed0c000 WB
> > RUNTIME DATA     00000000bed0c000-00000000bed0d000 WB|RT
> > LOADER DATA      00000000bed0d000-00000000bff4f000 WB
> > RUNTIME CODE     00000000bff4f000-00000000bff50000 WB|RT
> > LOADER DATA      00000000bff50000-00000000c0000000 WB
> > RESERVED         00000000e0000000-00000000f0000000 WB
> > BOOT DATA        0000000100000000-0000000240000000 WB
> >
> > Close to 3GB in low memory and 5 GB above the 4GB boundary.
> >
> > Tested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > @Bin:
> > If you plan to create a pull request for RC4, you can take both patches to avoid
> > unnecessary dependencies.
> >
> > Otherwise I will try to get patch 1/2 into RC4.
> >
> In U-Boot master branch, it looks Heinrich did pull request with patch 1/2 and
> it was already merged. Can you merge patch 2/2 as well? This blocks OS booting
> on real hardware. Appreciate your help.
>

Yes, I will take the other one. Thanks!

Regards,
Bin
Bin Meng Sept. 10, 2019, 6:31 a.m. UTC | #4 
On Wed, Sep 4, 2019 at 1:43 AM Park, Aiden <aiden.park@intel.com> wrote:
>
> Use efi_add_conventional_memory_map() to configure EFI conventional memory
> properly with ram_top value. This will give 32bit mode U-Boot proper

nits: 32-bit

> conventional memory regions even if e820 has a entry which is greater than

nits: an entry

> 32bit address space.

32-bit

>
> Signed-off-by: Aiden Park <aiden.park@intel.com>
> ---
> Changes in v3:
>   * Split a single commit to two relevant commits
>
> Changes in v2:
>   * Add efi_add_conventional_memory_map() for common code re-use
>
>  arch/x86/lib/e820.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
>

Fixed the above nits, and

applied to u-boot-x86, thanks!
Park, Aiden Sept. 10, 2019, 7:22 a.m. UTC | #5 
Thanks Bin!

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn@gmail.com]
> Sent: Monday, September 9, 2019 11:31 PM
> To: Park, Aiden <aiden.park@intel.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>; Alexander Graf
> <agraf@csgraf.de>; u-boot@lists.denx.de
> Subject: Re: [PATCH v3 2/2] x86: efi_loader: Use
> efi_add_conventional_memory_map()
> 
> On Wed, Sep 4, 2019 at 1:43 AM Park, Aiden <aiden.park@intel.com> wrote:
> >
> > Use efi_add_conventional_memory_map() to configure EFI conventional
> > memory properly with ram_top value. This will give 32bit mode U-Boot
> > proper
> 
> nits: 32-bit
> 
> > conventional memory regions even if e820 has a entry which is greater
> > than
> 
> nits: an entry
> 
> > 32bit address space.
> 
> 32-bit
> 
> >
> > Signed-off-by: Aiden Park <aiden.park@intel.com>
> > ---
> > Changes in v3:
> >   * Split a single commit to two relevant commits
> >
> > Changes in v2:
> >   * Add efi_add_conventional_memory_map() for common code re-use
> >
> >  arch/x86/lib/e820.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> 
> Fixed the above nits, and
> 
> applied to u-boot-x86, thanks!

Best Regards,
Aiden
diff mbox series

Patch

diff --git a/arch/x86/lib/e820.c b/arch/x86/lib/e820.c
index d6ae2c4e9d..26da4d2f27 100644
--- a/arch/x86/lib/e820.c
+++ b/arch/x86/lib/e820.c
@@ -41,14 +41,17 @@  void efi_add_known_memory(void)
 {
 	struct e820_entry e820[E820MAX];
 	unsigned int i, num;
-	u64 start, pages;
+	u64 start, pages, ram_top;
 	int type;
 
 	num = install_e820_map(ARRAY_SIZE(e820), e820);
 
+	ram_top = (u64)gd->ram_top & ~EFI_PAGE_MASK;
+	if (!ram_top)
+		ram_top = 0x100000000ULL;
+
 	for (i = 0; i < num; ++i) {
 		start = e820[i].addr;
-		pages = ALIGN(e820[i].size, EFI_PAGE_SIZE) >> EFI_PAGE_SHIFT;
 
 		switch (e820[i].type) {
 		case E820_RAM:
@@ -69,7 +72,15 @@  void efi_add_known_memory(void)
 			break;
 		}
 
-		efi_add_memory_map(start, pages, type, false);
+		if (type == EFI_CONVENTIONAL_MEMORY) {
+			efi_add_conventional_memory_map(start,
+							start + e820[i].size,
+							ram_top);
+		} else {
+			pages = ALIGN(e820[i].size, EFI_PAGE_SIZE)
+				>> EFI_PAGE_SHIFT;
+			efi_add_memory_map(start, pages, type, false);
+		}
 	}
 }
 #endif /* CONFIG_IS_ENABLED(EFI_LOADER) */