public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC 1/1] efi_loader: in situ relocation
Date: Mon, 18 Feb 2019 09:52:01 +0900	[thread overview]
Message-ID: <20190218005200.GW20286@linaro.org> (raw)
In-Reply-To: <20190216195044.8639-1-xypron.glpk@gmx.de>

Heinrich,

On Sat, Feb 16, 2019 at 08:50:43PM +0100, Heinrich Schuchardt wrote:
> All code and data sections of PE images are already in the correct relative
> location when loaded into memory. There is not need to copy them once
> again.

While I'm not very familiar with how PE image is created (in EDK2),
what I understand in Alex's code is
* All the code and data are located starting 0x0 (in virtual space)
* Sections in PE image may not be sorted in ascending order
* There may be some gaps (more than one page) between sections,
  probably, due to alignment requirements or BSS

Do you say that those assumptions are no longer correct?

Thanks,
-Takahiro Akashi

> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_image_loader.c | 64 ++++++-------------------------
>  1 file changed, 11 insertions(+), 53 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index fe66e7b9ffe..6146acc6b6f 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -2,7 +2,7 @@
>  /*
>   *  EFI image loader
>   *
> - *  based partly on wine code
> + *  based partly on Wine code
>   *
>   *  Copyright (c) 2016 Alexander Graf
>   */
> @@ -219,16 +219,12 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>  {
>  	IMAGE_NT_HEADERS32 *nt;
>  	IMAGE_DOS_HEADER *dos;
> -	IMAGE_SECTION_HEADER *sections;
> -	int num_sections;
> -	void *efi_reloc;
>  	int i;
>  	const IMAGE_BASE_RELOCATION *rel;
>  	unsigned long rel_size;
>  	int rel_idx = IMAGE_DIRECTORY_ENTRY_BASERELOC;
>  	uint64_t image_base;
>  	uint64_t image_size;
> -	unsigned long virt_size = 0;
>  	int supported = 0;
>  
>  	dos = efi;
> @@ -255,85 +251,47 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, void *efi,
>  		return EFI_LOAD_ERROR;
>  	}
>  
> -	/* Calculate upper virtual address boundary */
> -	num_sections = nt->FileHeader.NumberOfSections;
> -	sections = (void *)&nt->OptionalHeader +
> -			    nt->FileHeader.SizeOfOptionalHeader;
> -
> -	for (i = num_sections - 1; i >= 0; i--) {
> -		IMAGE_SECTION_HEADER *sec = &sections[i];
> -		virt_size = max_t(unsigned long, virt_size,
> -				  sec->VirtualAddress + sec->Misc.VirtualSize);
> -	}
> -
>  	/* Read 32/64bit specific header bits */
>  	if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>  		IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
>  		IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
> +
>  		image_base = opt->ImageBase;
>  		image_size = opt->SizeOfImage;
>  		efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
> -		efi_reloc = efi_alloc(virt_size,
> -				      loaded_image_info->image_code_type);
> -		if (!efi_reloc) {
> -			printf("%s: Could not allocate %lu bytes\n",
> -			       __func__, virt_size);
> -			return EFI_OUT_OF_RESOURCES;
> -		}
> -		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
> +		handle->entry = efi + opt->AddressOfEntryPoint;
>  		rel_size = opt->DataDirectory[rel_idx].Size;
> -		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
> -		virt_size = ALIGN(virt_size, opt->SectionAlignment);
> +		rel = efi + opt->DataDirectory[rel_idx].VirtualAddress;
>  	} else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>  		IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
> +
>  		image_base = opt->ImageBase;
>  		image_size = opt->SizeOfImage;
>  		efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
> -		efi_reloc = efi_alloc(virt_size,
> -				      loaded_image_info->image_code_type);
> -		if (!efi_reloc) {
> -			printf("%s: Could not allocate %lu bytes\n",
> -			       __func__, virt_size);
> -			return EFI_OUT_OF_RESOURCES;
> -		}
> -		handle->entry = efi_reloc + opt->AddressOfEntryPoint;
> +		handle->entry = efi + opt->AddressOfEntryPoint;
>  		rel_size = opt->DataDirectory[rel_idx].Size;
> -		rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress;
> -		virt_size = ALIGN(virt_size, opt->SectionAlignment);
> +		rel = efi + opt->DataDirectory[rel_idx].VirtualAddress;
>  	} else {
>  		printf("%s: Invalid optional header magic %x\n", __func__,
>  		       nt->OptionalHeader.Magic);
>  		return EFI_LOAD_ERROR;
>  	}
>  
> -	/* Load sections into RAM */
> -	for (i = num_sections - 1; i >= 0; i--) {
> -		IMAGE_SECTION_HEADER *sec = &sections[i];
> -		memset(efi_reloc + sec->VirtualAddress, 0,
> -		       sec->Misc.VirtualSize);
> -		memcpy(efi_reloc + sec->VirtualAddress,
> -		       efi + sec->PointerToRawData,
> -		       sec->SizeOfRawData);
> -	}
> -
>  	/* Run through relocations */
> -	if (efi_loader_relocate(rel, rel_size, efi_reloc,
> +	if (efi_loader_relocate(rel, rel_size, efi,
>  				(unsigned long)image_base) != EFI_SUCCESS) {
> -		efi_free_pages((uintptr_t) efi_reloc,
> -			       (virt_size + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT);
>  		return EFI_LOAD_ERROR;
>  	}
>  
>  	/* Flush cache */
> -	flush_cache((ulong)efi_reloc,
> -		    ALIGN(virt_size, EFI_CACHELINE_SIZE));
> +	flush_cache((ulong)efi, ALIGN(image_size, EFI_CACHELINE_SIZE));
>  	invalidate_icache_all();
>  
>  	/* Populate the loaded image interface bits */
>  	loaded_image_info->image_base = efi;
>  	loaded_image_info->image_size = image_size;
> -	handle->reloc_base = efi_reloc;
> -	handle->reloc_size = virt_size;
> +	handle->reloc_base = efi;
> +	handle->reloc_size = image_size;
>  
>  	return EFI_SUCCESS;
>  }
> -- 
> 2.20.1
> 

  parent reply	other threads:[~2019-02-18  0:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-16 19:50 [U-Boot] [RFC 1/1] efi_loader: in situ relocation Heinrich Schuchardt
2019-02-16 19:50 ` Heinrich Schuchardt
2019-02-18  0:52 ` AKASHI Takahiro [this message]
2019-02-20 18:12   ` Heinrich Schuchardt
2019-02-21 10:21     ` Alexander Graf
2019-02-21 18:55       ` Heinrich Schuchardt
2019-02-22  1:17         ` AKASHI Takahiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190218005200.GW20286@linaro.org \
    --to=takahiro.akashi@linaro.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox