[PATCH v2 0/2] riscv: access_ok() optimization

[PATCH v2 0/2] riscv: access_ok() optimization

[Samuel Holland]

This series optimizes access_ok() by defining TASK_SIZE_MAX. At Alex's
suggestion, I also tried making TASK_SIZE constant (specifically by
making PGDIR_SHIFT a variable instead of a ternary expression, then
replacing the load with an immediate using ALTERNATIVE). This appeared
to slightly improve performance on some implementations (C906) but
regressed it on others (FU740). So I am leaving further optimizations to
a later series.

Changes in v2:
 - Add a patch removing PGDIR_SIZE_L3 and TASK_SIZE_MIN
 - Set TASK_SIZE_MAX to LONG_MAX to optimize the comparison
 - Reword the commit message

Samuel Holland (2):
  riscv: Remove PGDIR_SIZE_L3 and TASK_SIZE_MIN
  riscv: Define TASK_SIZE_MAX for __access_ok()

 arch/riscv/include/asm/pgtable-64.h | 2 --
 arch/riscv/include/asm/pgtable.h    | 3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

-- 
2.43.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/2] riscv: Remove PGDIR_SIZE_L3 and TASK_SIZE_MIN

[Samuel Holland]

TASK_SIZE_MIN is unused since commit 085e2ff9aeb0 ("efi: libstub: Drop
randomization of runtime memory map"). PGDIR_SIZE_L3 is only used in the
definition of TASK_SIZE_MIN.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

Changes in v2:
 - New patch for v2

 arch/riscv/include/asm/pgtable-64.h | 2 --
 arch/riscv/include/asm/pgtable.h    | 2 --
 2 files changed, 4 deletions(-)

diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 221a5c1ee287..8c36a8818432 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -16,8 +16,6 @@ extern bool pgtable_l5_enabled;
 #define PGDIR_SHIFT_L3  30
 #define PGDIR_SHIFT_L4  39
 #define PGDIR_SHIFT_L5  48
-#define PGDIR_SIZE_L3   (_AC(1, UL) << PGDIR_SHIFT_L3)
-
 #define PGDIR_SHIFT     (pgtable_l5_enabled ? PGDIR_SHIFT_L5 : \
 		(pgtable_l4_enabled ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3))
 /* Size of region mapped by a page global directory */
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 97fcde30e247..f5cc8bcc7f8d 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -870,7 +870,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) - PAGE_SIZE)
@@ -882,7 +881,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 */
-- 
2.43.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/2] riscv: Remove PGDIR_SIZE_L3 and TASK_SIZE_MIN

[Arnd Bergmann]

On Wed, Mar 27, 2024, at 15:38, Samuel Holland wrote:
> TASK_SIZE_MIN is unused since commit 085e2ff9aeb0 ("efi: libstub: Drop
> randomization of runtime memory map"). PGDIR_SIZE_L3 is only used in the
> definition of TASK_SIZE_MIN.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/2] riscv: Remove PGDIR_SIZE_L3 and TASK_SIZE_MIN

[Alexandre Ghiti]

Hi Samuel,

On 27/03/2024 15:38, Samuel Holland wrote:
> TASK_SIZE_MIN is unused since commit 085e2ff9aeb0 ("efi: libstub: Drop
> randomization of runtime memory map"). PGDIR_SIZE_L3 is only used in the
> definition of TASK_SIZE_MIN.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
> Changes in v2:
>   - New patch for v2
>
>   arch/riscv/include/asm/pgtable-64.h | 2 --
>   arch/riscv/include/asm/pgtable.h    | 2 --
>   2 files changed, 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 221a5c1ee287..8c36a8818432 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -16,8 +16,6 @@ extern bool pgtable_l5_enabled;
>   #define PGDIR_SHIFT_L3  30
>   #define PGDIR_SHIFT_L4  39
>   #define PGDIR_SHIFT_L5  48
> -#define PGDIR_SIZE_L3   (_AC(1, UL) << PGDIR_SHIFT_L3)
> -
>   #define PGDIR_SHIFT     (pgtable_l5_enabled ? PGDIR_SHIFT_L5 : \
>   		(pgtable_l4_enabled ? PGDIR_SHIFT_L4 : PGDIR_SHIFT_L3))
>   /* Size of region mapped by a page global directory */
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 97fcde30e247..f5cc8bcc7f8d 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -870,7 +870,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) - PAGE_SIZE)
> @@ -882,7 +881,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 */


I noticed loongarch still had this definition, but up to you to remove 
it too :)

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/2] riscv: Define TASK_SIZE_MAX for __access_ok()

[Samuel Holland]

TASK_SIZE_MAX should be set to a constant value, at least the largest
valid userspace address under any runtime configuration. This optimizes
the check in __access_ok(), which no longer needs to compute the runtime
value of TASK_SIZE. The check does not need to be exact, as long as it
accepts all valid userspace addresses and rejects all valid kernel
addresses; well-behaved programs will never fail the access_ok() check.

For RISC-V, which requires all virtual addresses to be sign extended,
the optimal choice is LONG_MAX because it simplifies the limit
comparison to a sign bit test.

This removes about half of the references to pgtable_l[45]_enabled.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
---

Changes in v2:
 - Set TASK_SIZE_MAX to LONG_MAX to optimize the comparison
 - Reword the commit message

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

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index f5cc8bcc7f8d..762a85551764 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -870,6 +870,7 @@ 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_MAX	LONG_MAX
 
 #ifdef CONFIG_COMPAT
 #define TASK_SIZE_32	(_AC(0x80000000, UL) - PAGE_SIZE)
-- 
2.43.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] riscv: Define TASK_SIZE_MAX for __access_ok()

[Arnd Bergmann]

On Wed, Mar 27, 2024, at 15:38, Samuel Holland wrote:
> TASK_SIZE_MAX should be set to a constant value, at least the largest
> valid userspace address under any runtime configuration. This optimizes
> the check in __access_ok(), which no longer needs to compute the runtime
> value of TASK_SIZE. The check does not need to be exact, as long as it
> accepts all valid userspace addresses and rejects all valid kernel
> addresses; well-behaved programs will never fail the access_ok() check.
>
> For RISC-V, which requires all virtual addresses to be sign extended,
> the optimal choice is LONG_MAX because it simplifies the limit
> comparison to a sign bit test.
>
> This removes about half of the references to pgtable_l[45]_enabled.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/2] riscv: Define TASK_SIZE_MAX for __access_ok()

[Alexandre Ghiti]

On 27/03/2024 15:38, Samuel Holland wrote:
> TASK_SIZE_MAX should be set to a constant value, at least the largest
> valid userspace address under any runtime configuration. This optimizes
> the check in __access_ok(), which no longer needs to compute the runtime
> value of TASK_SIZE. The check does not need to be exact, as long as it
> accepts all valid userspace addresses and rejects all valid kernel
> addresses; well-behaved programs will never fail the access_ok() check.
>
> For RISC-V, which requires all virtual addresses to be sign extended,
> the optimal choice is LONG_MAX because it simplifies the limit
> comparison to a sign bit test.
>
> This removes about half of the references to pgtable_l[45]_enabled.
>
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
>
> Changes in v2:
>   - Set TASK_SIZE_MAX to LONG_MAX to optimize the comparison
>   - Reword the commit message
>
>   arch/riscv/include/asm/pgtable.h | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index f5cc8bcc7f8d..762a85551764 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -870,6 +870,7 @@ 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_MAX	LONG_MAX
>   
>   #ifdef CONFIG_COMPAT
>   #define TASK_SIZE_32	(_AC(0x80000000, UL) - PAGE_SIZE)


You can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 0/2] riscv: access_ok() optimization

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Wed, 27 Mar 2024 07:38:11 -0700 you wrote:
> This series optimizes access_ok() by defining TASK_SIZE_MAX. At Alex's
> suggestion, I also tried making TASK_SIZE constant (specifically by
> making PGDIR_SHIFT a variable instead of a ternary expression, then
> replacing the load with an immediate using ALTERNATIVE). This appeared
> to slightly improve performance on some implementations (C906) but
> regressed it on others (FU740). So I am leaving further optimizations to
> a later series.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] riscv: Remove PGDIR_SIZE_L3 and TASK_SIZE_MIN
    https://git.kernel.org/riscv/c/9ad6bb3298d1
  - [v2,2/2] riscv: Define TASK_SIZE_MAX for __access_ok()
    https://git.kernel.org/riscv/c/ad5643cf2f69

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv