Re: [musl] FDPIC on sh4?

On 2/16/24 06:25, Rob Landley wrote:
> On 2/15/24 10:47, Rich Felker wrote:
>> On Thu, Feb 15, 2024 at 03:53:53PM +0100, Geert Uytterhoeven wrote:
>>> Hi Rich,
>>> 
>>> On Thu, Feb 15, 2024 at 2:49 PM Rich Felker <[email protected]> wrote:
>>> > On Thu, Feb 15, 2024 at 04:31:00AM -0600, Rob Landley wrote:
>>> > > Is there any easy way to build FDPIC support for sh4 with-mmu? In theory ARM can
>>> >
>>> > On the userspace toolchain and musl side, yes, I see no reason you
>>> > shouldn't be able to build musl for sh4 with fdpic ABI or build a
>>> > toolchain for sh4 that defaults to fdpic and has fdpic target-libs. I
>>> > just tested passing -mfdpic to sh4-linux-musl-gcc and it seems to
>>> > produce correct fdpic code.
>>> >
>>> > On the kernel side, I'm not sure, but the normal ELF loader should be
>>> > able to load fdpic binaries on a system with mmu. It will not float
>>> > the data segment separately from text, but doesn't need to because it
>>> > has an mmu. If this is no longer working it's a kernel regression;
>>> > that's how I always tested sh2eb fdpic binaries on qemu-system-sh4eb.
>>> >
>>> > > do it, so I tried editing the kconfig BINFMT_ELF_FDPIC dependencies in
>>> > > fs/Kconfig.binfmt to move "SUPERH" out of the !MMU list and put it next to ARM,
>>> > > switched on the FDPIC loader, and the build broke with:
>>> > >
>>> > > fs/binfmt_elf_fdpic.o: in function `load_elf_fdpic_binary':
>>> > > binfmt_elf_fdpic.c:(.text+0x1b44): undefined reference to
>>> > > `elf_fdpic_arch_lay_out_mm'
>>> >
>>> > It looks like there's an arch-provided function that's conditional on
>>> > !MMU in arch/sh but that, now that fdpic loader is intended to be
>>> > supported on mmu-ful systems, should be changed to be conditional on
>>> > fdpic support (or maybe even unconditional if fdpic can be loaded as a
>>> > module). Just look for where it's defined in arch/sh. If it's in a
>>> > nommu-specific file it might need to be moved somewhere more
>>> > appropriate, or an mmu-ful variant may need to be written and placed
>>> > somewhere more appropriate.
>>> 
>>> ARM is the sole architecture that provides elf_fdpic_arch_lay_out_mm().
>> 
>> Ah, I missed that this was a new mmu-ful only function. So I guess
>> something like the ARM one is needed. I'm not clear why this is
>> expected to be arch-specific, so it would probably be nice for the
>> kernel to provide a generic version that archs can use unless they
>> need specific behaviors here, or just make it conditional whether it's
>> called at all (only if the arch declares its presence) and use
>> defaults otherwise.
> 
> It's in arch/arm/kernel/elf.c, and pretty short. Doesn't LOOK
> architecture-specific, although I'm not an expert. (Why SZ_16M instead of
> RLIM_STACK? No idea...)

arch/sh/kernel/elf.c doesn't exist so I cut and pasted the function into setup.c
just to see what would happen...

(Question about that file while I'm there: why are calibrate_delay(),
generic_mode_pins(), and test_mode_pin() not marked __init? And is marking
sh_fdt_init() as __ref equivalent to marking it __init?)

The build broke, I fixed it up to at least compile, and my sh2eb fdpic
filesystem didn't boot because...

Starting init: /bin/sh exists but couldn't execute it (error -8)

Exec format error, but I _think_ that's me trying to run a bit endian executable
on a little endian kernel build. :)

Anyway, to keep you posted, here's the "wrong fix" so far.  (Attached because
thunderbird "upgraded" itself and broke my wordrap disable plugin.)

Rob

diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
index d3175f09b3aa..effda8b21370 100644
--- a/arch/sh/kernel/setup.c
+++ b/arch/sh/kernel/setup.c
@@ -404,3 +404,28 @@ void __init arch_cpu_finalize_init(void)
 #endif
 	*p = '\0';
 }
+
+#if defined(CONFIG_MMU) && defined(CONFIG_BINFMT_ELF_FDPIC)
+
+#include <linux/personality.h>
+#include <linux/elf-fdpic.h>
+
+void elf_fdpic_arch_lay_out_mm(struct elf_fdpic_params *exec_params,
+                               struct elf_fdpic_params *interp_params,
+                               unsigned long *start_stack,
+                               unsigned long *start_brk)
+{
+	set_personality((current->personality & ~PER_MASK) | PER_LINUX);
+
+        exec_params->load_addr = 0x8000;
+        interp_params->load_addr = ELF_ET_DYN_BASE;
+        *start_stack = TASK_SIZE - SZ_16M;
+
+        if ((exec_params->flags & ELF_FDPIC_FLAG_ARRANGEMENT) == ELF_FDPIC_FLAG_INDEPENDENT) {
+                exec_params->flags &= ~ELF_FDPIC_FLAG_ARRANGEMENT;
+                exec_params->flags |= ELF_FDPIC_FLAG_CONSTDISP;
+        }
+}
+
+#endif
+