mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] arm64: refactor the rodata=xxx
@ 2024-12-09  7:20 Huang Shijie
  2024-12-09  7:20 ` [PATCH v3 1/2] " Huang Shijie
  2024-12-09  7:20 ` [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
  0 siblings, 2 replies; 10+ messages in thread
From: Huang Shijie @ 2024-12-09  7:20 UTC (permalink / raw)
  To: catalin.marinas, will, anshuman.khandual, corbet
  Cc: patches, cl, akpm, thuth, rostedt, xiongwei.song, ardb, inux-doc,
	linux-kernel, linux-arm-kernel, Huang Shijie

From Documentation/admin-guide/kernel-parameters.txt, we know that:
 rodata=	[KNL,EARLY]
	on	Mark read-only kernel memory as read-only (default).
	off	Leave read-only kernel memory writable for debugging.
	full	Mark read-only kernel memory and aliases as read-only
		[arm64]

So the "rodata=on" is the default.

But the current code does not follow the document, it makes "rodata=full"
as the default.

This patch set follows Anshuman Khandual's suggetions. 
It makes the "rodata=on" as the default, and removes the CONFIG_RODATA_FULL_DEFAULT_ENABLED.

v3:
  Follows Anshuman Khandual's suggetions:
  - Merge patch 1 and patch 3 into one patch.
  - Remove patch 4 
  - update comments and document.

v2:
  Follows Will's suggetions.
  Add a new file fine-tuning-tips.rst for the expert users.
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-November/981190.html

v1:
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-October/971415.html


Huang Shijie (2):
  arm64: refactor the rodata=xxx
  arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED

 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/Kconfig                            | 14 -----------
 arch/arm64/include/asm/setup.h                | 25 +++++++++++++++++--
 arch/arm64/mm/pageattr.c                      |  2 +-
 4 files changed, 25 insertions(+), 18 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-09  7:20 [PATCH v3 0/2] arm64: refactor the rodata=xxx Huang Shijie
@ 2024-12-09  7:20 ` Huang Shijie
  2024-12-09 17:05   ` Christoph Lameter (Ampere)
  2024-12-10  7:17   ` Anshuman Khandual
  2024-12-09  7:20 ` [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
  1 sibling, 2 replies; 10+ messages in thread
From: Huang Shijie @ 2024-12-09  7:20 UTC (permalink / raw)
  To: catalin.marinas, will, anshuman.khandual, corbet
  Cc: patches, cl, akpm, thuth, rostedt, xiongwei.song, ardb, inux-doc,
	linux-kernel, linux-arm-kernel, Huang Shijie

From Documentation/admin-guide/kernel-parameters.txt:
	rodata=		[KNL,EARLY]
		on	Mark read-only kernel memory as read-only (default).

So "rodata=on" is the default.

But the current code does not follow the document, it makes "rodata=full"
as the default.

This patch refactors the "rodata=xxx" :
      1.) Make rodata=on behaviour be the original rodata=full.
          (Drop the original rodata=full.)
      2.) Introduce "rodata=noalias" which is the original "rodata=on"
      3.) Add more comment for arch_parse_debug_rodata(),
          update kernel-parameters.txt.

After this patch, the rodata=on will be the default.

Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/include/asm/setup.h                | 25 +++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a22b7e621007..51bce7b9d805 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5901,7 +5901,7 @@
 	rodata=		[KNL,EARLY]
 		on	Mark read-only kernel memory as read-only (default).
 		off	Leave read-only kernel memory writable for debugging.
-		full	Mark read-only kernel memory and aliases as read-only
+		noalias	Use more block mappings,may have better performance.
 		        [arm64]
 
 	rockchip.usb_uart
diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
index ba269a7a3201..b3063326b79a 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -13,6 +13,27 @@
 extern phys_addr_t __fdt_pointer __initdata;
 extern u64 __cacheline_aligned boot_args[4];
 
+/*
+ * rodata=on (default):
+ *    Apply read-only attributes of VM areas to the linear alias of
+ *    the backing pages as well. This prevents code or read-only data
+ *    from being modified (inadvertently or intentionally) via another
+ *    mapping of the same memory page.
+ *
+ *    This requires the linear region to be mapped down to pages,
+ *    which may adversely affect performance in some cases.
+ *
+ * rodata=off:
+ *    It provides us more block mappings and contiguous hits
+ *    to map the linear region which minimize the TLB footprint.
+ *    Leave read-only kernel memory writable for debugging.
+ *
+ * rodata=noalias:
+ *    It provides us more block mappings and contiguous hits
+ *    to map the linear region which minimize the TLB footprint.
+ *    And the linear aliases of pages belonging to read-only mappings
+ *    in vmalloc region are also marked as read-only.
+ */
 static inline bool arch_parse_debug_rodata(char *arg)
 {
 	extern bool rodata_enabled;
@@ -21,7 +42,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 	if (!arg)
 		return false;
 
-	if (!strcmp(arg, "full")) {
+	if (!strcmp(arg, "on")) {
 		rodata_enabled = rodata_full = true;
 		return true;
 	}
@@ -31,7 +52,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 		return true;
 	}
 
-	if (!strcmp(arg, "on")) {
+	if (!strcmp(arg, "noalias")) {
 		rodata_enabled = true;
 		rodata_full = false;
 		return true;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED
  2024-12-09  7:20 [PATCH v3 0/2] arm64: refactor the rodata=xxx Huang Shijie
  2024-12-09  7:20 ` [PATCH v3 1/2] " Huang Shijie
@ 2024-12-09  7:20 ` Huang Shijie
  2024-12-10  7:27   ` Anshuman Khandual
  1 sibling, 1 reply; 10+ messages in thread
From: Huang Shijie @ 2024-12-09  7:20 UTC (permalink / raw)
  To: catalin.marinas, will, anshuman.khandual, corbet
  Cc: patches, cl, akpm, thuth, rostedt, xiongwei.song, ardb, inux-doc,
	linux-kernel, linux-arm-kernel, Huang Shijie

After patch "arm64: refacotr the rodata=xxx",
the "rodata=on" becomes the default.

     ......................................
	if (!strcmp(arg, "on")) {
		rodata_enabled = rodata_full = true;
		return true;
	}
     ......................................

The rodata_full is always "true" via "rodata=on" and does not
depend on the config RODATA_FULL_DEFAULT_ENABLED anymore,
so it can be dropped.

Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
 arch/arm64/Kconfig       | 14 --------------
 arch/arm64/mm/pageattr.c |  2 +-
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cbfd357f94a6..1c69982302ed 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1666,20 +1666,6 @@ config MITIGATE_SPECTRE_BRANCH_HISTORY
 	  When taking an exception from user-space, a sequence of branches
 	  or a firmware call overwrites the branch history.
 
-config RODATA_FULL_DEFAULT_ENABLED
-	bool "Apply r/o permissions of VM areas also to their linear aliases"
-	default y
-	help
-	  Apply read-only attributes of VM areas to the linear alias of
-	  the backing pages as well. This prevents code or read-only data
-	  from being modified (inadvertently or intentionally) via another
-	  mapping of the same memory page. This additional enhancement can
-	  be turned off at runtime by passing rodata=[off|on] (and turned on
-	  with rodata=full if this option is set to 'n')
-
-	  This requires the linear region to be mapped down to pages,
-	  which may adversely affect performance in some cases.
-
 config ARM64_SW_TTBR0_PAN
 	bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
 	depends on !KCSAN
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 39fd1f7ff02a..6eef08d8451e 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -20,7 +20,7 @@ struct page_change_data {
 	pgprot_t clear_mask;
 };
 
-bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
+bool rodata_full __ro_after_init = true;
 
 bool can_set_direct_map(void)
 {
-- 
2.40.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-09  7:20 ` [PATCH v3 1/2] " Huang Shijie
@ 2024-12-09 17:05   ` Christoph Lameter (Ampere)
  2024-12-10  3:22     ` Shijie Huang
  2024-12-10  7:17   ` Anshuman Khandual
  1 sibling, 1 reply; 10+ messages in thread
From: Christoph Lameter (Ampere) @ 2024-12-09 17:05 UTC (permalink / raw)
  To: Huang Shijie
  Cc: catalin.marinas, will, anshuman.khandual, corbet, patches, akpm,
	thuth, rostedt, xiongwei.song, ardb, inux-doc, linux-kernel,
	linux-arm-kernel

On Mon, 9 Dec 2024, Huang Shijie wrote:

> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5901,7 +5901,7 @@
>  	rodata=		[KNL,EARLY]
>  		on	Mark read-only kernel memory as read-only (default).
>  		off	Leave read-only kernel memory writable for debugging.
> -		full	Mark read-only kernel memory and aliases as read-only
> +		noalias	Use more block mappings,may have better performance.
>  		        [arm64]

Maybe use
		noalias Do not check aliases in order to allow larger kernel page
                        sizes on a platforms without FEAT_BBM2 support[arm64])

instead?

>
>  	rockchip.usb_uart
> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> index ba269a7a3201..b3063326b79a 100644
> --- a/arch/arm64/include/asm/setup.h
> +++ b/arch/arm64/include/asm/setup.h
> @@ -13,6 +13,27 @@
>  extern phys_addr_t __fdt_pointer __initdata;
>  extern u64 __cacheline_aligned boot_args[4];
>
> +/*
> + * rodata=on (default):
> + *    Apply read-only attributes of VM areas to the linear alias of
> + *    the backing pages as well. This prevents code or read-only data
> + *    from being modified (inadvertently or intentionally) via another
> + *    mapping of the same memory page.
> + *
> + *    This requires the linear region to be mapped down to pages,
> + *    which may adversely affect performance in some cases.
> + *
> + * rodata=off:
> + *    It provides us more block mappings and contiguous hits
> + *    to map the linear region which minimize the TLB footprint.
> + *    Leave read-only kernel memory writable for debugging.
> + *
> + * rodata=noalias:
> + *    It provides us more block mappings and contiguous hits
> + *    to map the linear region which minimize the TLB footprint.
> + *    And the linear aliases of pages belonging to read-only mappings
> + *    in vmalloc region are also marked as read-only.
> + */
>  static inline bool arch_parse_debug_rodata(char *arg)
>  {
>  	extern bool rodata_enabled;
> @@ -21,7 +42,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
>  	if (!arg)
>  		return false;
>
> -	if (!strcmp(arg, "full")) {
> +	if (!strcmp(arg, "on")) {
>  		rodata_enabled = rodata_full = true;
>  		return true;
>  	}
> @@ -31,7 +52,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
>  		return true;
>  	}
>
> -	if (!strcmp(arg, "on")) {
> +	if (!strcmp(arg, "noalias")) {
>  		rodata_enabled = true;
>  		rodata_full = false;
>  		return true;
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-09 17:05   ` Christoph Lameter (Ampere)
@ 2024-12-10  3:22     ` Shijie Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Shijie Huang @ 2024-12-10  3:22 UTC (permalink / raw)
  To: Christoph Lameter (Ampere), Huang Shijie
  Cc: catalin.marinas, will, anshuman.khandual, corbet, patches, akpm,
	thuth, rostedt, xiongwei.song, ardb, inux-doc, linux-kernel,
	linux-arm-kernel


On 2024/12/10 1:05, Christoph Lameter (Ampere) wrote:
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5901,7 +5901,7 @@
>>   	rodata=		[KNL,EARLY]
>>   		on	Mark read-only kernel memory as read-only (default).
>>   		off	Leave read-only kernel memory writable for debugging.
>> -		full	Mark read-only kernel memory and aliases as read-only
>> +		noalias	Use more block mappings,may have better performance.
>>   		        [arm64]
> Maybe use
> 		noalias Do not check aliases in order to allow larger kernel page
>                          sizes on a platforms without FEAT_BBM2 support[arm64])
>
> instead?

The "rodata=noalias" here is equal to original "rodata=on".

But Yang Shi's patch set works with original "rodata=full" :

https://lists.infradead.org/pipermail/linux-arm-kernel/2024-November/979770.html


IMHO, it's not proper to mention the FEAT_BBM in "rodata=noalias".



Thanks

Huang Shijie




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-09  7:20 ` [PATCH v3 1/2] " Huang Shijie
  2024-12-09 17:05   ` Christoph Lameter (Ampere)
@ 2024-12-10  7:17   ` Anshuman Khandual
  2024-12-10  7:21     ` Ard Biesheuvel
  1 sibling, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2024-12-10  7:17 UTC (permalink / raw)
  To: Huang Shijie, catalin.marinas, will, corbet
  Cc: patches, cl, akpm, thuth, rostedt, xiongwei.song, ardb, inux-doc,
	linux-kernel, linux-arm-kernel

Hello Huang,

On 12/9/24 12:50, Huang Shijie wrote:
>>From Documentation/admin-guide/kernel-parameters.txt:
> 	rodata=		[KNL,EARLY]
> 		on	Mark read-only kernel memory as read-only (default).
> 
> So "rodata=on" is the default.
> 
> But the current code does not follow the document, it makes "rodata=full"
> as the default.
> 
> This patch refactors the "rodata=xxx" :
>       1.) Make rodata=on behaviour be the original rodata=full.
>           (Drop the original rodata=full.)
>       2.) Introduce "rodata=noalias" which is the original "rodata=on"
>       3.) Add more comment for arch_parse_debug_rodata(),
>           update kernel-parameters.txt.
> 
> After this patch, the rodata=on will be the default.

Could this commit message be reworded/reformatted something like the
following instead ? Although feel free to improve this as required.

-----------------------------------------------------------------------
As per admin guide documentation, "rodata=on" should be the default on
platforms. Documentation/admin-guide/kernel-parameters.txt describes
these options as

        rodata=         [KNL,EARLY]
                on      Mark read-only kernel memory as read-only (default).
                off     Leave read-only kernel memory writable for debugging.
                full    Mark read-only kernel memory and aliases as read-only
                        [arm64]

But on arm64 platform, "rodata=full" is the default instead. This patch
implements the following changes.

- Make "rodata=on" behaviour same as the original "rodata=full"
- Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
- Drop the original "rodata=full"
- Add comment for arch_parse_debug_rodata()
- Update kernel-parameters.txt as required

After this patch, the "rodata=on" will be the default on arm64 platform
as well.
-----------------------------------------------------------------------


> 
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  2 +-
>  arch/arm64/include/asm/setup.h                | 25 +++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a22b7e621007..51bce7b9d805 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5901,7 +5901,7 @@
>  	rodata=		[KNL,EARLY]
>  		on	Mark read-only kernel memory as read-only (default).
>  		off	Leave read-only kernel memory writable for debugging.
> -		full	Mark read-only kernel memory and aliases as read-only
> +		noalias	Use more block mappings,may have better performance.
>  		        [arm64]
>  
>  	rockchip.usb_uart
> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> index ba269a7a3201..b3063326b79a 100644
> --- a/arch/arm64/include/asm/setup.h
> +++ b/arch/arm64/include/asm/setup.h
> @@ -13,6 +13,27 @@
>  extern phys_addr_t __fdt_pointer __initdata;
>  extern u64 __cacheline_aligned boot_args[4];
>  
> +/*
> + * rodata=on (default):
> + *    Apply read-only attributes of VM areas to the linear alias of
> + *    the backing pages as well. This prevents code or read-only data
> + *    from being modified (inadvertently or intentionally) via another
> + *    mapping of the same memory page.
> + *
> + *    This requires the linear region to be mapped down to pages,
> + *    which may adversely affect performance in some cases.
> + *
> + * rodata=off:
> + *    It provides us more block mappings and contiguous hits
> + *    to map the linear region which minimize the TLB footprint.
> + *    Leave read-only kernel memory writable for debugging.
> + *
> + * rodata=noalias:
> + *    It provides us more block mappings and contiguous hits
> + *    to map the linear region which minimize the TLB footprint.
> + *    And the linear aliases of pages belonging to read-only mappings
> + *    in vmalloc region are also marked as read-only.
> + */

Reformatted and cleaned up the above comment a bit but feel free to
improve it further.

/*
 * rodata=on (default)
 *
 *    This applies read-only attributes to VM areas and to the linear
 *    alias of the backing pages as well. This prevents code or read-
 *    only data from being modified (inadvertently or intentionally),
 *    via another mapping for the same memory page.
 *
 *    But this might cause linear map region to be mapped down to base
 *    pages, which may adversely affect performance in some cases.
 *
 * rodata=off
 *
 *    This provides more block mappings and contiguous hints for linear
 *    map region which would minimize TLB footprint. This also leaves
 *    read-only kernel memory writable for debugging.
 *
 * rodata=noalias
 *
 *    This provides more block mappings and contiguous hints for linear
 *    map region which would minimize TLB footprint. Linear aliases of
 *    pages belonging to read-only mappings in vmalloc region are also
 *    marked as read-only.


>  static inline bool arch_parse_debug_rodata(char *arg)
>  {
>  	extern bool rodata_enabled;
> @@ -21,7 +42,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
>  	if (!arg)
>  		return false;
>  
> -	if (!strcmp(arg, "full")) {
> +	if (!strcmp(arg, "on")) {
>  		rodata_enabled = rodata_full = true;
>  		return true;
>  	}
> @@ -31,7 +52,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
>  		return true;
>  	}
>  
> -	if (!strcmp(arg, "on")) {
> +	if (!strcmp(arg, "noalias")) {
>  		rodata_enabled = true;
>  		rodata_full = false;
>  		return true;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-10  7:17   ` Anshuman Khandual
@ 2024-12-10  7:21     ` Ard Biesheuvel
  2024-12-10  8:38       ` Anshuman Khandual
  0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2024-12-10  7:21 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Huang Shijie, catalin.marinas, will, corbet, patches, cl, akpm,
	thuth, rostedt, xiongwei.song, inux-doc, linux-kernel,
	linux-arm-kernel

On Tue, 10 Dec 2024 at 08:17, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
...
>
> Reformatted and cleaned up the above comment a bit but feel free to
> improve it further.
>
> /*
>  * rodata=on (default)
>  *
>  *    This applies read-only attributes to VM areas and to the linear
>  *    alias of the backing pages as well. This prevents code or read-
>  *    only data from being modified (inadvertently or intentionally),
>  *    via another mapping for the same memory page.
>  *
>  *    But this might cause linear map region to be mapped down to base
>  *    pages, which may adversely affect performance in some cases.
>  *
>  * rodata=off
>  *
>  *    This provides more block mappings and contiguous hints for linear
>  *    map region which would minimize TLB footprint. This also leaves
>  *    read-only kernel memory writable for debugging.
>  *
>  * rodata=noalias
>  *
>  *    This provides more block mappings and contiguous hints for linear
>  *    map region which would minimize TLB footprint. Linear aliases of
>  *    pages belonging to read-only mappings in vmalloc region are also
>  *    marked as read-only.
>

If linear aliases are marked as read-only, how does 'noalias' differ from 'on'?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED
  2024-12-09  7:20 ` [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
@ 2024-12-10  7:27   ` Anshuman Khandual
  0 siblings, 0 replies; 10+ messages in thread
From: Anshuman Khandual @ 2024-12-10  7:27 UTC (permalink / raw)
  To: Huang Shijie, catalin.marinas, will, corbet
  Cc: patches, cl, akpm, thuth, rostedt, xiongwei.song, ardb, inux-doc,
	linux-kernel, linux-arm-kernel

A small nit:

arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED

On 12/9/24 12:50, Huang Shijie wrote:
> After patch "arm64: refacotr the rodata=xxx",
> the "rodata=on" becomes the default.
> 
>      ......................................
> 	if (!strcmp(arg, "on")) {
> 		rodata_enabled = rodata_full = true;
> 		return true;
> 	}
>      ......................................
> 
> The rodata_full is always "true" via "rodata=on" and does not
> depend on the config RODATA_FULL_DEFAULT_ENABLED anymore,
> so it can be dropped.
> 
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
>  arch/arm64/Kconfig       | 14 --------------
>  arch/arm64/mm/pageattr.c |  2 +-
>  2 files changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index cbfd357f94a6..1c69982302ed 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1666,20 +1666,6 @@ config MITIGATE_SPECTRE_BRANCH_HISTORY
>  	  When taking an exception from user-space, a sequence of branches
>  	  or a firmware call overwrites the branch history.
>  
> -config RODATA_FULL_DEFAULT_ENABLED
> -	bool "Apply r/o permissions of VM areas also to their linear aliases"
> -	default y
> -	help
> -	  Apply read-only attributes of VM areas to the linear alias of
> -	  the backing pages as well. This prevents code or read-only data
> -	  from being modified (inadvertently or intentionally) via another
> -	  mapping of the same memory page. This additional enhancement can
> -	  be turned off at runtime by passing rodata=[off|on] (and turned on
> -	  with rodata=full if this option is set to 'n')
> -
> -	  This requires the linear region to be mapped down to pages,
> -	  which may adversely affect performance in some cases.
> -
>  config ARM64_SW_TTBR0_PAN
>  	bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
>  	depends on !KCSAN
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 39fd1f7ff02a..6eef08d8451e 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -20,7 +20,7 @@ struct page_change_data {
>  	pgprot_t clear_mask;
>  };
>  
> -bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
> +bool rodata_full __ro_after_init = true;
>  
>  bool can_set_direct_map(void)
>  {

Otherwise LGTM

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-10  7:21     ` Ard Biesheuvel
@ 2024-12-10  8:38       ` Anshuman Khandual
  2024-12-10  9:47         ` Shijie Huang
  0 siblings, 1 reply; 10+ messages in thread
From: Anshuman Khandual @ 2024-12-10  8:38 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huang Shijie, catalin.marinas, will, corbet, patches, cl, akpm,
	thuth, rostedt, xiongwei.song, inux-doc, linux-kernel,
	linux-arm-kernel



On 12/10/24 12:51, Ard Biesheuvel wrote:
> On Tue, 10 Dec 2024 at 08:17, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
> ...
>>
>> Reformatted and cleaned up the above comment a bit but feel free to
>> improve it further.
>>
>> /*
>>  * rodata=on (default)
>>  *
>>  *    This applies read-only attributes to VM areas and to the linear
>>  *    alias of the backing pages as well. This prevents code or read-
>>  *    only data from being modified (inadvertently or intentionally),
>>  *    via another mapping for the same memory page.
>>  *
>>  *    But this might cause linear map region to be mapped down to base
>>  *    pages, which may adversely affect performance in some cases.
>>  *
>>  * rodata=off
>>  *
>>  *    This provides more block mappings and contiguous hints for linear
>>  *    map region which would minimize TLB footprint. This also leaves
>>  *    read-only kernel memory writable for debugging.
>>  *
>>  * rodata=noalias
>>  *
>>  *    This provides more block mappings and contiguous hints for linear
>>  *    map region which would minimize TLB footprint. Linear aliases of
>>  *    pages belonging to read-only mappings in vmalloc region are also
>>  *    marked as read-only.
>>
> 
> If linear aliases are marked as read-only, how does 'noalias' differ from 'on'?

Right, the last sentence can be can dropped.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/2] arm64: refactor the rodata=xxx
  2024-12-10  8:38       ` Anshuman Khandual
@ 2024-12-10  9:47         ` Shijie Huang
  0 siblings, 0 replies; 10+ messages in thread
From: Shijie Huang @ 2024-12-10  9:47 UTC (permalink / raw)
  To: Anshuman Khandual, Ard Biesheuvel
  Cc: Huang Shijie, catalin.marinas, will, corbet, patches, cl, akpm,
	thuth, rostedt, xiongwei.song, inux-doc, linux-kernel,
	linux-arm-kernel


On 2024/12/10 16:38, Anshuman Khandual wrote:
>
> On 12/10/24 12:51, Ard Biesheuvel wrote:
>> On Tue, 10 Dec 2024 at 08:17, Anshuman Khandual
>> <anshuman.khandual@arm.com> wrote:
>> ...
>>> Reformatted and cleaned up the above comment a bit but feel free to
>>> improve it further.
>>>
>>> /*
>>>   * rodata=on (default)
>>>   *
>>>   *    This applies read-only attributes to VM areas and to the linear
>>>   *    alias of the backing pages as well. This prevents code or read-
>>>   *    only data from being modified (inadvertently or intentionally),
>>>   *    via another mapping for the same memory page.
>>>   *
>>>   *    But this might cause linear map region to be mapped down to base
>>>   *    pages, which may adversely affect performance in some cases.
>>>   *
>>>   * rodata=off
>>>   *
>>>   *    This provides more block mappings and contiguous hints for linear
>>>   *    map region which would minimize TLB footprint. This also leaves
>>>   *    read-only kernel memory writable for debugging.
>>>   *
>>>   * rodata=noalias
>>>   *
>>>   *    This provides more block mappings and contiguous hints for linear
>>>   *    map region which would minimize TLB footprint. Linear aliases of
>>>   *    pages belonging to read-only mappings in vmalloc region are also
>>>   *    marked as read-only.
>>>
>> If linear aliases are marked as read-only, how does 'noalias' differ from 'on'?
> Right, the last sentence can be can dropped.

Okay, no problem.


Thanks

Huang Shijie


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-12-10  9:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-09  7:20 [PATCH v3 0/2] arm64: refactor the rodata=xxx Huang Shijie
2024-12-09  7:20 ` [PATCH v3 1/2] " Huang Shijie
2024-12-09 17:05   ` Christoph Lameter (Ampere)
2024-12-10  3:22     ` Shijie Huang
2024-12-10  7:17   ` Anshuman Khandual
2024-12-10  7:21     ` Ard Biesheuvel
2024-12-10  8:38       ` Anshuman Khandual
2024-12-10  9:47         ` Shijie Huang
2024-12-09  7:20 ` [PATCH v3 2/2] arm64: remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
2024-12-10  7:27   ` Anshuman Khandual

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®