diff mbox

[v2,4/4] arm64: Add support for binary image files

Message ID f352dcfc827a4cbb3878dd1043877a4154b1bd08.1469559530.git.geoff@infradead.org (mailing list archive)
State New, archived
Headers show

Commit Message

Geoff Levand July 26, 2016, 8:18 p.m. UTC 
From: Pratyush Anand <panand@redhat.com>

Signed-off-by: Pratyush Anand <panand@redhat.com>
[Reworked and cleaned up]
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
 kexec/arch/arm64/kexec-image-arm64.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

Comments

Pratyush Anand July 28, 2016, 4:01 a.m. UTC | #1 
Hi Geoff,

On 26/07/2016:08:18:57 PM, Geoff Levand wrote:
> From: Pratyush Anand <panand@redhat.com>
> 
> Signed-off-by: Pratyush Anand <panand@redhat.com>
> [Reworked and cleaned up]

You removed page_offset calculation in rework.
I am unable to understand that how arm64_mem.page_offset will be filled up now
for binary image case.

I think, we still need to keep page_offset user input. You may take following
two patches which does this, or you may leave binary image support patch which I
will send after your patches are merged.

https://github.com/pratyushanand/kexec-tools/commit/5b7e49a75d1d6cd4ac846f50ff10275fd54cb545
https://github.com/pratyushanand/kexec-tools/commit/a0ce0ce673755c4061c1f081170a3a75dfa1d1fb

~Pratyush

> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
>  kexec/arch/arm64/kexec-image-arm64.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/kexec/arch/arm64/kexec-image-arm64.c b/kexec/arch/arm64/kexec-image-arm64.c
> index 84386f7..cad7c73 100644
> --- a/kexec/arch/arm64/kexec-image-arm64.c
> +++ b/kexec/arch/arm64/kexec-image-arm64.c
> @@ -24,21 +24,40 @@ int image_arm64_probe(const char *kernel_buf, off_t kernel_size)
>  		return -1;
>  	}
>  
> -	fprintf(stderr, "kexec: ARM64 binary image files are currently NOT SUPPORTED.\n");
> -
> -	return -1;
> +	return 0;
>  }
>  
>  int image_arm64_load(int argc, char **argv, const char *kernel_buf,
>  	off_t kernel_size, struct kexec_info *info)
>  {
> -	return -EFAILED;
> +	const struct arm64_image_header *h;
> +	unsigned long image_base;
> +
> +	h = (const struct arm64_image_header *)(kernel_buf);
> +
> +	if (arm64_process_image_header(h))
> +		return -EINVAL;
> +
> +	dbgprintf("%s: text_offset:   %016lx\n", __func__,
> +		arm64_mem.text_offset);
> +	dbgprintf("%s: image_size:    %016lx\n", __func__,
> +		arm64_mem.image_size);
> +	dbgprintf("%s: phys_offset:   %016lx\n", __func__,
> +		arm64_mem.phys_offset);
> +	dbgprintf("%s: PE format:     %s\n", __func__,
> +		(arm64_header_check_pe_sig(h) ? "yes" : "no"));
> +
> +	image_base = get_phys_offset() + arm64_mem.text_offset;
> +	
> +	add_segment_phys_virt(info, kernel_buf, kernel_size, image_base,
> +		arm64_mem.image_size, 0);
> +
> +	return arm64_load_other_segments(info, image_base);
>  }
>  
>  void image_arm64_usage(void)
>  {
>  	printf(
>  "     An ARM64 binary image, compressed or not, big or little endian.\n"
> -"     Typically an Image, Image.gz or Image.lzma file.\n"
> -"     This file type is currently NOT SUPPORTED.\n\n");
> +"     Typically an Image, Image.gz or Image.lzma file.\n\n");
>  }
> -- 
> 2.5.0
Geoff Levand July 28, 2016, 6:04 p.m. UTC | #2 
Hi Pratyush,

On Thu, 2016-07-28 at 09:31 +0530, Pratyush Anand wrote:
> On 26/07/2016:08:18:57 PM, Geoff Levand wrote:

> You removed page_offset calculation in rework.
> I am unable to understand that how arm64_mem.page_offset will be filled up now
> for binary image case.
> 
> I think, we still need to keep page_offset user input. 

The current kernel linker scripts are setup to output virtual
address values to the vmlinux elf file.  

To load the elf sections of vmlinux using the provided elf
file loader, elf_exec_load(), we need to convert those elf
header values to physical addresses since we work with
physical addresses in the kernel's kimage structure.

That conversion is 

  p = v - page_offset + phys_offset;

We get phys_offset from the memory ranges, and we calculate
page_offset from the elf header info.

As for the binary image file, we load that with
add_segment_phys_virt(), which can put the binary data of the
Image file into memory at a physical address we specify:

  image_base = phys_offset + text_offset.

phys_offset is again from the memory ranges, and text_offset
from the arm64 image header.


> You may take following
> two patches which does this, or you may leave binary image support patch which I
> will send after your patches are merged.
> 
> https://github.com/pratyushanand/kexec-tools/commit/5b7e49a75d1d6cd4ac846f50ff10275fd54cb545

This one may be useful, for debugging if anything.

> https://github.com/pratyushanand/kexec-tools/commit/a0ce0ce673755c4061c1f081170a3a75dfa1d1fb

We do not need, nor want, a PAGE_OFFSET option.

-Geoff
Pratyush Anand July 29, 2016, 3:35 a.m. UTC | #3 
Hi Geoff,

On 28/07/2016:11:04:43 AM, Geoff Levand wrote:
> Hi Pratyush,
> 
> On Thu, 2016-07-28 at 09:31 +0530, Pratyush Anand wrote:
> > On 26/07/2016:08:18:57 PM, Geoff Levand wrote:
> 
> > You removed page_offset calculation in rework.
> > I am unable to understand that how arm64_mem.page_offset will be filled up now
> > for binary image case.
> > 
> > I think, we still need to keep page_offset user input. 
> 
> The current kernel linker scripts are setup to output virtual
> address values to the vmlinux elf file.  
> 
> To load the elf sections of vmlinux using the provided elf
> file loader, elf_exec_load(), we need to convert those elf
> header values to physical addresses since we work with
> physical addresses in the kernel's kimage structure.
> 
> That conversion is 
> 
>   p = v - page_offset + phys_offset;
> 
> We get phys_offset from the memory ranges, and we calculate
> page_offset from the elf header info.
> 
> As for the binary image file, we load that with
> add_segment_phys_virt(), which can put the binary data of the
> Image file into memory at a physical address we specify:
> 
>   image_base = phys_offset + text_offset.
> 
> phys_offset is again from the memory ranges, and text_offset
> from the arm64 image header.
> 
> 
> > You may take following
> > two patches which does this, or you may leave binary image support patch which I
> > will send after your patches are merged.
> > 
> > https://github.com/pratyushanand/kexec-tools/commit/5b7e49a75d1d6cd4ac846f50ff10275fd54cb545
> 
> This one may be useful, for debugging if anything.
> 
> > https://github.com/pratyushanand/kexec-tools/commit/a0ce0ce673755c4061c1f081170a3a75dfa1d1fb
> 
> We do not need, nor want, a PAGE_OFFSET option.

See kexec/crashdump-elf.c:FUNC()
We have:
223                 phdr->p_vaddr = phys_to_virt(elf_info, mstart);

Now, if we do not have page_offset then we will not have correct p_vaddr, and
then vmcore-dmesg/vmcore-dmesg.c:vaddr_to_offset() fails with

No program header covering vaddr 0xfffffc0008c312f0found kexec bug?

~Pratyush
Geoff Levand July 29, 2016, 5:02 p.m. UTC | #4 
On Fri, 2016-07-29 at 09:05 +0530, Pratyush Anand wrote:

> See kexec/crashdump-elf.c:FUNC()
> We have:
> 223                 phdr->p_vaddr = phys_to_virt(elf_info, mstart);
> 

These patches don't support kdump.

-Geoff
Pratyush Anand July 29, 2016, 5:15 p.m. UTC | #5 
Hi Geoff,

On 29/07/2016:10:02:40 AM, Geoff Levand wrote:
> On Fri, 2016-07-29 at 09:05 +0530, Pratyush Anand wrote:
> 
> > See kexec/crashdump-elf.c:FUNC()
> > We have:
> > 223                 phdr->p_vaddr = phys_to_virt(elf_info, mstart);
> > 
> 
> These patches don't support kdump.

OK, so you suggest that I should keep this patch as it is and add --page-offset
in corresponding kdump patch. I can do that, no issue.

~Pratyush
Geoff Levand July 29, 2016, 5:19 p.m. UTC | #6 
On Fri, 2016-07-29 at 22:45 +0530, Pratyush Anand wrote:
> Hi Geoff,
> 
> On 29/07/2016:10:02:40 AM, Geoff Levand wrote:
> > On Fri, 2016-07-29 at 09:05 +0530, Pratyush Anand wrote:
> > 
> > > See kexec/crashdump-elf.c:FUNC()
> > > We have:
> > > 223                 phdr->p_vaddr = phys_to_virt(elf_info,
> > > mstart);
> > > 
> > 
> > These patches don't support kdump.
> 
> OK, so you suggest that I should keep this patch as it is and add -
> -page-offset
> in corresponding kdump patch. I can do that, no issue.

Yes, this is an issue related to kdump, so let's discuss it in that
context.

-Geoff
AKASHI Takahiro Aug. 4, 2016, 4:51 a.m. UTC | #7 
Pratyush,

On Fri, Jul 29, 2016 at 10:45:52PM +0530, Pratyush Anand wrote:
> Hi Geoff,
> 
> On 29/07/2016:10:02:40 AM, Geoff Levand wrote:
> > On Fri, 2016-07-29 at 09:05 +0530, Pratyush Anand wrote:
> > 
> > > See kexec/crashdump-elf.c:FUNC()
> > > We have:
> > > 223                 phdr->p_vaddr = phys_to_virt(elf_info, mstart);
> > > 
> > 
> > These patches don't support kdump.
> 
> OK, so you suggest that I should keep this patch as it is and add --page-offset
> in corresponding kdump patch. I can do that, no issue.

I will add necessary hunks from your old patch when I submit
a next version of patches for arm64, probably next Monday.

I hope that you don't mind.

Thanks,
-Takahiro AKASHI

> ~Pratyush
Pratyush Anand Aug. 4, 2016, 5:11 a.m. UTC | #8 
Hi Takahiro,

On 04/08/2016:01:51:05 PM, AKASHI Takahiro wrote:
> Pratyush,
> 
> On Fri, Jul 29, 2016 at 10:45:52PM +0530, Pratyush Anand wrote:
> > Hi Geoff,
> > 
> > On 29/07/2016:10:02:40 AM, Geoff Levand wrote:
> > > On Fri, 2016-07-29 at 09:05 +0530, Pratyush Anand wrote:
> > > 
> > > > See kexec/crashdump-elf.c:FUNC()
> > > > We have:
> > > > 223                 phdr->p_vaddr = phys_to_virt(elf_info, mstart);
> > > > 
> > > 
> > > These patches don't support kdump.
> > 
> > OK, so you suggest that I should keep this patch as it is and add --page-offset
> > in corresponding kdump patch. I can do that, no issue.
> 
> I will add necessary hunks from your old patch when I submit
> a next version of patches for arm64, probably next Monday.
> 
> I hope that you don't mind.

Thanks, no issue.

May be you can just cherry-pick following two commits.

https://github.com/pratyushanand/kexec-tools/commit/1e4eea5798de312c7ac90043361a889516da0aaf
https://github.com/pratyushanand/kexec-tools/commit/fd19e4b9c1423d24fcd4355277835f0bf01c33e8

They are in upstream_arm64_devel branch of https://github.com/pratyushanand/kexec-tools.git

~Pratyush
diff mbox

Patch

diff --git a/kexec/arch/arm64/kexec-image-arm64.c b/kexec/arch/arm64/kexec-image-arm64.c
index 84386f7..cad7c73 100644
--- a/kexec/arch/arm64/kexec-image-arm64.c
+++ b/kexec/arch/arm64/kexec-image-arm64.c
@@ -24,21 +24,40 @@  int image_arm64_probe(const char *kernel_buf, off_t kernel_size)
 		return -1;
 	}
 
-	fprintf(stderr, "kexec: ARM64 binary image files are currently NOT SUPPORTED.\n");
-
-	return -1;
+	return 0;
 }
 
 int image_arm64_load(int argc, char **argv, const char *kernel_buf,
 	off_t kernel_size, struct kexec_info *info)
 {
-	return -EFAILED;
+	const struct arm64_image_header *h;
+	unsigned long image_base;
+
+	h = (const struct arm64_image_header *)(kernel_buf);
+
+	if (arm64_process_image_header(h))
+		return -EINVAL;
+
+	dbgprintf("%s: text_offset:   %016lx\n", __func__,
+		arm64_mem.text_offset);
+	dbgprintf("%s: image_size:    %016lx\n", __func__,
+		arm64_mem.image_size);
+	dbgprintf("%s: phys_offset:   %016lx\n", __func__,
+		arm64_mem.phys_offset);
+	dbgprintf("%s: PE format:     %s\n", __func__,
+		(arm64_header_check_pe_sig(h) ? "yes" : "no"));
+
+	image_base = get_phys_offset() + arm64_mem.text_offset;
+	
+	add_segment_phys_virt(info, kernel_buf, kernel_size, image_base,
+		arm64_mem.image_size, 0);
+
+	return arm64_load_other_segments(info, image_base);
 }
 
 void image_arm64_usage(void)
 {
 	printf(
 "     An ARM64 binary image, compressed or not, big or little endian.\n"
-"     Typically an Image, Image.gz or Image.lzma file.\n"
-"     This file type is currently NOT SUPPORTED.\n\n");
+"     Typically an Image, Image.gz or Image.lzma file.\n\n");
 }