2023-07-08 01:22:02

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 0/4] RISC-V: mm: Make SV48 the default address space

Make sv48 the default address space for mmap as some applications
currently depend on this assumption. Also enable users to select
desired address space using a non-zero hint address to mmap. Previous
kernel changes caused Java and other applications to be broken on sv57
which this patch fixes.

Documentation is also added to the RISC-V virtual memory section to explain
these changes.

-Charlie

---
v4:
- Split testcases/document patch into test cases, in-code documentation, and
formal documentation patches
- Modified the mmap_base macro to be more legible and better represent memory
layout
- Fixed documentation to better reflect the implmentation
- Renamed DEFAULT_VA_BITS to MMAP_VA_BITS
- Added additional test case for rlimit changes
---

Charlie Jenkins (4):
RISC-V: mm: Restrict address space for sv39,sv48,sv57
RISC-V: mm: Add tests for RISC-V mm
RISC-V: mm: Update pgtable comment documentation
RISC-V: mm: Document mmap changes

Documentation/riscv/vm-layout.rst | 22 +++
arch/riscv/include/asm/elf.h | 2 +-
arch/riscv/include/asm/pgtable.h | 21 ++-
arch/riscv/include/asm/processor.h | 43 +++++-
tools/testing/selftests/riscv/Makefile | 2 +-
tools/testing/selftests/riscv/mm/.gitignore | 1 +
tools/testing/selftests/riscv/mm/Makefile | 21 +++
.../selftests/riscv/mm/testcases/mmap.c | 133 ++++++++++++++++++
8 files changed, 232 insertions(+), 13 deletions(-)
create mode 100644 tools/testing/selftests/riscv/mm/.gitignore
create mode 100644 tools/testing/selftests/riscv/mm/Makefile
create mode 100644 tools/testing/selftests/riscv/mm/testcases/mmap.c

--
2.41.0



2023-07-08 01:25:50

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 2/4] RISC-V: mm: Add tests for RISC-V mm

Add tests that enforce mmap hint address behavior. mmap should default
to sv48. mmap will provide an address at the highest address space that
can fit into the hint address, unless the hint address is less than sv39
and not 0, then it will return a sv39 address. In addition, ensure that
rlimit changes do not cause mmap to fail.

Signed-off-by: Charlie Jenkins <[email protected]>
---
tools/testing/selftests/riscv/Makefile | 2 +-
tools/testing/selftests/riscv/mm/.gitignore | 1 +
tools/testing/selftests/riscv/mm/Makefile | 21 +++
.../selftests/riscv/mm/testcases/mmap.c | 133 ++++++++++++++++++
4 files changed, 156 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/riscv/mm/.gitignore
create mode 100644 tools/testing/selftests/riscv/mm/Makefile
create mode 100644 tools/testing/selftests/riscv/mm/testcases/mmap.c

diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile
index 9dd629cc86aa..1b79da90396e 100644
--- a/tools/testing/selftests/riscv/Makefile
+++ b/tools/testing/selftests/riscv/Makefile
@@ -5,7 +5,7 @@
ARCH ?= $(shell uname -m 2>/dev/null || echo not)

ifneq (,$(filter $(ARCH),riscv))
-RISCV_SUBTARGETS ?= hwprobe vector
+RISCV_SUBTARGETS ?= hwprobe vector mm
else
RISCV_SUBTARGETS :=
endif
diff --git a/tools/testing/selftests/riscv/mm/.gitignore b/tools/testing/selftests/riscv/mm/.gitignore
new file mode 100644
index 000000000000..9a6f303edcd3
--- /dev/null
+++ b/tools/testing/selftests/riscv/mm/.gitignore
@@ -0,0 +1 @@
+mmap
diff --git a/tools/testing/selftests/riscv/mm/Makefile b/tools/testing/selftests/riscv/mm/Makefile
new file mode 100644
index 000000000000..cf68e63e7495
--- /dev/null
+++ b/tools/testing/selftests/riscv/mm/Makefile
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+# Originally tools/testing/selftests/arm64/signal
+
+# Additional include paths needed by kselftest.h and local headers
+CFLAGS += -D_GNU_SOURCE -std=gnu99 -I.
+
+SRCS := $(filter-out testcases/testcases.c,$(wildcard testcases/*.c))
+PROGS := $(patsubst %.c,%,$(SRCS))
+
+# Generated binaries to be installed by top KSFT script
+TEST_GEN_PROGS := $(notdir $(PROGS))
+
+# Get Kernel headers installed and use them.
+
+# Including KSFT lib.mk here will also mangle the TEST_GEN_PROGS list
+# to account for any OUTPUT target-dirs optionally provided by
+# the toplevel makefile
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): $(PROGS)
+ cp $(PROGS) $(OUTPUT)/
diff --git a/tools/testing/selftests/riscv/mm/testcases/mmap.c b/tools/testing/selftests/riscv/mm/testcases/mmap.c
new file mode 100644
index 000000000000..d8e751f7b8c9
--- /dev/null
+++ b/tools/testing/selftests/riscv/mm/testcases/mmap.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <sys/mman.h>
+#include <sys/resource.h>
+#include <sys/time.h>
+
+#include "../../kselftest_harness.h"
+struct addresses {
+ int *no_hint;
+ int *on_37_addr;
+ int *on_38_addr;
+ int *on_46_addr;
+ int *on_47_addr;
+ int *on_55_addr;
+ int *on_56_addr;
+};
+
+void do_mmaps(struct addresses *mmap_addresses)
+{
+ // Place all of the hint addresses on the boundaries of mmap
+ // sv39, sv48, sv57
+ // User addresses end at 1<<38, 1<<47, 1<<56 respectively
+ void *on_37_bits = (void *)(1UL << 37);
+ void *on_38_bits = (void *)(1UL << 38);
+ void *on_46_bits = (void *)(1UL << 46);
+ void *on_47_bits = (void *)(1UL << 47);
+ void *on_55_bits = (void *)(1UL << 55);
+ void *on_56_bits = (void *)(1UL << 56);
+
+ int prot = PROT_READ | PROT_WRITE;
+ int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+
+ mmap_addresses->no_hint =
+ mmap(NULL, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_37_addr =
+ mmap(on_37_bits, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_38_addr =
+ mmap(on_38_bits, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_46_addr =
+ mmap(on_46_bits, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_47_addr =
+ mmap(on_47_bits, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_55_addr =
+ mmap(on_55_bits, 5 * sizeof(int), prot, flags, 0, 0);
+ mmap_addresses->on_56_addr =
+ mmap(on_56_bits, 5 * sizeof(int), prot, flags, 0, 0);
+}
+
+TEST(default_rlimit)
+{
+// Only works on 64 bit
+#if __riscv_xlen == 64
+ struct addresses mmap_addresses;
+
+ do_mmaps(&mmap_addresses);
+
+ EXPECT_NE(mmap_addresses.no_hint, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_37_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_38_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_46_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_47_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_55_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_56_addr, MAP_FAILED);
+
+ EXPECT_LT((unsigned long)mmap_addresses.no_hint, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_37_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_38_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_46_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_47_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_55_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_56_addr, 1UL << 56);
+#endif
+}
+
+TEST(zero_rlimit)
+{
+// Only works on 64 bit
+#if __riscv_xlen == 64
+ struct addresses mmap_addresses;
+ struct rlimit rlim_new = { .rlim_cur = 0, .rlim_max = RLIM_INFINITY };
+
+ setrlimit(RLIMIT_STACK, &rlim_new);
+
+ do_mmaps(&mmap_addresses);
+
+ EXPECT_NE(mmap_addresses.no_hint, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_37_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_38_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_46_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_47_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_55_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_56_addr, MAP_FAILED);
+
+ EXPECT_LT((unsigned long)mmap_addresses.no_hint, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_37_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_38_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_46_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_47_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_55_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_56_addr, 1UL << 56);
+#endif
+}
+
+TEST(infinite_rlimit)
+{
+// Only works on 64 bit
+#if __riscv_xlen == 64
+ struct addresses mmap_addresses;
+ struct rlimit rlim_new = { .rlim_cur = RLIM_INFINITY,
+ .rlim_max = RLIM_INFINITY };
+
+ setrlimit(RLIMIT_STACK, &rlim_new);
+
+ do_mmaps(&mmap_addresses);
+
+ EXPECT_NE(mmap_addresses.no_hint, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_37_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_38_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_46_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_47_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_55_addr, MAP_FAILED);
+ EXPECT_NE(mmap_addresses.on_56_addr, MAP_FAILED);
+
+ EXPECT_LT((unsigned long)mmap_addresses.no_hint, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_37_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_38_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_46_addr, 1UL << 38);
+ EXPECT_LT((unsigned long)mmap_addresses.on_47_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_55_addr, 1UL << 47);
+ EXPECT_LT((unsigned long)mmap_addresses.on_56_addr, 1UL << 56);
+#endif
+}
+
+TEST_HARNESS_MAIN
--
2.41.0


2023-07-08 01:26:33

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v4 1/4] RISC-V: mm: Restrict address space for sv39,sv48,sv57

Make sv48 the default address space for mmap as some applications
currently depend on this assumption. A hint address passed to mmap will
cause the largest address space that fits entirely into the hint to be
used. If the hint is less than or equal to 1<<38, an sv39 address will
be used. An exception is that if the hint address is 0, then a sv48
address will be used. After an address space is completely full, the next
smallest address space will be used.

Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/include/asm/elf.h | 2 +-
arch/riscv/include/asm/pgtable.h | 13 ++++++++-
arch/riscv/include/asm/processor.h | 43 +++++++++++++++++++++++++-----
3 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index c24280774caf..5d3368d5585c 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -49,7 +49,7 @@ extern bool compat_elf_check_arch(Elf32_Ehdr *hdr);
* the loader. We need to make sure that it is out of the way of the program
* that it will "exec", and that there is sufficient room for the brk.
*/
-#define ELF_ET_DYN_BASE ((TASK_SIZE / 3) * 2)
+#define ELF_ET_DYN_BASE ((DEFAULT_MAP_WINDOW / 3) * 2)

#ifdef CONFIG_64BIT
#ifdef CONFIG_COMPAT
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 75970ee2bda2..a8090ebea705 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -57,18 +57,29 @@
#define MODULES_END (PFN_ALIGN((unsigned long)&_start))
#endif

+
/*
* Roughly size the vmemmap space to be large enough to fit enough
* struct pages to map half the virtual address space. Then
* position vmemmap directly below the VMALLOC region.
*/
#ifdef CONFIG_64BIT
+#define VA_BITS_SV39 39
+#define VA_BITS_SV48 48
+#define VA_BITS_SV57 57
+
+#define VA_USER_SV39 (UL(1) << (VA_BITS_SV39 - 1))
+#define VA_USER_SV48 (UL(1) << (VA_BITS_SV48 - 1))
+#define VA_USER_SV57 (UL(1) << (VA_BITS_SV57 - 1))
+
#define VA_BITS (pgtable_l5_enabled ? \
- 57 : (pgtable_l4_enabled ? 48 : 39))
+ VA_BITS_SV57 : (pgtable_l4_enabled ? VA_BITS_SV48 : VA_BITS_SV39))
#else
#define VA_BITS 32
#endif

+#define MMAP_VA_BITS ((VA_BITS >= VA_BITS_SV48) ? VA_BITS_SV48 : VA_BITS)
+
#define VMEMMAP_SHIFT \
(VA_BITS - PAGE_SHIFT - 1 + STRUCT_PAGE_MAX_SHIFT)
#define VMEMMAP_SIZE BIT(VMEMMAP_SHIFT)
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index c950a8d9edef..63715c071e1b 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -13,20 +13,49 @@

#include <asm/ptrace.h>

-/*
- * This decides where the kernel will search for a free chunk of vm
- * space during mmap's.
- */
-#define TASK_UNMAPPED_BASE PAGE_ALIGN(TASK_SIZE / 3)
-
-#define STACK_TOP TASK_SIZE
#ifdef CONFIG_64BIT
+#define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1))
#define STACK_TOP_MAX TASK_SIZE_64
+
+#define arch_get_mmap_end(addr, len, flags) \
+({ \
+ unsigned long mmap_end; \
+ if ((addr) >= VA_USER_SV57) \
+ mmap_end = STACK_TOP_MAX; \
+ else if ((((addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
+ mmap_end = VA_USER_SV48; \
+ else \
+ mmap_end = VA_USER_SV39; \
+ mmap_end; \
+})
+
+#define arch_get_mmap_base(addr, base) \
+({ \
+ unsigned long mmap_base; \
+ if ((addr >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \
+ mmap_base = base + (VA_USER_SV57 - DEFAULT_MAP_WINDOW); \
+ else if ((((addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
+ mmap_base = base + (VA_USER_SV48 - DEFAULT_MAP_WINDOW); \
+ else \
+ mmap_base = base + (VA_USER_SV39 - DEFAULT_MAP_WINDOW); \
+ mmap_base; \
+})
+
#else
+#define DEFAULT_MAP_WINDOW TASK_SIZE
#define STACK_TOP_MAX TASK_SIZE
#endif
#define STACK_ALIGN 16

+
+#define STACK_TOP DEFAULT_MAP_WINDOW
+
+/*
+ * This decides where the kernel will search for a free chunk of vm
+ * space during mmap's.
+ */
+#define TASK_UNMAPPED_BASE PAGE_ALIGN(DEFAULT_MAP_WINDOW / 3)
+
#ifndef __ASSEMBLY__

struct task_struct;
--
2.41.0