Re: [PATCH v1 2/3] arm64: mm: Handle invalid large leaf mappings correctly

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 23/03/2026 14:03, Ryan Roberts wrote:
> [...]
>
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 358d1dc9a576f..87dfe4c82fa92 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -25,6 +25,11 @@ static ptdesc_t set_pageattr_masks(ptdesc_t val, struct mm_walk *walk)
>  {
>  	struct page_change_data *masks = walk->private;
>  
> +	/*
> +	 * Some users clear and set bits which alias eachother (e.g. PTE_NG and

Nit: "each other"

> +	 * PTE_PRESENT_INVALID). It is therefore important that we always clear
> +	 * first then set.
> +	 */
>  	val &= ~(pgprot_val(masks->clear_mask));
>  	val |= (pgprot_val(masks->set_mask));
>  
> @@ -36,7 +41,7 @@ static int pageattr_pud_entry(pud_t *pud, unsigned long addr,
>  {
>  	pud_t val = pudp_get(pud);
>  
> -	if (pud_sect(val)) {
> +	if (pud_leaf(val)) {
>  		if (WARN_ON_ONCE((next - addr) != PUD_SIZE))
>  			return -EINVAL;
>  		val = __pud(set_pageattr_masks(pud_val(val), walk));
> @@ -52,7 +57,7 @@ static int pageattr_pmd_entry(pmd_t *pmd, unsigned long addr,
>  {
>  	pmd_t val = pmdp_get(pmd);
>  
> -	if (pmd_sect(val)) {
> +	if (pmd_leaf(val)) {
>  		if (WARN_ON_ONCE((next - addr) != PMD_SIZE))
>  			return -EINVAL;
>  		val = __pmd(set_pageattr_masks(pmd_val(val), walk));
> @@ -132,11 +137,12 @@ static int __change_memory_common(unsigned long start, unsigned long size,
>  	ret = update_range_prot(start, size, set_mask, clear_mask);
>  
>  	/*
> -	 * If the memory is being made valid without changing any other bits
> -	 * then a TLBI isn't required as a non-valid entry cannot be cached in
> -	 * the TLB.
> +	 * If the memory is being switched from present-invalid to valid without
> +	 * changing any other bits then a TLBI isn't required as a non-valid
> +	 * entry cannot be cached in the TLB.
>  	 */
> -	if (pgprot_val(set_mask) != PTE_VALID || pgprot_val(clear_mask))
> +	if (pgprot_val(set_mask) != (PTE_MAYBE_NG | PTE_VALID) ||

It isn't obvious to understand where all those PTE_MAYBE_NG come from if
one hasn't realised that PTE_PRESENT_INVALID overlays PTE_NG.

Since for this purpose we always set/clear both PTE_VALID and
PTE_MAYBE_NG, maybe we could define some macro as PTE_VALID |
PTE_MAYBE_NG, as a counterpart to PTE_PRESENT_INVALID?

- Kevin

> [...]