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

On Tue, Feb 1, 2022 at 7:48 PM Arnd Bergmann [off-list ref] wrote:

On Tue, Feb 1, 2022 at 11:26 AM Guo Ren [off-list ref] wrote:

quoted

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

quoted

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

quoted

+}

quoted

+void compat_mode_detect(void)

__init

Okay

quoted

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

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/