public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] arm64: mm: Use generic enum pgtable_level
@ 2026-03-16 14:01 Kevin Brodsky
  2026-03-16 14:07 ` David Hildenbrand (Arm)
  2026-03-16 14:22 ` Ryan Roberts
  0 siblings, 2 replies; 7+ messages in thread
From: Kevin Brodsky @ 2026-03-16 14:01 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, Kevin Brodsky, David Hildenbrand (Arm),
	Catalin Marinas, Ryan Roberts, Will Deacon

enum pgtable_type was introduced for arm64 by commit c64f46ee1377
("arm64: mm: use enum to identify pgtable level instead of
*_SHIFT"). In the meantime, the generic enum pgtable_level got
introduced by commit b22cc9a9c7ff ("mm/rmap: convert "enum
rmap_level" to "enum pgtable_level"").

Let's switch to the generic enum pgtable_level. The only difference
is that it also includes PGD level - this isn't relevant in
__pgd_pgtable_alloc() so we just add a default statement.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/mmu.h |  7 -----
 arch/arm64/mm/mmu.c          | 58 +++++++++++++++++++-----------------
 2 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 137a173df1ff..f28e6e215da4 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -17,13 +17,6 @@
 #include <linux/refcount.h>
 #include <asm/cpufeature.h>
 
-enum pgtable_type {
-	TABLE_PTE,
-	TABLE_PMD,
-	TABLE_PUD,
-	TABLE_P4D,
-};
-
 typedef struct {
 	atomic64_t	id;
 #ifdef CONFIG_COMPAT
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index a6a00accf4f9..3a22e8f3c9fe 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -112,7 +112,7 @@ pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
 }
 EXPORT_SYMBOL(phys_mem_access_prot);
 
-static phys_addr_t __init early_pgtable_alloc(enum pgtable_type pgtable_type)
+static phys_addr_t __init early_pgtable_alloc(enum pgtable_level pgtable_level)
 {
 	phys_addr_t phys;
 
@@ -197,7 +197,7 @@ static void init_pte(pte_t *ptep, unsigned long addr, unsigned long end,
 static int alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 			       unsigned long end, phys_addr_t phys,
 			       pgprot_t prot,
-			       phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+			       phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 			       int flags)
 {
 	unsigned long next;
@@ -212,7 +212,7 @@ static int alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 		if (flags & NO_EXEC_MAPPINGS)
 			pmdval |= PMD_TABLE_PXN;
 		BUG_ON(!pgtable_alloc);
-		pte_phys = pgtable_alloc(TABLE_PTE);
+		pte_phys = pgtable_alloc(PGTABLE_LEVEL_PTE);
 		if (pte_phys == INVALID_PHYS_ADDR)
 			return -ENOMEM;
 		ptep = pte_set_fixmap(pte_phys);
@@ -252,7 +252,7 @@ static int alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 
 static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
 		    phys_addr_t phys, pgprot_t prot,
-		    phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags)
+		    phys_addr_t (*pgtable_alloc)(enum pgtable_level), int flags)
 {
 	unsigned long next;
 
@@ -292,7 +292,7 @@ static int init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
 static int alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 			       unsigned long end, phys_addr_t phys,
 			       pgprot_t prot,
-			       phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+			       phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 			       int flags)
 {
 	int ret;
@@ -311,7 +311,7 @@ static int alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 		if (flags & NO_EXEC_MAPPINGS)
 			pudval |= PUD_TABLE_PXN;
 		BUG_ON(!pgtable_alloc);
-		pmd_phys = pgtable_alloc(TABLE_PMD);
+		pmd_phys = pgtable_alloc(PGTABLE_LEVEL_PMD);
 		if (pmd_phys == INVALID_PHYS_ADDR)
 			return -ENOMEM;
 		pmdp = pmd_set_fixmap(pmd_phys);
@@ -349,7 +349,7 @@ static int alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 
 static int alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
 			  phys_addr_t phys, pgprot_t prot,
-			  phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+			  phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 			  int flags)
 {
 	int ret = 0;
@@ -364,7 +364,7 @@ static int alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
 		if (flags & NO_EXEC_MAPPINGS)
 			p4dval |= P4D_TABLE_PXN;
 		BUG_ON(!pgtable_alloc);
-		pud_phys = pgtable_alloc(TABLE_PUD);
+		pud_phys = pgtable_alloc(PGTABLE_LEVEL_PUD);
 		if (pud_phys == INVALID_PHYS_ADDR)
 			return -ENOMEM;
 		pudp = pud_set_fixmap(pud_phys);
@@ -415,7 +415,7 @@ static int alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
 
 static int alloc_init_p4d(pgd_t *pgdp, unsigned long addr, unsigned long end,
 			  phys_addr_t phys, pgprot_t prot,
-			  phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+			  phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 			  int flags)
 {
 	int ret;
@@ -430,7 +430,7 @@ static int alloc_init_p4d(pgd_t *pgdp, unsigned long addr, unsigned long end,
 		if (flags & NO_EXEC_MAPPINGS)
 			pgdval |= PGD_TABLE_PXN;
 		BUG_ON(!pgtable_alloc);
-		p4d_phys = pgtable_alloc(TABLE_P4D);
+		p4d_phys = pgtable_alloc(PGTABLE_LEVEL_P4D);
 		if (p4d_phys == INVALID_PHYS_ADDR)
 			return -ENOMEM;
 		p4dp = p4d_set_fixmap(p4d_phys);
@@ -467,7 +467,7 @@ static int alloc_init_p4d(pgd_t *pgdp, unsigned long addr, unsigned long end,
 static int __create_pgd_mapping_locked(pgd_t *pgdir, phys_addr_t phys,
 				       unsigned long virt, phys_addr_t size,
 				       pgprot_t prot,
-				       phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+				       phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 				       int flags)
 {
 	int ret;
@@ -500,7 +500,7 @@ static int __create_pgd_mapping_locked(pgd_t *pgdir, phys_addr_t phys,
 static int __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 				unsigned long virt, phys_addr_t size,
 				pgprot_t prot,
-				phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+				phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 				int flags)
 {
 	int ret;
@@ -516,7 +516,7 @@ static int __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 static void early_create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 				     unsigned long virt, phys_addr_t size,
 				     pgprot_t prot,
-				     phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+				     phys_addr_t (*pgtable_alloc)(enum pgtable_level),
 				     int flags)
 {
 	int ret;
@@ -528,7 +528,7 @@ static void early_create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 }
 
 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;
 	}
 
 	return pa;
 }
 
 static phys_addr_t
-pgd_pgtable_alloc_init_mm_gfp(enum pgtable_type pgtable_type, gfp_t gfp)
+pgd_pgtable_alloc_init_mm_gfp(enum pgtable_level pgtable_level, gfp_t gfp)
 {
-	return __pgd_pgtable_alloc(&init_mm, gfp, pgtable_type);
+	return __pgd_pgtable_alloc(&init_mm, gfp, pgtable_level);
 }
 
 static phys_addr_t __maybe_unused
-pgd_pgtable_alloc_init_mm(enum pgtable_type pgtable_type)
+pgd_pgtable_alloc_init_mm(enum pgtable_level pgtable_level)
 {
-	return pgd_pgtable_alloc_init_mm_gfp(pgtable_type, GFP_PGTABLE_KERNEL);
+	return pgd_pgtable_alloc_init_mm_gfp(pgtable_level, GFP_PGTABLE_KERNEL);
 }
 
 static phys_addr_t
-pgd_pgtable_alloc_special_mm(enum pgtable_type pgtable_type)
+pgd_pgtable_alloc_special_mm(enum pgtable_level pgtable_level)
 {
-	return  __pgd_pgtable_alloc(NULL, GFP_PGTABLE_KERNEL, pgtable_type);
+	return  __pgd_pgtable_alloc(NULL, GFP_PGTABLE_KERNEL, pgtable_level);
 }
 
 static void split_contpte(pte_t *ptep)
@@ -593,7 +595,7 @@ static int split_pmd(pmd_t *pmdp, pmd_t pmd, gfp_t gfp, bool to_cont)
 	pte_t *ptep;
 	int i;
 
-	pte_phys = pgd_pgtable_alloc_init_mm_gfp(TABLE_PTE, gfp);
+	pte_phys = pgd_pgtable_alloc_init_mm_gfp(PGTABLE_LEVEL_PTE, gfp);
 	if (pte_phys == INVALID_PHYS_ADDR)
 		return -ENOMEM;
 	ptep = (pte_t *)phys_to_virt(pte_phys);
@@ -638,7 +640,7 @@ static int split_pud(pud_t *pudp, pud_t pud, gfp_t gfp, bool to_cont)
 	pmd_t *pmdp;
 	int i;
 
-	pmd_phys = pgd_pgtable_alloc_init_mm_gfp(TABLE_PMD, gfp);
+	pmd_phys = pgd_pgtable_alloc_init_mm_gfp(PGTABLE_LEVEL_PMD, gfp);
 	if (pmd_phys == INVALID_PHYS_ADDR)
 		return -ENOMEM;
 	pmdp = (pmd_t *)phys_to_virt(pmd_phys);
@@ -1226,7 +1228,7 @@ static void __init declare_vma(struct vm_struct *vma,
 
 static phys_addr_t kpti_ng_temp_alloc __initdata;
 
-static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
+static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_level pgtable_level)
 {
 	kpti_ng_temp_alloc -= PAGE_SIZE;
 	return kpti_ng_temp_alloc;
-- 
2.51.2



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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-16 14:01 [PATCH] arm64: mm: Use generic enum pgtable_level Kevin Brodsky
@ 2026-03-16 14:07 ` David Hildenbrand (Arm)
  2026-03-16 14:22 ` Ryan Roberts
  1 sibling, 0 replies; 7+ messages in thread
From: David Hildenbrand (Arm) @ 2026-03-16 14:07 UTC (permalink / raw)
  To: Kevin Brodsky, linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Ryan Roberts, Will Deacon

On 3/16/26 15:01, Kevin Brodsky wrote:
> enum pgtable_type was introduced for arm64 by commit c64f46ee1377
> ("arm64: mm: use enum to identify pgtable level instead of
> *_SHIFT"). In the meantime, the generic enum pgtable_level got
> introduced by commit b22cc9a9c7ff ("mm/rmap: convert "enum
> rmap_level" to "enum pgtable_level"").
> 
> Let's switch to the generic enum pgtable_level. The only difference
> is that it also includes PGD level - this isn't relevant in
> __pgd_pgtable_alloc() so we just add a default statement.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

Thanks!

-- 
Cheers,

David


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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-16 14:01 [PATCH] arm64: mm: Use generic enum pgtable_level Kevin Brodsky
  2026-03-16 14:07 ` David Hildenbrand (Arm)
@ 2026-03-16 14:22 ` Ryan Roberts
  2026-03-16 14:38   ` Kevin Brodsky
  1 sibling, 1 reply; 7+ messages in thread
From: Ryan Roberts @ 2026-03-16 14:22 UTC (permalink / raw)
  To: Kevin Brodsky, linux-arm-kernel
  Cc: linux-kernel, David Hildenbrand (Arm), Catalin Marinas,
	Will Deacon

On 16/03/2026 14:01, Kevin Brodsky wrote:
> enum pgtable_type was introduced for arm64 by commit c64f46ee1377
> ("arm64: mm: use enum to identify pgtable level instead of
> *_SHIFT"). In the meantime, the generic enum pgtable_level got
> introduced by commit b22cc9a9c7ff ("mm/rmap: convert "enum
> rmap_level" to "enum pgtable_level"").
> 
> Let's switch to the generic enum pgtable_level. The only difference
> is that it also includes PGD level - this isn't relevant in
> __pgd_pgtable_alloc() so we just add a default statement.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/mmu.h |  7 -----
>  arch/arm64/mm/mmu.c          | 58 +++++++++++++++++++-----------------
>  2 files changed, 30 insertions(+), 35 deletions(-)
> 

[...]

>  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().

Probably simplest just to call BUG() in the default path?

With this addressed:

Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

Thanks,
Ryan




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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-16 14:22 ` Ryan Roberts
@ 2026-03-16 14:38   ` Kevin Brodsky
  2026-03-16 14:45     ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Brodsky @ 2026-03-16 14:38 UTC (permalink / raw)
  To: Ryan Roberts, linux-arm-kernel
  Cc: linux-kernel, David Hildenbrand (Arm), Catalin Marinas,
	Will Deacon

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

> With this addressed:
>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

Thanks!

- Kevin


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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-16 14:38   ` Kevin Brodsky
@ 2026-03-16 14:45     ` David Hildenbrand (Arm)
  2026-03-17 12:47       ` Kevin Brodsky
  0 siblings, 1 reply; 7+ messages in thread
From: David Hildenbrand (Arm) @ 2026-03-16 14:45 UTC (permalink / raw)
  To: Kevin Brodsky, Ryan Roberts, linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Will Deacon

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().

	VM_WARN_ON()

might be good enough I guess.

-- 
Cheers,

David


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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-16 14:45     ` David Hildenbrand (Arm)
@ 2026-03-17 12:47       ` Kevin Brodsky
  2026-03-17 15:10         ` Ryan Roberts
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Brodsky @ 2026-03-17 12:47 UTC (permalink / raw)
  To: David Hildenbrand (Arm), Ryan Roberts, linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Will Deacon

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.

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

- Kevin


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

* Re: [PATCH] arm64: mm: Use generic enum pgtable_level
  2026-03-17 12:47       ` Kevin Brodsky
@ 2026-03-17 15:10         ` Ryan Roberts
  0 siblings, 0 replies; 7+ messages in thread
From: Ryan Roberts @ 2026-03-17 15:10 UTC (permalink / raw)
  To: Kevin Brodsky, David Hildenbrand (Arm), linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Will Deacon

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



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

end of thread, other threads:[~2026-03-17 15:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 14:01 [PATCH] arm64: mm: Use generic enum pgtable_level Kevin Brodsky
2026-03-16 14:07 ` David Hildenbrand (Arm)
2026-03-16 14:22 ` Ryan Roberts
2026-03-16 14:38   ` Kevin Brodsky
2026-03-16 14:45     ` David Hildenbrand (Arm)
2026-03-17 12:47       ` Kevin Brodsky
2026-03-17 15:10         ` Ryan Roberts

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