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

[Palmer Dabbelt <palmer@rivosinc.com>]

On Thu, 15 Feb 2024 11:09:13 PST (-0800), jrtc27@jrtc27.com wrote:
> On 15 Feb 2024, at 18:52, enh <enh@google.com> wrote:
>> 
>> On Thu, Feb 15, 2024 at 9:42 AM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>>> 
>>> 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.
>> 
>> oh, yeah, obviously on the Android side i'm only talking about API,
>> not ABI. but i think Android-glibc source compatibility is more
>> meaningful -- by virtue of them both using Linux -- than anything
>> else.
>> 
>>> 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.
>> 
>> _not_ passing the function pointer to the ifunc resolver only
>> addresses source compatibility as far as the function _signature_
>> goes, but does nothing for the _body_ of the function unless BSD
>> copies the Linux __riscv_hwprobe() syscall exactly. and tracks every
>> change after that. that seems unlikely?
>
> Yes, nobody’s going to copy that, but that doesn’t mean it’s going to

Why not?  It's about the simplest interface that's extensible, 
essentially just a list of key-value pairs and some scaffolding to get 
them past a protection domain.

> forever be the only interface that you could use. One function existing
> doesn’t stop another from doing so, but one argument being used for one
> purpose does stop it being used for another. That is, in the proposed
> world where you just call __riscv_hwprobe directly and it works, there
> is a path towards having a cross-platform solution, namely defining a
> simplified, OS-independent interface for querying what extensions you
> have (which won’t cover all use cases, and doesn’t need to, so long as
> it can deal with the ones that IFUNCs typically care about, i.e. simple
> questions like “do I have V/Zb*/etc?”), which could just be a simple
> wrapper around __riscv_hwprobe on Linux. But if you pass
> __riscv_hwprobe as a function pointer as the solution to the problem,
> you’re tied to the Linux interface, and specifically the
> __riscv_hwprobe one at that, unless you want to add an argument every
> time you come up with an interface that software might want to use in a
> resolver for querying RISC-V information. I’m trying to look at the
> bigger picture here rather than the very narrow view of “what’s the
> easiest path to being able to write an IFUNCed memcpy in glibc for
> Linux”.

I think this is just going around in circles, it's basically the same 
discussion we had when adding hwprobe to Linux.  We have a real concrete 
solution to real concrete problems, progress has been blocked for years 
without any concrete proposal aside from just doing something different.

So like I said in some other thread, I'm inclined to just merge this.  
There's some feedback from Florian that looks pretty easy to resolve, 
and looks like Evan should have a new version soon.

>> plus, like i keep saying --- iOS and macOS don't have ifuncs (and i
>> don't think Windows does either?), so for any code that actually cares
>> about portability, handling your own function pointer is the only
>> truly portable game in town, and all this fussing about a niche
>> alternative is just noise.
>
> Firstly, none of those support RISC-V. Secondly, just because some OSes
> are more different than others doesn’t mean we shouldn’t aim for as
> cross-platform interfaces as possible. Otherwise why are we bothering
> to have a RISC-V psABI at all rather than just letting glibc define
> whatever it wants?

We have already defacto forked from the psABI pretty much everywhere due 
to all the unresolved issues, we just try to keep things compatible in 
practice -- or at least where we can, some of it just gets ignored.

>> (and we see that in practice, not just theory: open source libraries
>> have voted with their feet, and nothing we do here will affect that.)
>> 
>> i don't see that anyone actually gains anything from your proposal,
>> whereas the "pass the function pointer to the ifunc resolver"
>> workaround addresses actual problems with currently-shipping systems
>> in a way that doesn't require large and complex behavior changes (for
>> no gain _in the context of ifuncs_).
>
> Well, it’s a general solution to the specific problem seen here, which
> arguably makes the glibc implementation of IFUNCs on RISC-V less
> special, as you can just call the normal __riscv_hwprobe if you want to.
> The gain is generality, which includes, but isn’t limited to, being
> cross-platform.

I guess it's kind of hard to evaluate hypothetical interface proposals, 
but it's not clear how merging this prevents a new interface from being 
added.  Unless I'm missing something we've already got a NULL at the end 
of the argument list, so we should be safe to extend it.  If users of 
this new interface can also call hwprobe then we'll need to maintain it 
forever, having an argument seems like a pretty straight-forward 
solution.

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