Re: [musl] [PATCH 1/1] feat(x86_64): use wrfsbase if AT_HWCAP2 allows usage

[Rich Felker <dalias@libc.org>]

On Wed, Mar 30, 2022 at 08:57:55AM +0200, Harald Hoyer wrote:
> Am 29.03.22 um 17:54 schrieb Rich Felker:
> >On Tue, Mar 29, 2022 at 02:24:16PM +0200, Harald Hoyer wrote:
> >>If `AT_HWCAP2` has `HWCAP2_FSGSBASE` set, then instead of calling
> >>`arch_prctl()`, the `wrfsbase` instruction will be used.
> >>
> >>This is helpful in SGX contexts, where inside the enclave no other
> >>mechanism is possible.
> >
> >Thanks for including this motivation, since otherwise I don't think it
> >makes any sense to use this feature. BTW what happens with other
> >syscalls in such a context (at least set_tid_address is called
> >unconditionally), and how does the process communicate any information
> >or even exit?
> 
> In our project (enarx) we catch the `syscall` exception and handle it by proxying
> it to the host (with filtering and sanity checks). Because `arch_prctl` sets a
> hardware register, which is handled special in enclave context switching,
> we can't do that for this syscall.

Is there no way to make it jump the guest to a host-provided (ala
vdso) helper that would juse use the instruction?

> >>diff --git a/src/thread/x86_64/__set_thread_area.c b/src/thread/x86_64/__set_thread_area.c
> >>new file mode 100644
> >>index 00000000..dcc5d116
> >>--- /dev/null
> >>+++ b/src/thread/x86_64/__set_thread_area.c
> >>@@ -0,0 +1,14 @@
> >>+#include <libc.h>
> >>+#include <syscall.h>
> >>+#include <bits/hwcap.h>
> >>+
> >>+hidden int __set_thread_area(void *p)
> >>+{
> >>+	if (__hwcap2 & HWCAP2_FSGSBASE) {
> >>+		__asm__ ("wrfsbase %0" :: "r" (p) : "memory");
> >>+		return 0;
> >>+	}
> >>+
> >>+	// arch_prctl(SET_FS, arg)
> >>+	return syscall(__NR_arch_prctl, 0x1002, p);
> >>+}
> >
> >I'm guessing this breaks build on anything but recent assembler
> >versions, no? If so, it should probably be written with a .byte
> >directive or something.
> 
> Is gcc-4.6.0 old enough?
> 
> commit 4ee89d5fb78b48b62b507a29d3a576c63ae22505
> Author: H.J. Lu <hongjiu.lu@intel.com>
> Date:   Mon Jul 5 21:57:55 2010 +0000

It's a matter of binutils not gcc. And I'm not sure. Previously I
don't think we've had any minimum required binutils version for
x86_64, so my leaning would be to do this a way that does not create a
dependency on the assembler knowing new insns from a -march greater
than what you're compiling for.

Using .byte does preclude letting the compiler pick an input register,
but "D"(p) instead of "r"(p) should be zero-cost anyway since p is
always going to be in the first argument register anyway.

> or llvm 3.1.0?
> 
> commit 228d9131aad9f3553c9c69abf010f41ce43c8423
> Author: Craig Topper <craig.topper@gmail.com>
> Date:   Sun Oct 30 19:57:21 2011 +0000
> 
> >
> >There's also a question of whether the existence of the hwcap flag is
> >intended to document a contract for the kernel to permit the process
> >to perform this operation, and a contract for the kernel to accept it
> >being set this way (rather than creating a possible inconsistency
> >between the kernel's idea of the process's %fs base and the actual %fs
> >base that's active. Is this documented somewhere on the kernel side?
> >If so then this should be okay, but this needs checking before it can
> >be merged.
> >
> >Rich
> 
> linux Documentation/x86/x86_64/fsgs.rst:
> 
>  The kernel provides reliable information about the enabled state in the
>  ELF AUX vector. If the HWCAP2_FSGSBASE bit is set in the AUX vector, the
>  kernel has FSGSBASE instructions enabled and applications can use them.

Thanks! I just found that myself, along with:

https://lwn.net/Articles/821723/

which explains some of the history and motivation.

Short of any concern others may raise that I'm not seeing, I think
this change is almost surely okay with care taken not to bump the
assembler version requirements. I'll plan to take this shortly after
release.

Rich