diff mbox series

[V4,16/17] riscv: compat: Add COMPAT Kbuild skeletal support

Message ID 20220129121728.1079364-17-guoren@kernel.org (mailing list archive)
State Superseded
Headers show
Series riscv: compat: Add COMPAT mode support for rv64 | expand

Commit Message

Guo Ren Jan. 29, 2022, 12:17 p.m. UTC
From: Guo Ren <guoren@linux.alibaba.com>

Adds initial skeletal COMPAT Kbuild (Runing 32bit U-mode on 64bit
S-mode) support.
 - Setup kconfig & dummy functions for compiling.
 - Implement compat_start_thread by the way.

Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@kernel.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
---
 arch/riscv/Kconfig | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Christoph Hellwig Jan. 31, 2022, 12:26 p.m. UTC | #1
Given that most rv64 implementations can't run in rv32 mode, what is the
failure mode if someone tries it with the compat mode enabled?
Guo Ren Jan. 31, 2022, 1:50 p.m. UTC | #2
On Mon, Jan 31, 2022 at 8:26 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> Given that most rv64 implementations can't run in rv32 mode, what is the
> failure mode if someone tries it with the compat mode enabled?
A static linked simple hello_world could still run on a non-compat
support hardware. But most rv32 apps would meet different userspace
segment faults.

Current code would let the machine try the rv32 apps without detecting
whether hw support or not.
Christoph Hellwig Feb. 1, 2022, 7:44 a.m. UTC | #3
On Mon, Jan 31, 2022 at 09:50:58PM +0800, Guo Ren wrote:
> On Mon, Jan 31, 2022 at 8:26 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > Given that most rv64 implementations can't run in rv32 mode, what is the
> > failure mode if someone tries it with the compat mode enabled?
> A static linked simple hello_world could still run on a non-compat
> support hardware. But most rv32 apps would meet different userspace
> segment faults.
> 
> Current code would let the machine try the rv32 apps without detecting
> whether hw support or not.

Hmm, we probably want some kind of check for not even offer running
rv32 binaries.  I guess trying to write UXL some time during early
boot and catching the resulting exception would be the way to go?

> 
> 
> -- 
> Best Regards
>  Guo Ren
> 
> ML: https://lore.kernel.org/linux-csky/
---end quoted text---
Guo Ren Feb. 1, 2022, 9:13 a.m. UTC | #4
On Tue, Feb 1, 2022 at 3:45 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Jan 31, 2022 at 09:50:58PM +0800, Guo Ren wrote:
> > On Mon, Jan 31, 2022 at 8:26 PM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > Given that most rv64 implementations can't run in rv32 mode, what is the
> > > failure mode if someone tries it with the compat mode enabled?
> > A static linked simple hello_world could still run on a non-compat
> > support hardware. But most rv32 apps would meet different userspace
> > segment faults.
> >
> > Current code would let the machine try the rv32 apps without detecting
> > whether hw support or not.
>
> Hmm, we probably want some kind of check for not even offer running
> rv32 binaries.  I guess trying to write UXL some time during early
> boot and catching the resulting exception would be the way to go?

Emm... I think it's unnecessary. Free rv32 app running won't cause
system problem, just as a wrong elf running. They are U-mode
privileged.
>
> >
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> ---end quoted text---
Arnd Bergmann Feb. 1, 2022, 9:36 a.m. UTC | #5
On Tue, Feb 1, 2022 at 10:13 AM Guo Ren <guoren@kernel.org> wrote:
> On Tue, Feb 1, 2022 at 3:45 PM Christoph Hellwig <hch@lst.de> wrote:
> > On Mon, Jan 31, 2022 at 09:50:58PM +0800, Guo Ren wrote:
> > > On Mon, Jan 31, 2022 at 8:26 PM Christoph Hellwig <hch@infradead.org> wrote:
> > > >
> > > > Given that most rv64 implementations can't run in rv32 mode, what is the
> > > > failure mode if someone tries it with the compat mode enabled?
> > > A static linked simple hello_world could still run on a non-compat
> > > support hardware. But most rv32 apps would meet different userspace
> > > segment faults.
> > >
> > > Current code would let the machine try the rv32 apps without detecting
> > > whether hw support or not.
> >
> > Hmm, we probably want some kind of check for not even offer running
> > rv32 binaries.  I guess trying to write UXL some time during early
> > boot and catching the resulting exception would be the way to go?
>
> Emm... I think it's unnecessary. Free rv32 app running won't cause
> system problem, just as a wrong elf running. They are U-mode
> privileged.

While it's not a security issue, I think it would be helpful to get a
user-readable error message and a machine-readable /proc/cpuinfo
flag to see if a particular system can run rv32 binaries rather than
relying on SIGILL to kill a process.

        Arnd
Guo Ren Feb. 1, 2022, 10:26 a.m. UTC | #6
Hi Arnd & Christoph,

The UXL field controls the value of XLEN for U-mode, termed UXLEN,
which may differ from the
value of XLEN for S-mode, termed SXLEN. The encoding of UXL is the
same as that of the MXL
field of misa, shown in Table 3.1.

Here is the patch. (We needn't exception helper, because we are in
S-mode and UXL wouldn't affect.)

 arch/riscv/include/asm/elf.h       |  5 ++++-
 arch/riscv/include/asm/processor.h |  1 +
 arch/riscv/kernel/process.c        | 22 ++++++++++++++++++++++
 arch/riscv/kernel/setup.c          |  5 +++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index 37f1cbdaa242..6baa49c4fba1 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -35,7 +35,10 @@
  */
 #define elf_check_arch(x) ((x)->e_machine == EM_RISCV)

-#define compat_elf_check_arch(x) ((x)->e_machine == EM_RISCV)
+#ifdef CONFIG_COMPAT
+#define compat_elf_check_arch compat_elf_check_arch
+extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
+#endif

 #define CORE_DUMP_USE_REGSET
 #define ELF_EXEC_PAGESIZE (PAGE_SIZE)
diff --git a/arch/riscv/include/asm/processor.h
b/arch/riscv/include/asm/processor.h
index 9544c138d9ce..8b288ac0d704 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -64,6 +64,7 @@ extern void start_thread(struct pt_regs *regs,
 #ifdef CONFIG_COMPAT
 extern void compat_start_thread(struct pt_regs *regs,
  unsigned long pc, unsigned long sp);
+extern void compat_mode_detect(void);

 #define DEFAULT_MAP_WINDOW_64 TASK_SIZE_64
 #else
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 9ebf9a95e5ea..496d09c5d384 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -101,6 +101,28 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
 }

 #ifdef CONFIG_COMPAT
+static bool compat_mode_support __read_mostly = false;
+
+bool compat_elf_check_arch(Elf32_Ehdr *hdr)
+{
+ if (compat_mode_support && (hdr->e_machine == EM_RISCV))
+ return true;
+
+ return false;
+}
+
+void compat_mode_detect(void)
+{
+ unsigned long tmp = csr_read(CSR_STATUS);
+ csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
+
+ if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
+ csr_write(CSR_STATUS, tmp);
+ return;
+ }
+
+ csr_write(CSR_STATUS, tmp);
+ compat_mode_support = true;
+
+ pr_info("riscv: compat: 32bit U-mode applications support\n");
+}
+
 void compat_start_thread(struct pt_regs *regs, unsigned long pc,
  unsigned long sp)
 {
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index b42bfdc67482..be131219d549 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -12,6 +12,7 @@
 #include <linux/mm.h>
 #include <linux/memblock.h>
 #include <linux/sched.h>
+#include <linux/compat.h>
 #include <linux/console.h>
 #include <linux/screen_info.h>
 #include <linux/of_fdt.h>
@@ -294,6 +295,10 @@ void __init setup_arch(char **cmdline_p)
  setup_smp();
 #endif

+#ifdef CONFIG_COMPAT
+ compat_mode_detect();
+#endif
+
  riscv_fill_hwcap();
 }
On Tue, Feb 1, 2022 at 5:36 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 1, 2022 at 10:13 AM Guo Ren <guoren@kernel.org> wrote:
> > On Tue, Feb 1, 2022 at 3:45 PM Christoph Hellwig <hch@lst.de> wrote:
> > > On Mon, Jan 31, 2022 at 09:50:58PM +0800, Guo Ren wrote:
> > > > On Mon, Jan 31, 2022 at 8:26 PM Christoph Hellwig <hch@infradead.org> wrote:
> > > > >
> > > > > Given that most rv64 implementations can't run in rv32 mode, what is the
> > > > > failure mode if someone tries it with the compat mode enabled?
> > > > A static linked simple hello_world could still run on a non-compat
> > > > support hardware. But most rv32 apps would meet different userspace
> > > > segment faults.
> > > >
> > > > Current code would let the machine try the rv32 apps without detecting
> > > > whether hw support or not.
> > >
> > > Hmm, we probably want some kind of check for not even offer running
> > > rv32 binaries.  I guess trying to write UXL some time during early
> > > boot and catching the resulting exception would be the way to go?
> >
> > Emm... I think it's unnecessary. Free rv32 app running won't cause
> > system problem, just as a wrong elf running. They are U-mode
> > privileged.
>
> While it's not a security issue, I think it would be helpful to get a
> user-readable error message and a machine-readable /proc/cpuinfo
> flag to see if a particular system can run rv32 binaries rather than
> relying on SIGILL to kill a process.
--
2.25.1


>
>         Arnd



--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/
Arnd Bergmann Feb. 1, 2022, 11:48 a.m. UTC | #7
On Tue, Feb 1, 2022 at 11:26 AM Guo Ren <guoren@kernel.org> wrote:
>
> Hi Arnd & Christoph,
>
> The UXL field controls the value of XLEN for U-mode, termed UXLEN,
> which may differ from the
> value of XLEN for S-mode, termed SXLEN. The encoding of UXL is the
> same as that of the MXL
> field of misa, shown in Table 3.1.
>
> Here is the patch. (We needn't exception helper, because we are in
> S-mode and UXL wouldn't affect.)

Looks good to me, just a few details that could be improved

> -#define compat_elf_check_arch(x) ((x)->e_machine == EM_RISCV)
> +#ifdef CONFIG_COMPAT
> +#define compat_elf_check_arch compat_elf_check_arch
> +extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
> +#endif

No need for the #ifdef
> +}

> +void compat_mode_detect(void)

__init

> +{
> + unsigned long tmp = csr_read(CSR_STATUS);
> + csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
> +
> + if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
> + csr_write(CSR_STATUS, tmp);
> + return;
> + }
> +
> + csr_write(CSR_STATUS, tmp);
> + compat_mode_support = true;
> +
> + pr_info("riscv: compat: 32bit U-mode applications support\n");
> +}

I think an entry in /proc/cpuinfo would be more helpful than the pr_info at
boot time. Maybe a follow-up patch though, as there is no obvious place
to put it. On other architectures, you typically have a set of space
separated feature names, but riscv has a single string that describes
the ISA, and this feature is technically the support for a second ISA.

         Arnd
Guo Ren Feb. 1, 2022, 1:56 p.m. UTC | #8
On Tue, Feb 1, 2022 at 7:48 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 1, 2022 at 11:26 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > Hi Arnd & Christoph,
> >
> > The UXL field controls the value of XLEN for U-mode, termed UXLEN,
> > which may differ from the
> > value of XLEN for S-mode, termed SXLEN. The encoding of UXL is the
> > same as that of the MXL
> > field of misa, shown in Table 3.1.
> >
> > Here is the patch. (We needn't exception helper, because we are in
> > S-mode and UXL wouldn't affect.)
>
> Looks good to me, just a few details that could be improved
>
> > -#define compat_elf_check_arch(x) ((x)->e_machine == EM_RISCV)
> > +#ifdef CONFIG_COMPAT
> > +#define compat_elf_check_arch compat_elf_check_arch
> > +extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
> > +#endif
>
> No need for the #ifdef
Okay

> > +}
>
> > +void compat_mode_detect(void)
>
> __init
Okay

>
> > +{
> > + unsigned long tmp = csr_read(CSR_STATUS);
> > + csr_write(CSR_STATUS, (tmp & ~SR_UXL) | SR_UXL_32);
> > +
> > + if ((csr_read(CSR_STATUS) & SR_UXL) != SR_UXL_32) {
> > + csr_write(CSR_STATUS, tmp);
> > + return;
> > + }
> > +
> > + csr_write(CSR_STATUS, tmp);
> > + compat_mode_support = true;
> > +
> > + pr_info("riscv: compat: 32bit U-mode applications support\n");
> > +}
>
> I think an entry in /proc/cpuinfo would be more helpful than the pr_info at
> boot time. Maybe a follow-up patch though, as there is no obvious place
> to put it. On other architectures, you typically have a set of space
> separated feature names, but riscv has a single string that describes
> the ISA, and this feature is technically the support for a second ISA.
Yes, it should be another patch after discussion.

>
>          Arnd
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 5adcbd9b5e88..6f11df8c189f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -73,6 +73,7 @@  config RISCV
 	select HAVE_ARCH_KGDB if !XIP_KERNEL
 	select HAVE_ARCH_KGDB_QXFER_PKT
 	select HAVE_ARCH_MMAP_RND_BITS if MMU
+	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARCH_TRANSPARENT_HUGEPAGE if 64BIT && MMU
@@ -123,12 +124,18 @@  config ARCH_MMAP_RND_BITS_MIN
 	default 18 if 64BIT
 	default 8
 
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+	default 8
+
 # max bits determined by the following formula:
 #  VA_BITS - PAGE_SHIFT - 3
 config ARCH_MMAP_RND_BITS_MAX
 	default 24 if 64BIT # SV39 based
 	default 17
 
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+	default 17
+
 # set if we run in machine mode, cleared if we run in supervisor mode
 config RISCV_M_MODE
 	bool
@@ -406,6 +413,18 @@  config CRASH_DUMP
 
 	  For more details see Documentation/admin-guide/kdump/kdump.rst
 
+config COMPAT
+	bool "Kernel support for 32-bit U-mode"
+	default 64BIT
+	depends on 64BIT && MMU
+	help
+	  This option enables support for a 32-bit U-mode running under a 64-bit
+	  kernel at S-mode. riscv32-specific components such as system calls,
+	  the user helper functions (vdso), signal rt_frame functions and the
+	  ptrace interface are handled appropriately by the kernel.
+
+	  If you want to execute 32-bit userspace applications, say Y.
+
 endmenu
 
 menu "Boot options"