[U-Boot] [PATCH v2] efi_loader: Reserve unaccessible memory

[Heinrich Schuchardt]

On 11/30/18 2:11 AM, Alexander Graf wrote:
> On some systems, not all RAM may be usable within U-Boot. Maybe the
> memory maps are incomplete, maybe it's used as workaround for broken
> DMA. But whatever the reason may be, a platform can say that it does
> not wish to have its RAM accessed above a certain address by defining
> board_get_usable_ram_top().
> 
> In the efi_loader world, we ignored that hint, mostly because very few
> boards actually have real restrictions around this.
> 
> So let's honor the board's wish to not access high addresses during
> boot time. The best way to do so is by indicating the respective pages
> as "allocated by firmware". That way, Operating Systems will still
> use the pages after boot, but before boot no allocation will use them.
> 
> Reported-by: Baruch Siach <baruch at tkos.co.il>
> Signed-off-by: Alexander Graf <agraf at suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - Reserve banks that start above ram_top
>   - Improve inline comments
>   - Fix 32bit target with ram_top = 0 (>=4G)
> ---
>  include/common.h            | 11 +++++++++++
>  lib/efi_loader/efi_memory.c | 32 +++++++++++++++++++++++++++++---
>  2 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/include/common.h b/include/common.h
> index 3f69943887..8f295c2f30 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -106,6 +106,17 @@ int mdm_init(void);
>  void board_show_dram(phys_size_t size);
>  
>  /**
> + * Get the uppermost pointer that is valid to access
> + *
> + * Some systems may not map all of their address space. This function allows
> + * boards to indicate what their highest support pointer value is for DRAM
> + * access.
> + *
> + * @param total_size	Size of U-Boot (unused?)
> + */
> +ulong board_get_usable_ram_top(ulong total_size);
> +
> +/**
>   * arch_fixup_fdt() - Write arch-specific information to fdt
>   *
>   * Defined in arch/$(ARCH)/lib/bootm-fdt.c
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index f225a9028c..3392af59a1 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -551,8 +551,13 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size,
>  
>  __weak void efi_add_known_memory(void)
>  {
> +	u64 ram_top = board_get_usable_ram_top(0) & ~EFI_PAGE_MASK;
>  	int i;
>  
> +	/* Fix for 32bit targets with ram_top at 4G */
> +	if (!ram_top)
> +		ram_top = U32_MAX;

Why don't you set ram_top to (1 << 32)? ram_top is u64 anyway. We do not
want something ending with 0xfff to be passed to efi_add_memory_map.

Are we sure there will never be 64bit machines with a memory bank ending
at 1 << 64? What would be the value of ram_end and ram_top in such a case?

Regards

Heinrich

> +
>  	/* Add RAM */
>  	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>  		u64 ram_end, ram_start, pages;
> @@ -564,11 +569,32 @@ __weak void efi_add_known_memory(void)
>  		ram_end &= ~EFI_PAGE_MASK;
>  		ram_start = (ram_start + EFI_PAGE_MASK) & ~EFI_PAGE_MASK;
>  
> -		if (ram_end > ram_start) {
> -			pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT;
> +		if (ram_end <= ram_start) {
> +			/* Invalid mapping, keep going. */
> +			continue;
> +		}
> +
> +		pages = (ram_end - ram_start) >> EFI_PAGE_SHIFT;
>  
> +		efi_add_memory_map(ram_start, pages,
> +				   EFI_CONVENTIONAL_MEMORY, false);
> +
> +		/*
> +		 * Boards may indicate to the U-Boot memory core that they
> +		 * can not support memory above ram_top. Let's honor this
> +		 * in the efi_loader subsystem too by declaring any memory
> +		 * above ram_top as "already occupied by firmware".
> +		 */
> +		if (ram_top < ram_start) {
> +			/* ram_top is before this region, reserve all */
>  			efi_add_memory_map(ram_start, pages,
> -					   EFI_CONVENTIONAL_MEMORY, false);
> +					   EFI_BOOT_SERVICES_DATA, true);
> +		} else if ((ram_top >= ram_start) && (ram_top < ram_end)) {
> +			/* ram_top is inside this region, reserve parts */
> +			pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT;
> +
> +			efi_add_memory_map(ram_top, pages,
> +					   EFI_BOOT_SERVICES_DATA, true);
>  		}
>  	}
>  }
>