Re: [PATCH] arm64: mm: Use generic enum pgtable_level

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

On 17/03/2026 12:47, Kevin Brodsky wrote:
> On 16/03/2026 15:45, David Hildenbrand (Arm) wrote:
>> On 3/16/26 15:38, Kevin Brodsky wrote:
>>> On 16/03/2026 15:22, Ryan Roberts wrote:
>>>>>  static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm, gfp_t gfp,
>>>>> -				       enum pgtable_type pgtable_type)
>>>>> +				       enum pgtable_level pgtable_level)
>>>>>  {
>>>>>  	/* Page is zeroed by init_clear_pgtable() so don't duplicate effort. */
>>>>>  	struct ptdesc *ptdesc = pagetable_alloc(gfp & ~__GFP_ZERO, 0);
>>>>> @@ -539,40 +539,42 @@ static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm, gfp_t gfp,
>>>>>  
>>>>>  	pa = page_to_phys(ptdesc_page(ptdesc));
>>>>>  
>>>>> -	switch (pgtable_type) {
>>>>> -	case TABLE_PTE:
>>>>> +	switch (pgtable_level) {
>>>>> +	case PGTABLE_LEVEL_PTE:
>>>>>  		BUG_ON(!pagetable_pte_ctor(mm, ptdesc));
>>>>>  		break;
>>>>> -	case TABLE_PMD:
>>>>> +	case PGTABLE_LEVEL_PMD:
>>>>>  		BUG_ON(!pagetable_pmd_ctor(mm, ptdesc));
>>>>>  		break;
>>>>> -	case TABLE_PUD:
>>>>> +	case PGTABLE_LEVEL_PUD:
>>>>>  		pagetable_pud_ctor(ptdesc);
>>>>>  		break;
>>>>> -	case TABLE_P4D:
>>>>> +	case PGTABLE_LEVEL_P4D:
>>>>>  		pagetable_p4d_ctor(ptdesc);
>>>>>  		break;
>>>>> +	default:
>>>>> +		break;
>>>> nit: I think we should either explicitly support pgd or explicitly bug/warn. Now
>>>> that the enum has PGTABLE_LEVEL_PGD it looks legit to call __pgd_pgtable_alloc()
>>>> to allocate one. But it will currently silently fail to call pagetable_pgd_ctor().
>>> I hesitated there as well, eventually I concluded that we're dealing
>>> with kernel page tables so we'll never allocate a PGD anyway...
>>>
>>>> Probably simplest just to call BUG() in the default path?
>>> ... but that's certainly fine by me :)
>> If we could force it to be inline, we could turn it into a BUILD_BUG().
> 
> I don't think this would help, pgd_pgtable_alloc_init_mm() is passed
> around as a function pointer so the compiler can't know the value of
> pgtable_level at compile time.
> 
>> 	VM_WARN_ON()
>>
>> might be good enough I guess.

Fine by me.

> 
> I think so, not calling a ctor at PGD level has no real consequence (it
> wasn't done at all until recently).
> 
> - Kevin