Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro

2019-07-19 Thread Michael Ellerman
Sven Schnelle  writes:
> Hi Michael,
>
> On Thu, Jul 11, 2019 at 09:08:51PM +1000, Michael Ellerman wrote:
>> Sven Schnelle  writes:
>> > On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote:
>> >> Le 10/07/2019 à 16:29, Sven Schnelle a écrit :
>> >> > It had only one definition, so just use the function directly.
>> >> 
>> >> It had only one definition because it was for ppc64 only.
>> >> But as far as I understand (at least from the name of the new file), you
>> >> want it to be generic, don't you ? Therefore I get on 32 bits it would be
>> >> elf32_to_cpu().
>> >
>> > That brings up the question whether we need those endianess conversions. I 
>> > would
>> > assume that the ELF file has always the same endianess as the running 
>> > kernel. So
>> > i think we could just drop them. What do you think?
>> 
>> We should be able to kexec from big to little endian or vice versa, so
>> they are necessary.
>
> I'll update the patch to check for a needed 32/64 bit conversion during 
> runtime,
> so we can also kexec from 32 to 64 bit kernels and vice versa. Don't know
> whether that's possible on powerpc, but at least on parisc it is.

On some of the Freescale (NXP) machines that should actually be
possible, the hardware can run a 64 or 32-bit kernel, but I'm not sure
if anyone has actually tested kexec'ing from one to the other.

cheers


Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro

2019-07-15 Thread Sven Schnelle
Hi Michael,

On Thu, Jul 11, 2019 at 09:08:51PM +1000, Michael Ellerman wrote:
> Sven Schnelle  writes:
> > On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote:
> >> Le 10/07/2019 à 16:29, Sven Schnelle a écrit :
> >> > It had only one definition, so just use the function directly.
> >> 
> >> It had only one definition because it was for ppc64 only.
> >> But as far as I understand (at least from the name of the new file), you
> >> want it to be generic, don't you ? Therefore I get on 32 bits it would be
> >> elf32_to_cpu().
> >
> > That brings up the question whether we need those endianess conversions. I 
> > would
> > assume that the ELF file has always the same endianess as the running 
> > kernel. So
> > i think we could just drop them. What do you think?
> 
> We should be able to kexec from big to little endian or vice versa, so
> they are necessary.

I'll update the patch to check for a needed 32/64 bit conversion during runtime,
so we can also kexec from 32 to 64 bit kernels and vice versa. Don't know
whether that's possible on powerpc, but at least on parisc it is.

Regards
Sven


Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro

2019-07-11 Thread Michael Ellerman
Sven Schnelle  writes:
> On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote:
>> Le 10/07/2019 à 16:29, Sven Schnelle a écrit :
>> > It had only one definition, so just use the function directly.
>> 
>> It had only one definition because it was for ppc64 only.
>> But as far as I understand (at least from the name of the new file), you
>> want it to be generic, don't you ? Therefore I get on 32 bits it would be
>> elf32_to_cpu().
>
> That brings up the question whether we need those endianess conversions. I 
> would
> assume that the ELF file has always the same endianess as the running kernel. 
> So
> i think we could just drop them. What do you think?

We should be able to kexec from big to little endian or vice versa, so
they are necessary.

cheers


Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro

2019-07-10 Thread Sven Schnelle
Hi Christophe,

On Wed, Jul 10, 2019 at 05:09:29PM +0200, Christophe Leroy wrote:
> 
> 
> Le 10/07/2019 à 16:29, Sven Schnelle a écrit :
> > It had only one definition, so just use the function directly.
> 
> It had only one definition because it was for ppc64 only.
> But as far as I understand (at least from the name of the new file), you
> want it to be generic, don't you ? Therefore I get on 32 bits it would be
> elf32_to_cpu().

That brings up the question whether we need those endianess conversions. I would
assume that the ELF file has always the same endianess as the running kernel. So
i think we could just drop them. What do you think?

Regards
Sven


Re: [PATCH v3 5/7] kexec_elf: remove elf_addr_to_cpu macro

2019-07-10 Thread Christophe Leroy




Le 10/07/2019 à 16:29, Sven Schnelle a écrit :

It had only one definition, so just use the function directly.


It had only one definition because it was for ppc64 only.
But as far as I understand (at least from the name of the new file), you 
want it to be generic, don't you ? Therefore I get on 32 bits it would 
be elf32_to_cpu().


Christophe



Signed-off-by: Sven Schnelle 
---
  kernel/kexec_elf.c | 20 +---
  1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/kernel/kexec_elf.c b/kernel/kexec_elf.c
index 70d31b8feeae..99e6d63b5dfc 100644
--- a/kernel/kexec_elf.c
+++ b/kernel/kexec_elf.c
@@ -8,8 +8,6 @@
  #include 
  #include 
  
-#define elf_addr_to_cpu	elf64_to_cpu

-
  #ifndef Elf_Rel
  #define Elf_Rel   Elf64_Rel
  #endif /* Elf_Rel */
@@ -143,9 +141,9 @@ static int elf_read_ehdr(const char *buf, size_t len, 
struct elfhdr *ehdr)
ehdr->e_type  = elf16_to_cpu(ehdr, buf_ehdr->e_type);
ehdr->e_machine   = elf16_to_cpu(ehdr, buf_ehdr->e_machine);
ehdr->e_version   = elf32_to_cpu(ehdr, buf_ehdr->e_version);
-   ehdr->e_entry = elf_addr_to_cpu(ehdr, buf_ehdr->e_entry);
-   ehdr->e_phoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_phoff);
-   ehdr->e_shoff = elf_addr_to_cpu(ehdr, buf_ehdr->e_shoff);
+   ehdr->e_entry = elf64_to_cpu(ehdr, buf_ehdr->e_entry);
+   ehdr->e_phoff = elf64_to_cpu(ehdr, buf_ehdr->e_phoff);
+   ehdr->e_shoff = elf64_to_cpu(ehdr, buf_ehdr->e_shoff);
ehdr->e_flags = elf32_to_cpu(ehdr, buf_ehdr->e_flags);
ehdr->e_phentsize = elf16_to_cpu(ehdr, buf_ehdr->e_phentsize);
ehdr->e_phnum = elf16_to_cpu(ehdr, buf_ehdr->e_phnum);
@@ -190,18 +188,18 @@ static int elf_read_phdr(const char *buf, size_t len,
buf_phdr = (struct elf_phdr *) pbuf;
  
  	phdr->p_type   = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_type);

-   phdr->p_offset = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_offset);
-   phdr->p_paddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_paddr);
-   phdr->p_vaddr  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr);
+   phdr->p_offset = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_offset);
+   phdr->p_paddr  = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_paddr);
+   phdr->p_vaddr  = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_vaddr);
phdr->p_flags  = elf32_to_cpu(elf_info->ehdr, buf_phdr->p_flags);
  
  	/*

 * The following fields have a type equivalent to Elf_Addr
 * both in 32 bit and 64 bit ELF.
 */
-   phdr->p_filesz = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_filesz);
-   phdr->p_memsz  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_memsz);
-   phdr->p_align  = elf_addr_to_cpu(elf_info->ehdr, buf_phdr->p_align);
+   phdr->p_filesz = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_filesz);
+   phdr->p_memsz  = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_memsz);
+   phdr->p_align  = elf64_to_cpu(elf_info->ehdr, buf_phdr->p_align);
  
  	return elf_is_phdr_sane(phdr, len) ? 0 : -ENOEXEC;

  }