| 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 |
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?
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.
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---
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---
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
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/
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
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 --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"