Received-SPF: pass (google.com: domain of linux-kernel+bounces-25342-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <tencent_F9568C6D8872E30EDFAF20ADF686A31D6E06@qq.com>
From: Yangyu Chen <cyy@cyyself.name>
To: charlie@rivosinc.com
Cc: alexghiti@rivosinc.com,
	anup@brainfault.org,
	aou@eecs.berkeley.edu,
	conor@kernel.org,
	jrtc27@jrtc27.com,
	konstantin@linuxfoundation.org,
	linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	linux-mm@kvack.org,
	linux-riscv@lists.infradead.org,
	mick@ics.forth.gr,
	palmer@rivosinc.com,
	paul.walmsley@sifive.com,
	rdunlap@infradead.org
Subject: Re: [PATCH v10 0/4] RISC-V: mm: Make SV48 the default address space
Date: Sun, 14 Jan 2024 01:26:57 +0800
In-Reply-To: <20230809232218.849726-1-charlie@rivosinc.com>
References: <20230809232218.849726-1-charlie@rivosinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Hi, Charlie

Although this patchset has been merged I still have some questions about
this patchset. Because it breaks regular mmap if address >= 38 bits on
sv48 / sv57 capable systems like qemu. For example, If a userspace program
wants to mmap an anonymous page to addr=(1<<45) on an sv48 capable system,
it will fail and kernel will mmaped to another sv39 address since it does
not meet the requirement to use sv48 as you wrote:

>	else if ((((_addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) \
>		mmap_end = VA_USER_SV48;			\
>	else							\
>		mmap_end = VA_USER_SV39;			\

Then, How can a userspace program create a mmap with a hint if the address
>= (1<<38) after your patch without MAP_FIXED? The only way to do this is
to pass a hint >= (1<<47) on mmap syscall then kernel will return a random
address in sv48 address space but the hint address gets lost. I think this
violate the principle of mmap syscall as kernel should take the hint and
attempt to create the mapping there.

I don't think patching in this way is right. However, if we only revert
this patch, some programs relying on mmap to return address with effective
bits <= 48 will still be an issue and it might expand to other ISAs if
they implement larger virtual address space like RISC-V sv57. A better way
to solve this might be adding a MAP_48BIT flag to mmap like MAP_32BIT has
been introduced for decades.

Thanks,
Yangyu Chen