Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V6 0/2] arm64: refactor the rodata=xxx
@ 2025-07-03  9:42 Huang Shijie
  2025-07-03  9:42 ` [PATCH V6 1/2] " Huang Shijie
  2025-07-03  9:42 ` [PATCH V6 2/2] arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
  0 siblings, 2 replies; 5+ messages in thread
From: Huang Shijie @ 2025-07-03  9:42 UTC (permalink / raw)
  To: catalin.marinas, will, corbet
  Cc: patches, cl, yang, akpm, paulmck, rostedt, Neeraj.Upadhyay, bp,
	ardb, anshuman.khandual, suzuki.poulose, gshan, linux-doc,
	linux-kernel, linux-arm-kernel, rdunlap, 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.

v6:
  Fix a small issue pointed by Randy Dunlap.

v5:
  Rebase this patch set with linux-next20250627
  https://lists.infradead.org/pipermail/linux-arm-kernel/2025-June/1040297.html

v4:
  Follows Anshuman Khandual/Ard Biesheuvel's suggetions:
  - Change commit message format.
  - Change the titile name.
  - others
  https://lists.infradead.org/pipermail/linux-arm-kernel/2024-December/985629.html

v3:
  Follows Anshuman Khandual's suggetions:
  - Merge patch 1 and patch 3 into one patch.
  - Remove patch 4
  - update comments and document.
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-December/984344.html

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/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED

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

-- 
2.40.1



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

* [PATCH V6 1/2] arm64: refactor the rodata=xxx
  2025-07-03  9:42 [PATCH V6 0/2] arm64: refactor the rodata=xxx Huang Shijie
@ 2025-07-03  9:42 ` Huang Shijie
  2025-09-08 11:35   ` Will Deacon
  2025-07-03  9:42 ` [PATCH V6 2/2] arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie
  1 sibling, 1 reply; 5+ messages in thread
From: Huang Shijie @ 2025-07-03  9:42 UTC (permalink / raw)
  To: catalin.marinas, will, corbet
  Cc: patches, cl, yang, akpm, paulmck, rostedt, Neeraj.Upadhyay, bp,
	ardb, anshuman.khandual, suzuki.poulose, gshan, linux-doc,
	linux-kernel, linux-arm-kernel, rdunlap, Huang Shijie,
	Christoph Lameter

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.

Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ee0735c6b8e2..3590bdc8d9a5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6354,7 +6354,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..6b994d0881d1 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -13,6 +13,30 @@
 extern phys_addr_t __fdt_pointer __initdata;
 extern u64 __cacheline_aligned boot_args[4];
 
+/*
+ * 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. This leaves the linear
+ *    alias of read-only mappings in the vmalloc space writeable, making
+ *    them susceptible to inadvertent modification by software.
+ */
 static inline bool arch_parse_debug_rodata(char *arg)
 {
 	extern bool rodata_enabled;
@@ -21,7 +45,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 +55,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 related	[flat|nested] 5+ messages in thread

* [PATCH V6 2/2] arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED
  2025-07-03  9:42 [PATCH V6 0/2] arm64: refactor the rodata=xxx Huang Shijie
  2025-07-03  9:42 ` [PATCH V6 1/2] " Huang Shijie
@ 2025-07-03  9:42 ` Huang Shijie
  1 sibling, 0 replies; 5+ messages in thread
From: Huang Shijie @ 2025-07-03  9:42 UTC (permalink / raw)
  To: catalin.marinas, will, corbet
  Cc: patches, cl, yang, akpm, paulmck, rostedt, Neeraj.Upadhyay, bp,
	ardb, anshuman.khandual, suzuki.poulose, gshan, linux-doc,
	linux-kernel, linux-arm-kernel, rdunlap, Huang Shijie,
	Christoph Lameter

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.

Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
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 f9f988c2cab7..12a70f10f7bb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1694,20 +1694,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 04d4a8f676db..667aff1efe49 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 related	[flat|nested] 5+ messages in thread

* Re: [PATCH V6 1/2] arm64: refactor the rodata=xxx
  2025-07-03  9:42 ` [PATCH V6 1/2] " Huang Shijie
@ 2025-09-08 11:35   ` Will Deacon
  2025-09-09  2:52     ` Shijie Huang
  0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2025-09-08 11:35 UTC (permalink / raw)
  To: Huang Shijie
  Cc: catalin.marinas, corbet, patches, cl, yang, akpm, paulmck,
	rostedt, Neeraj.Upadhyay, bp, ardb, anshuman.khandual,
	suzuki.poulose, gshan, linux-doc, linux-kernel, linux-arm-kernel,
	rdunlap, Christoph Lameter

On Thu, Jul 03, 2025 at 05:42:11PM +0800, Huang Shijie wrote:
> 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.

Please mention RODATA_FULL_DEFAULT_ENABLED here.

> This patch implements the following changes.
> 
>  - Make "rodata=on" behaviour same as the original "rodata=full"

You should mention that this gives us parity with x86.

>  - 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

These last two are self-evident from the code and don't need to be listed
here.

> After this patch, the "rodata=on" will be the default on arm64 platform
> as well.
> 
> Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
>  .../admin-guide/kernel-parameters.txt         |  2 +-
>  arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ee0735c6b8e2..3590bdc8d9a5 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6354,7 +6354,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]

This isn't particularly helpful documentation and I think we need to mention
the linear alias rather than talk about the page-table structure.

How about:

	noalias	Mark read-only kernel memory as read-only but retain
		writable aliases in the direct map for regions outside
		of the kernel image. [arm64]

?

> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> index ba269a7a3201..6b994d0881d1 100644
> --- a/arch/arm64/include/asm/setup.h
> +++ b/arch/arm64/include/asm/setup.h
> @@ -13,6 +13,30 @@
>  extern phys_addr_t __fdt_pointer __initdata;
>  extern u64 __cacheline_aligned boot_args[4];
>  
> +/*
> + * 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. This leaves the linear
> + *    alias of read-only mappings in the vmalloc space writeable, making
> + *    them susceptible to inadvertent modification by software.
> + */

Please remove this comment. If you want to keep it, this information
belongs either in the commit message (to justify the performance impact)
or the Documentation (to describe the functional impact) but there's
little point having it hidden away here.

With those changes, this looks good and I can pick it up for 6.18 if
you respin.

Cheers,

Will


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

* Re: [PATCH V6 1/2] arm64: refactor the rodata=xxx
  2025-09-08 11:35   ` Will Deacon
@ 2025-09-09  2:52     ` Shijie Huang
  0 siblings, 0 replies; 5+ messages in thread
From: Shijie Huang @ 2025-09-09  2:52 UTC (permalink / raw)
  To: Will Deacon, Huang Shijie
  Cc: catalin.marinas, corbet, patches, cl, yang, akpm, paulmck,
	rostedt, Neeraj.Upadhyay, bp, ardb, anshuman.khandual,
	suzuki.poulose, gshan, linux-doc, linux-kernel, linux-arm-kernel,
	rdunlap, Christoph Lameter


On 08/09/2025 19:35, Will Deacon wrote:
> On Thu, Jul 03, 2025 at 05:42:11PM +0800, Huang Shijie wrote:
>> 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.
> Please mention RODATA_FULL_DEFAULT_ENABLED here.
okay.
>> This patch implements the following changes.
>>
>>   - Make "rodata=on" behaviour same as the original "rodata=full"
> You should mention that this gives us parity with x86.
No problem.
>>   - 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
> These last two are self-evident from the code and don't need to be listed
> here.
>
>> After this patch, the "rodata=on" will be the default on arm64 platform
>> as well.
>>
>> Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
>> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  2 +-
>>   arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
>>   2 files changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index ee0735c6b8e2..3590bdc8d9a5 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -6354,7 +6354,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]
> This isn't particularly helpful documentation and I think we need to mention
> the linear alias rather than talk about the page-table structure.
>
> How about:
>
> 	noalias	Mark read-only kernel memory as read-only but retain
> 		writable aliases in the direct map for regions outside
> 		of the kernel image. [arm64]
>
> ?
Okay, thanks.
>> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
>> index ba269a7a3201..6b994d0881d1 100644
>> --- a/arch/arm64/include/asm/setup.h
>> +++ b/arch/arm64/include/asm/setup.h
>> @@ -13,6 +13,30 @@
>>   extern phys_addr_t __fdt_pointer __initdata;
>>   extern u64 __cacheline_aligned boot_args[4];
>>   
>> +/*
>> + * 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. This leaves the linear
>> + *    alias of read-only mappings in the vmalloc space writeable, making
>> + *    them susceptible to inadvertent modification by software.
>> + */
> Please remove this comment. If you want to keep it, this information
> belongs either in the commit message (to justify the performance impact)

Okay, I can move it to commit message.


Thanks

Huang Shijie



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

end of thread, other threads:[~2025-09-09  6:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-03  9:42 [PATCH V6 0/2] arm64: refactor the rodata=xxx Huang Shijie
2025-07-03  9:42 ` [PATCH V6 1/2] " Huang Shijie
2025-09-08 11:35   ` Will Deacon
2025-09-09  2:52     ` Shijie Huang
2025-07-03  9:42 ` [PATCH V6 2/2] arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED Huang Shijie

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