Re: [PATCH v12 0/7] RISC-V: ifunced memcpy using new kernel hwprobe interface

[Jessica Clarke <jrtc27@jrtc27.com>]

On 15 Feb 2024, at 16:50, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> On Thu, 15 Feb 2024 07:48:03 PST (-0800), Evan Green wrote:
>> On Wed, Feb 14, 2024 at 10:16 AM Adhemerval Zanella Netto
>> <adhemerval.zanella@linaro.org> wrote:
>>> 
>>> 
>>> 
>>> On 14/02/24 11:31, Evan Green wrote:
>>> >
>>> > This series illustrates the use of a recently accepted Linux syscall that
>>> > enumerates architectural information about the RISC-V cores the system
>>> > is running on. In this series we expose a small wrapper function around
>>> > the syscall. An ifunc selector for memcpy queries it to see if unaligned
>>> > access is "fast" on this hardware. If it is, it selects a newly provided
>>> > implementation of memcpy that doesn't work hard at aligning the src and
>>> > destination buffers.
>>> >
>>> > For applications and libraries outside of glibc that want to use
>>> > __riscv_hwprobe() in ifunc selectors, this series also sends a pointer
>>> > to the riscv_hwprobe() function in as the second argument to ifunc
>>> > selectors. A new inline convenience function can help application and
>>> > library callers to check for validity and quickly probe a single key.
>>> 
>>> I still think we should address Jessica Clarke remarks for the ifunc ABI [1].
>>> I recall that Florian has tried to address the ifunc ordering and that
>>> Jessica proposed solutions was not fully sufficient to address all the
>>> ifunc corner cases.
>>> 
>>> [1] https://sourceware.org/pipermail/libc-alpha/2024-January/154082.html
>> 
>> I haven't invested the time yet in studying the resolver to understand
>> how feasible Jessica's suggestion is. I was sort of hoping Florian
>> would chime in with an "oh yeah let's do that" or "no, it doesn't
>> work". I suppose I still am :)
> 
> The discussion's over here: https://inbox.sourceware.org/libc-alpha/20231017044641.pw2ccr6exvhtmhkk@google.com/
> I was inclined to just ignore it: Florian explained what's going on with the multiple libraries constraint, so we're kind of just going around in circles at that point.
> 
> I'm also not sure the argument makes a whole lot of sense in the first place.  The core of the argument seems to be around binary compatibility with other kernels/libcs,

Not binary compatibility, source compatibility. Yes, it’s part of that
platform’s ABI, but it leaks into the source code by means of the
resolver’s type and arguments. I would much rather see a standard IFUNC
resolver interface that can be specified in the psABI as a
cross-platform standard for any ELF-using OS that implements IFUNCs,
since the moment glibc adopts this as its implementation of choice
there’s no going back (without ugly “ignore that magic -1 argument,
it’s there for historical reasons” kinds of situations). Adopting the
stronger guarantees provided by FreeBSD’s rtld is one way to achieve
that, and seems the simplest to me, but I’m not wedded to that, other
suggestions that achieve the same end goal are just as welcome. Of
course glibc is free to ignore me, too, with a “not our problem, deal
with it, we’re the dominant OS”, but that would seem sad.

Jess

> but there's a ton of reasons why binaries aren't compatible between those systems.  So if glibc just has a more complex IFUNC resolving scheme and that ends up requiring the extra agrument, then users of glibc have to go deal with that -- other systems/libcs might make different decisions, but that's just how these things go.
> 
> Looks like Maskray is proposing making glibc's IFUNC resolving.  That's a generic glibc decision and I don't really understand this stuff well enough to have much of an opinion -- sure maybe the multi-library IFUNC resolving is overkill, but aside from doing some similar grep of all the sources (or trying some builds and such) I'm not sure how to tell if we're safe breaking the ABI there.
> 
> That said, we're not comitting to ABI stability until the release.  So if we decide to make a generic glibc change we can always go drop the argument, that should be easy.  Maybe we even just throw out a RFC patch to do it, unless I'm missing something the code is the easy part here (we're essentially just going back to one of the early versions, from before we knew about this resolving order complexity).
> 
>> Alternatively, patches 1-3 of this series stand on their own. If the
>> ifunc aspect of this is gated on me doing a bunch of research, it
>> might at least make sense to land the first half now, to get Linux
>> users easy access to the __riscv_hwprobe() syscall and vDSO.
> 
> I'm OK with that, too.
> 
>> 
>> -Evan