[PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Anshuman Khandual]

There is an implicit assumption about bm_pud[] being single entry PUD table
just like bp_pmd[] which is a single entry PMD table. Though only the later
gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
table as well.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/fixmap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
index c5c5425791da..e02594be8b8e 100644
--- a/arch/arm64/mm/fixmap.c
+++ b/arch/arm64/mm/fixmap.c
@@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
 	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
 #define NR_BM_PMD_TABLES \
 	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
+#define NR_BM_PUD_TABLES \
+	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
 
 static_assert(NR_BM_PMD_TABLES == 1);
+static_assert(NR_BM_PUD_TABLES == 1);
 
 #define __BM_TABLE_IDX(addr, shift) \
 	(((addr) >> (shift)) - (FIXADDR_TOT_START >> (shift)))
-- 
2.30.2

Re: [PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Ryan Roberts]

On 07/01/2026 06:16, Anshuman Khandual wrote:
> There is an implicit assumption about bm_pud[] being single entry PUD table
> just like bp_pmd[] which is a single entry PMD table. Though only the later
> gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
> table as well.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/mm/fixmap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
> index c5c5425791da..e02594be8b8e 100644
> --- a/arch/arm64/mm/fixmap.c
> +++ b/arch/arm64/mm/fixmap.c
> @@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
>  #define NR_BM_PMD_TABLES \
>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
> +#define NR_BM_PUD_TABLES \
> +	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
>  
>  static_assert(NR_BM_PMD_TABLES == 1);
> +static_assert(NR_BM_PUD_TABLES == 1);

But if there is only one PMD table, there must only be one PUD table (and one
P4D table, and one PGD table) by definition. So I don't think this is checking
anything that the existing PMD table check is not.

Thanks,
Ryan

>  
>  #define __BM_TABLE_IDX(addr, shift) \
>  	(((addr) >> (shift)) - (FIXADDR_TOT_START >> (shift)))

Re: [PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Anshuman Khandual]

On 07/01/26 2:03 PM, Ryan Roberts wrote:
> On 07/01/2026 06:16, Anshuman Khandual wrote:
>> There is an implicit assumption about bm_pud[] being single entry PUD table
>> just like bp_pmd[] which is a single entry PMD table. Though only the later
>> gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
>> table as well.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/mm/fixmap.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
>> index c5c5425791da..e02594be8b8e 100644
>> --- a/arch/arm64/mm/fixmap.c
>> +++ b/arch/arm64/mm/fixmap.c
>> @@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
>>  #define NR_BM_PMD_TABLES \
>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
>> +#define NR_BM_PUD_TABLES \
>> +	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
>>  
>>  static_assert(NR_BM_PMD_TABLES == 1);
>> +static_assert(NR_BM_PUD_TABLES == 1);
> 
> But if there is only one PMD table, there must only be one PUD table (and one
> P4D table, and one PGD table) by definition. So I don't think this is checking
> anything that the existing PMD table check is not.

Agreed but should not this check help tighten this code up given that bm_pud[]
array gets used for possible PUD entries ?

> 
> Thanks,
> Ryan
> 
>>  
>>  #define __BM_TABLE_IDX(addr, shift) \
>>  	(((addr) >> (shift)) - (FIXADDR_TOT_START >> (shift)))
> 

Re: [PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Ryan Roberts]

On 07/01/2026 09:07, Anshuman Khandual wrote:
> 
> 
> On 07/01/26 2:03 PM, Ryan Roberts wrote:
>> On 07/01/2026 06:16, Anshuman Khandual wrote:
>>> There is an implicit assumption about bm_pud[] being single entry PUD table
>>> just like bp_pmd[] which is a single entry PMD table. Though only the later
>>> gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
>>> table as well.
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>  arch/arm64/mm/fixmap.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
>>> index c5c5425791da..e02594be8b8e 100644
>>> --- a/arch/arm64/mm/fixmap.c
>>> +++ b/arch/arm64/mm/fixmap.c
>>> @@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
>>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
>>>  #define NR_BM_PMD_TABLES \
>>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
>>> +#define NR_BM_PUD_TABLES \
>>> +	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
>>>  
>>>  static_assert(NR_BM_PMD_TABLES == 1);
>>> +static_assert(NR_BM_PUD_TABLES == 1);
>>
>> But if there is only one PMD table, there must only be one PUD table (and one
>> P4D table, and one PGD table) by definition. So I don't think this is checking
>> anything that the existing PMD table check is not.
> 
> Agreed but should not this check help tighten this code up given that bm_pud[]
> array gets used for possible PUD entries ?

I don't see the value of it myself, given the PMD assert is already doing the
required check.

> 
>>
>> Thanks,
>> Ryan
>>
>>>  
>>>  #define __BM_TABLE_IDX(addr, shift) \
>>>  	(((addr) >> (shift)) - (FIXADDR_TOT_START >> (shift)))
>>
> 

Re: [PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Mark Rutland]

On Wed, Jan 07, 2026 at 02:37:51PM +0530, Anshuman Khandual wrote:
> 
> 
> On 07/01/26 2:03 PM, Ryan Roberts wrote:
> > On 07/01/2026 06:16, Anshuman Khandual wrote:
> >> There is an implicit assumption about bm_pud[] being single entry PUD table
> >> just like bp_pmd[] which is a single entry PMD table. Though only the later
> >> gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
> >> table as well.
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >>  arch/arm64/mm/fixmap.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
> >> index c5c5425791da..e02594be8b8e 100644
> >> --- a/arch/arm64/mm/fixmap.c
> >> +++ b/arch/arm64/mm/fixmap.c
> >> @@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
> >>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
> >>  #define NR_BM_PMD_TABLES \
> >>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
> >> +#define NR_BM_PUD_TABLES \
> >> +	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
> >>  
> >>  static_assert(NR_BM_PMD_TABLES == 1);
> >> +static_assert(NR_BM_PUD_TABLES == 1);
> > 
> > But if there is only one PMD table, there must only be one PUD table (and one
> > P4D table, and one PGD table) by definition. So I don't think this is checking
> > anything that the existing PMD table check is not.
> 
> Agreed but should not this check help tighten this code up given that bm_pud[]
> array gets used for possible PUD entries ?

An assert isn't going to help with code generation, so this patch alone is not
useful. When I last looked at actually ommiting the table in various
configurations, it was more trouble than it was worth.

As Ryan said, we deliberately only added the assert for the PMD level,
and it's sufficient as-is. That happened in commit:

  414c109bdf49 ("arm64: mm: always map fixmap at page granularity")

... where the commit message said:

    Our VA layout means that the fixmap will always fall within a single PMD
    table (and consequently, within a single PUD/P4D/PGD entry), which we
    can verify at compile time with a static_assert(). With that assert a
    number of runtime warnings become impossible, and are removed.

Please leave this as-is.

Mark.

Re: [PATCH] arm64/mm: Assert NR_BM_PUD_TABLES for bm_pud[]

[Anshuman Khandual]

On 07/01/26 3:41 PM, Mark Rutland wrote:
> On Wed, Jan 07, 2026 at 02:37:51PM +0530, Anshuman Khandual wrote:
>>
>>
>> On 07/01/26 2:03 PM, Ryan Roberts wrote:
>>> On 07/01/2026 06:16, Anshuman Khandual wrote:
>>>> There is an implicit assumption about bm_pud[] being single entry PUD table
>>>> just like bp_pmd[] which is a single entry PMD table. Though only the later
>>>> gets asserted via NR_BM_PMD_TABLES. Hence let's add a similar check for PUD
>>>> table as well.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>  arch/arm64/mm/fixmap.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/mm/fixmap.c b/arch/arm64/mm/fixmap.c
>>>> index c5c5425791da..e02594be8b8e 100644
>>>> --- a/arch/arm64/mm/fixmap.c
>>>> +++ b/arch/arm64/mm/fixmap.c
>>>> @@ -23,8 +23,11 @@ static_assert(FIXADDR_TOT_START > PCI_IO_END);
>>>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PMD_SHIFT)
>>>>  #define NR_BM_PMD_TABLES \
>>>>  	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, PUD_SHIFT)
>>>> +#define NR_BM_PUD_TABLES \
>>>> +	SPAN_NR_ENTRIES(FIXADDR_TOT_START, FIXADDR_TOP, P4D_SHIFT)
>>>>  
>>>>  static_assert(NR_BM_PMD_TABLES == 1);
>>>> +static_assert(NR_BM_PUD_TABLES == 1);
>>>
>>> But if there is only one PMD table, there must only be one PUD table (and one
>>> P4D table, and one PGD table) by definition. So I don't think this is checking
>>> anything that the existing PMD table check is not.
>>
>> Agreed but should not this check help tighten this code up given that bm_pud[]
>> array gets used for possible PUD entries ?
> 
> An assert isn't going to help with code generation, so this patch alone is not
> useful. When I last looked at actually ommiting the table in various
> configurations, it was more trouble than it was worth.
> 
> As Ryan said, we deliberately only added the assert for the PMD level,
> and it's sufficient as-is. That happened in commit:
> 
>   414c109bdf49 ("arm64: mm: always map fixmap at page granularity")
> 
> ... where the commit message said:
> 
>     Our VA layout means that the fixmap will always fall within a single PMD
>     table (and consequently, within a single PUD/P4D/PGD entry), which we
>     can verify at compile time with a static_assert(). With that assert a
>     number of runtime warnings become impossible, and are removed.
> 
> Please leave this as-is.

Alright, makes sense.