[PATCH v4] um: Enable preemption in UML

[PATCH v4] um: Enable preemption in UML

[anton.ivanov]

From: Anton Ivanov <anton.ivanov@cambridgegreys.com>

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

Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>
---
 arch/um/Kconfig                         |  1 -
 arch/um/include/asm/fpu/api.h           |  9 ++-
 arch/um/include/asm/processor-generic.h |  3 +
 arch/um/kernel/Makefile                 |  4 ++
 arch/um/kernel/fpu.c                    | 77 +++++++++++++++++++++++++
 arch/um/kernel/irq.c                    |  2 +
 6 files changed, 92 insertions(+), 4 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/include/asm/fpu/api.h b/arch/um/include/asm/fpu/api.h
index 71bfd9ef3938..9e7680bf48f0 100644
--- a/arch/um/include/asm/fpu/api.h
+++ b/arch/um/include/asm/fpu/api.h
@@ -4,12 +4,15 @@
 
 /* Copyright (c) 2020 Cambridge Greys Ltd
  * Copyright (c) 2020 Red Hat Inc.
- * A set of "dummy" defines to allow the direct inclusion
- * of x86 optimized copy, xor, etc routines into the
- * UML code tree. */
+ */
 
+#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
+extern void kernel_fpu_begin(void);
+extern void kernel_fpu_end(void);
+#else
 #define kernel_fpu_begin() (void)0
 #define kernel_fpu_end() (void)0
+#endif
 
 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..c08e8c5b0249 100644
--- a/arch/um/include/asm/processor-generic.h
+++ b/arch/um/include/asm/processor-generic.h
@@ -44,6 +44,9 @@ struct thread_struct {
 			} cb;
 		} u;
 	} request;
+#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
+    u8 fpu[2048] __aligned(64); /* Intel docs require xsave/xrestore area to be aligned to 64 bytes */
+#endif
 };
 
 #define INIT_THREAD \
diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
index 811188be954c..c616e884a488 100644
--- a/arch/um/kernel/Makefile
+++ b/arch/um/kernel/Makefile
@@ -26,9 +26,13 @@ obj-$(CONFIG_OF) += dtb.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-$(CONFIG_STACKTRACE) += stacktrace.o
 obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o
+obj-$(CONFIG_PREEMPT) += fpu.o
+obj-$(CONFIG_PREEMPT_VOLUNTARY) += fpu.o
 
 USER_OBJS := config.o
 
+CFLAGS_fpu.o += -mxsave -mxsaveopt
+
 include $(srctree)/arch/um/scripts/Makefile.rules
 
 targets := config.c config.tmp capflags.c
diff --git a/arch/um/kernel/fpu.c b/arch/um/kernel/fpu.c
new file mode 100644
index 000000000000..346c07236185
--- /dev/null
+++ b/arch/um/kernel/fpu.c
@@ -0,0 +1,77 @@
+// 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>
+#include <asm/cpufeature.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);
+
+/* UML and driver code it pulls out of the x86 tree knows about 387 features
+ * up to and including AVX512. TILE, etc are not yet supported.
+ */
+
+#define KNOWN_387_FEATURES 0xFF
+
+
+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
+	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
+		__builtin_ia32_xsaveopt64(&current->thread.fpu, KNOWN_387_FEATURES);
+	else {
+		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
+			__builtin_ia32_xsave64(&current->thread.fpu, KNOWN_387_FEATURES);
+		else
+			__builtin_ia32_fxsave64(&current->thread.fpu);
+	}
+#else
+	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
+		__builtin_ia32_xsaveopt(&current->thread.fpu, KNOWN_387_FEATURES);
+	else {
+		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
+			__builtin_ia32_xsave(&current->thread.fpu, KNOWN_387_FEATURES);
+		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
+	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
+		__builtin_ia32_xrstor64(&current->thread.fpu, KNOWN_387_FEATURES);
+	else
+		__builtin_ia32_fxrstor64(&current->thread.fpu);
+#else
+	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
+		__builtin_ia32_xrstor(&current->thread.fpu, KNOWN_387_FEATURES);
+	else
+		__builtin_ia32_fxrstor(&current->thread.fpu);
+#endif
+	this_cpu_write(in_kernel_fpu, false);
+
+	preempt_enable();
+}
+EXPORT_SYMBOL_GPL(kernel_fpu_end);
+
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index 635d44606bfe..c02525da45df 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -195,7 +195,9 @@ static void _sigio_handler(struct uml_pt_regs *regs,
 
 void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
 {
+	preempt_disable();
 	_sigio_handler(regs, irqs_suspended);
+	preempt_enable();
 }
 
 static struct irq_entry *get_irq_entry_by_fd(int fd)
-- 
2.30.2


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

Re: [PATCH v4] um: Enable preemption in UML

[Anton Ivanov]

On 21/09/2023 16:55, anton.ivanov@cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
> 
> Preemption requires saving/restoring FPU state. This patch
> adds support for it using GCC intrinsics.
> 
> Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>
> ---
>   arch/um/Kconfig                         |  1 -
>   arch/um/include/asm/fpu/api.h           |  9 ++-
>   arch/um/include/asm/processor-generic.h |  3 +
>   arch/um/kernel/Makefile                 |  4 ++
>   arch/um/kernel/fpu.c                    | 77 +++++++++++++++++++++++++
>   arch/um/kernel/irq.c                    |  2 +
>   6 files changed, 92 insertions(+), 4 deletions(-)
>   create mode 100644 arch/um/kernel/fpu.c

Cleanup of typos and stale comments from older versions.

> 
> 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/include/asm/fpu/api.h b/arch/um/include/asm/fpu/api.h
> index 71bfd9ef3938..9e7680bf48f0 100644
> --- a/arch/um/include/asm/fpu/api.h
> +++ b/arch/um/include/asm/fpu/api.h
> @@ -4,12 +4,15 @@
>   
>   /* Copyright (c) 2020 Cambridge Greys Ltd
>    * Copyright (c) 2020 Red Hat Inc.
> - * A set of "dummy" defines to allow the direct inclusion
> - * of x86 optimized copy, xor, etc routines into the
> - * UML code tree. */
> + */
>   
> +#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
> +extern void kernel_fpu_begin(void);
> +extern void kernel_fpu_end(void);
> +#else
>   #define kernel_fpu_begin() (void)0
>   #define kernel_fpu_end() (void)0
> +#endif
>   
>   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..c08e8c5b0249 100644
> --- a/arch/um/include/asm/processor-generic.h
> +++ b/arch/um/include/asm/processor-generic.h
> @@ -44,6 +44,9 @@ struct thread_struct {
>   			} cb;
>   		} u;
>   	} request;
> +#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
> +    u8 fpu[2048] __aligned(64); /* Intel docs require xsave/xrestore area to be aligned to 64 bytes */
> +#endif
>   };
>   
>   #define INIT_THREAD \
> diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
> index 811188be954c..c616e884a488 100644
> --- a/arch/um/kernel/Makefile
> +++ b/arch/um/kernel/Makefile
> @@ -26,9 +26,13 @@ obj-$(CONFIG_OF) += dtb.o
>   obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>   obj-$(CONFIG_STACKTRACE) += stacktrace.o
>   obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o
> +obj-$(CONFIG_PREEMPT) += fpu.o
> +obj-$(CONFIG_PREEMPT_VOLUNTARY) += fpu.o
>   
>   USER_OBJS := config.o
>   
> +CFLAGS_fpu.o += -mxsave -mxsaveopt
> +
>   include $(srctree)/arch/um/scripts/Makefile.rules
>   
>   targets := config.c config.tmp capflags.c
> diff --git a/arch/um/kernel/fpu.c b/arch/um/kernel/fpu.c
> new file mode 100644
> index 000000000000..346c07236185
> --- /dev/null
> +++ b/arch/um/kernel/fpu.c
> @@ -0,0 +1,77 @@
> +// 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>
> +#include <asm/cpufeature.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);
> +
> +/* UML and driver code it pulls out of the x86 tree knows about 387 features
> + * up to and including AVX512. TILE, etc are not yet supported.
> + */
> +
> +#define KNOWN_387_FEATURES 0xFF
> +
> +
> +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
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> +		__builtin_ia32_xsaveopt64(&current->thread.fpu, KNOWN_387_FEATURES);
> +	else {
> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +			__builtin_ia32_xsave64(&current->thread.fpu, KNOWN_387_FEATURES);
> +		else
> +			__builtin_ia32_fxsave64(&current->thread.fpu);
> +	}
> +#else
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> +		__builtin_ia32_xsaveopt(&current->thread.fpu, KNOWN_387_FEATURES);
> +	else {
> +		if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +			__builtin_ia32_xsave(&current->thread.fpu, KNOWN_387_FEATURES);
> +		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
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +		__builtin_ia32_xrstor64(&current->thread.fpu, KNOWN_387_FEATURES);
> +	else
> +		__builtin_ia32_fxrstor64(&current->thread.fpu);
> +#else
> +	if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> +		__builtin_ia32_xrstor(&current->thread.fpu, KNOWN_387_FEATURES);
> +	else
> +		__builtin_ia32_fxrstor(&current->thread.fpu);
> +#endif
> +	this_cpu_write(in_kernel_fpu, false);
> +
> +	preempt_enable();
> +}
> +EXPORT_SYMBOL_GPL(kernel_fpu_end);
> +
> diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
> index 635d44606bfe..c02525da45df 100644
> --- a/arch/um/kernel/irq.c
> +++ b/arch/um/kernel/irq.c
> @@ -195,7 +195,9 @@ static void _sigio_handler(struct uml_pt_regs *regs,
>   
>   void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
>   {
> +	preempt_disable();
>   	_sigio_handler(regs, irqs_suspended);
> +	preempt_enable();
>   }
>   
>   static struct irq_entry *get_irq_entry_by_fd(int fd)

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

Re: [PATCH v4] um: Enable preemption in UML

[Peter Lafreniere]

On Thu, Sep 21, 2023 at 11:55, anton.ivanov@cambridgegreys.com wrote:
> 
> From: Anton Ivanov anton.ivanov@cambridgegreys.com
> 
> 
> Preemption requires saving/restoring FPU state. This patch
> adds support for it using GCC intrinsics.
> 
> Signed-off-by: Anton Ivanov anton.ivanov@cambridgegreys.com
> 
> ---
> arch/um/Kconfig | 1 -
> arch/um/include/asm/fpu/api.h | 9 ++-
> arch/um/include/asm/processor-generic.h | 3 +
> arch/um/kernel/Makefile | 4 ++
> arch/um/kernel/fpu.c | 77 +++++++++++++++++++++++++
> arch/um/kernel/irq.c | 2 +
> 6 files changed, 92 insertions(+), 4 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/include/asm/fpu/api.h b/arch/um/include/asm/fpu/api.h
> index 71bfd9ef3938..9e7680bf48f0 100644
> --- a/arch/um/include/asm/fpu/api.h
> +++ b/arch/um/include/asm/fpu/api.h
> @@ -4,12 +4,15 @@
> 
> /* Copyright (c) 2020 Cambridge Greys Ltd
> * Copyright (c) 2020 Red Hat Inc.
> - * A set of "dummy" defines to allow the direct inclusion
> - * of x86 optimized copy, xor, etc routines into the
> - * UML code tree. */
> + /
> 
> +#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
> +extern void kernel_fpu_begin(void);
> +extern void kernel_fpu_end(void);
> +#else
> #define kernel_fpu_begin() (void)0
> #define kernel_fpu_end() (void)0
> +#endif
> 
> 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..c08e8c5b0249 100644
> --- a/arch/um/include/asm/processor-generic.h
> +++ b/arch/um/include/asm/processor-generic.h
> @@ -44,6 +44,9 @@ struct thread_struct {
> } cb;
> } u;
> } request;
> +#if defined(CONFIG_PREEMPT) || defined(CONFIG_PREEMPT_VOLUNTARY)
> + u8 fpu[2048] __aligned(64); / Intel docs require xsave/xrestore area to be aligned to 64 bytes /
> +#endif
> };
> 
> #define INIT_THREAD \
> diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
> index 811188be954c..c616e884a488 100644
> --- a/arch/um/kernel/Makefile
> +++ b/arch/um/kernel/Makefile
> @@ -26,9 +26,13 @@ obj-$(CONFIG_OF) += dtb.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> obj-$(CONFIG_STACKTRACE) += stacktrace.o
> obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o
> +obj-$(CONFIG_PREEMPT) += fpu.o
> +obj-$(CONFIG_PREEMPT_VOLUNTARY) += fpu.o
> 
> USER_OBJS := config.o
> 
> +CFLAGS_fpu.o += -mxsave -mxsaveopt
> +
> include $(srctree)/arch/um/scripts/Makefile.rules
> 
> targets := config.c config.tmp capflags.c
> diff --git a/arch/um/kernel/fpu.c b/arch/um/kernel/fpu.c
> new file mode 100644
> index 000000000000..346c07236185
> --- /dev/null
> +++ b/arch/um/kernel/fpu.c
> @@ -0,0 +1,77 @@
> +// 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>
> 
> +#include <asm/cpufeature.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);
> +
> +/ UML and driver code it pulls out of the x86 tree knows about 387 features
> + * up to and including AVX512. TILE, etc are not yet supported.
> + */

Tiny nit, but you use two different styles of block comments here.
You did the same with the copyright declarations.

> +
> +#define KNOWN_387_FEATURES 0xFF
> +
> +
> +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
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> + __builtin_ia32_xsaveopt64(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + else {
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> + __builtin_ia32_xsave64(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + else
> + __builtin_ia32_fxsave64(&current->thread.fpu);
> 
> + }
> +#else
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVEOPT)))
> + __builtin_ia32_xsaveopt(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + else {
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> + __builtin_ia32_xsave(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + 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
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> + __builtin_ia32_xrstor64(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + else
> + __builtin_ia32_fxrstor64(&current->thread.fpu);
> 
> +#else
> + if (likely(cpu_has(&boot_cpu_data, X86_FEATURE_XSAVE)))
> + __builtin_ia32_xrstor(&current->thread.fpu, KNOWN_387_FEATURES);
> 
> + else
> + __builtin_ia32_fxrstor(&current->thread.fpu);
> 
> +#endif
> + this_cpu_write(in_kernel_fpu, false);
> +
> + preempt_enable();
> +}
> +EXPORT_SYMBOL_GPL(kernel_fpu_end);
> +
> diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
> index 635d44606bfe..c02525da45df 100644
> --- a/arch/um/kernel/irq.c
> +++ b/arch/um/kernel/irq.c
> @@ -195,7 +195,9 @@ static void _sigio_handler(struct uml_pt_regs *regs,
> 
> void sigio_handler(int sig, struct siginfo *unused_si, struct uml_pt_regs *regs)
> {
> + preempt_disable();
> _sigio_handler(regs, irqs_suspended);
> + preempt_enable();
> }
> 
> static struct irq_entry *get_irq_entry_by_fd(int fd)
> --
> 2.30.2

I'm happy with this as it is. The issue with AVX registers being clobbered
when xsave isn't supported is unlikely to matter in practice, and can
be fixed with a patch afterwards.

Reviewed-by: Peter Lafreniere <peter@n8pjl.ca>

Cheers,
Peter


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

Re: [PATCH v4] um: Enable preemption in UML

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
> An: "linux-um" <linux-um@lists.infradead.org>
> CC: "Johannes Berg" <johannes@sipsolutions.net>, "richard" <richard@nod.at>, "anton ivanov"
> <anton.ivanov@cambridgegreys.com>
> Gesendet: Donnerstag, 21. September 2023 17:55:22
> Betreff: [PATCH v4] um: Enable preemption in UML

> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
> 
> Preemption requires saving/restoring FPU state. This patch
> adds support for it using GCC intrinsics.

This patch triggers here the following splat:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at init/main.c:992 start_kernel+0x541/0x6b6
Interrupts were enabled early
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc2-ga74d8f8592e5 #128
Stack:
 60381b94 00000001 604d3d80 6002b6f6
 60444bdd 00000001 604d3e20 6037ecfa
 604d3db0 60389b2f 604d3da0 60381b94
Call Trace:
 [<60381b94>] ? _printk+0x0/0x94
 [<600212b1>] show_stack+0x13c/0x14b
 [<60381b94>] ? _printk+0x0/0x94
 [<6002b6f6>] ? um_set_signals+0x0/0x3f
 [<6037ecfa>] ? sprintf+0x0/0x95
 [<60389b2f>] dump_stack_lvl+0x4d/0x5a
 [<60381b94>] ? _printk+0x0/0x94
 [<60389b56>] dump_stack+0x1a/0x1c
 [<60033ebc>] __warn+0xd9/0x109
 [<60033fbd>] warn_slowpath_fmt+0xd1/0xdf
 [<60033eec>] ? warn_slowpath_fmt+0x0/0xdf
 [<60381b94>] ? _printk+0x0/0x94
 [<60050e2c>] ? __blocking_notifier_chain_register+0x33/0x78
 [<60381b94>] ? _printk+0x0/0x94
 [<60050e83>] ? blocking_notifier_chain_register+0x12/0x14
 [<6005fa92>] ? register_pm_notifier+0x1d/0x1f
 [<600152c9>] ? random_init+0xc2/0x126
 [<60381b94>] ? _printk+0x0/0x94
 [<60001c40>] start_kernel+0x541/0x6b6
 [<6004deb0>] ? parse_args+0x0/0x2b9
 [<6002b609>] ? block_signals+0x0/0x16
 [<60003b9d>] start_kernel_proc+0x49/0x4d
 [<60061863>] ? kmsg_dump_register+0x6d/0x75
 [<6001ffdb>] new_thread_handler+0x81/0xb2
 [<60003b52>] ? kmsg_dumper_stdout_init+0x1a/0x1c
 [<60022e52>] uml_finishsetup+0x54/0x59
---[ end trace 0000000000000000 ]---

x86_64 defconfig + CONFIG_PREEMPT_VOLUNTARY=y

Thanks,
//richard

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

Re: [PATCH v4] um: Enable preemption in UML

[Anton Ivanov]

On 21/09/2023 22:02, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
>> An: "linux-um" <linux-um@lists.infradead.org>
>> CC: "Johannes Berg" <johannes@sipsolutions.net>, "richard" <richard@nod.at>, "anton ivanov"
>> <anton.ivanov@cambridgegreys.com>
>> Gesendet: Donnerstag, 21. September 2023 17:55:22
>> Betreff: [PATCH v4] um: Enable preemption in UML
>> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
>>
>> Preemption requires saving/restoring FPU state. This patch
>> adds support for it using GCC intrinsics.
> This patch triggers here the following splat:

Ack. I will look into it tomorrow.

There will be places where it will bomb out. F.E. I am chasing one is 
triggered by the NFS server (it may be the same as in this trace - it 
happens around the end of the initial setup).

All in all it looks better than I expected.

defconfig + vector + nfs client  + CONFIG_PREEMPT - I had the kernel 
build without any issues from a source tree on NFS and with reasonable 
performance too.

> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at init/main.c:992 start_kernel+0x541/0x6b6
> Interrupts were enabled early
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-rc2-ga74d8f8592e5 #128
> Stack:
>   60381b94 00000001 604d3d80 6002b6f6
>   60444bdd 00000001 604d3e20 6037ecfa
>   604d3db0 60389b2f 604d3da0 60381b94
> Call Trace:
>   [<60381b94>] ? _printk+0x0/0x94
>   [<600212b1>] show_stack+0x13c/0x14b
>   [<60381b94>] ? _printk+0x0/0x94
>   [<6002b6f6>] ? um_set_signals+0x0/0x3f
>   [<6037ecfa>] ? sprintf+0x0/0x95
>   [<60389b2f>] dump_stack_lvl+0x4d/0x5a
>   [<60381b94>] ? _printk+0x0/0x94
>   [<60389b56>] dump_stack+0x1a/0x1c
>   [<60033ebc>] __warn+0xd9/0x109
>   [<60033fbd>] warn_slowpath_fmt+0xd1/0xdf
>   [<60033eec>] ? warn_slowpath_fmt+0x0/0xdf
>   [<60381b94>] ? _printk+0x0/0x94
>   [<60050e2c>] ? __blocking_notifier_chain_register+0x33/0x78
>   [<60381b94>] ? _printk+0x0/0x94
>   [<60050e83>] ? blocking_notifier_chain_register+0x12/0x14
>   [<6005fa92>] ? register_pm_notifier+0x1d/0x1f
>   [<600152c9>] ? random_init+0xc2/0x126
>   [<60381b94>] ? _printk+0x0/0x94
>   [<60001c40>] start_kernel+0x541/0x6b6
>   [<6004deb0>] ? parse_args+0x0/0x2b9
>   [<6002b609>] ? block_signals+0x0/0x16
>   [<60003b9d>] start_kernel_proc+0x49/0x4d
>   [<60061863>] ? kmsg_dump_register+0x6d/0x75
>   [<6001ffdb>] new_thread_handler+0x81/0xb2
>   [<60003b52>] ? kmsg_dumper_stdout_init+0x1a/0x1c
>   [<60022e52>] uml_finishsetup+0x54/0x59
> ---[ end trace 0000000000000000 ]---
>
> x86_64 defconfig + CONFIG_PREEMPT_VOLUNTARY=y
>
> Thanks,
> //richard
>

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

Re: [PATCH v4] um: Enable preemption in UML

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
> An: "richard" <richard@nod.at>
> CC: "linux-um" <linux-um@lists.infradead.org>, "Johannes Berg" <johannes@sipsolutions.net>
> Gesendet: Donnerstag, 21. September 2023 23:41:16
> Betreff: Re: [PATCH v4] um: Enable preemption in UML

> On 21/09/2023 22:02, Richard Weinberger wrote:
>> ----- Ursprüngliche Mail -----
>>> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
>>> An: "linux-um" <linux-um@lists.infradead.org>
>>> CC: "Johannes Berg" <johannes@sipsolutions.net>, "richard" <richard@nod.at>,
>>> "anton ivanov"
>>> <anton.ivanov@cambridgegreys.com>
>>> Gesendet: Donnerstag, 21. September 2023 17:55:22
>>> Betreff: [PATCH v4] um: Enable preemption in UML
>>> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
>>>
>>> Preemption requires saving/restoring FPU state. This patch
>>> adds support for it using GCC intrinsics.
>> This patch triggers here the following splat:
> 
> Ack. I will look into it tomorrow.

Thx!
I had a brief look, UML's init_IRQ() seems to return now with interrupts enabled.
Dunno why, I need some sleep now.

> 
> There will be places where it will bomb out. F.E. I am chasing one is
> triggered by the NFS server (it may be the same as in this trace - it
> happens around the end of the initial setup).
> 
> All in all it looks better than I expected.

True that! :-)

Thanks,
//richard

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

Re: [PATCH v4] um: Enable preemption in UML

[Anton Ivanov]

On 21/09/2023 22:55, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
>> An: "richard" <richard@nod.at>
>> CC: "linux-um" <linux-um@lists.infradead.org>, "Johannes Berg" <johannes@sipsolutions.net>
>> Gesendet: Donnerstag, 21. September 2023 23:41:16
>> Betreff: Re: [PATCH v4] um: Enable preemption in UML
>> On 21/09/2023 22:02, Richard Weinberger wrote:
>>> ----- Ursprüngliche Mail -----
>>>> Von: "anton ivanov" <anton.ivanov@cambridgegreys.com>
>>>> An: "linux-um" <linux-um@lists.infradead.org>
>>>> CC: "Johannes Berg" <johannes@sipsolutions.net>, "richard" <richard@nod.at>,
>>>> "anton ivanov"
>>>> <anton.ivanov@cambridgegreys.com>
>>>> Gesendet: Donnerstag, 21. September 2023 17:55:22
>>>> Betreff: [PATCH v4] um: Enable preemption in UML
>>>> From: Anton Ivanov <anton.ivanov@cambridgegreys.com>
>>>>
>>>> Preemption requires saving/restoring FPU state. This patch
>>>> adds support for it using GCC intrinsics.
>>> This patch triggers here the following splat:
>> Ack. I will look into it tomorrow.
> Thx!
> I had a brief look, UML's init_IRQ() seems to return now with interrupts enabled.
> Dunno why, I need some sleep now.

Tlb. Looking at other architectures (and specifically x86) it needs preempt_disable/enable around critical sections.

 From there you may end up in the wrong state across the board.

>
>> There will be places where it will bomb out. F.E. I am chasing one is
>> triggered by the NFS server (it may be the same as in this trace - it
>> happens around the end of the initial setup).
>>
>> All in all it looks better than I expected.
> True that! :-)
>
> Thanks,
> //richard
>
> _______________________________________________
> linux-um mailing list
> linux-um@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um

-- 
Anton R. Ivanov
https://www.kot-begemot.co.uk/


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