| Message ID | 20190710142944.2774-6-svens@stackframe.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kexec: add generic support for elf kernel images | expand |
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 <svens@stackframe.org> > --- > 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 <linux/slab.h> > #include <linux/types.h> > > -#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; > } >
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
Sven Schnelle <svens@stackframe.org> 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
Hi Michael, On Thu, Jul 11, 2019 at 09:08:51PM +1000, Michael Ellerman wrote: > Sven Schnelle <svens@stackframe.org> 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
Sven Schnelle <svens@stackframe.org> writes: > Hi Michael, > > On Thu, Jul 11, 2019 at 09:08:51PM +1000, Michael Ellerman wrote: >> Sven Schnelle <svens@stackframe.org> 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
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 <linux/slab.h> #include <linux/types.h> -#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; }
It had only one definition, so just use the function directly. Signed-off-by: Sven Schnelle <svens@stackframe.org> --- kernel/kexec_elf.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)