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

[Palmer Dabbelt <palmer@rivosinc.com>]

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, 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