Message ID | 20231221154702.2267684-2-guoren@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:2483:b0:fb:cd0c:d3e with SMTP id q3csp499626dyi; Thu, 21 Dec 2023 07:47:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IFqsSfeMNcXWBjcBIcJ1j/j1ZQmfshCUZWm0sfnNTsRQA9rT83gRwV7JBQXgWxk5YaaWt4g X-Received: by 2002:a05:6808:6488:b0:3ba:d81:841e with SMTP id fh8-20020a056808648800b003ba0d81841emr24328275oib.55.1703173660032; Thu, 21 Dec 2023 07:47:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703173660; cv=none; d=google.com; s=arc-20160816; b=voiqlVOWT1SUKSoNN+hOHWGuSNahpaKsVq9k1U4UsonEnDM1yHTEflaApyj/NXUlq3 Qr53GvAXmPboTDMPoF8CNd9fiwKkBOCikPgLNDTseQaH8n9M/Kv9rn2zGdB3yaGyvQs6 T/t/6TnBhiCxufVdpbgupP3ANPzJ08lHA7sTtJOkCfejG/mhqNpy0UTKXXW31VBq0S15 se1H3cxSjdfUtlAvxcp7mtQmZjERVV65q6PM0Omsyo0zLKgpX7JE0vaFmqrfmCyecM9t dED3U68CZakGyagB9OhXeb6k3g5OVtxuh7sYYRbwXqAtlRm5WNucQD9Qjp1k1wagYEpL a2pA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=R7v7KRN5kF3f6T8ivYjW/QyKlDyLwS+koztQTaV2Ui0=; fh=3YQLS1cPsLhMKuGGotL7Ux7rpkwBoh7ZT2nN27VXc5c=; b=CVm7LDiNp5nBkLjWvLhyMSNFUnwdrGjND3255Iai1DhhfPsKKwr7dIodVMmmB5+Xlx QTI/i69CQ3LSWU6i/1tJmm2Fsi3Nl2/gPBn/v8FKhTxFb4/bbwhGg7sZPfjWhIL/uBM3 yROto85QBJzGPMfvC4qnwAPhELBqYN1UQkO/fc/18xgiFl5v7zFR4I5hl3zLX7RAEYRp glW+YQ+DpWjLOQVE/fN6NoALY6GqDgDAyEPEaIxEIzqolrlyiIKg1aIL/dRe4xTqtEi3 +7GGhFNkpCz0tzA01QYdBgLs3ysPZhbuvx767RXDrlFaB9PD75sAwoxv1YuAxoUnqfHX 5lgg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=foMQzB4V; spf=pass (google.com: domain of linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id r5-20020a0ce285000000b0067f6107bfb2si2313049qvl.403.2023.12.21.07.47.39 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Dec 2023 07:47:40 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=foMQzB4V; spf=pass (google.com: domain of linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-8703-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id CC19B1C25E14 for <ouuuleilei@gmail.com>; Thu, 21 Dec 2023 15:47:39 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 6545555E6F; Thu, 21 Dec 2023 15:47:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="foMQzB4V" X-Original-To: linux-kernel@vger.kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D40EE55E41; Thu, 21 Dec 2023 15:47:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 028E0C433C9; Thu, 21 Dec 2023 15:47:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1703173634; bh=1VkNb9SSj6ijZ3M72NIaFsxAoNnYaBQ2VS47upbIECw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=foMQzB4VeCSYCK//dcfGdemiuC7dKk0zuG1BDQxCkZdTqaRrEwvgQCiJ0pXugK9Qa LlUugpTV6s3JWWoRtIg3kD00AvVPEJP5ILsdBTWtHMZDygSEHTpeduzLfOCjCD3mUD K3IOHDR4zp4rOF+qKjzuLt5nVJFScnxaGbvVMet34PxjkKCWbSg2cZHqJJI/e/AMc/ /UaclkeRKp4EkIdtq18KfuTuwxLqSAFS+T4dthVtNZ1Tyl0Nwfu6r4+m9FZkW4q9rG X7uiwUiXnuQy4VWUUAkrbx6BAI1wIIVJ2ppeNoQPmqW7SH+zUe85KzXJjgyMQMwOfw FmXjWEdP3pzJQ== From: guoren@kernel.org To: linux-kernel@vger.kernel.org, paul.walmsley@sifive.com, palmer@dabbelt.com, alexghiti@rivosinc.com, charlie@rivosinc.com, xiao.w.wang@intel.com, guoren@kernel.org, david@redhat.com, panqinglin2020@iscas.ac.cn, rick.p.edgecombe@intel.com, willy@infradead.org, bjorn@rivosinc.com, conor.dooley@microchip.com, cleger@rivosinc.com, leobras@redhat.com Cc: linux-riscv@lists.infradead.org, Guo Ren <guoren@linux.alibaba.com>, stable@vger.kernel.org Subject: [PATCH V2 1/4] riscv: mm: Fixup compat mode boot failure Date: Thu, 21 Dec 2023 10:46:58 -0500 Message-Id: <20231221154702.2267684-2-guoren@kernel.org> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20231221154702.2267684-1-guoren@kernel.org> References: <20231221154702.2267684-1-guoren@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785907023742897099 X-GMAIL-MSGID: 1785907023742897099 |
Series |
riscv: mm: Fixup & Optimize COMPAT code
|
|
Commit Message
Guo Ren
Dec. 21, 2023, 3:46 p.m. UTC
From: Guo Ren <guoren@linux.alibaba.com> In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user segment fault. Sometimes, it would cause boot failure when the whole rootfs is rv32. Freeing unused kernel image (initmem) memory: 2236K Run /sbin/init as init process Starting init: /sbin/init exists but couldn't execute it (error -14) Run /etc/init as init process ... Cc: stable@vger.kernel.org Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") Signed-off-by: Guo Ren <guoren@linux.alibaba.com> Signed-off-by: Guo Ren <guoren@kernel.org> --- arch/riscv/include/asm/pgtable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Hello Guo Ren, On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > segment fault. Sometimes, it would cause boot failure when the whole > rootfs is rv32. Checking if I get the scenario: In pgtable.h: #ifdef CONFIG_64BIT #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) #ifdef CONFIG_COMPAT #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ TASK_SIZE_32 : TASK_SIZE_64) #else [...] Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. from processor.h: #ifdef CONFIG_64BIT #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) #define STACK_TOP_MAX TASK_SIZE_64 [...] #define STACK_TOP DEFAULT_MAP_WINDOW where: #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) with MMAP_VA_BITS_64 being either 48 or 37. In compat mode, STACK_TOP = 1 << (32 - 1) -> 0x80000000 TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. Then why not: #ifdef CONFIG_COMPAT #define TASK_SIZE_32 STACK_TOP With some comments explaining why there is no need to reserve a PAGE_SIZE in the TASK_SIZE_32. Does that make sense? Thanks! Leo > > Freeing unused kernel image (initmem) memory: 2236K > Run /sbin/init as init process > Starting init: /sbin/init exists but couldn't execute it (error -14) > Run /etc/init as init process > ... > > Cc: stable@vger.kernel.org > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > --- > arch/riscv/include/asm/pgtable.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > index ab00235b018f..74ffb2178f54 100644 > --- a/arch/riscv/include/asm/pgtable.h > +++ b/arch/riscv/include/asm/pgtable.h > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > #ifdef CONFIG_COMPAT > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > TASK_SIZE_32 : TASK_SIZE_64) > #else > -- > 2.40.1 >
On Fri, Dec 22, 2023 at 9:51 AM Leonardo Bras <leobras@redhat.com> wrote: > > Hello Guo Ren, > > On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > > segment fault. Sometimes, it would cause boot failure when the whole > > rootfs is rv32. > > Checking if I get the scenario: > > In pgtable.h: > #ifdef CONFIG_64BIT > #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > #ifdef CONFIG_COMPAT > #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > TASK_SIZE_32 : TASK_SIZE_64) > #else > [...] > > Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in > compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. > > from processor.h: > #ifdef CONFIG_64BIT > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > #define STACK_TOP_MAX TASK_SIZE_64 > [...] > #define STACK_TOP DEFAULT_MAP_WINDOW > > > where: > #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) > with MMAP_VA_BITS_64 being either 48 or 37. > > In compat mode, > STACK_TOP = 1 << (32 - 1) -> 0x80000000 > TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 > > IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. Yes, it causes the problem, which causes the boot to fail. > > Then why not: > #ifdef CONFIG_COMPAT > #define TASK_SIZE_32 STACK_TOP Yes, it's the solution that I think at first. But I didn't find any problem with 0x7ffff000 ~ 0x80000000, and then I removed this gap to unify it with Sv39 and Sv48. > > With some comments explaining why there is no need to reserve a PAGE_SIZE > in the TASK_SIZE_32. At first, I wanted to put a invalid page between the user & kernel space, but it seems useless. > > Does that make sense? > > Thanks! > Leo > > > > > Freeing unused kernel image (initmem) memory: 2236K > > Run /sbin/init as init process > > Starting init: /sbin/init exists but couldn't execute it (error -14) > > Run /etc/init as init process > > ... > > > > Cc: stable@vger.kernel.org > > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > --- > > arch/riscv/include/asm/pgtable.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > index ab00235b018f..74ffb2178f54 100644 > > --- a/arch/riscv/include/asm/pgtable.h > > +++ b/arch/riscv/include/asm/pgtable.h > > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > #ifdef CONFIG_COMPAT > > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > > > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > TASK_SIZE_32 : TASK_SIZE_64) > > #else > > -- > > 2.40.1 > > >
On Fri, Dec 22, 2023 at 10:57:16AM +0800, Guo Ren wrote: > On Fri, Dec 22, 2023 at 9:51 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > Hello Guo Ren, > > > > On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > > > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > > > segment fault. Sometimes, it would cause boot failure when the whole > > > rootfs is rv32. > > > > Checking if I get the scenario: > > > > In pgtable.h: > > #ifdef CONFIG_64BIT > > #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > #ifdef CONFIG_COMPAT > > #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > TASK_SIZE_32 : TASK_SIZE_64) > > #else > > [...] > > > > Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in > > compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. > > > > from processor.h: > > #ifdef CONFIG_64BIT > > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > > #define STACK_TOP_MAX TASK_SIZE_64 > > [...] > > #define STACK_TOP DEFAULT_MAP_WINDOW > > > > > > where: > > #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) > > with MMAP_VA_BITS_64 being either 48 or 37. > > > > In compat mode, > > STACK_TOP = 1 << (32 - 1) -> 0x80000000 > > TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 > > > > IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. > Yes, it causes the problem, which causes the boot to fail. I think what Leonardo is getting at is that it is odd that it would cause boot issues if TASK_SIZE is not equal STACK_TOP. This seems indicative of a different problem. While this may fix the issue, it should be valid for TASK_SIZE to be less than STACK_TOP. - Charlie > > > > > Then why not: > > #ifdef CONFIG_COMPAT > > #define TASK_SIZE_32 STACK_TOP > Yes, it's the solution that I think at first. But I didn't find any > problem with 0x7ffff000 ~ 0x80000000, and then I removed this gap to > unify it with Sv39 and Sv48. > > > > > With some comments explaining why there is no need to reserve a PAGE_SIZE > > in the TASK_SIZE_32. > At first, I wanted to put a invalid page between the user & kernel > space, but it seems useless. > > > > > Does that make sense? > > > > Thanks! > > Leo > > > > > > > > Freeing unused kernel image (initmem) memory: 2236K > > > Run /sbin/init as init process > > > Starting init: /sbin/init exists but couldn't execute it (error -14) > > > Run /etc/init as init process > > > ... > > > > > > Cc: stable@vger.kernel.org > > > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > > --- > > > arch/riscv/include/asm/pgtable.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > > index ab00235b018f..74ffb2178f54 100644 > > > --- a/arch/riscv/include/asm/pgtable.h > > > +++ b/arch/riscv/include/asm/pgtable.h > > > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > #ifdef CONFIG_COMPAT > > > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > > > > > > > > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > TASK_SIZE_32 : TASK_SIZE_64) > > > #else > > > -- > > > 2.40.1 > > > > > > > > -- > Best Regards > Guo Ren
On Thu, Dec 21, 2023 at 07:50:27PM -0800, Charlie Jenkins wrote: > On Fri, Dec 22, 2023 at 10:57:16AM +0800, Guo Ren wrote: > > On Fri, Dec 22, 2023 at 9:51 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > Hello Guo Ren, > > > > > > On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > > > > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > > > > segment fault. Sometimes, it would cause boot failure when the whole > > > > rootfs is rv32. > > > > > > Checking if I get the scenario: > > > > > > In pgtable.h: > > > #ifdef CONFIG_64BIT > > > #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > #ifdef CONFIG_COMPAT > > > #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > TASK_SIZE_32 : TASK_SIZE_64) > > > #else > > > [...] > > > > > > Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in > > > compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. > > > > > > from processor.h: > > > #ifdef CONFIG_64BIT > > > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > > > #define STACK_TOP_MAX TASK_SIZE_64 > > > [...] > > > #define STACK_TOP DEFAULT_MAP_WINDOW > > > > > > > > > where: > > > #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) > > > with MMAP_VA_BITS_64 being either 48 or 37. > > > > > > In compat mode, > > > STACK_TOP = 1 << (32 - 1) -> 0x80000000 > > > TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 > > > > > > IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. > > Yes, it causes the problem, which causes the boot to fail. > > I think what Leonardo is getting at is that it is odd that it would > cause boot issues if TASK_SIZE is not equal STACK_TOP. This seems > indicative of a different problem. While this may fix the issue, it > should be valid for TASK_SIZE to be less than STACK_TOP. > > - Charlie > That is also a good point, but I am not that acquainted to this to actually propose this. I was thinking more on these questions: Is TASK_SIZE and STACK_TOP related somehow? If so, would not be better to describe one in terms of the other, like #define TASK_SIZE (STACK_TOP - PAGE_SIZE) Or the other way around. I mean, if they have any relation it would be much easier to represent them that way, and it would avoid having two magical numbers. Thanks! Leo > > > > > > > > Then why not: > > > #ifdef CONFIG_COMPAT > > > #define TASK_SIZE_32 STACK_TOP > > Yes, it's the solution that I think at first. But I didn't find any > > problem with 0x7ffff000 ~ 0x80000000, and then I removed this gap to > > unify it with Sv39 and Sv48. > > > > > > > > With some comments explaining why there is no need to reserve a PAGE_SIZE > > > in the TASK_SIZE_32. > > At first, I wanted to put a invalid page between the user & kernel > > space, but it seems useless. > > > > > > > > Does that make sense? > > > > > > Thanks! > > > Leo > > > > > > > > > > > Freeing unused kernel image (initmem) memory: 2236K > > > > Run /sbin/init as init process > > > > Starting init: /sbin/init exists but couldn't execute it (error -14) > > > > Run /etc/init as init process > > > > ... > > > > > > > > Cc: stable@vger.kernel.org > > > > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > > > --- > > > > arch/riscv/include/asm/pgtable.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > > > index ab00235b018f..74ffb2178f54 100644 > > > > --- a/arch/riscv/include/asm/pgtable.h > > > > +++ b/arch/riscv/include/asm/pgtable.h > > > > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > > > #ifdef CONFIG_COMPAT > > > > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > > > > > > > > > > > > > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > > TASK_SIZE_32 : TASK_SIZE_64) > > > > #else > > > > -- > > > > 2.40.1 > > > > > > > > > > > > > -- > > Best Regards > > Guo Ren >
On Fri, Dec 22, 2023 at 12:33 PM Leonardo Bras <leobras@redhat.com> wrote: > > On Thu, Dec 21, 2023 at 07:50:27PM -0800, Charlie Jenkins wrote: > > On Fri, Dec 22, 2023 at 10:57:16AM +0800, Guo Ren wrote: > > > On Fri, Dec 22, 2023 at 9:51 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > > > Hello Guo Ren, > > > > > > > > On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > > > > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > > > > > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > > > > > segment fault. Sometimes, it would cause boot failure when the whole > > > > > rootfs is rv32. > > > > > > > > Checking if I get the scenario: > > > > > > > > In pgtable.h: > > > > #ifdef CONFIG_64BIT > > > > #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) > > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > > > #ifdef CONFIG_COMPAT > > > > #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > > TASK_SIZE_32 : TASK_SIZE_64) > > > > #else > > > > [...] > > > > > > > > Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in > > > > compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. > > > > > > > > from processor.h: > > > > #ifdef CONFIG_64BIT > > > > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > > > > #define STACK_TOP_MAX TASK_SIZE_64 > > > > [...] > > > > #define STACK_TOP DEFAULT_MAP_WINDOW > > > > > > > > > > > > where: > > > > #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) > > > > with MMAP_VA_BITS_64 being either 48 or 37. > > > > > > > > In compat mode, > > > > STACK_TOP = 1 << (32 - 1) -> 0x80000000 > > > > TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 > > > > > > > > IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. > > > Yes, it causes the problem, which causes the boot to fail. > > > > I think what Leonardo is getting at is that it is odd that it would > > cause boot issues if TASK_SIZE is not equal STACK_TOP. This seems > > indicative of a different problem. While this may fix the issue, it > > should be valid for TASK_SIZE to be less than STACK_TOP. > > > > - Charlie > > > > That is also a good point, but I am not that acquainted to this to > actually propose this. > > I was thinking more on these questions: > Is TASK_SIZE and STACK_TOP related somehow? > If so, would not be better to describe one in terms of the other, like > #define TASK_SIZE (STACK_TOP - PAGE_SIZE) TASK_SIZE means the maximum user address space, so it's the limitation to any kind of mmap or stack ... So STACK_TOP <= TASK_SIZE Follow your idea. The question is: #define TASK_SIZE ((UL(1) << (VA_BITS - 1)) - PAGE_SIZE) Do we need to reserve one page between userspace & kernel? > > Or the other way around. > > I mean, if they have any relation it would be much easier to represent them > that way, and it would avoid having two magical numbers. > > Thanks! > Leo > > > > > > > > > > > > Then why not: > > > > #ifdef CONFIG_COMPAT > > > > #define TASK_SIZE_32 STACK_TOP > > > Yes, it's the solution that I think at first. But I didn't find any > > > problem with 0x7ffff000 ~ 0x80000000, and then I removed this gap to > > > unify it with Sv39 and Sv48. > > > > > > > > > > > With some comments explaining why there is no need to reserve a PAGE_SIZE > > > > in the TASK_SIZE_32. > > > At first, I wanted to put a invalid page between the user & kernel > > > space, but it seems useless. > > > > > > > > > > > Does that make sense? > > > > > > > > Thanks! > > > > Leo > > > > > > > > > > > > > > Freeing unused kernel image (initmem) memory: 2236K > > > > > Run /sbin/init as init process > > > > > Starting init: /sbin/init exists but couldn't execute it (error -14) > > > > > Run /etc/init as init process > > > > > ... > > > > > > > > > > Cc: stable@vger.kernel.org > > > > > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > > > > --- > > > > > arch/riscv/include/asm/pgtable.h | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > > > > index ab00235b018f..74ffb2178f54 100644 > > > > > --- a/arch/riscv/include/asm/pgtable.h > > > > > +++ b/arch/riscv/include/asm/pgtable.h > > > > > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > > > > > #ifdef CONFIG_COMPAT > > > > > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > > > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > > > > > > > > > > > > > > > > > > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > > > TASK_SIZE_32 : TASK_SIZE_64) > > > > > #else > > > > > -- > > > > > 2.40.1 > > > > > > > > > > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > > -- Best Regards Guo Ren
Hello Charlie, On Fri, Dec 22, 2023 at 11:50 AM Charlie Jenkins <charlie@rivosinc.com> wrote: > > On Fri, Dec 22, 2023 at 10:57:16AM +0800, Guo Ren wrote: > > On Fri, Dec 22, 2023 at 9:51 AM Leonardo Bras <leobras@redhat.com> wrote: > > > > > > Hello Guo Ren, > > > > > > On Thu, Dec 21, 2023 at 10:46:58AM -0500, guoren@kernel.org wrote: > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > > > > > In COMPAT mode, the STACK_TOP is 0x80000000, but the TASK_SIZE is > > > > 0x7fff000. When the user stack is upon 0x7fff000, it will cause a user > > > > segment fault. Sometimes, it would cause boot failure when the whole > > > > rootfs is rv32. > > > > > > Checking if I get the scenario: > > > > > > In pgtable.h: > > > #ifdef CONFIG_64BIT > > > #define TASK_SIZE_64 (PGDIR_SIZE * PTRS_PER_PGD / 2) > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > #ifdef CONFIG_COMPAT > > > #define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > TASK_SIZE_32 : TASK_SIZE_64) > > > #else > > > [...] > > > > > > Meaning CONFIG_COMPAT is only available in CONFIG_64BIT, and TASK_SIZE in > > > compat mode is either TASK_SIZE_32 or TASK_SIZE_64 depending on the thread_flag. > > > > > > from processor.h: > > > #ifdef CONFIG_64BIT > > > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > > > #define STACK_TOP_MAX TASK_SIZE_64 > > > [...] > > > #define STACK_TOP DEFAULT_MAP_WINDOW > > > > > > > > > where: > > > #define MMAP_VA_BITS (is_compat_task() ? VA_BITS_SV32 : MMAP_VA_BITS_64) > > > with MMAP_VA_BITS_64 being either 48 or 37. > > > > > > In compat mode, > > > STACK_TOP = 1 << (32 - 1) -> 0x80000000 > > > TASK_SIZE = 0x8000000 - 4k -> 0x7ffff000 > > > > > > IIUC, your suggestion is to make TASK_SIZE = STACK_TOP in compat mode only. > > Yes, it causes the problem, which causes the boot to fail. > > I think what Leonardo is getting at is that it is odd that it would > cause boot issues if TASK_SIZE is not equal STACK_TOP. This seems > indicative of a different problem. While this may fix the issue, it > should be valid for TASK_SIZE to be less than STACK_TOP. Sorry, I don't make sense here. Why do you think STACK_TOP could > TASK_SIZE? TASK_SIZE is the highest priority of the address limitation for user-space address, right? Do you mean: STACK_TOP could > MMAP_END? > > - Charlie > > > > > > > > > Then why not: > > > #ifdef CONFIG_COMPAT > > > #define TASK_SIZE_32 STACK_TOP > > Yes, it's the solution that I think at first. But I didn't find any > > problem with 0x7ffff000 ~ 0x80000000, and then I removed this gap to > > unify it with Sv39 and Sv48. > > > > > > > > With some comments explaining why there is no need to reserve a PAGE_SIZE > > > in the TASK_SIZE_32. > > At first, I wanted to put a invalid page between the user & kernel > > space, but it seems useless. > > > > > > > > Does that make sense? > > > > > > Thanks! > > > Leo > > > > > > > > > > > Freeing unused kernel image (initmem) memory: 2236K > > > > Run /sbin/init as init process > > > > Starting init: /sbin/init exists but couldn't execute it (error -14) > > > > Run /etc/init as init process > > > > ... > > > > > > > > Cc: stable@vger.kernel.org > > > > Fixes: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > > > --- > > > > arch/riscv/include/asm/pgtable.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h > > > > index ab00235b018f..74ffb2178f54 100644 > > > > --- a/arch/riscv/include/asm/pgtable.h > > > > +++ b/arch/riscv/include/asm/pgtable.h > > > > @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) > > > > #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) > > > > > > > > #ifdef CONFIG_COMPAT > > > > -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) > > > > +#define TASK_SIZE_32 (_AC(0x80000000, UL)) > > > > > > > > > > > > > > > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > > > > TASK_SIZE_32 : TASK_SIZE_64) > > > > #else > > > > -- > > > > 2.40.1 > > > > > > > > > > > > > -- > > Best Regards > > Guo Ren
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h index ab00235b018f..74ffb2178f54 100644 --- a/arch/riscv/include/asm/pgtable.h +++ b/arch/riscv/include/asm/pgtable.h @@ -881,7 +881,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte) #define TASK_SIZE_MIN (PGDIR_SIZE_L3 * PTRS_PER_PGD / 2) #ifdef CONFIG_COMPAT -#define TASK_SIZE_32 (_AC(0x80000000, UL) - PAGE_SIZE) +#define TASK_SIZE_32 (_AC(0x80000000, UL)) #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ TASK_SIZE_32 : TASK_SIZE_64) #else