Re: [RFC PATCH v6 1/4] arm64: Enable permission change on arm64 kernel block mappings

[Ryan Roberts <ryan.roberts@arm.com>]

On 28/08/2025 17:26, Catalin Marinas wrote:
> On Tue, Aug 05, 2025 at 09:13:46AM +0100, Ryan Roberts wrote:
>> From: Dev Jain <dev.jain@arm.com>
>>
>> This patch paves the path to enable huge mappings in vmalloc space and
>> linear map space by default on arm64. For this we must ensure that we
>> can handle any permission games on the kernel (init_mm) pagetable.
>> Currently, __change_memory_common() uses apply_to_page_range() which
>> does not support changing permissions for block mappings. We attempt to
>> move away from this by using the pagewalk API, similar to what riscv
>> does right now; however, it is the responsibility of the caller to
>> ensure that we do not pass a range overlapping a partial block mapping
>> or cont mapping; in such a case, the system must be able to support
>> range splitting.
>>
>> This patch is tied with Yang Shi's attempt [1] at using huge mappings in
>> the linear mapping in case the system supports BBML2, in which case we
>> will be able to split the linear mapping if needed without
>> break-before-make. Thus, Yang's series, IIUC, will be one such user of
>> my patch; suppose we are changing permissions on a range of the linear
>> map backed by PMD-hugepages, then the sequence of operations should look
>> like the following:
>>
>> split_range(start)
>> split_range(end);
>> __change_memory_common(start, end);
>>
>> However, this patch can be used independently of Yang's; since currently
>> permission games are being played only on pte mappings (due to
>> apply_to_page_range not supporting otherwise), this patch provides the
>> mechanism for enabling huge mappings for various kernel mappings like
>> linear map and vmalloc.
> [...]
> 
> I think some of this text needs to be trimmed down, avoid references to
> other series if they are merged at the same time.
> 
>> diff --git a/include/linux/pagewalk.h b/include/linux/pagewalk.h
>> index 682472c15495..8212e8f2d2d5 100644
>> --- a/include/linux/pagewalk.h
>> +++ b/include/linux/pagewalk.h
>> @@ -134,6 +134,9 @@ int walk_page_range(struct mm_struct *mm, unsigned long start,
>>  int walk_kernel_page_table_range(unsigned long start,
>>  		unsigned long end, const struct mm_walk_ops *ops,
>>  		pgd_t *pgd, void *private);
>> +int walk_kernel_page_table_range_lockless(unsigned long start,
>> +		unsigned long end, const struct mm_walk_ops *ops,
>> +		void *private);
>>  int walk_page_range_vma(struct vm_area_struct *vma, unsigned long start,
>>  			unsigned long end, const struct mm_walk_ops *ops,
>>  			void *private);
>> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
>> index 648038247a8d..18a675ab87cf 100644
>> --- a/mm/pagewalk.c
>> +++ b/mm/pagewalk.c
>> @@ -633,6 +633,30 @@ int walk_kernel_page_table_range(unsigned long start, unsigned long end,
>>  	return walk_pgd_range(start, end, &walk);
>>  }
>>  
>> +/*
>> + * Use this function to walk the kernel page tables locklessly. It should be
>> + * guaranteed that the caller has exclusive access over the range they are
>> + * operating on - that there should be no concurrent access, for example,
>> + * changing permissions for vmalloc objects.
>> + */
>> +int walk_kernel_page_table_range_lockless(unsigned long start, unsigned long end,
>> +		const struct mm_walk_ops *ops, void *private)
>> +{
>> +	struct mm_walk walk = {
>> +		.ops		= ops,
>> +		.mm		= &init_mm,
>> +		.private	= private,
>> +		.no_vma		= true
>> +	};
>> +
>> +	if (start >= end)
>> +		return -EINVAL;
>> +	if (!check_ops_valid(ops))
>> +		return -EINVAL;
>> +
>> +	return walk_pgd_range(start, end, &walk);
>> +}
> 
> More of a nit: we could change walk_kernel_page_table_range() to call
> this function after checking the mm lock as they look nearly identical.
> The existing function has a pgd argument but it doesn't seem to be used
> anywhere and could be removed (or add it here for consistency).

Good point. I've done this refactoring in my new version, adding pgd to the
_lockless() variant, since it's used by x86. Let's see what Lorenzo and co think
in the context of the next version (incomming shortly).

> 
> Either way, the patch looks fine.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>