On riscv, mmap currently returns an address from the largest address
space that can fit entirely inside of the hint address. This makes it
such that the hint address is almost never returned. This patch raises
the mappable area up to and including the hint address. This allows mmap
to often return the hint address, which allows a performance improvement
over searching for a valid address as well as making the behavior more
similar to other architectures.
Signed-off-by: Charlie Jenkins <[email protected]>
---
Charlie Jenkins (3):
riscv: mm: Use hint address in mmap if available
selftests: riscv: Generalize mm selftests
docs: riscv: Define behavior of mmap
Documentation/arch/riscv/vm-layout.rst | 16 ++--
arch/riscv/include/asm/processor.h | 21 ++----
tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
5 files changed, 66 insertions(+), 104 deletions(-)
---
base-commit: 556e2d17cae620d549c5474b1ece053430cd50bc
change-id: 20240119-use_mmap_hint_address-f9f4b1b6f5f1
--
- Charlie
On riscv it is guaranteed that the address returned by mmap is less than
the hint address. Allow mmap to return an address all the way up to
addr, if provided, rather than just up to the lower address space.
This provides a performance benefit as well, allowing mmap to exit after
checking that the address is in range rather than searching for a valid
address.
It is possible to provide an address that uses at most the same number
of bits, however it is significantly more computationally expensive to
provide that number rather than setting the max to be the hint address.
There is the instruction clz/clzw in Zbb that returns the highest set bit
which could be used to performantly implement this, but it would still
be slower than the current implementation. At worst case, half of the
address would not be able to be allocated when a hint address is
provided.
Signed-off-by: Charlie Jenkins <[email protected]>
---
arch/riscv/include/asm/processor.h | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index f19f861cda54..f3ea5166e3b2 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -22,14 +22,11 @@
({ \
unsigned long mmap_end; \
typeof(addr) _addr = (addr); \
- if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
- mmap_end = STACK_TOP_MAX; \
- else 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; \
+ if ((_addr) == 0 || \
+ (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
+ ((_addr + len) > BIT(VA_BITS - 1))) \
else \
- mmap_end = VA_USER_SV39; \
+ mmap_end = (_addr + len); \
mmap_end; \
})
@@ -39,14 +36,12 @@
typeof(addr) _addr = (addr); \
typeof(base) _base = (base); \
unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \
- if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
+ if ((_addr) == 0 || \
+ (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
+ ((_addr + len) > BIT(VA_BITS - 1))) \
mmap_base = (_base); \
- else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \
- mmap_base = VA_USER_SV57 - rnd_gap; \
- else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
- mmap_base = VA_USER_SV48 - rnd_gap; \
else \
- mmap_base = VA_USER_SV39 - rnd_gap; \
+ mmap_base = (_addr + len) - rnd_gap; \
mmap_base; \
})
--
2.43.0
The behavior of mmap on riscv is defined to not provide an address that
uses more bits than the hint address, if provided. Make the tests
reflect that.
Signed-off-by: Charlie Jenkins <[email protected]>
---
tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
3 files changed, 53 insertions(+), 80 deletions(-)
diff --git a/tools/testing/selftests/riscv/mm/mmap_bottomup.c b/tools/testing/selftests/riscv/mm/mmap_bottomup.c
index 1757d19ca89b..bad8e854263d 100644
--- a/tools/testing/selftests/riscv/mm/mmap_bottomup.c
+++ b/tools/testing/selftests/riscv/mm/mmap_bottomup.c
@@ -8,27 +8,9 @@ TEST(infinite_rlimit)
{
// Only works on 64 bit
#if __riscv_xlen == 64
- struct addresses mmap_addresses;
-
EXPECT_EQ(BOTTOM_UP, memory_layout());
- do_mmaps(&mmap_addresses);
-
- EXPECT_NE(MAP_FAILED, mmap_addresses.no_hint);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_37_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_38_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_46_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_47_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_55_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_56_addr);
-
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.no_hint);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_37_addr);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_38_addr);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_46_addr);
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.on_47_addr);
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.on_55_addr);
- EXPECT_GT(1UL << 56, (unsigned long)mmap_addresses.on_56_addr);
+ TEST_MMAPS;
#endif
}
diff --git a/tools/testing/selftests/riscv/mm/mmap_default.c b/tools/testing/selftests/riscv/mm/mmap_default.c
index c63c60b9397e..a3874778d795 100644
--- a/tools/testing/selftests/riscv/mm/mmap_default.c
+++ b/tools/testing/selftests/riscv/mm/mmap_default.c
@@ -8,27 +8,9 @@ TEST(default_rlimit)
{
// Only works on 64 bit
#if __riscv_xlen == 64
- struct addresses mmap_addresses;
-
EXPECT_EQ(TOP_DOWN, memory_layout());
- do_mmaps(&mmap_addresses);
-
- EXPECT_NE(MAP_FAILED, mmap_addresses.no_hint);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_37_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_38_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_46_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_47_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_55_addr);
- EXPECT_NE(MAP_FAILED, mmap_addresses.on_56_addr);
-
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.no_hint);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_37_addr);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_38_addr);
- EXPECT_GT(1UL << 38, (unsigned long)mmap_addresses.on_46_addr);
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.on_47_addr);
- EXPECT_GT(1UL << 47, (unsigned long)mmap_addresses.on_55_addr);
- EXPECT_GT(1UL << 56, (unsigned long)mmap_addresses.on_56_addr);
+ TEST_MMAPS;
#endif
}
diff --git a/tools/testing/selftests/riscv/mm/mmap_test.h b/tools/testing/selftests/riscv/mm/mmap_test.h
index 9b8434f62f57..93face2b3118 100644
--- a/tools/testing/selftests/riscv/mm/mmap_test.h
+++ b/tools/testing/selftests/riscv/mm/mmap_test.h
@@ -4,60 +4,69 @@
#include <sys/mman.h>
#include <sys/resource.h>
#include <stddef.h>
+#include <strings.h>
+#include "../../kselftest_harness.h"
#define TOP_DOWN 0
#define BOTTOM_UP 1
-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;
+uint64_t random_addresses[] = {
+ 0x19764f0d73b3a9f0, 0x016049584cecef59, 0x3580bdd3562f4acd,
+ 0x1164219f20b17da0, 0x07d97fcb40ff2373, 0x76ec528921272ee7,
+ 0x4dd48c38a3de3f70, 0x2e11415055f6997d, 0x14b43334ac476c02,
+ 0x375a60795aff19f6, 0x47f3051725b8ee1a, 0x4e697cf240494a9f,
+ 0x456b59b5c2f9e9d1, 0x101724379d63cb96, 0x7fe9ad31619528c1,
+ 0x2f417247c495c2ea, 0x329a5a5b82943a5e, 0x06d7a9d6adcd3827,
+ 0x327b0b9ee37f62d5, 0x17c7b1851dfd9b76, 0x006ebb6456ec2cd9,
+ 0x00836cd14146a134, 0x00e5c4dcde7126db, 0x004c29feadf75753,
+ 0x00d8b20149ed930c, 0x00d71574c269387a, 0x0006ebe4a82acb7a,
+ 0x0016135df51f471b, 0x00758bdb55455160, 0x00d0bdd949b13b32,
+ 0x00ecea01e7c5f54b, 0x00e37b071b9948b1, 0x0011fdd00ff57ab3,
+ 0x00e407294b52f5ea, 0x00567748c200ed20, 0x000d073084651046,
+ 0x00ac896f4365463c, 0x00eb0d49a0b26216, 0x0066a2564a982a31,
+ 0x002e0d20237784ae, 0x0000554ff8a77a76, 0x00006ce07a54c012,
+ 0x000009570516d799, 0x00000954ca15b84d, 0x0000684f0d453379,
+ 0x00002ae5816302b5, 0x0000042403fb54bf, 0x00004bad7392bf30,
+ 0x00003e73bfa4b5e3, 0x00005442c29978e0, 0x00002803f11286b6,
+ 0x000073875d745fc6, 0x00007cede9cb8240, 0x000027df84cc6a4f,
+ 0x00006d7e0e74242a, 0x00004afd0b836e02, 0x000047d0e837cd82,
+ 0x00003b42405efeda, 0x00001531bafa4c95, 0x00007172cae34ac4,
+ 0x0000002732f06b2b, 0x00000012cbf8fd0b, 0x0000001fcc6af0e8,
};
-static inline 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;
+#define PROT (PROT_READ | PROT_WRITE)
+#define FLAGS (MAP_PRIVATE | MAP_ANONYMOUS)
+
+/* mmap must return a value that doesn't use more bits than the hint address. */
+static inline unsigned long get_max_value(unsigned long input)
+{
+ unsigned long max_bit = (1UL << (ffsl(input) - 1));
- 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);
+ return max_bit + (max_bit - 1);
}
+#define TEST_MMAPS \
+ ({ \
+ void *mmap_addr; \
+ for (int i = 0; i < ARRAY_SIZE(random_addresses); i++) { \
+ mmap_addr = mmap((void *)random_addresses[i], \
+ 5 * sizeof(int), PROT, FLAGS, 0, 0); \
+ EXPECT_NE(MAP_FAILED, mmap_addr); \
+ EXPECT_GE((void *)get_max_value(random_addresses[i]), \
+ mmap_addr); \
+ mmap_addr = mmap((void *)random_addresses[i], \
+ 5 * sizeof(int), PROT, FLAGS, 0, 0); \
+ EXPECT_NE(MAP_FAILED, mmap_addr); \
+ EXPECT_GE((void *)get_max_value(random_addresses[i]), \
+ mmap_addr); \
+ } \
+ })
+
static inline int memory_layout(void)
{
- int prot = PROT_READ | PROT_WRITE;
- int flags = MAP_PRIVATE | MAP_ANONYMOUS;
-
- void *value1 = mmap(NULL, sizeof(int), prot, flags, 0, 0);
- void *value2 = mmap(NULL, sizeof(int), prot, flags, 0, 0);
+ void *value1 = mmap(NULL, sizeof(int), PROT, FLAGS, 0, 0);
+ void *value2 = mmap(NULL, sizeof(int), PROT, FLAGS, 0, 0);
return value2 > value1;
}
--
2.43.0
Define mmap on riscv to not provide an address that uses more bits than
the hint address, if provided.
Signed-off-by: Charlie Jenkins <[email protected]>
---
Documentation/arch/riscv/vm-layout.rst | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/Documentation/arch/riscv/vm-layout.rst b/Documentation/arch/riscv/vm-layout.rst
index 69ff6da1dbf8..e476b4386bd9 100644
--- a/Documentation/arch/riscv/vm-layout.rst
+++ b/Documentation/arch/riscv/vm-layout.rst
@@ -144,14 +144,8 @@ passing 0 into the hint address parameter of mmap. On CPUs with an address space
smaller than sv48, the CPU maximum supported address space will be the default.
Software can "opt-in" to receiving VAs from another VA space by providing
-a hint address to mmap. A hint address passed to mmap will cause the largest
-address space that fits entirely into the hint to be used, unless there is no
-space left in the address space. If there is no space available in the requested
-address space, an address in the next smallest available address space will be
-returned.
-
-For example, in order to obtain 48-bit VA space, a hint address greater than
-:code:`1 << 47` must be provided. Note that this is 47 due to sv48 userspace
-ending at :code:`1 << 47` and the addresses beyond this are reserved for the
-kernel. Similarly, to obtain 57-bit VA space addresses, a hint address greater
-than or equal to :code:`1 << 56` must be provided.
+a hint address to mmap. When a hint address is passed to mmap, the returned
+address will never use more bits than the hint address. For example, if a hint
+address of `1 << 40` is passed to mmap, a valid returned address will never use
+bits 41 through 63. If no mappable addresses are available in that range, mmap
+will return `MAP_FAILED`.
--
2.43.0
On Mon, Jan 29, 2024, at 7:37 PM, Charlie Jenkins wrote:
> On riscv it is guaranteed that the address returned by mmap is less than
> the hint address. Allow mmap to return an address all the way up to
> addr, if provided, rather than just up to the lower address space.
>
> This provides a performance benefit as well, allowing mmap to exit after
> checking that the address is in range rather than searching for a valid
> address.
>
> It is possible to provide an address that uses at most the same number
> of bits, however it is significantly more computationally expensive to
> provide that number rather than setting the max to be the hint address.
> There is the instruction clz/clzw in Zbb that returns the highest set bit
> which could be used to performantly implement this, but it would still
> be slower than the current implementation. At worst case, half of the
> address would not be able to be allocated when a hint address is
> provided.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> arch/riscv/include/asm/processor.h | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/arch/riscv/include/asm/processor.h
> b/arch/riscv/include/asm/processor.h
> index f19f861cda54..f3ea5166e3b2 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -22,14 +22,11 @@
> ({ \
> unsigned long mmap_end; \
> typeof(addr) _addr = (addr); \
> - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> - mmap_end = STACK_TOP_MAX; \
Setting mmap_end in the no-hint case seems to have been lost?
-s
> - else 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; \
> + if ((_addr) == 0 || \
> + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> + ((_addr + len) > BIT(VA_BITS - 1))) \
> else \
> - mmap_end = VA_USER_SV39; \
> + mmap_end = (_addr + len); \
> mmap_end; \
> })
>
> @@ -39,14 +36,12 @@
> typeof(addr) _addr = (addr); \
> typeof(base) _base = (base); \
> unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \
> - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> + if ((_addr) == 0 || \
> + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> + ((_addr + len) > BIT(VA_BITS - 1))) \
> mmap_base = (_base); \
> - else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \
> - mmap_base = VA_USER_SV57 - rnd_gap; \
> - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
> - mmap_base = VA_USER_SV48 - rnd_gap; \
> else \
> - mmap_base = VA_USER_SV39 - rnd_gap; \
> + mmap_base = (_addr + len) - rnd_gap; \
> mmap_base; \
> })
>
>
> --
> 2.43.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Jan 29, 2024, at 7:36 PM, Charlie Jenkins wrote:
> On riscv, mmap currently returns an address from the largest address
> space that can fit entirely inside of the hint address. This makes it
> such that the hint address is almost never returned. This patch raises
> the mappable area up to and including the hint address. This allows mmap
> to often return the hint address, which allows a performance improvement
> over searching for a valid address as well as making the behavior more
> similar to other architectures.
This means that if an application or library opts in to Sv48 support by
passing a nonzero hint, it will lose the benefits of ASLR.
Allowing applications to opt in to a VA space smaller than the
architectural minimum seems like an independently useful feature.
Is there a reason to only add it for riscv64?
-s
> Signed-off-by: Charlie Jenkins <[email protected]>
> ---
> Charlie Jenkins (3):
> riscv: mm: Use hint address in mmap if available
> selftests: riscv: Generalize mm selftests
> docs: riscv: Define behavior of mmap
>
> Documentation/arch/riscv/vm-layout.rst | 16 ++--
> arch/riscv/include/asm/processor.h | 21 ++----
> tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
> tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
> tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
> 5 files changed, 66 insertions(+), 104 deletions(-)
> ---
> base-commit: 556e2d17cae620d549c5474b1ece053430cd50bc
> change-id: 20240119-use_mmap_hint_address-f9f4b1b6f5f1
> --
> - Charlie
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Jan 29, 2024 at 08:53:31PM -0500, Stefan O'Rear wrote:
> On Mon, Jan 29, 2024, at 7:37 PM, Charlie Jenkins wrote:
> > On riscv it is guaranteed that the address returned by mmap is less than
> > the hint address. Allow mmap to return an address all the way up to
> > addr, if provided, rather than just up to the lower address space.
> >
> > This provides a performance benefit as well, allowing mmap to exit after
> > checking that the address is in range rather than searching for a valid
> > address.
> >
> > It is possible to provide an address that uses at most the same number
> > of bits, however it is significantly more computationally expensive to
> > provide that number rather than setting the max to be the hint address.
> > There is the instruction clz/clzw in Zbb that returns the highest set bit
> > which could be used to performantly implement this, but it would still
> > be slower than the current implementation. At worst case, half of the
> > address would not be able to be allocated when a hint address is
> > provided.
> >
> > Signed-off-by: Charlie Jenkins <[email protected]>
> > ---
> > arch/riscv/include/asm/processor.h | 21 ++++++++-------------
> > 1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/processor.h
> > b/arch/riscv/include/asm/processor.h
> > index f19f861cda54..f3ea5166e3b2 100644
> > --- a/arch/riscv/include/asm/processor.h
> > +++ b/arch/riscv/include/asm/processor.h
> > @@ -22,14 +22,11 @@
> > ({ \
> > unsigned long mmap_end; \
> > typeof(addr) _addr = (addr); \
> > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> > - mmap_end = STACK_TOP_MAX; \
>
> Setting mmap_end in the no-hint case seems to have been lost?
>
> -s
Thanks for catching that, will fix in v2.
- Charlie
>
> > - else 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; \
> > + if ((_addr) == 0 || \
> > + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> > + ((_addr + len) > BIT(VA_BITS - 1))) \
> > else \
> > - mmap_end = VA_USER_SV39; \
> > + mmap_end = (_addr + len); \
> > mmap_end; \
> > })
> >
> > @@ -39,14 +36,12 @@
> > typeof(addr) _addr = (addr); \
> > typeof(base) _base = (base); \
> > unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \
> > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> > + if ((_addr) == 0 || \
> > + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> > + ((_addr + len) > BIT(VA_BITS - 1))) \
> > mmap_base = (_base); \
> > - else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \
> > - mmap_base = VA_USER_SV57 - rnd_gap; \
> > - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
> > - mmap_base = VA_USER_SV48 - rnd_gap; \
> > else \
> > - mmap_base = VA_USER_SV39 - rnd_gap; \
> > + mmap_base = (_addr + len) - rnd_gap; \
> > mmap_base; \
> > })
> >
> >
> > --
> > 2.43.0
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, Jan 29, 2024 at 09:04:50PM -0500, Stefan O'Rear wrote:
> On Mon, Jan 29, 2024, at 7:36 PM, Charlie Jenkins wrote:
> > On riscv, mmap currently returns an address from the largest address
> > space that can fit entirely inside of the hint address. This makes it
> > such that the hint address is almost never returned. This patch raises
> > the mappable area up to and including the hint address. This allows mmap
> > to often return the hint address, which allows a performance improvement
> > over searching for a valid address as well as making the behavior more
> > similar to other architectures.
>
> This means that if an application or library opts in to Sv48 support by
> passing a nonzero hint, it will lose the benefits of ASLR.
sv48 is default. However your statement stands for opting into sv57.
If they always pass the same hint address, only the first address will
be deterministic though, correct?
>
> Allowing applications to opt in to a VA space smaller than the
> architectural minimum seems like an independently useful feature.
> Is there a reason to only add it for riscv64?
>
If there is interest, it can be added to other architectures as well.
- Charlie
> -s
>
> > Signed-off-by: Charlie Jenkins <[email protected]>
> > ---
> > Charlie Jenkins (3):
> > riscv: mm: Use hint address in mmap if available
> > selftests: riscv: Generalize mm selftests
> > docs: riscv: Define behavior of mmap
> >
> > Documentation/arch/riscv/vm-layout.rst | 16 ++--
> > arch/riscv/include/asm/processor.h | 21 ++----
> > tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
> > tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
> > tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
> > 5 files changed, 66 insertions(+), 104 deletions(-)
> > ---
> > base-commit: 556e2d17cae620d549c5474b1ece053430cd50bc
> > change-id: 20240119-use_mmap_hint_address-f9f4b1b6f5f1
> > --
> > - Charlie
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jan 30, 2024 at 10:34:03AM +0800, Yangyu Chen wrote:
>
> > On Jan 30, 2024, at 08:37, Charlie Jenkins <[email protected]> wrote:
> >
> > On riscv it is guaranteed that the address returned by mmap is less than
> > the hint address. Allow mmap to return an address all the way up to
> > addr, if provided, rather than just up to the lower address space.
> >
> > This provides a performance benefit as well, allowing mmap to exit after
> > checking that the address is in range rather than searching for a valid
> > address.
> >
> > It is possible to provide an address that uses at most the same number
> > of bits, however it is significantly more computationally expensive to
> > provide that number rather than setting the max to be the hint address.
> > There is the instruction clz/clzw in Zbb that returns the highest set bit
> > which could be used to performantly implement this, but it would still
> > be slower than the current implementation. At worst case, half of the
> > address would not be able to be allocated when a hint address is
> > provided.
> >
> > Signed-off-by: Charlie Jenkins <[email protected]>
> > ---
> > arch/riscv/include/asm/processor.h | 21 ++++++++-------------
> > 1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > index f19f861cda54..f3ea5166e3b2 100644
> > --- a/arch/riscv/include/asm/processor.h
> > +++ b/arch/riscv/include/asm/processor.h
> > @@ -22,14 +22,11 @@
> > ({ \
> > unsigned long mmap_end; \
> > typeof(addr) _addr = (addr); \
> > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> > - mmap_end = STACK_TOP_MAX; \
> > - else 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; \
> > + if ((_addr) == 0 || \
> > + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> > + ((_addr + len) > BIT(VA_BITS - 1))) \
>
> How about replacing BIT(VA_BITS-1) to DEFAULT_MAP_WINDOW to make the code
> more general.
>
> > else \
> > - mmap_end = VA_USER_SV39; \
> > + mmap_end = (_addr + len); \
> > mmap_end; \
> > })
> >
> > @@ -39,14 +36,12 @@
> > typeof(addr) _addr = (addr); \
> > typeof(base) _base = (base); \
> > unsigned long rnd_gap = DEFAULT_MAP_WINDOW - (_base); \
> > - if ((_addr) == 0 || (IS_ENABLED(CONFIG_COMPAT) && is_compat_task())) \
> > + if ((_addr) == 0 || \
> > + (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
> > + ((_addr + len) > BIT(VA_BITS - 1))) \
>
> Same here.
>
> > mmap_base = (_base); \
> > - else if (((_addr) >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) \
> > - mmap_base = VA_USER_SV57 - rnd_gap; \
> > - else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
> > - mmap_base = VA_USER_SV48 - rnd_gap; \
> > else \
> > - mmap_base = VA_USER_SV39 - rnd_gap; \
> > + mmap_base = (_addr + len) - rnd_gap; \
> > mmap_base; \
> > })
> >
> >
>
> What about not setting the upper bound as x86/arm/powerpc as [1] did?
> In this case, user space can directly pass a constant hint address >
> BIT(47) to get a mapping in sv57. If you want this, this code also allows
> user-space to pass any address larger than TASK_SIZE. You should also
> limit the mmap_base to (base) + TASK_SIZE - DEFAULT_MAP_WINDOW.
No. This suggestion causes a random address to be used if the hint
address is not available. That is why I didn't simply go with your
patch.
This patch both gives your application the benefit of being able to use
a hint address in the hopes that the address is available, as well as
continuing to support the guarantee that an address using more bits than
the hint address is not returned.
- Charlie
>
> I’m also aware of the rnd_gap if it is not 0, then we will not get
> address mapped to VA_USER_SV39 - rnd_gap.
>
> [1]. https://lore.kernel.org/linux-riscv/[email protected]/
>
> > --
> > 2.43.0
> >
>
Hi Charlie,
kernel test robot noticed the following build errors:
[auto build test ERROR on 556e2d17cae620d549c5474b1ece053430cd50bc]
url: https://github.com/intel-lab-lkp/linux/commits/Charlie-Jenkins/riscv-mm-Use-hint-address-in-mmap-if-available/20240130-084208
base: 556e2d17cae620d549c5474b1ece053430cd50bc
patch link: https://lore.kernel.org/r/20240129-use_mmap_hint_address-v1-1-4c74da813ba1%40rivosinc.com
patch subject: [PATCH 1/3] riscv: mm: Use hint address in mmap if available
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240131/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240131/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
In file included from arch/riscv/include/asm/irqflags.h:10,
from include/linux/irqflags.h:18,
from arch/riscv/include/asm/bitops.h:14,
from include/linux/bitops.h:68,
from include/linux/kernel.h:23,
from mm/mmap.c:12:
mm/mmap.c: In function 'generic_get_unmapped_area':
>> arch/riscv/include/asm/processor.h:28:9: error: expected expression before 'else'
28 | else \
| ^~~~
mm/mmap.c:1703:40: note: in expansion of macro 'arch_get_mmap_end'
1703 | const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
| ^~~~~~~~~~~~~~~~~
mm/mmap.c: In function 'generic_get_unmapped_area_topdown':
>> arch/riscv/include/asm/processor.h:28:9: error: expected expression before 'else'
28 | else \
| ^~~~
mm/mmap.c:1751:40: note: in expansion of macro 'arch_get_mmap_end'
1751 | const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
| ^~~~~~~~~~~~~~~~~
--
In file included from arch/riscv/include/asm/irqflags.h:10,
from include/linux/irqflags.h:18,
from arch/riscv/include/asm/bitops.h:14,
from include/linux/bitops.h:68,
from include/linux/thread_info.h:27,
from fs/hugetlbfs/inode.c:12:
fs/hugetlbfs/inode.c: In function 'hugetlb_get_unmapped_area_bottomup':
>> arch/riscv/include/asm/processor.h:28:9: error: expected expression before 'else'
28 | else \
| ^~~~
fs/hugetlbfs/inode.c:173:27: note: in expansion of macro 'arch_get_mmap_end'
173 | info.high_limit = arch_get_mmap_end(addr, len, flags);
| ^~~~~~~~~~~~~~~~~
fs/hugetlbfs/inode.c: In function 'hugetlb_get_unmapped_area_topdown':
>> arch/riscv/include/asm/processor.h:28:9: error: expected expression before 'else'
28 | else \
| ^~~~
fs/hugetlbfs/inode.c:204:35: note: in expansion of macro 'arch_get_mmap_end'
204 | info.high_limit = arch_get_mmap_end(addr, len, flags);
| ^~~~~~~~~~~~~~~~~
fs/hugetlbfs/inode.c: In function 'generic_hugetlb_get_unmapped_area':
>> arch/riscv/include/asm/processor.h:28:9: error: expected expression before 'else'
28 | else \
| ^~~~
fs/hugetlbfs/inode.c:219:40: note: in expansion of macro 'arch_get_mmap_end'
219 | const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
| ^~~~~~~~~~~~~~~~~
vim +/else +28 arch/riscv/include/asm/processor.h
add2cc6b6515f7 Charlie Jenkins 2023-08-09 20
add2cc6b6515f7 Charlie Jenkins 2023-08-09 21 #define arch_get_mmap_end(addr, len, flags) \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 22 ({ \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 23 unsigned long mmap_end; \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 24 typeof(addr) _addr = (addr); \
c5712238cfe3f5 Charlie Jenkins 2024-01-29 25 if ((_addr) == 0 || \
c5712238cfe3f5 Charlie Jenkins 2024-01-29 26 (IS_ENABLED(CONFIG_COMPAT) && is_compat_task()) || \
c5712238cfe3f5 Charlie Jenkins 2024-01-29 27 ((_addr + len) > BIT(VA_BITS - 1))) \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 @28 else \
c5712238cfe3f5 Charlie Jenkins 2024-01-29 29 mmap_end = (_addr + len); \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 30 mmap_end; \
add2cc6b6515f7 Charlie Jenkins 2023-08-09 31 })
add2cc6b6515f7 Charlie Jenkins 2023-08-09 32
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, Jan 29, 2024, at 9:13 PM, Charlie Jenkins wrote:
> On Mon, Jan 29, 2024 at 09:04:50PM -0500, Stefan O'Rear wrote:
>> On Mon, Jan 29, 2024, at 7:36 PM, Charlie Jenkins wrote:
>> > On riscv, mmap currently returns an address from the largest address
>> > space that can fit entirely inside of the hint address. This makes it
>> > such that the hint address is almost never returned. This patch raises
>> > the mappable area up to and including the hint address. This allows mmap
>> > to often return the hint address, which allows a performance improvement
>> > over searching for a valid address as well as making the behavior more
>> > similar to other architectures.
>>
>> This means that if an application or library opts in to Sv48 support by
>> passing a nonzero hint, it will lose the benefits of ASLR.
>
> sv48 is default. However your statement stands for opting into sv57.
> If they always pass the same hint address, only the first address will
> be deterministic though, correct?
I think this is correct.
>>
>> Allowing applications to opt in to a VA space smaller than the
>> architectural minimum seems like an independently useful feature.
>> Is there a reason to only add it for riscv64?
>>
>
> If there is interest, it can be added to other architectures as well.
I meant as opposed to riscv32.
-s
> - Charlie
>
>> -s
>>
>> > Signed-off-by: Charlie Jenkins <[email protected]>
>> > ---
>> > Charlie Jenkins (3):
>> > riscv: mm: Use hint address in mmap if available
>> > selftests: riscv: Generalize mm selftests
>> > docs: riscv: Define behavior of mmap
>> >
>> > Documentation/arch/riscv/vm-layout.rst | 16 ++--
>> > arch/riscv/include/asm/processor.h | 21 ++----
>> > tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
>> > tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
>> > tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
>> > 5 files changed, 66 insertions(+), 104 deletions(-)
>> > ---
>> > base-commit: 556e2d17cae620d549c5474b1ece053430cd50bc
>> > change-id: 20240119-use_mmap_hint_address-f9f4b1b6f5f1
>> > --
>> > - Charlie
>> >
>> >
>> > _______________________________________________
>> > linux-riscv mailing list
>> > [email protected]
>> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jan 30, 2024 at 05:04:55PM -0500, Stefan O'Rear wrote:
> On Mon, Jan 29, 2024, at 9:13 PM, Charlie Jenkins wrote:
> > On Mon, Jan 29, 2024 at 09:04:50PM -0500, Stefan O'Rear wrote:
> >> On Mon, Jan 29, 2024, at 7:36 PM, Charlie Jenkins wrote:
> >> > On riscv, mmap currently returns an address from the largest address
> >> > space that can fit entirely inside of the hint address. This makes it
> >> > such that the hint address is almost never returned. This patch raises
> >> > the mappable area up to and including the hint address. This allows mmap
> >> > to often return the hint address, which allows a performance improvement
> >> > over searching for a valid address as well as making the behavior more
> >> > similar to other architectures.
> >>
> >> This means that if an application or library opts in to Sv48 support by
> >> passing a nonzero hint, it will lose the benefits of ASLR.
> >
> > sv48 is default. However your statement stands for opting into sv57.
> > If they always pass the same hint address, only the first address will
> > be deterministic though, correct?
>
> I think this is correct.
>
> >>
> >> Allowing applications to opt in to a VA space smaller than the
> >> architectural minimum seems like an independently useful feature.
> >> Is there a reason to only add it for riscv64?
> >>
> >
> > If there is interest, it can be added to other architectures as well.
>
> I meant as opposed to riscv32.
That is a good point. I can include rv32 as well.
- Charlie
>
> -s
>
> > - Charlie
> >
> >> -s
> >>
> >> > Signed-off-by: Charlie Jenkins <[email protected]>
> >> > ---
> >> > Charlie Jenkins (3):
> >> > riscv: mm: Use hint address in mmap if available
> >> > selftests: riscv: Generalize mm selftests
> >> > docs: riscv: Define behavior of mmap
> >> >
> >> > Documentation/arch/riscv/vm-layout.rst | 16 ++--
> >> > arch/riscv/include/asm/processor.h | 21 ++----
> >> > tools/testing/selftests/riscv/mm/mmap_bottomup.c | 20 +----
> >> > tools/testing/selftests/riscv/mm/mmap_default.c | 20 +----
> >> > tools/testing/selftests/riscv/mm/mmap_test.h | 93 +++++++++++++-----------
> >> > 5 files changed, 66 insertions(+), 104 deletions(-)
> >> > ---
> >> > base-commit: 556e2d17cae620d549c5474b1ece053430cd50bc
> >> > change-id: 20240119-use_mmap_hint_address-f9f4b1b6f5f1
> >> > --
> >> > - Charlie
> >> >
> >> >
> >> > _______________________________________________
> >> > linux-riscv mailing list
> >> > [email protected]
> >> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Charlie,
kernel test robot noticed the following build errors:
[auto build test ERROR on 556e2d17cae620d549c5474b1ece053430cd50bc]
url: https://github.com/intel-lab-lkp/linux/commits/Charlie-Jenkins/riscv-mm-Use-hint-address-in-mmap-if-available/20240130-084208
base: 556e2d17cae620d549c5474b1ece053430cd50bc
patch link: https://lore.kernel.org/r/20240129-use_mmap_hint_address-v1-1-4c74da813ba1%40rivosinc.com
patch subject: [PATCH 1/3] riscv: mm: Use hint address in mmap if available
config: riscv-allnoconfig (https://download.01.org/0day-ci/archive/20240131/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project fdac7d0b6f74f919d319b31a0680c77f66732586)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240131/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> mm/mmap.c:1703:33: error: expected expression
1703 | const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
| ^
arch/riscv/include/asm/processor.h:28:2: note: expanded from macro 'arch_get_mmap_end'
28 | else \
| ^
mm/mmap.c:1751:33: error: expected expression
1751 | const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
| ^
arch/riscv/include/asm/processor.h:28:2: note: expanded from macro 'arch_get_mmap_end'
28 | else \
| ^
2 errors generated.
vim +1703 mm/mmap.c
f6795053dac8d4d Steve Capper 2018-12-06 1683
^1da177e4c3f415 Linus Torvalds 2005-04-16 1684 /* Get an address range which is currently unmapped.
^1da177e4c3f415 Linus Torvalds 2005-04-16 1685 * For shmat() with addr=0.
^1da177e4c3f415 Linus Torvalds 2005-04-16 1686 *
^1da177e4c3f415 Linus Torvalds 2005-04-16 1687 * Ugly calling convention alert:
^1da177e4c3f415 Linus Torvalds 2005-04-16 1688 * Return value with the low bits set means error value,
^1da177e4c3f415 Linus Torvalds 2005-04-16 1689 * ie
^1da177e4c3f415 Linus Torvalds 2005-04-16 1690 * if (ret & ~PAGE_MASK)
^1da177e4c3f415 Linus Torvalds 2005-04-16 1691 * error = ret;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1692 *
^1da177e4c3f415 Linus Torvalds 2005-04-16 1693 * This function "knows" that -ENOMEM has the bits set.
^1da177e4c3f415 Linus Torvalds 2005-04-16 1694 */
^1da177e4c3f415 Linus Torvalds 2005-04-16 1695 unsigned long
4b439e25e29ec33 Christophe Leroy 2022-04-09 1696 generic_get_unmapped_area(struct file *filp, unsigned long addr,
4b439e25e29ec33 Christophe Leroy 2022-04-09 1697 unsigned long len, unsigned long pgoff,
4b439e25e29ec33 Christophe Leroy 2022-04-09 1698 unsigned long flags)
^1da177e4c3f415 Linus Torvalds 2005-04-16 1699 {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1700 struct mm_struct *mm = current->mm;
1be7107fbe18eed Hugh Dickins 2017-06-19 1701 struct vm_area_struct *vma, *prev;
db4fbfb9523c935 Michel Lespinasse 2012-12-11 1702 struct vm_unmapped_area_info info;
2cb4de085f383cb Christophe Leroy 2022-04-09 @1703 const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
^1da177e4c3f415 Linus Torvalds 2005-04-16 1704
f6795053dac8d4d Steve Capper 2018-12-06 1705 if (len > mmap_end - mmap_min_addr)
^1da177e4c3f415 Linus Torvalds 2005-04-16 1706 return -ENOMEM;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1707
06abdfb47ee745a Benjamin Herrenschmidt 2007-05-06 1708 if (flags & MAP_FIXED)
06abdfb47ee745a Benjamin Herrenschmidt 2007-05-06 1709 return addr;
06abdfb47ee745a Benjamin Herrenschmidt 2007-05-06 1710
^1da177e4c3f415 Linus Torvalds 2005-04-16 1711 if (addr) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 1712 addr = PAGE_ALIGN(addr);
1be7107fbe18eed Hugh Dickins 2017-06-19 1713 vma = find_vma_prev(mm, addr, &prev);
f6795053dac8d4d Steve Capper 2018-12-06 1714 if (mmap_end - len >= addr && addr >= mmap_min_addr &&
1be7107fbe18eed Hugh Dickins 2017-06-19 1715 (!vma || addr + len <= vm_start_gap(vma)) &&
1be7107fbe18eed Hugh Dickins 2017-06-19 1716 (!prev || addr >= vm_end_gap(prev)))
^1da177e4c3f415 Linus Torvalds 2005-04-16 1717 return addr;
^1da177e4c3f415 Linus Torvalds 2005-04-16 1718 }
^1da177e4c3f415 Linus Torvalds 2005-04-16 1719
db4fbfb9523c935 Michel Lespinasse 2012-12-11 1720 info.flags = 0;
db4fbfb9523c935 Michel Lespinasse 2012-12-11 1721 info.length = len;
4e99b02131b280b Heiko Carstens 2013-11-12 1722 info.low_limit = mm->mmap_base;
f6795053dac8d4d Steve Capper 2018-12-06 1723 info.high_limit = mmap_end;
db4fbfb9523c935 Michel Lespinasse 2012-12-11 1724 info.align_mask = 0;
09ef5283fd96ac4 Jaewon Kim 2020-04-10 1725 info.align_offset = 0;
db4fbfb9523c935 Michel Lespinasse 2012-12-11 1726 return vm_unmapped_area(&info);
^1da177e4c3f415 Linus Torvalds 2005-04-16 1727 }
4b439e25e29ec33 Christophe Leroy 2022-04-09 1728
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki