Message ID | 20231213211142.1543025-1-evan@rivosinc.com |
---|---|
Headers | show |
Series | RISC-V: ifunced memcpy using new kernel hwprobe interface | expand |
Any last thoughts on this series? I think the plan is to land it shortly.
On Mon, 08 Jan 2024 09:06:10 PST (-0800), Evan Green wrote: > Any last thoughts on this series? I think the plan is to land it shortly. Sorry for benig slow here. It's good on my end, so as long as nobody has lurking issues I'm happy to take it. Acked-by: Palmer Dabbelt <palmer@rivosinc.com> in case someone else wants to pick it up, otherwise let's give it a day and see? There's no branch date listed on the wiki, but I'd like to get this in sooner rather than later so we can get the distro folks testing it and such.
On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote:
> Any last thoughts on this series? I think the plan is to land it shortly.
I am still wondering what the intended ABI is for the second parameter on
non-Linux ELF systems which do not provide riscv_hwprobe as a system call.
Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic
linkers expected to provide some emulation of riscv_hwprobe of some variable
fidelity? In the latter case, what features of riscv_hwprobe must be emulated
for the dynamic linker to claim ABI conformance?
-s
On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> wrote: > > On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: > > Any last thoughts on this series? I think the plan is to land it shortly. > > I am still wondering what the intended ABI is for the second parameter on > non-Linux ELF systems which do not provide riscv_hwprobe as a system call. > Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic > linkers expected to provide some emulation of riscv_hwprobe of some variable > fidelity? In the latter case, what features of riscv_hwprobe must be emulated > for the dynamic linker to claim ABI conformance? NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so code that uses hwprobe (either inside or outside an ifunc selector) won't by default be portable across OSes. If Linux is consistently good at defining the hwprobe bits and not breaking our own ABI, other OSes could in theory emulate the interface by exposing the same keys/values. Though at least from our perspective that's not a goal. -Evan > > -s
On Tue, Jan 9, 2024, at 12:10 PM, Evan Green wrote: > On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> wrote: >> >> On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: >> > Any last thoughts on this series? I think the plan is to land it shortly. >> >> I am still wondering what the intended ABI is for the second parameter on >> non-Linux ELF systems which do not provide riscv_hwprobe as a system call. >> Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic >> linkers expected to provide some emulation of riscv_hwprobe of some variable >> fidelity? In the latter case, what features of riscv_hwprobe must be emulated >> for the dynamic linker to claim ABI conformance? > > NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so > code that uses hwprobe (either inside or outside an ifunc selector) > won't by default be portable across OSes. If Linux is consistently > good at defining the hwprobe bits and not breaking our own ABI, other > OSes could in theory emulate the interface by exposing the same > keys/values. Though at least from our perspective that's not a goal. > > -Evan NULL was a mistake; we need to pass a non-NULL value in a1 to signal that a2 is defined, since the current implementations pass NULL in a1 and garbage in a2. If a dynamic linker does not provide a Linux-compatible riscv_hwprobe but does support features that are passed in a2..a7, would it be better to pass (long(*)())(-1) or a stub function that just returns 38 (+ENOSYS)? -s
On Tue, Jan 9, 2024 at 10:30 AM Stefan O'Rear <sorear@fastmail.com> wrote: > > On Tue, Jan 9, 2024, at 12:10 PM, Evan Green wrote: > > On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> wrote: > >> > >> On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: > >> > Any last thoughts on this series? I think the plan is to land it shortly. > >> > >> I am still wondering what the intended ABI is for the second parameter on > >> non-Linux ELF systems which do not provide riscv_hwprobe as a system call. > >> Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic > >> linkers expected to provide some emulation of riscv_hwprobe of some variable > >> fidelity? In the latter case, what features of riscv_hwprobe must be emulated > >> for the dynamic linker to claim ABI conformance? > > > > NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so > > code that uses hwprobe (either inside or outside an ifunc selector) > > won't by default be portable across OSes. If Linux is consistently > > good at defining the hwprobe bits and not breaking our own ABI, other > > OSes could in theory emulate the interface by exposing the same > > keys/values. Though at least from our perspective that's not a goal. > > > > -Evan > > NULL was a mistake; we need to pass a non-NULL value in a1 to signal that > a2 is defined, since the current implementations pass NULL in a1 and > garbage in a2. > > If a dynamic linker does not provide a Linux-compatible riscv_hwprobe but > does support features that are passed in a2..a7, would it be better to pass > (long(*)())(-1) or a stub function that just returns 38 (+ENOSYS)? Oh great point, I hadn't connected those dots. I'm fine with either. -1 would let them distinguish this case a little more explicitly, so maybe that's better? Is there a good spot to document this? Should I defend against that -1 value in my static helper function as well by checking for it? It seems like I shouldn't need to since it's in a Linux-specific header, but if there's a scenario for it then I'll add it. -Evan > > -s
On Tue, Jan 9, 2024, at 1:41 PM, Evan Green wrote: > On Tue, Jan 9, 2024 at 10:30 AM Stefan O'Rear <sorear@fastmail.com> wrote: >> >> On Tue, Jan 9, 2024, at 12:10 PM, Evan Green wrote: >> > On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> wrote: >> >> >> >> On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: >> >> > Any last thoughts on this series? I think the plan is to land it shortly. >> >> >> >> I am still wondering what the intended ABI is for the second parameter on >> >> non-Linux ELF systems which do not provide riscv_hwprobe as a system call. >> >> Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic >> >> linkers expected to provide some emulation of riscv_hwprobe of some variable >> >> fidelity? In the latter case, what features of riscv_hwprobe must be emulated >> >> for the dynamic linker to claim ABI conformance? >> > >> > NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so >> > code that uses hwprobe (either inside or outside an ifunc selector) >> > won't by default be portable across OSes. If Linux is consistently >> > good at defining the hwprobe bits and not breaking our own ABI, other >> > OSes could in theory emulate the interface by exposing the same >> > keys/values. Though at least from our perspective that's not a goal. >> > >> > -Evan >> >> NULL was a mistake; we need to pass a non-NULL value in a1 to signal that >> a2 is defined, since the current implementations pass NULL in a1 and >> garbage in a2. >> >> If a dynamic linker does not provide a Linux-compatible riscv_hwprobe but >> does support features that are passed in a2..a7, would it be better to pass >> (long(*)())(-1) or a stub function that just returns 38 (+ENOSYS)? > > Oh great point, I hadn't connected those dots. I'm fine with either. > -1 would let them distinguish this case a little more explicitly, so > maybe that's better? +1 might be a better choice (match SIG_IGN rather than MAP_FAILED, already a non-function in the uABI, and slightly fewer bytes for the branch). The stub function has a disadvantage of polluting the global ABI with Linux-specific error constants. No strong feelings here. > Is there a good spot to document this? The details of the IRELATIVE resolution process should probably go in riscv-elf-psabi-doc/riscv-elf.adoc . > Should I defend against that -1 value in my static helper function as > well by checking for it? It seems like I shouldn't need to since it's > in a Linux-specific header, but if there's a scenario for it then I'll > add it. > -Evan Do you mean select_memcpy_ifunc in patch 7? I'm inclined to say that it should be robust in case someone copies it into a library other than glibc and it can no longer depend on Linux and a specific version of the glibc dynamic linker. -s
On Tue, Jan 9, 2024, 11:10 Stefan O'Rear <sorear@fastmail.com> wrote: > On Tue, Jan 9, 2024, at 1:41 PM, Evan Green wrote: > > On Tue, Jan 9, 2024 at 10:30 AM Stefan O'Rear <sorear@fastmail.com> > wrote: > >> > >> On Tue, Jan 9, 2024, at 12:10 PM, Evan Green wrote: > >> > On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> > wrote: > >> >> > >> >> On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: > >> >> > Any last thoughts on this series? I think the plan is to land it > shortly. > >> >> > >> >> I am still wondering what the intended ABI is for the second > parameter on > >> >> non-Linux ELF systems which do not provide riscv_hwprobe as a system > call. > >> >> Should it be passed as a null (or 1/-1) pointer in that case, or are > dynamic > >> >> linkers expected to provide some emulation of riscv_hwprobe of some > variable > >> >> fidelity? In the latter case, what features of riscv_hwprobe must > be emulated > >> >> for the dynamic linker to claim ABI conformance? > >> > > >> > NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so > >> > code that uses hwprobe (either inside or outside an ifunc selector) > >> > won't by default be portable across OSes. If Linux is consistently > >> > good at defining the hwprobe bits and not breaking our own ABI, other > >> > OSes could in theory emulate the interface by exposing the same > >> > keys/values. Though at least from our perspective that's not a goal. > >> > > >> > -Evan > >> > >> NULL was a mistake; we need to pass a non-NULL value in a1 to signal > that > >> a2 is defined, since the current implementations pass NULL in a1 and > >> garbage in a2. > >> > >> If a dynamic linker does not provide a Linux-compatible riscv_hwprobe > but > >> does support features that are passed in a2..a7, would it be better to > pass > >> (long(*)())(-1) or a stub function that just returns 38 (+ENOSYS)? > > > > Oh great point, I hadn't connected those dots. I'm fine with either. > > -1 would let them distinguish this case a little more explicitly, so > > maybe that's better? > > +1 might be a better choice (match SIG_IGN rather than MAP_FAILED, already > a non-function in the uABI, and slightly fewer bytes for the branch). > > The stub function has a disadvantage of polluting the global ABI with > Linux-specific error constants. > (and i'd still argue that no-one really needs it and, worse, folks who think they need it are probably pessimizing their code.) No strong feelings here. > > > Is there a good spot to document this? > > The details of the IRELATIVE resolution process should probably go in > riscv-elf-psabi-doc/riscv-elf.adoc . > > > Should I defend against that -1 value in my static helper function as > > well by checking for it? It seems like I shouldn't need to since it's > > in a Linux-specific header, but if there's a scenario for it then I'll > > add it. > > -Evan > > Do you mean select_memcpy_ifunc in patch 7? I'm inclined to say that it > should be robust in case someone copies it into a library other than glibc > and it can no longer depend on Linux and a specific version of the glibc > dynamic linker. > > -s >
On Tue, Jan 9, 2024 at 11:10 AM Stefan O'Rear <sorear@fastmail.com> wrote: > > On Tue, Jan 9, 2024, at 1:41 PM, Evan Green wrote: > > On Tue, Jan 9, 2024 at 10:30 AM Stefan O'Rear <sorear@fastmail.com> wrote: > >> > >> On Tue, Jan 9, 2024, at 12:10 PM, Evan Green wrote: > >> > On Tue, Jan 9, 2024 at 4:06 AM Stefan O'Rear <sorear@fastmail.com> wrote: > >> >> > >> >> On Mon, Jan 8, 2024, at 12:06 PM, Evan Green wrote: > >> >> > Any last thoughts on this series? I think the plan is to land it shortly. > >> >> > >> >> I am still wondering what the intended ABI is for the second parameter on > >> >> non-Linux ELF systems which do not provide riscv_hwprobe as a system call. > >> >> Should it be passed as a null (or 1/-1) pointer in that case, or are dynamic > >> >> linkers expected to provide some emulation of riscv_hwprobe of some variable > >> >> fidelity? In the latter case, what features of riscv_hwprobe must be emulated > >> >> for the dynamic linker to claim ABI conformance? > >> > > >> > NULL would make the most sense IMO. Hwprobe is a Linuxism for sure, so > >> > code that uses hwprobe (either inside or outside an ifunc selector) > >> > won't by default be portable across OSes. If Linux is consistently > >> > good at defining the hwprobe bits and not breaking our own ABI, other > >> > OSes could in theory emulate the interface by exposing the same > >> > keys/values. Though at least from our perspective that's not a goal. > >> > > >> > -Evan > >> > >> NULL was a mistake; we need to pass a non-NULL value in a1 to signal that > >> a2 is defined, since the current implementations pass NULL in a1 and > >> garbage in a2. > >> > >> If a dynamic linker does not provide a Linux-compatible riscv_hwprobe but > >> does support features that are passed in a2..a7, would it be better to pass > >> (long(*)())(-1) or a stub function that just returns 38 (+ENOSYS)? > > > > Oh great point, I hadn't connected those dots. I'm fine with either. > > -1 would let them distinguish this case a little more explicitly, so > > maybe that's better? > > +1 might be a better choice (match SIG_IGN rather than MAP_FAILED, already > a non-function in the uABI, and slightly fewer bytes for the branch). > > The stub function has a disadvantage of polluting the global ABI with > Linux-specific error constants. > > No strong feelings here. Sure, the arguments here make sense to me, so +1 seems fine. > > > Is there a good spot to document this? > > The details of the IRELATIVE resolution process should probably go in > riscv-elf-psabi-doc/riscv-elf.adoc . > > > Should I defend against that -1 value in my static helper function as > > well by checking for it? It seems like I shouldn't need to since it's > > in a Linux-specific header, but if there's a scenario for it then I'll > > add it. > > -Evan > > Do you mean select_memcpy_ifunc in patch 7? I'm inclined to say that it > should be robust in case someone copies it into a library other than glibc > and it can no longer depend on Linux and a specific version of the glibc > dynamic linker. Specifically I meant the __riscv_hwprobe_one() static inline helper in patch 6, which is what's used by patch 7. Yeah, as far as I can tell the only way to run into trouble is copying that function over to a substantially different world. Maybe a comment would be sufficient then? -Evan