> On Aug 28, 2024, at 00:33, Palmer Dabbelt <pal...@rivosinc.com> wrote:
> 
> On Tue, 27 Aug 2024 01:05:15 PDT (-0700), c...@cyyself.name wrote:
>> Previous patch series[1][2] changes a mmap behavior that treats the hint
>> address as the upper bound of the mmap address range. The motivation of the
>> previous patch series is that some user space software may assume 48-bit
>> address space and use higher bits to encode some information, which may
>> collide with large virtual address space mmap may return. However, to make
>> sv48 by default, we don't need to change the meaning of the hint address on
>> mmap as the upper bound of the mmap address range. This behavior breaks
>> some user space software like Chromium that gets ENOMEM error when the hint
>> address + size is not big enough, as specified in [3].
>> 
>> Other ISAs with larger than 48-bit virtual address space like x86, arm64,
>> and powerpc do not have this special mmap behavior on hint address. They
>> all just make 48-bit / 47-bit virtual address space by default, and if a
>> user space software wants to large virtual address space, it only need to
>> specify a hint address larger than 48-bit / 47-bit.
>> 
>> Thus, this patch series change mmap to use sv48 by default but does not
>> treat the hint address as the upper bound of the mmap address range. After
>> this patch, the behavior of mmap will align with existing behavior on other
>> ISAs with larger than 48-bit virtual address space like x86, arm64, and
>> powerpc. The user space software will no longer need to rewrite their code
>> to fit with this special mmap behavior only on RISC-V.
> 
> So it actually looks like we just screwed up the original version of this: 
> the reason we went with the more complicated address splits were than we 
> actually started with a defacto 39-bit page table uABI (ie 38-bit user VAs), 
> and moving to even 48-bit page tables (ie, 47-bit user VAs) broke users 
> (here's an ASAN bug, for example: 
> https://github.com/google/android-riscv64/issues/64).  
> Unless I'm missing something, though, the code doesn't actually do that.  I 
> remember having that discussion at some point, but I must have forgotten to 
> make sure it worked.  As far as I can tell we've just moved to the 48-bit VAs 
> by default, which breaks the whole point of doing the compatibilty stuff.  
> Probably a good sign I need to pay more attention to this stuff.
> 

It seems the issues have been solved in LLVM D139823 [1] and LLVM D152895 [2].

[1] https://reviews.llvm.org/D139823
[2] https://reviews.llvm.org/D152895

> So I'm not really sure what to do here: we can just copy the arm64 behavior 
> at tell the other users that's just how things work, but then we're just 
> pushing around breakages.  At a certain point all we can really do with this 
> hint stuff is push around problems, though, and at least if we copy arm64 
> then most of those problems get reported as bugs for us.
> 
>> Note: Charlie also created another series [4] to completely remove the
>> arch_get_mmap_end and arch_get_mmap_base behavior based on the hint address
>> and size. However, this will cause programs like Go and Java, which need to
>> store information in the higher bits of the pointer, to fail on Sv57
>> machines.
>> 
>> Changes in v3:
>> - Rebase to newest master
>> - Changes some information in cover letter after patchset [2]
>> - Use patch [5] to patch selftests
>> - Link to v2: 
>> https://lore.kernel.org/linux-riscv/tencent_b2d0435bc011135736262764b511994f4...@qq.com/
>> 
>> Changes in v2:
>> - correct arch_get_mmap_end and arch_get_mmap_base
>> - Add description in documentation about mmap behavior on kernel v6.6-6.7.
>> - Improve commit message and cover letter
>> - Rebase to newest riscv/for-next branch
>> - Link to v1: 
>> https://lore.kernel.org/linux-riscv/tencent_f3b3b5ab1c9d704763ca423e1a41f8be0...@qq.com/
>> 
>> [1] 
>> https://lore.kernel.org/linux-riscv/20230809232218.849726-1-char...@rivosinc.com/
>> [2] 
>> https://lore.kernel.org/linux-riscv/20240130-use_mmap_hint_address-v3-0-8a655cfa8...@rivosinc.com/
>> [3] 
>> https://lore.kernel.org/linux-riscv/meyp282mb2312a08ff95d44014ab78411c6...@meyp282mb2312.ausp282.prod.outlook.com/
>> [4] 
>> https://lore.kernel.org/linux-riscv/20240826-riscv_mmap-v1-0-cd8962afe...@rivosinc.com/
>> [5] 
>> https://lore.kernel.org/linux-riscv/20240826-riscv_mmap-v1-2-cd8962afe...@rivosinc.com/
>> 
>> Charlie Jenkins (1):
>>  riscv: selftests: Remove mmap hint address checks
>> 
>> Yangyu Chen (2):
>>  RISC-V: mm: not use hint addr as upper bound
>>  Documentation: riscv: correct sv57 kernel behavior
>> 
>> Documentation/arch/riscv/vm-layout.rst        | 43 ++++++++----
>> arch/riscv/include/asm/processor.h            | 20 ++----
>> .../selftests/riscv/mm/mmap_bottomup.c        |  2 -
>> .../testing/selftests/riscv/mm/mmap_default.c |  2 -
>> tools/testing/selftests/riscv/mm/mmap_test.h  | 67 -------------------
>> 5 files changed, 36 insertions(+), 98 deletions(-)



Reply via email to