Re: [PATCH 0/3] riscv: mm: Use hint address in mmap if available

From: Charlie Jenkins <charlie@rivosinc.com>
To: Stefan O'Rear <sorear@fastmail.com>
Cc: Alexandre Ghiti <alexghiti@rivosinc.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Shuah Khan <shuah@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>, Yangyu Chen <cyy@cyyself.name>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH 0/3] riscv: mm: Use hint address in mmap if available
Date: Tue, 30 Jan 2024 14:10:07 -0800	[thread overview]
Message-ID: <Zblzv0KqlIZQ0wbF@ghost> (raw)
In-Reply-To: <6352ff7f-1e69-4f7a-b793-f69d223d4684@app.fastmail.com>

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 <charlie@rivosinc.com>
> >> > ---
> >> > 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
> >> > linux-riscv@lists.infradead.org
> >> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv