[U-Boot] [PATCH 1/3] efi: fix issue for 32bit targets with ram_top at 4G

[U-Boot] [PATCH 1/3] efi: fix issue for 32bit targets with ram_top at 4G

[Patrick Delaunay]

Avoid ram_end = 0 on 32bit targets with ram_end at 4G.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---
example of issue in stm32mp1 board ev1:
  ram_start = c0000000
  size = 40000000
  ram_end = 100000000
  ram_end &= ~EFI_PAGE_MASK => result is 0

 lib/efi_loader/efi_memory.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 55622d2..81dc5fc 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -574,6 +574,10 @@ __weak void efi_add_known_memory(void)
 
 		/* Remove partial pages */
 		ram_end &= ~EFI_PAGE_MASK;
+		/* Fix for 32bit targets with ram_top at 4G */
+		if (!ram_end)
+			ram_end = 0x100000000ULL;
+
 		ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
 
 		if (ram_end <= ram_start) {
-- 
2.7.4

[U-Boot] [PATCH 2/3] efi: add protection for block_dev

[Patrick Delaunay]

Check the value of block_dev before to use this pointer.

This patch solves problem for the command "load" when ubifs
is previously mounted: in this case the function
blk_get_device_part_str("ubi 0") don't return error but return
block_dev = NULL and then data abort.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

To reproduce the issue, I have a boot script 'boot.scr.uimg'
with a load command executed during ubi boot:

load ${devtype} ${devnum}:${distro_bootpart} ${m4fw_addr} ${m4fw_name}

I have a data abort for call stack:
- do_load_wrapper for "ubi 0"
-- efi_set_bootdev
--- efi_dp_from_name

=> desc = 0 and data abort for access to 'desc->*'

I also proposed a protection for the same issue in ums command
http://patchwork.ozlabs.org/project/uboot/list/?series=68096


 lib/efi_loader/efi_device_path.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 53b40c8..fd57be8 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -970,7 +970,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
 	if (!is_net) {
 		part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
 					       1);
-		if (part < 0)
+		if (part < 0 || !desc)
 			return EFI_INVALID_PARAMETER;
 
 		if (device)
-- 
2.7.4

[U-Boot] [PATCH 2/3] efi: add protection for block_dev

[Heinrich Schuchardt]

On 4/10/19 11:02 AM, Patrick Delaunay wrote:
> Check the value of block_dev before to use this pointer.
>
> This patch solves problem for the command "load" when ubifs
> is previously mounted: in this case the function
> blk_get_device_part_str("ubi 0") don't return error but return
> block_dev = NULL and then data abort.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> To reproduce the issue, I have a boot script 'boot.scr.uimg'
> with a load command executed during ubi boot:
>
> load ${devtype} ${devnum}:${distro_bootpart} ${m4fw_addr} ${m4fw_name}
>
> I have a data abort for call stack:
> - do_load_wrapper for "ubi 0"
> -- efi_set_bootdev
> --- efi_dp_from_name
>
> => desc = 0 and data abort for access to 'desc->*'

Thanks for reporting and analyzing the problem

Where exactly is the NULL dereference occurring?

Igor reported a similar bug for a USB device in
cmd: fs: fix data abort in load cmd
https://lists.denx.de/pipermail/u-boot/2019-April/364484.htmll

>
> I also proposed a protection for the same issue in ums command
> http://patchwork.ozlabs.org/project/uboot/list/?series=68096
>
>
>   lib/efi_loader/efi_device_path.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 53b40c8..fd57be8 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -970,7 +970,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   	if (!is_net) {
>   		part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
>   					       1);
> -		if (part < 0)
> +		if (part < 0 || !desc)

part = 0, desc = NULL occurs for UBI if the UBI file system is mounted.

Returning an error here means in the end that we will not be able to
install run GRUB from the UBI device because we cannot describe the boot
device.

I think that UBI volumes should be handled like any other block device.
This will avoid having separate program paths for UBI and not UBI.

Heiko and Kyungmin could you, please, explain why UBI currently is not
providing a struct blk_desc * block descriptor and how this can be fixed.

Best regards

Heinrich

 >   			return EFI_INVALID_PARAMETER;
>
>   		if (device)
>

[U-Boot] [PATCH 2/3] efi: add protection for block_dev

[Patrick DELAUNAY]

Hi Heinrich

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> On 4/10/19 11:02 AM, Patrick Delaunay wrote:
> > Check the value of block_dev before to use this pointer.
> >
> > This patch solves problem for the command "load" when ubifs is
> > previously mounted: in this case the function
> > blk_get_device_part_str("ubi 0") don't return error but return
> > block_dev = NULL and then data abort.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > To reproduce the issue, I have a boot script 'boot.scr.uimg'
> > with a load command executed during ubi boot:
> >
> > load ${devtype} ${devnum}:${distro_bootpart} ${m4fw_addr} ${m4fw_name}
> >
> > I have a data abort for call stack:
> > - do_load_wrapper for "ubi 0"
> > -- efi_set_bootdev
> > --- efi_dp_from_name
> >
> > => desc = 0 and data abort for access to 'desc->*'
> 
> Thanks for reporting and analyzing the problem
> 
> Where exactly is the NULL dereference occurring?

I see the issue on v2018.11 and I adapt the patch for v2019.04 (as efi_dp_from_name as be created in v2019.01)

On v2018.11, the stack frame was

do_load_wrapper
=> efi_set_bootdev
==> efi_dp_from_name
===> efi_dp_from_part
====> dp_part_size

With result for blk_get_device_part_str
  dev="ubi"
  devnr="0:"
  path="/boot.scr.uimg"

desc =00000000 part=0

And the crash occurs in the function dp_part_size / called from efi_dp_from_part

In trace :
	pc : [<ffc91d28>]	   lr : [<ffc92383>]
	reloc pc : [<c0166d28>]	   lr : [<c0167383>]

with symbol 

	System.map:c0167358 T efi_dp_is_multi_instance
	System.map:c0167378 T efi_dp_from_part
	System.map:c01673a4 T efi_dp_part_node
	System.map:c01673c8 T efi_dp_from_file

	System.map:c0166d22 t dp_part_size
	System.map:c0166d50 t dp_part_node


> Igor reported a similar bug for a USB device in
> cmd: fs: fix data abort in load cmd
> https://lists.denx.de/pipermail/u-boot/2019-April/364484.htmll
> 
> >
> > I also proposed a protection for the same issue in ums command
> > http://patchwork.ozlabs.org/project/uboot/list/?series=68096
> >
> >
> >   lib/efi_loader/efi_device_path.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_device_path.c
> > b/lib/efi_loader/efi_device_path.c
> > index 53b40c8..fd57be8 100644
> > --- a/lib/efi_loader/efi_device_path.c
> > +++ b/lib/efi_loader/efi_device_path.c
> > @@ -970,7 +970,7 @@ efi_status_t efi_dp_from_name(const char *dev, const
> char *devnr,
> >   	if (!is_net) {
> >   		part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
> >   					       1);
> > -		if (part < 0)
> > +		if (part < 0 || !desc)
> 
> part = 0, desc = NULL occurs for UBI if the UBI file system is mounted.
> 
> Returning an error here means in the end that we will not be able to install run
> GRUB from the UBI device because we cannot describe the boot device.
> 
> I think that UBI volumes should be handled like any other block device.
> This will avoid having separate program paths for UBI and not UBI.
> 
> Heiko and Kyungmin could you, please, explain why UBI currently is not providing
> a struct blk_desc * block descriptor and how this can be fixed.
> 
> Best regards
> 
> Heinrich
> 
>  >   			return EFI_INVALID_PARAMETER;
> >
> >   		if (device)
> >

Best regards

Patrick

[U-Boot] [PATCH 2/3] efi: add protection for block_dev

[Heinrich Schuchardt]

On 4/10/19 11:02 AM, Patrick Delaunay wrote:
> Check the value of block_dev before to use this pointer.
>
> This patch solves problem for the command "load" when ubifs
> is previously mounted: in this case the function
> blk_get_device_part_str("ubi 0") don't return error but return
> block_dev = NULL and then data abort.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Added to efi-2019-07 branch.

> ---
>
> To reproduce the issue, I have a boot script 'boot.scr.uimg'
> with a load command executed during ubi boot:
>
> load ${devtype} ${devnum}:${distro_bootpart} ${m4fw_addr} ${m4fw_name}
>
> I have a data abort for call stack:
> - do_load_wrapper for "ubi 0"
> -- efi_set_bootdev
> --- efi_dp_from_name
>
> => desc = 0 and data abort for access to 'desc->*'
>
> I also proposed a protection for the same issue in ums command
> http://patchwork.ozlabs.org/project/uboot/list/?series=68096
>
>
>   lib/efi_loader/efi_device_path.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 53b40c8..fd57be8 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -970,7 +970,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   	if (!is_net) {
>   		part = blk_get_device_part_str(dev, devnr, &desc, &fs_partition,
>   					       1);
> -		if (part < 0)
> +		if (part < 0 || !desc)
>   			return EFI_INVALID_PARAMETER;
>
>   		if (device)
>

[U-Boot] [PATCH 3/3] efi: update virtual address in efi_mem_carve_out

[Patrick Delaunay]

Handle virtual address in efi_mem_carve_out function
when a new region is created to avoid issue in EFI memory map.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Issue detected during test of test of UEFI on ev1/dk2 boards.

For debug purpose, I add the dump of the map in efi_mem_sort:

static void efi_mem_sort(void)
{
.....
	int i = 0;
	printf("-- %s -------------------\n", __func__ );
	list_for_each(lhandle, &efi_mem) {
		struct efi_mem_list *lmem;
		struct efi_mem_desc *cur;

		lmem = list_entry(lhandle, struct efi_mem_list, link);
		cur = &lmem->desc;

		printf("%02d: va %08x pa %08x size %08x (%04x pages) => va end %08x attribute = %llx type=%d\n",
			i++,
			(u32)cur->virtual_start,
			(u32)cur->physical_start,
			(u32)(cur->num_pages << EFI_PAGE_SHIFT),
			(u32)cur->num_pages,
			(u32) (cur->virtual_start + (cur->num_pages << EFI_PAGE_SHIFT)),
			cur->attribute,
			cur->type);
	}
}

I get the memory layout (for EV1):

00: va fcf87000 pa fff92000 size 0006e000 (006e pages) => va end fcff5000 attribute = 8 type=2
01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5
02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2
03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6
04: va fcf46000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf4f000 attribute = 8 type=0
05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6
06: va fcf46000 pa fcf77000 size 00005000 (0005 pages) => va end fcf4b000 attribute = 8 type=0
07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7

But for some region virtual address (va) != physical address (pa)

After check of the code, the virtual address is not correctly managed
when new region is created.

I don't sure that could be a issue, but I solve the issue
with the proposed patch:

00: va fff92000 pa fff92000 size 0006e000 (006e pages) => va end 00000000 attribute = 8 type=2
01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5
02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2
03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6
04: va fcf7d000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf86000 attribute = 8 type=0
05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6
06: va fcf77000 pa fcf77000 size 00005000 (0005 pages) => va end fcf7c000 attribute = 8 type=0
07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7

The virtual address are now correct.

PS: I don't found more elegant code to update the virtual address
    but I avoid to force virtual_address = physical address
    in this function.

 lib/efi_loader/efi_memory.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index 81dc5fc..a47adef 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -168,6 +168,16 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
 			free(map);
 		} else {
 			map->desc.physical_start = carve_end;
+			if (carve_end == map_end)
+				map->desc.virtual_start =
+					map_desc->virtual_start +
+					(map_desc->num_pages << EFI_PAGE_SHIFT);
+			else
+				map->desc.virtual_start =
+					carve_desc->virtual_start +
+					(carve_desc->num_pages <<
+					 EFI_PAGE_SHIFT);
+
 			map->desc.num_pages = (map_end - carve_end)
 					      >> EFI_PAGE_SHIFT;
 		}
@@ -186,6 +196,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
 	newmap = calloc(1, sizeof(*newmap));
 	newmap->desc = map->desc;
 	newmap->desc.physical_start = carve_start;
+	if (carve_start == map_start)
+		newmap->desc.virtual_start = map_desc->virtual_start;
+	else
+		newmap->desc.virtual_start = carve_desc->virtual_start;
+
 	newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT;
 	/* Insert before current entry (descending address order) */
 	list_add_tail(&newmap->link, &map->link);
-- 
2.7.4

[U-Boot] [PATCH 3/3] efi: update virtual address in efi_mem_carve_out

[Heinrich Schuchardt]

On 4/10/19 11:02 AM, Patrick Delaunay wrote:
> Handle virtual address in efi_mem_carve_out function
> when a new region is created to avoid issue in EFI memory map.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Issue detected during test of test of UEFI on ev1/dk2 boards.
>
> For debug purpose, I add the dump of the map in efi_mem_sort:
>
> static void efi_mem_sort(void)
> {
> .....
> 	int i = 0;
> 	printf("-- %s -------------------\n", __func__ );
> 	list_for_each(lhandle, &efi_mem) {
> 		struct efi_mem_list *lmem;
> 		struct efi_mem_desc *cur;
>
> 		lmem = list_entry(lhandle, struct efi_mem_list, link);
> 		cur = &lmem->desc;
>
> 		printf("%02d: va %08x pa %08x size %08x (%04x pages) => va end %08x attribute = %llx type=%d\n",
> 			i++,
> 			(u32)cur->virtual_start,
> 			(u32)cur->physical_start,
> 			(u32)(cur->num_pages << EFI_PAGE_SHIFT),
> 			(u32)cur->num_pages,
> 			(u32) (cur->virtual_start + (cur->num_pages << EFI_PAGE_SHIFT)),
> 			cur->attribute,
> 			cur->type);
> 	}
> }
>
> I get the memory layout (for EV1):
>
> 00: va fcf87000 pa fff92000 size 0006e000 (006e pages) => va end fcff5000 attribute = 8 type=2
> 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5
> 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2
> 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6
> 04: va fcf46000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf4f000 attribute = 8 type=0
> 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6
> 06: va fcf46000 pa fcf77000 size 00005000 (0005 pages) => va end fcf4b000 attribute = 8 type=0
> 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7
>
> But for some region virtual address (va) != physical address (pa)
>
> After check of the code, the virtual address is not correctly managed
> when new region is created.
>
> I don't sure that could be a issue, but I solve the issue
> with the proposed patch:
>
> 00: va fff92000 pa fff92000 size 0006e000 (006e pages) => va end 00000000 attribute = 8 type=2
> 01: va fff91000 pa fff91000 size 00001000 (0001 pages) => va end fff92000 attribute = 8000000000000008 type=5
> 02: va fcf87000 pa fcf87000 size 0300a000 (300a pages) => va end fff91000 attribute = 8 type=2
> 03: va fcf86000 pa fcf86000 size 00001000 (0001 pages) => va end fcf87000 attribute = 8000000000000008 type=6
> 04: va fcf7d000 pa fcf7d000 size 00009000 (0009 pages) => va end fcf86000 attribute = 8 type=0
> 05: va fcf7c000 pa fcf7c000 size 00001000 (0001 pages) => va end fcf7d000 attribute = 8000000000000008 type=6
> 06: va fcf77000 pa fcf77000 size 00005000 (0005 pages) => va end fcf7c000 attribute = 8 type=0
> 07: va c0000000 pa c0000000 size 3cf77000 (3cf77 pages) => va end fcf77000 attribute = 8 type=7
>
> The virtual address are now correct.
>
> PS: I don't found more elegant code to update the virtual address
>      but I avoid to force virtual_address = physical address
>      in this function.
>
>   lib/efi_loader/efi_memory.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 81dc5fc..a47adef 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -168,6 +168,16 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
>   			free(map);
>   		} else {
>   			map->desc.physical_start = carve_end;

Thanks for reporting the issue. I think you correctly identified the two
places where the update of the virtual address is missing.

The efi_mem_carve_out() function is called efi_add_memory_map() which is
only called at boottime. By definition the physical and virtual
addresses must be the same at boottime. So a single line

map->desc.virtual_start = carve_end;

should be enough here.

> +			if (carve_end == map_end)
> +				map->desc.virtual_start =
> +					map_desc->virtual_start +
> +					(map_desc->num_pages << EFI_PAGE_SHIFT);
> +			else
> +				map->desc.virtual_start =
> +					carve_desc->virtual_start +
> +					(carve_desc->num_pages <<
> +					 EFI_PAGE_SHIFT);
> +
>   			map->desc.num_pages = (map_end - carve_end)
>   					      >> EFI_PAGE_SHIFT;
>   		}
> @@ -186,6 +196,11 @@ static s64 efi_mem_carve_out(struct efi_mem_list *map,
>   	newmap = calloc(1, sizeof(*newmap));
>   	newmap->desc = map->desc;
>   	newmap->desc.physical_start = carve_start;

newmap->desc.virtual_start = carve_start;

should be ok here.

Best regards

Heinrich

> +	if (carve_start == map_start)
> +		newmap->desc.virtual_start = map_desc->virtual_start;
> +	else
> +		newmap->desc.virtual_start = carve_desc->virtual_start;
> +
>   	newmap->desc.num_pages = (map_end - carve_start) >> EFI_PAGE_SHIFT;
>   	/* Insert before current entry (descending address order) */
>   	list_add_tail(&newmap->link, &map->link);
>

[U-Boot] [PATCH 1/3] efi: fix issue for 32bit targets with ram_top at 4G

[Patrick Wildt]

On Wed, Apr 10, 2019 at 11:02:57AM +0200, Patrick Delaunay wrote:
> Avoid ram_end = 0 on 32bit targets with ram_end at 4G.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> example of issue in stm32mp1 board ev1:
>   ram_start = c0000000
>   size = 40000000
>   ram_end = 100000000
>   ram_end &= ~EFI_PAGE_MASK => result is 0
> 
>  lib/efi_loader/efi_memory.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 55622d2..81dc5fc 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -574,6 +574,10 @@ __weak void efi_add_known_memory(void)
>  
>  		/* Remove partial pages */
>  		ram_end &= ~EFI_PAGE_MASK;
> +		/* Fix for 32bit targets with ram_top at 4G */
> +		if (!ram_end)
> +			ram_end = 0x100000000ULL;
> +
>  		ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
>  
>  		if (ram_end <= ram_start) {
> -- 
> 2.7.4
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Hi,

I had the exact same issue yesterday and posted a diff which I think
is a better approach:

https://patchwork.ozlabs.org/patch/1082739/

Best regards,
another Patrick

[U-Boot] [PATCH 1/3] efi: fix issue for 32bit targets with ram_top at 4G

[Patrick DELAUNAY]

Hi Patrick,

> From: Patrick Wildt <patrick@blueri.se>
> 
> On Wed, Apr 10, 2019 at 11:02:57AM +0200, Patrick Delaunay wrote:
> > Avoid ram_end = 0 on 32bit targets with ram_end at 4G.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> > example of issue in stm32mp1 board ev1:
> >   ram_start = c0000000
> >   size = 40000000
> >   ram_end = 100000000
> >   ram_end &= ~EFI_PAGE_MASK => result is 0
> >
> >  lib/efi_loader/efi_memory.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 55622d2..81dc5fc 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -574,6 +574,10 @@ __weak void efi_add_known_memory(void)
> >
> >  		/* Remove partial pages */
> >  		ram_end &= ~EFI_PAGE_MASK;
> > +		/* Fix for 32bit targets with ram_top at 4G */
> > +		if (!ram_end)
> > +			ram_end = 0x100000000ULL;
> > +
> >  		ram_start = (ram_start + EFI_PAGE_MASK) &
> ~EFI_PAGE_MASK;
> >
> >  		if (ram_end <= ram_start) {
> > --
> > 2.7.4
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> 
> Hi,
> 
> I had the exact same issue yesterday and posted a diff which I think is a better
> approach:

I tested your patch and yes that also correct my issue.

And I am agree that it is a better approach: So I will hack your patch and I abandon this patch (1/3) of this serie.

Regards

Patrick
 
> https://patchwork.ozlabs.org/patch/1082739/
> 
> Best regards,
> another Patrick