All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS: Fix computation on entry point
@ 2018-08-28 15:52 Philippe Reynes
  2018-08-29 17:57 ` Paul Burton
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe Reynes @ 2018-08-28 15:52 UTC (permalink / raw)
  To: ralf, paul.burton, jhogan; +Cc: linux-mips, linux-kernel, Philippe Reynes

Since commit 27c524d17430 ("MIPS: Use the entry point from the ELF
file header"), the kernel entry point is computed with a grep on
"start address" on the output of objdump. It works fine when the
default language is english but it may fail on other language (for
example in French, the grep should be done on "adresse de départ").
To fix this computation on most machine, I propose to force the
language to english with "LC_ALL=C".

Fixes: 27c524d17430 ("MIPS: Use the entry point from the ELF file header")

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 arch/mips/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index d74b374..835aa8f 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -258,7 +258,7 @@ load-y					= $(CONFIG_PHYSICAL_START)
 endif
 
 # Sign-extend the entry point to 64 bits if retrieved as a 32-bit number.
-entry-y		= $(shell $(OBJDUMP) -f vmlinux 2>/dev/null \
+entry-y		= $(shell LC_ALL=C $(OBJDUMP) -f vmlinux 2>/dev/null \
 			| sed -n '/^start address / { \
 				s/^.* //; \
 				s/0x\([0-7].......\)$$/0x00000000\1/; \
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: Fix computation on entry point
  2018-08-28 15:52 [PATCH] MIPS: Fix computation on entry point Philippe Reynes
@ 2018-08-29 17:57 ` Paul Burton
  2018-08-29 18:01   ` [PATCH] MIPS: Use a custom elf-entry program to find kernel " Paul Burton
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Burton @ 2018-08-29 17:57 UTC (permalink / raw)
  To: Philippe Reynes; +Cc: ralf, jhogan, linux-mips, linux-kernel

Hi Philippe,

On Tue, Aug 28, 2018 at 05:52:50PM +0200, Philippe Reynes wrote:
> Since commit 27c524d17430 ("MIPS: Use the entry point from the ELF
> file header"), the kernel entry point is computed with a grep on
> "start address" on the output of objdump. It works fine when the
> default language is english but it may fail on other language (for
> example in French, the grep should be done on "adresse de départ").

D'oh!

> To fix this computation on most machine, I propose to force the
> language to english with "LC_ALL=C".

I wonder if it's time to bite the bullet & just write a little custom
program to do what we need, rather than trying to wrangle standard tools
that *almost* do what we need but not quite... Patch incoming.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] MIPS: Use a custom elf-entry program to find kernel entry point
  2018-08-29 17:57 ` Paul Burton
@ 2018-08-29 18:01   ` Paul Burton
  2018-08-30 12:22     ` Philippe REYNES
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Burton @ 2018-08-29 18:01 UTC (permalink / raw)
  To: Philippe Reynes, linux-mips; +Cc: Paul Burton, James Hogan, Ralf Baechle

For a long time arch/mips/Makefile used nm to discover the kernel entry
point by looking for the address of the kernel_entry symbol. This
doesn't work for systems which make use of bit 0 of the PC to reflect
the ISA mode - ie. microMIPS (and MIPS16, but we don't support building
kernels that target MIPS16 anyway).

So for a while with commit 5fc9484f5e41 ("MIPS: Set ISA bit in entry-y
for microMIPS kernels") we manually modified the last nibble of the
output from nm, which worked but wasn't particularly pretty.

Commit 27c524d17430 ("MIPS: Use the entry point from the ELF file
header") then cleaned this up by using objdump to print the ELF entry
point which includes the ISA bit, rather than using nm to print the
address of the kernel_entry symbol which doesn't. That removed the ugly
replacement of the last nibble, but added its own ugliness by needing to
manually sign extend in the 32 bit case.

Unfortunately it has been pointed out that objdump's output is
localised, and therefore grepping for its "start address" output doesn't
work when the user's language settings are such that objdump doesn't
print in English.

We could simply revert commit 27c524d17430 ("MIPS: Use the entry point
from the ELF file header") and return to the manual replacement of the
last nibble of entry-y, but it seems that was found sufficiently
unpalatable to avoid. We could attempt to force the language used by
objdump by setting an environment variable such as LC_ALL, but that
seems fragile. Instead we add a small tool named elf-entry which simply
prints out the entry point of the kernel in the format we require.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Reported-by: Philippe Reynes <philippe.reynes@softathome.com>
Fixes: 27c524d17430 ("MIPS: Use the entry point from the ELF file header")
Cc: James Hogan <jhogan@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
---
 arch/mips/Makefile          |  9 +---
 arch/mips/tools/.gitignore  |  1 +
 arch/mips/tools/Makefile    |  5 ++
 arch/mips/tools/elf-entry.c | 96 +++++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+), 7 deletions(-)
 create mode 100644 arch/mips/tools/.gitignore
 create mode 100644 arch/mips/tools/Makefile
 create mode 100644 arch/mips/tools/elf-entry.c

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index d74b3742fa5d..053e1c314f9e 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -13,6 +13,7 @@
 #
 
 archscripts: scripts_basic
+	$(Q)$(MAKE) $(build)=arch/mips/tools elf-entry
 	$(Q)$(MAKE) $(build)=arch/mips/boot/tools relocs
 
 KBUILD_DEFCONFIG := 32r2el_defconfig
@@ -257,13 +258,7 @@ ifdef CONFIG_PHYSICAL_START
 load-y					= $(CONFIG_PHYSICAL_START)
 endif
 
-# Sign-extend the entry point to 64 bits if retrieved as a 32-bit number.
-entry-y		= $(shell $(OBJDUMP) -f vmlinux 2>/dev/null \
-			| sed -n '/^start address / { \
-				s/^.* //; \
-				s/0x\([0-7].......\)$$/0x00000000\1/; \
-				s/0x\(........\)$$/0xffffffff\1/; p }')
-
+entry-y				= $(shell $(objtree)/arch/mips/tools/elf-entry vmlinux)
 cflags-y			+= -I$(srctree)/arch/mips/include/asm/mach-generic
 drivers-$(CONFIG_PCI)		+= arch/mips/pci/
 
diff --git a/arch/mips/tools/.gitignore b/arch/mips/tools/.gitignore
new file mode 100644
index 000000000000..56d34ccccce4
--- /dev/null
+++ b/arch/mips/tools/.gitignore
@@ -0,0 +1 @@
+elf-entry
diff --git a/arch/mips/tools/Makefile b/arch/mips/tools/Makefile
new file mode 100644
index 000000000000..3baee4bc6775
--- /dev/null
+++ b/arch/mips/tools/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+hostprogs-y := elf-entry
+PHONY += elf-entry
+elf-entry: $(obj)/elf-entry
+	@:
diff --git a/arch/mips/tools/elf-entry.c b/arch/mips/tools/elf-entry.c
new file mode 100644
index 000000000000..adde79ce7fc0
--- /dev/null
+++ b/arch/mips/tools/elf-entry.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <byteswap.h>
+#include <elf.h>
+#include <endian.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#ifdef be32toh
+/* If libc provides [bl]e{32,64}toh() then we'll use them */
+#elif BYTE_ORDER == LITTLE_ENDIAN
+# define be32toh(x)	bswap_32(x)
+# define le32toh(x)	(x)
+# define be64toh(x)	bswap_64(x)
+# define le64toh(x)	(x)
+#elif BYTE_ORDER == BIG_ENDIAN
+# define be32toh(x)	(x)
+# define le32toh(x)	bswap_32(x)
+# define be64toh(x)	(x)
+# define le64toh(x)	bswap_64(x)
+#endif
+
+__attribute__((noreturn))
+static void die(const char *msg)
+{
+	fputs(msg, stderr);
+	exit(EXIT_FAILURE);
+}
+
+int main(int argc, const char *argv[])
+{
+	uint64_t entry;
+	size_t nread;
+	FILE *file;
+	union {
+		Elf32_Ehdr ehdr32;
+		Elf64_Ehdr ehdr64;
+	} hdr;
+
+	if (argc != 2)
+		die("Usage: elf-entry <elf-file>\n");
+
+	file = fopen(argv[1], "r");
+	if (!file) {
+		perror("Unable to open input file");
+		return EXIT_FAILURE;
+	}
+
+	nread = fread(&hdr, 1, sizeof(hdr), file);
+	if (nread != sizeof(hdr)) {
+		perror("Unable to read input file");
+		return EXIT_FAILURE;
+	}
+
+	if (memcmp(hdr.ehdr32.e_ident, ELFMAG, SELFMAG))
+		die("Input is not an ELF\n");
+
+	switch (hdr.ehdr32.e_ident[EI_CLASS]) {
+	case ELFCLASS32:
+		switch (hdr.ehdr32.e_ident[EI_DATA]) {
+		case ELFDATA2LSB:
+			entry = le32toh(hdr.ehdr32.e_entry);
+			break;
+		case ELFDATA2MSB:
+			entry = be32toh(hdr.ehdr32.e_entry);
+			break;
+		default:
+			die("Invalid ELF encoding\n");
+		}
+
+		/* Sign extend to form a canonical address */
+		entry = (int64_t)(int32_t)entry;
+		break;
+
+	case ELFCLASS64:
+		switch (hdr.ehdr32.e_ident[EI_DATA]) {
+		case ELFDATA2LSB:
+			entry = le64toh(hdr.ehdr64.e_entry);
+			break;
+		case ELFDATA2MSB:
+			entry = be64toh(hdr.ehdr64.e_entry);
+			break;
+		default:
+			die("Invalid ELF encoding\n");
+		}
+		break;
+
+	default:
+		die("Invalid ELF class\n");
+	}
+
+	printf("0x%016" PRIx64 "\n", entry);
+	return EXIT_SUCCESS;
+}
-- 
2.18.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: Use a custom elf-entry program to find kernel entry point
  2018-08-29 18:01   ` [PATCH] MIPS: Use a custom elf-entry program to find kernel " Paul Burton
@ 2018-08-30 12:22     ` Philippe REYNES
  2018-08-30 17:34       ` Paul Burton
  0 siblings, 1 reply; 5+ messages in thread
From: Philippe REYNES @ 2018-08-30 12:22 UTC (permalink / raw)
  To: Paul Burton; +Cc: linux-mips, James Hogan, Ralf Baechle

Hi Paul,

----- Le 29 Aoû 18, à 20:01, Paul Burton paul.burton@mips.com a écrit :

> For a long time arch/mips/Makefile used nm to discover the kernel entry
> point by looking for the address of the kernel_entry symbol. This
> doesn't work for systems which make use of bit 0 of the PC to reflect
> the ISA mode - ie. microMIPS (and MIPS16, but we don't support building
> kernels that target MIPS16 anyway).
> 
> So for a while with commit 5fc9484f5e41 ("MIPS: Set ISA bit in entry-y
> for microMIPS kernels") we manually modified the last nibble of the
> output from nm, which worked but wasn't particularly pretty.
> 
> Commit 27c524d17430 ("MIPS: Use the entry point from the ELF file
> header") then cleaned this up by using objdump to print the ELF entry
> point which includes the ISA bit, rather than using nm to print the
> address of the kernel_entry symbol which doesn't. That removed the ugly
> replacement of the last nibble, but added its own ugliness by needing to
> manually sign extend in the 32 bit case.
> 
> Unfortunately it has been pointed out that objdump's output is
> localised, and therefore grepping for its "start address" output doesn't
> work when the user's language settings are such that objdump doesn't
> print in English.
> 
> We could simply revert commit 27c524d17430 ("MIPS: Use the entry point
> from the ELF file header") and return to the manual replacement of the
> last nibble of entry-y, but it seems that was found sufficiently
> unpalatable to avoid. We could attempt to force the language used by
> objdump by setting an environment variable such as LC_ALL, but that
> seems fragile. Instead we add a small tool named elf-entry which simply
> prints out the entry point of the kernel in the format we require.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Reported-by: Philippe Reynes <philippe.reynes@softathome.com>
> Fixes: 27c524d17430 ("MIPS: Use the entry point from the ELF file header")
> Cc: James Hogan <jhogan@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org

I've tested on my "french" machine, and it works fine.

Tested-by: Philippe Reynes <philippe.reynes@softathome.com>

> ---
> arch/mips/Makefile | 9 +---
> arch/mips/tools/.gitignore | 1 +
> arch/mips/tools/Makefile | 5 ++
> arch/mips/tools/elf-entry.c | 96 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 104 insertions(+), 7 deletions(-)
> create mode 100644 arch/mips/tools/.gitignore
> create mode 100644 arch/mips/tools/Makefile
> create mode 100644 arch/mips/tools/elf-entry.c
> 
> diff --git a/arch/mips/Makefile b/arch/mips/Makefile
> index d74b3742fa5d..053e1c314f9e 100644
> --- a/arch/mips/Makefile
> +++ b/arch/mips/Makefile
> @@ -13,6 +13,7 @@
> #
> 
> archscripts: scripts_basic
> + $(Q)$(MAKE) $(build)=arch/mips/tools elf-entry
> $(Q)$(MAKE) $(build)=arch/mips/boot/tools relocs
> 
> KBUILD_DEFCONFIG := 32r2el_defconfig
> @@ -257,13 +258,7 @@ ifdef CONFIG_PHYSICAL_START
> load-y = $(CONFIG_PHYSICAL_START)
> endif
> 
> -# Sign-extend the entry point to 64 bits if retrieved as a 32-bit number.
> -entry-y = $(shell $(OBJDUMP) -f vmlinux 2>/dev/null \
> - | sed -n '/^start address / { \
> - s/^.* //; \
> - s/0x\([0-7].......\)$$/0x00000000\1/; \
> - s/0x\(........\)$$/0xffffffff\1/; p }')
> -
> +entry-y = $(shell $(objtree)/arch/mips/tools/elf-entry vmlinux)
> cflags-y += -I$(srctree)/arch/mips/include/asm/mach-generic
> drivers-$(CONFIG_PCI) += arch/mips/pci/
> 
> diff --git a/arch/mips/tools/.gitignore b/arch/mips/tools/.gitignore
> new file mode 100644
> index 000000000000..56d34ccccce4
> --- /dev/null
> +++ b/arch/mips/tools/.gitignore
> @@ -0,0 +1 @@
> +elf-entry
> diff --git a/arch/mips/tools/Makefile b/arch/mips/tools/Makefile
> new file mode 100644
> index 000000000000..3baee4bc6775
> --- /dev/null
> +++ b/arch/mips/tools/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-y := elf-entry
> +PHONY += elf-entry
> +elf-entry: $(obj)/elf-entry
> + @:
> diff --git a/arch/mips/tools/elf-entry.c b/arch/mips/tools/elf-entry.c
> new file mode 100644
> index 000000000000..adde79ce7fc0
> --- /dev/null
> +++ b/arch/mips/tools/elf-entry.c
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <byteswap.h>
> +#include <elf.h>
> +#include <endian.h>
> +#include <inttypes.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#ifdef be32toh
> +/* If libc provides [bl]e{32,64}toh() then we'll use them */
> +#elif BYTE_ORDER == LITTLE_ENDIAN
> +# define be32toh(x) bswap_32(x)
> +# define le32toh(x) (x)
> +# define be64toh(x) bswap_64(x)
> +# define le64toh(x) (x)
> +#elif BYTE_ORDER == BIG_ENDIAN
> +# define be32toh(x) (x)
> +# define le32toh(x) bswap_32(x)
> +# define be64toh(x) (x)
> +# define le64toh(x) bswap_64(x)
> +#endif
> +
> +__attribute__((noreturn))
> +static void die(const char *msg)
> +{
> + fputs(msg, stderr);
> + exit(EXIT_FAILURE);
> +}
> +
> +int main(int argc, const char *argv[])
> +{
> + uint64_t entry;
> + size_t nread;
> + FILE *file;
> + union {
> + Elf32_Ehdr ehdr32;
> + Elf64_Ehdr ehdr64;
> + } hdr;
> +
> + if (argc != 2)
> + die("Usage: elf-entry <elf-file>\n");
> +
> + file = fopen(argv[1], "r");
> + if (!file) {
> + perror("Unable to open input file");
> + return EXIT_FAILURE;
> + }
> +
> + nread = fread(&hdr, 1, sizeof(hdr), file);
> + if (nread != sizeof(hdr)) {
> + perror("Unable to read input file");
> + return EXIT_FAILURE;
> + }
> +
> + if (memcmp(hdr.ehdr32.e_ident, ELFMAG, SELFMAG))
> + die("Input is not an ELF\n");
> +
> + switch (hdr.ehdr32.e_ident[EI_CLASS]) {
> + case ELFCLASS32:
> + switch (hdr.ehdr32.e_ident[EI_DATA]) {
> + case ELFDATA2LSB:
> + entry = le32toh(hdr.ehdr32.e_entry);
> + break;
> + case ELFDATA2MSB:
> + entry = be32toh(hdr.ehdr32.e_entry);
> + break;
> + default:
> + die("Invalid ELF encoding\n");
> + }
> +
> + /* Sign extend to form a canonical address */
> + entry = (int64_t)(int32_t)entry;
> + break;
> +
> + case ELFCLASS64:
> + switch (hdr.ehdr32.e_ident[EI_DATA]) {
> + case ELFDATA2LSB:
> + entry = le64toh(hdr.ehdr64.e_entry);
> + break;
> + case ELFDATA2MSB:
> + entry = be64toh(hdr.ehdr64.e_entry);
> + break;
> + default:
> + die("Invalid ELF encoding\n");
> + }
> + break;
> +
> + default:
> + die("Invalid ELF class\n");
> + }
> +
> + printf("0x%016" PRIx64 "\n", entry);
> + return EXIT_SUCCESS;
> +}
> --
> 2.18.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] MIPS: Use a custom elf-entry program to find kernel entry point
  2018-08-30 12:22     ` Philippe REYNES
@ 2018-08-30 17:34       ` Paul Burton
  0 siblings, 0 replies; 5+ messages in thread
From: Paul Burton @ 2018-08-30 17:34 UTC (permalink / raw)
  To: Philippe REYNES; +Cc: linux-mips, James Hogan, Ralf Baechle

Hi Philippe,

On Thu, Aug 30, 2018 at 02:22:38PM +0200, Philippe REYNES wrote:
> I've tested on my "french" machine, and it works fine.
> 
> Tested-by: Philippe Reynes <philippe.reynes@softathome.com>

Merci beaucoup :)

Paul

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-08-30 17:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28 15:52 [PATCH] MIPS: Fix computation on entry point Philippe Reynes
2018-08-29 17:57 ` Paul Burton
2018-08-29 18:01   ` [PATCH] MIPS: Use a custom elf-entry program to find kernel " Paul Burton
2018-08-30 12:22     ` Philippe REYNES
2018-08-30 17:34       ` Paul Burton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.