Subject: Re: [musl] ppc64le and 32-bit LE userland compatibility

From: Will Springer <[email-masked]>
Date: Tue, 09 Jun 2020 10:29:57 +0000

On Saturday, May 30, 2020 3:56:47 PM PDT you wrote:
> On Friday, May 29, 2020 12:24:27 PM PDT Rich Felker wrote:
> > The argument passing for pread/pwrite is historically a mess and
> > differs between archs. musl has a dedicated macro that archs can
> > define to override it. But it looks like it should match regardless of
> > BE vs LE, and musl already defines it for powerpc with the default
> > definition, adding a zero arg to start on an even arg-slot index,
> > which is an odd register (since ppc32 args start with an odd one, r3).
> > 
> > > [6]:
> > > https://gist.github.com/Skirmisher/02891c1a8cafa0ff18b2460933ef4f3c
> > 
> > I don't think this is correct, but I'm confused about where it's
> > getting messed up because it looks like it should already be right.
> 
> Hmm, interesting. Will have to go back to it I guess...
> 
> > > This was enough to fix up the `file` bug. I'm no seasoned kernel
> > > hacker, though, and there is still concern over the right way to
> > > approach this, whether it should live in the kernel or libc, etc.
> > > Frankly, I don't know the ABI structure enough to understand why the
> > > register padding has to be different in this case, or what
> > > lower-level component is responsible for it.. For comparison, I had
> > > a
> > > look at the mips tree, since it's bi-endian and has a similar 32/64
> > > situation. There is a macro conditional upon endianness that is
> > > responsible for munging long longs; it uses __MIPSEB__ and
> > > __MIPSEL__
> > > instead of an if/else on the generic __LITTLE_ENDIAN__. Not sure
> > > what
> > > to make of that. (It also simply swaps registers for LE, unlike what
> > > I did for ppc.)
> > 
> > Indeed the problem is probably that you need to swap registers for LE,
> > not remove the padding slot. Did you check what happens if you pass a
> > value larger than 32 bits?
> > 
> > If so, the right way to fix this on the kernel side would be to
> > construct the value as a union rather than by bitwise ops so it's
> > 
> > endian-agnostic:
> > 	(union { u32 parts[2]; u64 val; }){{ arg1, arg2 }}.val
> > 
> > But the kernel folks might prefer endian ifdefs for some odd reason...
> 
> You are right, this does seem odd considering what the other archs do.
> It's quite possible I made a silly mistake, of course...
> 
> I haven't tested with values outside the 32-bit range yet; again, this
> is new territory for me, so I haven't exactly done exhaustive tests on
> everything. I'll give it a closer look.

I took some cues from the mips linux32 syscall setup, and drafted a new 
patch defining a macro to compose the hi/lo parts within the function, 
instead of swapping the args at the function definition. `file /bin/bash` 
and `truncate -s 5G test` both work correctly now. This appears to be the 
correct solution, so I'm not sure what silly mistake I made before, but 
apologies for the confusion. I've updated my gist with the new patch [1].

> > > Also worth noting is the one other outstanding bug, where the
> > > time-related syscalls in the 32-bit vDSO seem to return garbage. It
> > > doesn't look like an endian bug to me, and it doesn't affect
> > > standard
> > > syscalls (which is why if you run `date` on musl it prints the
> > > correct time, unlike on glibc). The vDSO time functions are
> > > implemented in ppc asm (arch/powerpc/kernel/vdso32/ gettimeofday.S),
> > > and I've never touched the stuff, so if anyone has a clue I'm all
> > > ears.
> > 
> > Not sure about this. Worst-case, just leave it disabled until someone
> > finds a fix.
> 
> Apparently these asm implementations are being replaced by the generic C
> ones [1], so it may be this fixes itself on its own.
> 
> Thanks,
> Will [she/her]
> 
> [1]:
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=173231

I mentioned in Christophe's thread the other day, but his patchset does 
solve the vdso32 issues, though it introduced problems in vdso64 in my 
testing. With that solved and the syscall situation established, I think 
the kernel state is stable enough to start looking at solidifying libc/
compiler stuff. I'll try to get a larger userland built in the near future 
to try to catch any remaining problems (before rebuilding it all when 
libc/ABI support becomes explicit).

Cheers,
Will [she/her]

[1]: https://gist.github.com/Skirmisher/02891c1a8cafa0ff18b2460933ef4f3c