From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF0ADC001B0 for ; Thu, 6 Jul 2023 23:56:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3BE7E8D0002; Thu, 6 Jul 2023 19:56:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3498E8D0001; Thu, 6 Jul 2023 19:56:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1C13E8D0002; Thu, 6 Jul 2023 19:56:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 059078D0001 for ; Thu, 6 Jul 2023 19:56:30 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id BE481A0CBC for ; Thu, 6 Jul 2023 23:56:29 +0000 (UTC) X-FDA: 80982848898.05.B865D87 Received: from mail-oa1-f42.google.com (mail-oa1-f42.google.com [209.85.160.42]) by imf06.hostedemail.com (Postfix) with ESMTP id C8676180003 for ; Thu, 6 Jul 2023 23:56:26 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b=27cUmVng; spf=pass (imf06.hostedemail.com: domain of charlie@rivosinc.com designates 209.85.160.42 as permitted sender) smtp.mailfrom=charlie@rivosinc.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688687786; a=rsa-sha256; cv=none; b=pRatwi9J7kefwjHbDiGG0jC2RcENEiSYq5Z7WCdtitBrFhxtPTri0Fz12sUjP9LgOtZG4z derjbet4iSBMW5nCkZwcDIocSqLQxpzVYRVNbp0MYBMuhvWl9EwtsUhsljebVve0ZHazED 52avWQ4Pqbxo4WAbmxYrjv9U1m7BnMI= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b=27cUmVng; spf=pass (imf06.hostedemail.com: domain of charlie@rivosinc.com designates 209.85.160.42 as permitted sender) smtp.mailfrom=charlie@rivosinc.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688687786; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=PnXWtVOLq2cNUHw/WdqqgS2x7oe3d4wm7IeGzzcAThk=; b=4jz5bmTcS4KOE89JTsl5SXzpbximeSlCusI1O3UR9KVmxU9MNq6/ybc8fOgkZdmRSkP4c7 xjNdKOZMxWCUYvUnm5iCdCMKPMAU9n8UB1bcKI24rCzV5nE08HO6W2bhEeGBCFKMNqhMHk 3+erDPgx6pSqyt6FWZbNbphqHXEODWw= Received: by mail-oa1-f42.google.com with SMTP id 586e51a60fabf-1b0156a1c49so1244339fac.0 for ; Thu, 06 Jul 2023 16:56:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1688687786; x=1691279786; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=PnXWtVOLq2cNUHw/WdqqgS2x7oe3d4wm7IeGzzcAThk=; b=27cUmVngr3rxnwZHmEULIXb7MtTzmPL3sECDNfTaMc0W8ANjxgmgp0wlGj7tASuwJs 4SAOYaar8Ng0s7P7jXWZh6KKvuGbIDcp0DAkqesL+URHgs4ukR5uijkcE5wEBFKiiJNm xJUI8fCzwMWTU3n4XjZ8rwyKUQjIfouPj5mPH7+pcPK91vbdoVPgBZBO26Tz5SDjup0z 2YErxOskK7Iip2U+ktlSiMqU6cx+9ObZkrZWvrE5ptMi50Akye0MziWCOGjRJgMKaHNB J7lFdKsMPz2FMB8J4SINFHMKDSbAuwlTrOaVbskcho++QRcyC6bFEAs5DqIlLkJsir09 JPIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688687786; x=1691279786; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=PnXWtVOLq2cNUHw/WdqqgS2x7oe3d4wm7IeGzzcAThk=; b=YzTaeSNVJw5Ijwwcf/IOIzPrvr5YG0MI+ZzjcFA45+yu5+VW26f3Tcc4fTxpi4kB+e uwJpApRAqyi3wP4Mr8hXydFEGXYCXorVGjz8hm12PwpXwQohuhgp7z69LodvziOdo/Ot UHm6EdgAFl/6rfm/M3X87GgHHOyGxDl6NPR8/OC0l6Qi/wO43KnlEKTiCUEFzYK/szm0 XSARLmmQCVqUUWtSp5KHXjCrjs2XYElKArvrfg959PYDdlP5jTjnlh/NBSXgqZVNJYx+ ViZYXrjFRCwwYDs1cm0v0bwlJMz9Ma3a/2+HthNbICoyfsAGhMNV9jvWW0nzpAZyysa7 qJUA== X-Gm-Message-State: ABy/qLbVv/U6el5/KkOn92eLX66EkHSHW6ikM5rJh0akDjJKVsrhKXus HHMlJxYexHHQaj7o7QNJ6qtdRg== X-Google-Smtp-Source: APBJJlFNRSchbur5eT8fqD4aKEhWFuO8ec2dvOejYvABwuGPLReD8I2FksZLB3206k2OhsKjtcQ08g== X-Received: by 2002:a05:6870:b625:b0:1b3:cd5c:d9ae with SMTP id cm37-20020a056870b62500b001b3cd5cd9aemr4165610oab.49.1688687785745; Thu, 06 Jul 2023 16:56:25 -0700 (PDT) Received: from ghost ([50.221.140.188]) by smtp.gmail.com with ESMTPSA id 20-20020a17090a199400b002639c4f81cesm367963pji.3.2023.07.06.16.56.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jul 2023 16:56:25 -0700 (PDT) Date: Thu, 6 Jul 2023 16:56:23 -0700 From: Charlie Jenkins To: Alexandre Ghiti Cc: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, conor@kernel.org, paul.walmsley@sifive.com, palmer@rivosinc.com, aou@eecs.berkeley.edu, anup@brainfault.org, konstantin@linuxfoundation.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, mick@ics.forth.gr, jrtc27@jrtc27.com Subject: Re: [RESEND PATCH v3 1/2] RISC-V: mm: Restrict address space for sv39,sv48,sv57 Message-ID: References: <20230705190002.384799-1-charlie@rivosinc.com> <20230705190002.384799-2-charlie@rivosinc.com> <2084462d-b11d-7a48-3049-6bafbe81e7b4@ghiti.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2084462d-b11d-7a48-3049-6bafbe81e7b4@ghiti.fr> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: C8676180003 X-Stat-Signature: g3zhpneb8gezn3jcwyd6e1z8j9syad9k X-Rspam-User: X-HE-Tag: 1688687786-814131 X-HE-Meta: U2FsdGVkX19/w/SChmaJY7pUuQM0cCn9IR+hTFTJxcoCSxNFhqLaun1zml+wIcsn6pKf3uXpC+xOQzDcuWcpaoASnNeCgwRIRjMwsTZ0ic6X7BYa+ZsV2uZZmEfgd3eCPk46yona/M72YP1siEzGT9707ceEblUDHfX3R2iGPK5G5vaM65fsMrMdeypnpBvny0324DGNA5ViSeE0WfnJmckG+E/pU6+8qC6lATErPv2DfkOkiUPqm2oLpQijkw7fjekmL+5D4sCSll1PBH+LC6K+Qk8j31hwtV5uw/1xRNP/u/mbRsy6FVjcGNea7Ziw4hpjEqVZyb5CMbOHJ2NJYKApCAmj5EHE28jonNj8yuoRmMQegcKBDFLYyY8lQ2WJySJG8LEq2KRDeTK08oJYHvVKJwo43Vnp7baCrQZ01BvfK1rG06oyK5ZwzBn30eaIUJrdcg91X5mkWw9z+VuV2KsLVo5NcgDtQN7IhW8DFoj7RrX2vAcodkzWdse3dpYQQmG6Uu8lwXyv97Ax4Qe/uHeItrTyFXlpSqxVPQ7vtEux0dAAWFNHL8kfJIfmcNYmktql/5fA0nunZzvffzu3AmGba7XLqxXRYYHbRyf+fDR7lHzZmhDD0A3aA8AppYoxSBiR3rs24+uDiJqIbW24ocy6PYEw9S/8Yuok6uZH/3ZmbBQEtu1tDWsal2CAzWhYmO5HDM9c06kbrjOVVOeOw3Ktvk0E3iqsU1jbmK/gwEXbm3uJ4/lHC8CshAa3ckQ+1dkbeg6BviiKSF2v6OXxS05dYszThfFfn0J+Dir7kdw/4N4OmDI9UbL691nd4f8tvaTYYwuXey64mdpuGk7Lr5WkV7J/CwRkLrzzhAHzaUOs+6sJwrDA4D93/TdeSqecvoJcQETDiBCoxxTq7Iis+g+55RXKNrNGLhQ1AO8lEWgfc6cxk6dLGY93Z/1edW3D/L4sEmpSagbYRv4q+Et cDDIvMn3 gL3o2s3bVKJDxN9QYYBp9cA6Kkd0edGpS5Fbt1c4dZ4+odrEfhe9Aw08qH459p3x8QBkSegrB09h/v3df8SojYSrDCvOciH5M4C6zQ3iaVv948UAgyQyKMtBznGrEacn2kj8ISrT/Ps/UTlg7ilIlcbI4KzU+W35gfKrl9zb5/bZHho50Dw8Z9WBuGE/8DgFHc7k/asjOQa6Il9PPBZQbM2MdPQBuNUZ3WGjt5lIvWtefRTo1S0WtapVzmNvBCWTCfIS++agBPcwTUuwVfr8b1zFmvXMUL5JOliXlsmfTNTqZ1hrOwLp5GTpK29TxGvTH9E/m8g8MllDdnN/R15//orawhJ8gaRPrPP9a+/F3yjdhVUFeMdghs+W+nk0mDDDwME60HX0N+RbYOZ4+VlE5KSSAN2I3uI5iJi7C2B/xgYY7pf4H7FXMv1kRYXh+QiuMAXEZrpl981VaPIKmpp6/p9tqyG7YV9rFWssLNXD0lSiKjT7Z3Gu+0Y4KHcRrKTJctZOGeEKOY9VrmGW8RCYrKp2IyJ/ecTbc5jga X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Jul 06, 2023 at 11:11:37AM +0200, Alexandre Ghiti wrote: > Hi Charlie, > > > On 05/07/2023 20:59, Charlie Jenkins wrote: > > Make sv48 the default address space for mmap as some applications > > currently depend on this assumption. The RISC-V specification enforces > > that bits outside of the virtual address range are not used, so > > restricting the size of the default address space as such should be > > temporary. > > > What do you mean in the last sentence above? > Applications like Java and Go broke when sv57 was implemented because they shove bits into the upper space of pointers. However riscv enforces that all of the upper bits in the virtual address are equal to the most significant bit. "Temporary" may not have been the best word, but this change would be irrelevant if application developers were following this rule, if I am understanding this requirement correctly. What this means to me is that riscv hardware is not guaranteed to not discard the bits in the virtual address that are not used in paging. > > > A hint address passed to mmap will cause the largest address > > space that fits entirely into the hint to be used. If the hint is less > > than or equal to 1<<38, an sv39 address will be used. An exception is > > that if the hint address is 0, then a sv48 address will be used.After > > an address space is completely full, the next smallest address space > > will be used. > > > > Signed-off-by: Charlie Jenkins > > --- > > arch/riscv/include/asm/elf.h | 2 +- > > arch/riscv/include/asm/pgtable.h | 13 +++++++++++- > > arch/riscv/include/asm/processor.h | 34 ++++++++++++++++++++++++------ > > 3 files changed, 40 insertions(+), 9 deletions(-) > > > > diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h > > index 30e7d2455960..1b57f13a1afd 100644 > > --- a/arch/riscv/include/asm/elf.h > > +++ b/arch/riscv/include/asm/elf.h > > @@ -49,7 +49,7 @@ extern bool compat_elf_check_arch(Elf32_Ehdr *hdr); > > * the loader. We need to make sure that it is out of the way of the program > > * that it will "exec", and that there is sufficient room for the brk. > > */ > > -#define ELF_ET_DYN_BASE ((TASK_SIZE / 3) * 2) > > +#define ELF_ET_DYN_BASE ((DEFAULT_MAP_WINDOW / 3) * 2) > > #ifdef CONFIG_64BIT > > #ifdef CONFIG_COMPAT > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > index 75970ee2bda2..752e210c7547 100644 > > --- a/arch/riscv/include/asm/pgtable.h > > +++ b/arch/riscv/include/asm/pgtable.h > > @@ -57,18 +57,29 @@ > > #define MODULES_END (PFN_ALIGN((unsigned long)&_start)) > > #endif > > + > > /* > > * Roughly size the vmemmap space to be large enough to fit enough > > * struct pages to map half the virtual address space. Then > > * position vmemmap directly below the VMALLOC region. > > */ > > #ifdef CONFIG_64BIT > > +#define VA_BITS_SV39 39 > > +#define VA_BITS_SV48 48 > > +#define VA_BITS_SV57 57 > > + > > +#define VA_USER_SV39 (UL(1) << (VA_BITS_SV39 - 1)) > > +#define VA_USER_SV48 (UL(1) << (VA_BITS_SV48 - 1)) > > +#define VA_USER_SV57 (UL(1) << (VA_BITS_SV57 - 1)) > > + > > #define VA_BITS (pgtable_l5_enabled ? \ > > - 57 : (pgtable_l4_enabled ? 48 : 39)) > > + VA_BITS_SV57 : (pgtable_l4_enabled ? VA_BITS_SV48 : VA_BITS_SV39)) > > #else > > #define VA_BITS 32 > > #endif > > +#define DEFAULT_VA_BITS ((VA_BITS >= VA_BITS_SV48) ? VA_BITS_SV48 : VA_BITS) > > > Maybe rename DEFAULT_VA_BITS into MMAP_VA_BITS? Or something similar? > > > > + > > #define VMEMMAP_SHIFT \ > > (VA_BITS - PAGE_SHIFT - 1 + STRUCT_PAGE_MAX_SHIFT) > > #define VMEMMAP_SIZE BIT(VMEMMAP_SHIFT) > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > > index 94a0590c6971..468a1f4b9da4 100644 > > --- a/arch/riscv/include/asm/processor.h > > +++ b/arch/riscv/include/asm/processor.h > > @@ -12,20 +12,40 @@ > > #include > > -/* > > - * This decides where the kernel will search for a free chunk of vm > > - * space during mmap's. > > - */ > > -#define TASK_UNMAPPED_BASE PAGE_ALIGN(TASK_SIZE / 3) > > - > > -#define STACK_TOP TASK_SIZE > > #ifdef CONFIG_64BIT > > +#define DEFAULT_MAP_WINDOW (UL(1) << (DEFAULT_VA_BITS - 1)) > > #define STACK_TOP_MAX TASK_SIZE_64 > > + > > +#define arch_get_mmap_end(addr, len, flags) \ > > + ((addr) >= VA_USER_SV57 ? STACK_TOP_MAX : \ > > + ((((addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) ? \ > > + VA_USER_SV48 : \ > > + VA_USER_SV39) > > + > > +#define arch_get_mmap_base(addr, base) \ > > + (((addr >= VA_USER_SV57) && (VA_BITS >= VA_BITS_SV57)) ? \ > > > So IIUC, a user must pass a hint larger than the max address of the mode the > user wants right? Shouldn't the user rather pass an address that is larger > than the previous mode? I mean if the user wants a 56-bit address, he should > just pass an address above 1<<47 no? > The rationale is that the hint address provided to mmap should signify all of the bits that the user is okay with being used for paging. Meaning that if they pass in 1<<50, they are okay with the first 51 bits being used in paging. The largest address space that fits within 51 bits is sv48, so that will be used. To use sv57, 1<<56 or larger will need to be used. > > > + VA_USER_SV57 - (DEFAULT_MAP_WINDOW - base) : \ > > + ((((addr) >= VA_USER_SV48)) && (VA_BITS >= VA_BITS_SV48)) ? \ > > + VA_USER_SV48 - (DEFAULT_MAP_WINDOW - base) : \ > > + (addr == 0) ? \ > > + base : \ > > + VA_USER_SV39 - (DEFAULT_MAP_WINDOW - base)) > > + > > > Can you turn that into a function or use if/else statement? It's very hard > to understand what happens there. > Yes, I can use statement expressions. > And riscv selects ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT which means the base > is at the top of the address space (minus the stack IIRC). But if > rlimit_stack is set to infinity (see mmap_base() > https://elixir.bootlin.com/linux/latest/source/mm/util.c#L412), mmap_base is > equal to TASK_UNMAPPED_BASE. Does that work in that case? It seems like > this: VA_USER_SV39 - (DEFAULT_MAP_WINDOW - base)) would be negative right? > > You should also add a rlimit test. > That is a good point. I think a better alternative will be to do base + (VA_USER_SV39 - DEFAULT_MAP_WINDOW). This will also work with the other address spaces by swapping out the 39 with 48 and 57. > > > #else > > +#define DEFAULT_MAP_WINDOW TASK_SIZE > > #define STACK_TOP_MAX TASK_SIZE > > #endif > > #define STACK_ALIGN 16 > > + > > +#define STACK_TOP DEFAULT_MAP_WINDOW > > + > > +/* > > + * This decides where the kernel will search for a free chunk of vm > > + * space during mmap's. > > + */ > > +#define TASK_UNMAPPED_BASE PAGE_ALIGN(DEFAULT_MAP_WINDOW / 3) > > + > > #ifndef __ASSEMBLY__ > > struct task_struct;