diff mbox series

efi_loader: Reserve unaccessible memory

Message ID 20181129234751.64224-1-agraf@suse.de
State Superseded
Headers show
Series efi_loader: Reserve unaccessible memory | expand

Commit Message

Alexander Graf Nov. 29, 2018, 11:47 p.m. UTC
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@tkos.co.il>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/common.h            | 11 +++++++++++
 lib/efi_loader/efi_memory.c |  9 +++++++++
 2 files changed, 20 insertions(+)

Comments

Heinrich Schuchardt Nov. 30, 2018, 12:05 a.m. UTC | #1
On 11/30/18 12:47 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@tkos.co.il>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  include/common.h            | 11 +++++++++++
>  lib/efi_loader/efi_memory.c |  9 +++++++++
>  2 files changed, 20 insertions(+)
> 
> 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..2f13bf8a75 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -551,6 +551,7 @@ 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;
>  
>  	/* Add RAM */
> @@ -570,6 +571,14 @@ __weak void efi_add_known_memory(void)
>  			efi_add_memory_map(ram_start, pages,
>  					   EFI_CONVENTIONAL_MEMORY, false);
>  		}
> +
> +		/* Reserve memory above ram_top, as that may not be mapped */
> +		if ((ram_top >= ram_start) && (ram_top < ram_end)) {
> +			pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT;

Please, do not assume that ram_end is page aligned. You could use the
macro efi_size_in_pages() here. See
https://patchwork.ozlabs.org/patch/999522/

Best regards

Heinrich

> +
> +			efi_add_memory_map(ram_top, pages,
> +					   EFI_BOOT_SERVICES_DATA, true);
> +		}
>  	}
>  }
>  
>
Alexander Graf Nov. 30, 2018, 12:19 a.m. UTC | #2
On 30.11.18 01:05, Heinrich Schuchardt wrote:
> On 11/30/18 12:47 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@tkos.co.il>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  include/common.h            | 11 +++++++++++
>>  lib/efi_loader/efi_memory.c |  9 +++++++++
>>  2 files changed, 20 insertions(+)
>>
>> 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..2f13bf8a75 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -551,6 +551,7 @@ 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;
>>  
>>  	/* Add RAM */
>> @@ -570,6 +571,14 @@ __weak void efi_add_known_memory(void)
>>  			efi_add_memory_map(ram_start, pages,
>>  					   EFI_CONVENTIONAL_MEMORY, false);
>>  		}
>> +
>> +		/* Reserve memory above ram_top, as that may not be mapped */
>> +		if ((ram_top >= ram_start) && (ram_top < ram_end)) {
>> +			pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT;
> 
> Please, do not assume that ram_end is page aligned.

See above. I align it manually, no?

Alex

> You could use the
> macro efi_size_in_pages() here. See
> https://patchwork.ozlabs.org/patch/999522/
> 
> Best regards
> 
> Heinrich
> 
>> +
>> +			efi_add_memory_map(ram_top, pages,
>> +					   EFI_BOOT_SERVICES_DATA, true);
>> +		}
>>  	}
>>  }
>>  
>>
>
diff mbox series

Patch

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..2f13bf8a75 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -551,6 +551,7 @@  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;
 
 	/* Add RAM */
@@ -570,6 +571,14 @@  __weak void efi_add_known_memory(void)
 			efi_add_memory_map(ram_start, pages,
 					   EFI_CONVENTIONAL_MEMORY, false);
 		}
+
+		/* Reserve memory above ram_top, as that may not be mapped */
+		if ((ram_top >= ram_start) && (ram_top < ram_end)) {
+			pages = (ram_end - ram_top) >> EFI_PAGE_SHIFT;
+
+			efi_add_memory_map(ram_top, pages,
+					   EFI_BOOT_SERVICES_DATA, true);
+		}
 	}
 }