riscv: mm: Fixup & Optimize COMPAT code

PATCH Superseded [email protected] 4 participants 13 messages Dec 22, 2023 – Jan 24, 2024

Subsystems (2)

Stable branch RISC-V

lore

Share & tools

lore

[email protected]

author

2023-12-22 11:57 UTC #0 lore raw

v3 cover riscv: mm: Fixup & Optimize COMPAT code

From: Guo Ren <[email protected]>

When the task is in COMPAT mode, the TASK_SIZE should be 2GB, so STACK_TOP_MAX and arch_get_mmap_end must be limited to 2 GB. This series fixes the problem made by commit: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") and optimizes the related coding convention of TASK_SIZE.

Changelog:
v3:
- Remove wrong modification of arch_get_mmap_end
- Remove is_compat_task optimization
- Update comment log

v2:
https://lore.kernel.org/linux-riscv/[email protected]/
- Separate rename from fixup
- Add STACK_TOP_MAX fixup for compat
- Add Cleanup & rename patches

v1:
https://lore.kernel.org/linux-riscv/[email protected]/

Guo Ren (4):
riscv: mm: Fixup compat mode boot failure
riscv: mm: Fixup compat arch_get_mmap_end
riscv: mm: Remove unused TASK_SIZE_MIN
riscv: mm: Optimize TASK_SIZE definition

 arch/riscv/include/asm/pgtable.h   | 7 +++----
 arch/riscv/include/asm/processor.h | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

Signature · 1 line

2.40.1

[email protected]

author

2023-12-22 11:57 UTC #1 lore raw

PATCH v3 1/4 riscv: mm: Fixup compat mode boot failure

From: Guo Ren <[email protected]>

In COMPAT mode, the STACK_TOP is DEFAULT_MAP_WINDOW (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
...

Increase the TASK_SIZE to cover STACK_TOP.

Cc [email protected]

Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/riscv/include/asm/pgtable.h +1 -1

index ab00235b018f..74ffb2178f54 100644
@@ -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

Signature · 1 line

2.40.1

Leonardo Bras

Reviewed-by on v3 1/4

2023-12-23 02:58 UTC #2 lore raw

Re: [PATCH V3 1/4] riscv: mm: Fixup compat mode boot failure

Quoted 39 lines · On Fri, Dec 22, 2023 at 06:57:00AM -0500, [email protected] wrote:

From: Guo Ren <[email protected]>
In COMPAT mode, the STACK_TOP is DEFAULT_MAP_WINDOW (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
...
Increase the TASK_SIZE to cover STACK_TOP.
Cc [email protected]
Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
---
 arch/riscv/include/asm/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/pgtable.h +1 -1
index ab00235b018f..74ffb2178f54 100644
@@ -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
Signature · 1 line
2.40.1

I am not really involved in the issue this is solving, so I have no technical opinion on the solution.

IIUC there should always be (TASK_SIZE >= STACK_TOP), so by itself this is fixing an issue.

I have reviewed the code and it does exactly as stated into the commit message, so FWIW:

Reviewed-by Leonardo Bras <[email protected]>

Guo Ren

author

2023-12-23 03:07 UTC #3 lore raw

Re: [PATCH V3 1/4] riscv: mm: Fixup compat mode boot failure

Quoted 45 lines · Leonardo Bras

Quoted 37 lines · On Fri, Dec 22, 2023 at 06:57:00AM -0500, [email protected] wrote:
From: Guo Ren <[email protected]>
In COMPAT mode, the STACK_TOP is DEFAULT_MAP_WINDOW (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
...
Increase the TASK_SIZE to cover STACK_TOP.
Cc [email protected]
Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
---
 arch/riscv/include/asm/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/pgtable.h +1 -2
index ab00235b018f..74ffb2178f54 100644
@@ -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
--
Quoted 2 lines
2.40.1
I am not really involved in the issue this is solving, so I have no technical opinion on the solution.

Quoted 6 lines

IIUC there should always be (TASK_SIZE >= STACK_TOP), so by itself this is fixing an issue.
I have reviewed the code and it does exactly as stated into the commit message, so FWIW:
Reviewed-by Leonardo Bras <[email protected]>

Thx,

I found this problem because it can't boot my rv32 buildroot-rootfs in v6.6. But it's okay in v6.5. So I used git bisect and found commit:
add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57"), which caused that.

Ping Charlie, I hope it can be fixed in the v6.6 long-term version.

Quoted 1 line

--
Best Regards
Guo Ren

Charlie Jenkins

Reviewed-by

2023-12-23 03:36 UTC #4 lore raw

Re: [PATCH V3 1/4] riscv: mm: Fixup compat mode boot failure

Quoted 52 lines · Guo Ren

Quoted 45 lines · Leonardo Bras
Quoted 37 lines · On Fri, Dec 22, 2023 at 06:57:00AM -0500, [email protected] wrote:
From: Guo Ren <[email protected]>
In COMPAT mode, the STACK_TOP is DEFAULT_MAP_WINDOW (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
...
Increase the TASK_SIZE to cover STACK_TOP.
Cc [email protected]
Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
---
 arch/riscv/include/asm/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/pgtable.h +1 -2
index ab00235b018f..74ffb2178f54 100644
@@ -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
--
Quoted 2 lines
2.40.1
I am not really involved in the issue this is solving, so I have no technical opinion on the solution.
Quoted 6 lines
IIUC there should always be (TASK_SIZE >= STACK_TOP), so by itself this is fixing an issue.
I have reviewed the code and it does exactly as stated into the commit message, so FWIW:
Reviewed-by Leonardo Bras <[email protected]>

Quoted 8 lines

Thx,
I found this problem because it can't boot my rv32 buildroot-rootfs in v6.6. But it's okay in v6.5. So I used git bisect and found commit:
add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57"), which caused that.
Ping Charlie, I hope it can be fixed in the v6.6 long-term version.

I have looked at this more I do agree that this change makes sense. I was thinking that it was valid to have STACK_TOP that was greater than TASK_SIZE but I am no longer convinced that is true. Thank you for looking into this.

Reviewed-by Charlie Jenkins <[email protected]>

Quoted 8 lines

Quoted 2 lines
--
Best Regards
Guo Ren

[email protected]

author

2023-12-22 11:57 UTC #5 lore raw

PATCH v3 2/4 riscv: mm: Fixup compat arch_get_mmap_end

In reply to [email protected]

From: Guo Ren <[email protected]>

When the task is in COMPAT mode, the arch_get_mmap_end should be 2GB, not TASK_SIZE_64. The TASK_SIZE has contained is_compat_mode() detection, so change the definition of STACK_TOP_MAX to TASK_SIZE directly.

Cc [email protected]

Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/riscv/include/asm/processor.h +1 -1

index f19f861cda54..e1944ff0757a 100644
@@ -16,7 +16,7 @@
 
 #ifdef CONFIG_64BIT
 #define DEFAULT_MAP_WINDOW	(UL(1) << (MMAP_VA_BITS - 1))
-#define STACK_TOP_MAX		TASK_SIZE_64
+#define STACK_TOP_MAX		TASK_SIZE
 
 #define arch_get_mmap_end(addr, len, flags)			\
 ({								\

Signature · 1 line

2.40.1

Leonardo Bras

Reviewed-by on v3 2/4

2023-12-23 02:52 UTC #6 lore raw

Re: [PATCH V3 2/4] riscv: mm: Fixup compat arch_get_mmap_end

Quoted 31 lines · On Fri, Dec 22, 2023 at 06:57:01AM -0500, [email protected] wrote:

From: Guo Ren <[email protected]>
When the task is in COMPAT mode, the arch_get_mmap_end should be 2GB, not TASK_SIZE_64. The TASK_SIZE has contained is_compat_mode() detection, so change the definition of STACK_TOP_MAX to TASK_SIZE directly.
Cc [email protected]
Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
---
 arch/riscv/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/processor.h +1 -1
index f19f861cda54..e1944ff0757a 100644
@@ -16,7 +16,7 @@
 
 #ifdef CONFIG_64BIT
 #define DEFAULT_MAP_WINDOW	(UL(1) << (MMAP_VA_BITS - 1))
-#define STACK_TOP_MAX		TASK_SIZE_64
+#define STACK_TOP_MAX		TASK_SIZE
 
 #define arch_get_mmap_end(addr, len, flags)			\
 ({								\
Signature · 1 line
2.40.1

LGTM.

FWIW:

Reviewed-by Leonardo Bras <[email protected]>

Charlie Jenkins

Reviewed-by

2023-12-23 03:39 UTC #7 lore raw

Re: [PATCH V3 2/4] riscv: mm: Fixup compat arch_get_mmap_end

Quoted 36 lines · Leonardo Bras

Quoted 31 lines · On Fri, Dec 22, 2023 at 06:57:01AM -0500, [email protected] wrote:
From: Guo Ren <[email protected]>
When the task is in COMPAT mode, the arch_get_mmap_end should be 2GB, not TASK_SIZE_64. The TASK_SIZE has contained is_compat_mode() detection, so change the definition of STACK_TOP_MAX to TASK_SIZE directly.
Cc [email protected]
Fixes add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57")
Signed-off-by Guo Ren <[email protected]>
Signed-off-by Guo Ren <[email protected]>
---
 arch/riscv/include/asm/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/processor.h +1 -1
index f19f861cda54..e1944ff0757a 100644
@@ -16,7 +16,7 @@
 
 #ifdef CONFIG_64BIT
 #define DEFAULT_MAP_WINDOW	(UL(1) << (MMAP_VA_BITS - 1))
-#define STACK_TOP_MAX		TASK_SIZE_64
+#define STACK_TOP_MAX		TASK_SIZE
 
 #define arch_get_mmap_end(addr, len, flags)			\
 ({								\
Signature · 1 line
2.40.1
LGTM.

Quoted 2 lines

FWIW:
Reviewed-by Leonardo Bras <[email protected]>

Reviewed-by Charlie Jenkins <[email protected]>

Quoted 1 line

[email protected]

author

2023-12-22 11:57 UTC #8 lore raw

PATCH v3 3/4 riscv: mm: Remove unused TASK_SIZE_MIN

In reply to [email protected]

From: Guo Ren <[email protected]>

Remove TASK_SIZE_MIN because it's not used anymore.

Reviewed-by Leonardo Bras <[email protected]>

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/pgtable.h | 2 --
 1 file changed, 2 deletions(-)

arch/riscv/include/asm/pgtable.h +0 -2

index 74ffb2178f54..e415582276ec 100644
@@ -878,7 +878,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  */
 #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))

@@ -890,7 +889,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
 
 #else
 #define TASK_SIZE	FIXADDR_START
-#define TASK_SIZE_MIN	TASK_SIZE
 #endif
 
 #else /* CONFIG_MMU */

Signature · 1 line

2.40.1

Charlie Jenkins

Reviewed-by on v3 3/4

2024-01-24 18:35 UTC #9 lore raw

Re: [PATCH V3 3/4] riscv: mm: Remove unused TASK_SIZE_MIN

Quoted 34 lines · On Fri, Dec 22, 2023 at 06:57:02AM -0500, [email protected] wrote:

From: Guo Ren <[email protected]>

Remove TASK_SIZE_MIN because it's not used anymore.

Reviewed-by Leonardo Bras <[email protected]>

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/pgtable.h | 2 --
 1 file changed, 2 deletions(-)

arch/riscv/include/asm/pgtable.h +0 -2

index 74ffb2178f54..e415582276ec 100644
@@ -878,7 +878,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  */
 #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))

@@ -890,7 +889,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
 
 #else
 #define TASK_SIZE	FIXADDR_START
-#define TASK_SIZE_MIN	TASK_SIZE
 #endif
 
 #else /* CONFIG_MMU */

Signature · 1 line

2.40.1

Reviewed-by Charlie Jenkins <[email protected]>

[email protected]

author

2023-12-22 11:57 UTC #10 lore raw

PATCH v3 4/4 riscv: mm: Optimize TASK_SIZE definition

In reply to [email protected]

From: Guo Ren <[email protected]>

Unify the TASK_SIZE definition with VA_BITS for better readability. Add COMPAT mode user address space info in the comment.

Reviewed-by Leonardo Bras <[email protected]>

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/pgtable.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

arch/riscv/include/asm/pgtable.h +3 -2

index e415582276ec..271565e1956f 100644
@@ -866,6 +866,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  * Note that PGDIR_SIZE must evenly divide TASK_SIZE.
  * Task size is:
  * -        0x9fc00000	(~2.5GB) for RV32.
+ * -        0x80000000	(   2GB) for RV64 compat mode
  * -      0x4000000000	( 256GB) for RV64 using SV39 mmu
  * -    0x800000000000	( 128TB) for RV64 using SV48 mmu
  * - 0x100000000000000	(  64PB) for RV64 using SV57 mmu

@@ -877,10 +878,10 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  * Similarly for SV57, bits 63–57 must be equal to bit 56.
  */
 #ifdef CONFIG_64BIT
-#define TASK_SIZE_64	(PGDIR_SIZE * PTRS_PER_PGD / 2)
+#define TASK_SIZE_64	(UL(1) << (VA_BITS - 1))
 
 #ifdef CONFIG_COMPAT
-#define TASK_SIZE_32	(_AC(0x80000000, UL))
+#define TASK_SIZE_32	(UL(1) << (VA_BITS_SV32 - 1))
 #define TASK_SIZE	(test_thread_flag(TIF_32BIT) ? \
 			 TASK_SIZE_32 : TASK_SIZE_64)
 #else

Signature · 1 line

2.40.1

Charlie Jenkins

Reviewed-by on v3 4/4

2024-01-24 18:36 UTC #11 lore raw

Re: [PATCH V3 4/4] riscv: mm: Optimize TASK_SIZE definition

Quoted 40 lines · On Fri, Dec 22, 2023 at 06:57:03AM -0500, [email protected] wrote:

From: Guo Ren <[email protected]>

Unify the TASK_SIZE definition with VA_BITS for better readability. Add COMPAT mode user address space info in the comment.

Reviewed-by Leonardo Bras <[email protected]>

Signed-off-by Guo Ren <[email protected]>

---
 arch/riscv/include/asm/pgtable.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

arch/riscv/include/asm/pgtable.h +3 -2

index e415582276ec..271565e1956f 100644
@@ -866,6 +866,7 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  * Note that PGDIR_SIZE must evenly divide TASK_SIZE.
  * Task size is:
  * -        0x9fc00000	(~2.5GB) for RV32.
+ * -        0x80000000	(   2GB) for RV64 compat mode
  * -      0x4000000000	( 256GB) for RV64 using SV39 mmu
  * -    0x800000000000	( 128TB) for RV64 using SV48 mmu
  * - 0x100000000000000	(  64PB) for RV64 using SV57 mmu

@@ -877,10 +878,10 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
  * Similarly for SV57, bits 63–57 must be equal to bit 56.
  */
 #ifdef CONFIG_64BIT
-#define TASK_SIZE_64	(PGDIR_SIZE * PTRS_PER_PGD / 2)
+#define TASK_SIZE_64	(UL(1) << (VA_BITS - 1))
 
 #ifdef CONFIG_COMPAT
-#define TASK_SIZE_32	(_AC(0x80000000, UL))
+#define TASK_SIZE_32	(UL(1) << (VA_BITS_SV32 - 1))
 #define TASK_SIZE	(test_thread_flag(TIF_32BIT) ? \
 			 TASK_SIZE_32 : TASK_SIZE_64)
 #else

Signature · 1 line

2.40.1

Reviewed-by Charlie Jenkins <[email protected]>

[email protected]

2024-01-12 06:30 UTC #12 lore raw

In reply to [email protected]

This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Hello:

This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Quoted 9 lines · On Fri, 22 Dec 2023 06:56:59 -0500 you wrote:

From: Guo Ren <[email protected]>
When the task is in COMPAT mode, the TASK_SIZE should be 2GB, so STACK_TOP_MAX and arch_get_mmap_end must be limited to 2 GB. This series fixes the problem made by commit: add2cc6b6515 ("RISC-V: mm: Restrict address space for sv39,sv48,sv57") and optimizes the related coding convention of TASK_SIZE.
[...]

Here is the summary with links:
- [V3,1/4] riscv: mm: Fixup compat mode boot failure

    https://git.kernel.org/riscv/c/5f449e245e5b
  - [V3,2/4] riscv: mm: Fixup compat arch_get_mmap_end
    https://git.kernel.org/riscv/c/97b7ac69be2e

- [V3,3/4] riscv: mm: Remove unused TASK_SIZE_MIN (no matching commit)
- [V3,4/4] riscv: mm: Optimize TASK_SIZE definition (no matching commit)

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html