Re: [PATCH V6 1/2] arm64: refactor the rodata=xxx

[Shijie Huang <shijie@amperemail.onmicrosoft.com>]

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