[U-Boot] [PATCH v2 8/9] efi_loader: do not use magic address for fdt

[Heinrich Schuchardt]

On 11/13/18 10:21 PM, Alexander Graf wrote:
> 
> 
> On 12.11.18 18:55, Heinrich Schuchardt wrote:
>> We currently place the flattened device tree 127 MiB from the start of the
>> first memory bank. This may be in a reserved memory area.
>>
>> So let's change the sequence: first create memory reservations and then
>> copy the device tree.
>>
>> Do not use any magic address.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
> This would be a regression over commit
> ad0c1a3d2cea03011091b07e9e066bf261d1556e no?

>From your commit in ad0c1a3d2cea03011091b07e9e066bf261d1556e message I
cannot infer which address would pose a problem and why.

You only mentioned it should be a place where the fdt can stay. Isn't
this any address in a reserved memory area?

You did not provide any reference to a document recommending that an
address around 128 MB should be favorable. Was the memory reservation
missing at that time?

Best regards

Heinrich

> 
> 
> Alex
> 
>> ---
>> v2:
>> 	Remove code handling memory reservation for the fdt.
>> ---
>>  cmd/bootefi.c | 32 +++++++-------------------------
>>  1 file changed, 7 insertions(+), 25 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 50253ea1859..251d8403bcc 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -189,32 +189,20 @@ static efi_status_t copy_fdt(ulong *fdt_addrp)
>>  	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
>>  	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
>>  
>> -	/*
>> -	 * Safe fdt location is at 127 MiB. On the sandbox convert from the
>> -	 * virtual address space.
>> -	 */
>> -	new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
>> -					     fdt_size, 0);
>> +	new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
>>  	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>>  				 EFI_RUNTIME_SERVICES_DATA, fdt_pages,
>>  				 &new_fdt_addr);
>>  	if (ret != EFI_SUCCESS) {
>> -		/* If we can't put it there, put it somewhere */
>> -		new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
>> -		ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
>> -					 EFI_RUNTIME_SERVICES_DATA, fdt_pages,
>> -					 &new_fdt_addr);
>> -		if (ret != EFI_SUCCESS) {
>> -			printf("ERROR: Failed to reserve space for FDT\n");
>> -			goto done;
>> -		}
>> +		printf("ERROR: Failed to allocate memory for FDT\n");
>> +		return ret;
>>  	}
>>  	new_fdt = (void *)(uintptr_t)new_fdt_addr;
>>  	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
>>  	fdt_set_totalsize(new_fdt, fdt_size);
>>  
>>  	*fdt_addrp = new_fdt_addr;
>> -done:
>> +
>>  	return ret;
>>  }
>>  
>> @@ -284,13 +272,6 @@ static void efi_carve_out_dt_rsv(void *fdt)
>>  		/* Convert from sandbox address space. */
>>  		addr = (uintptr_t)map_sysmem(addr, 0);
>>  
>> -		/*
>> -		 * Do not carve out the device tree. It is already marked as
>> -		 * EFI_RUNTIME_SERVICES_DATA
>> -		 */
>> -		if (addr == (uintptr_t)fdt)
>> -			continue;
>> -
>>  		pages = efi_size_in_pages(size + (addr & EFI_PAGE_MASK));
>>  		addr &= ~EFI_PAGE_MASK;
>>  		if (!efi_add_memory_map(addr, pages, EFI_RESERVED_MEMORY_TYPE,
>> @@ -310,6 +291,9 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
>>  		return EFI_INVALID_PARAMETER;
>>  	}
>>  
>> +	/* Create memory reservation as indicated by the device tree */
>> +	efi_carve_out_dt_rsv(fdt);
>> +
>>  	/* Prepare fdt for payload */
>>  	ret = copy_fdt(&fdt_addr);
>>  	if (ret)
>> @@ -321,8 +305,6 @@ static efi_status_t efi_install_fdt(ulong fdt_addr)
>>  		return EFI_LOAD_ERROR;
>>  	}
>>  
>> -	efi_carve_out_dt_rsv(fdt);
>> -
>>  	/* Link to it in the efi tables */
>>  	ret = efi_install_configuration_table(&efi_guid_fdt, fdt);
>>  	if (ret != EFI_SUCCESS)
>>
>