On 20/09/2023 15:49, Peter Lafreniere wrote:
On Wed, Sep 20, 2023 at 08:31, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov anton.iva...@cambridgegreys.com


Preemption requires saving/restoring FPU state. This patch
adds support for it using GCC intrinsics.

Signed-off-by: Anton Ivanov anton.iva...@cambridgegreys.com

---
arch/um/Kconfig | 1 -
arch/um/Makefile | 3 +-
arch/um/include/asm/fpu/api.h | 4 +-
arch/um/include/asm/processor-generic.h | 1 +
arch/um/kernel/Makefile | 2 +-
arch/um/kernel/fpu.c | 49 +++++++++++++++++++++++++
6 files changed, 55 insertions(+), 5 deletions(-)
create mode 100644 arch/um/kernel/fpu.c

diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index b5e179360534..603f5fd82293 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -11,7 +11,6 @@ config UML
select ARCH_HAS_KCOV
select ARCH_HAS_STRNCPY_FROM_USER
select ARCH_HAS_STRNLEN_USER
- select ARCH_NO_PREEMPT
select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_KASAN if X86_64
select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN
diff --git a/arch/um/Makefile b/arch/um/Makefile
index 82f05f250634..35a059baadeb 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -61,7 +61,8 @@ KBUILD_CFLAGS += $(CFLAGS) $(CFLAGS-y) -D__arch_um__ \
$(ARCH_INCLUDE) $(MODE_INCLUDE) -Dvmap=kernel_vmap \
-Dlongjmp=kernel_longjmp -Dsetjmp=kernel_setjmp \
-Din6addr_loopback=kernel_in6addr_loopback \
- -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr
+ -Din6addr_any=kernel_in6addr_any -Dstrrchr=kernel_strrchr \
+ -mxsave
Since we're now using a new isa extention, I would add a check in
linux_main() with a warning message if xsave is not available.

It also wouldn't hurt to only require xsave if CONFIG_PREEMPT is enabled.

Indeed. The full area ends up being quite large - half a page or thereabouts.


KBUILD_RUSTFLAGS += -Crelocation-model=pie

diff --git a/arch/um/include/asm/fpu/api.h b/arch/um/include/asm/fpu/api.h
index 71bfd9ef3938..0094624ae9b4 100644
--- a/arch/um/include/asm/fpu/api.h
+++ b/arch/um/include/asm/fpu/api.h
@@ -8,8 +8,8 @@
* of x86 optimized copy, xor, etc routines into the
* UML code tree. /

-#define kernel_fpu_begin() (void)0
-#define kernel_fpu_end() (void)0
+void kernel_fpu_begin(void);
+void kernel_fpu_end(void);

static inline bool irq_fpu_usable(void)
{
diff --git a/arch/um/include/asm/processor-generic.h 
b/arch/um/include/asm/processor-generic.h
index 7414154b8e9a..1c3287f1a444 100644
--- a/arch/um/include/asm/processor-generic.h
+++ b/arch/um/include/asm/processor-generic.h
@@ -44,6 +44,7 @@ struct thread_struct {
} cb;
} u;
} request;
+ u8 fpu[512] __aligned(16); / Intel docs require xsave/xrestore area to be 
aligned to 16 bytes /
};

#define INIT_THREAD \
diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
index 811188be954c..5d9fbaa544be 100644
--- a/arch/um/kernel/Makefile
+++ b/arch/um/kernel/Makefile
@@ -16,7 +16,7 @@ extra-y := vmlinux.lds

obj-y = config.o exec.o exitcode.o irq.o ksyms.o mem.o \
physmem.o process.o ptrace.o reboot.o sigio.o \
- signal.o sysrq.o time.o tlb.o trap.o \
+ signal.o sysrq.o time.o tlb.o trap.o fpu.o\
um_arch.o umid.o maccess.o kmsg_dump.o capflags.o skas/
obj-y += load_file.o

diff --git a/arch/um/kernel/fpu.c b/arch/um/kernel/fpu.c
new file mode 100644
index 000000000000..4142a08c474d
--- /dev/null
+++ b/arch/um/kernel/fpu.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/
+ * Copyright (C) 2023 Cambridge Greys Ltd
+ * Copyright (C) 2023 Red Hat Inc
+ */
+
+#include <linux/cpu.h>

+#include <linux/init.h>

+#include <asm/fpu/api.h>

+
+/*
+ * The critical section between kernel_fpu_begin() and kernel_fpu_end()
+ * is non-reentrant. It is the caller's responsibility to avoid reentrance.
+ */
+
+static DEFINE_PER_CPU(bool, in_kernel_fpu);
+
+void kernel_fpu_begin(void)
+{
+ preempt_disable();
+
+ WARN_ON(this_cpu_read(in_kernel_fpu));
+
+ this_cpu_write(in_kernel_fpu, true);
+
+#ifdef CONFIG_64BIT
+ __builtin_ia32_fxsave64(&current->thread.fpu);

+#else
+ __builtin_ia32_fxsave(&current->thread.fpu);

+#endif
+}
+
+EXPORT_SYMBOL_GPL(kernel_fpu_begin);
+
+void kernel_fpu_end(void)
+{
+ WARN_ON(!this_cpu_read(in_kernel_fpu));
+
+#ifdef CONFIG_64BIT
+ __builtin_ia32_fxrstor64(&current->thread.fpu);

+#else
+ __builtin_ia32_fxrstor(&current->thread.fpu);

+#endif
+ this_cpu_write(in_kernel_fpu, false);
+
+ preempt_enable();
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_end);
+
According to https://www.felixcloutier.com/x86/fxsave, the fxsave and fxrstor
instructions do not save ymm, zmm, or mask registers. Please make that explicit
in your comment, or replace intrinsics with inline assembler to support those
registers.

Several x86 crypto routines use these larger registers which could overwrite
userspace registers. These modules aren't built by default, and may not be
available on UML. Even so, this issue is worth a comment, at the least.

On my system, lscpu within uml shows all the feature flags that are available
on the host system, which includes AVX extentions.

That should be xsave, not fxsave upgrading to xsaveopt if available.


Cheers,
Peter Lafreniere


--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Reply via email to