2023-07-14 17:12:03

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v6 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. Users can now select a
desired address space using a non-zero hint address to mmap. Previously,
requesting the default address space from mmap by passing zero as the hint
address would result in using the largest address space possible. Some
applications depend on empty bits in the virtual address space, like Go and
Java, so this patch provides more flexibility for application developers.

-Charlie

---
v6:
- Rebase onto the correct base

v5:
- Minor wording change in documentation
- Change some parenthesis in arch_get_mmap_ macros
- Added case for addr==0 in arch_get_mmap_ because without this, programs would
crash if RLIMIT_STACK was modified before executing the program. This was
tested using the libhugetlbfs tests.

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 | 20 ++-
arch/riscv/include/asm/processor.h | 46 +++++-
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, 234 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-14 17:12:18

by Charlie Jenkins

[permalink] [raw]
Subject: [PATCH v6 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-20 08:56:58

by Alexandre Ghiti

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

On Fri, Jul 14, 2023 at 6:55 PM Charlie Jenkins <[email protected]> wrote:
>
> 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

Doesn't checkpatch complain about those comments? Shouldn't you use /*
*/ instead?

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