Re: [RFC PATCH 02/34] ARM: mm: make 2-level pgd_t a scalar

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/14/26 12:26, Pedro Falcato wrote:
> On Mon, Jul 13, 2026 at 02:55:41PM +0100, Yeoreum Yun wrote:
>> From: "David Hildenbrand (Arm)" <david@kernel.org>
>>
>> We don't want pgd_t to be an array, as it prohibits returning it from a
>> function, like pgdp_get().
>>
>> So let's just use an u64, and extract the right 32bit value in
>> pgd_val().
>>
>> Leave the STRICT_MM_TYPECHECKS case alone for now.
> 
> I have to ask: is there a good reason for the STRICT_MM_TYPECHECKS ifdef?
> 
> I see the compiler has an awkward time returning a u64 struct (see
> https://godbolt.org/z/qejbv6j9a), but if this doesn't work maybe we should
> get rid of the STRICT_MM_TYPECHECKS stuff? I seriously doubt anyone is
> purposefully toggling it on for testing from time to time.
> 
> If STRICT_MM_TYPEDEFS's worse codegen doesn't matter then maybe we should
> permanently toggle it on.
>>
>> As an alternative, we could use the STRICT_MM_TYPECHECKS approach here
>> as well, but using an u64 looks conceptually cleaner, even though
>> pgd_val() gets a bit more involved.
>>
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>> ---
>>  arch/arm/include/asm/pgtable-2level-types.h | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/pgtable-2level-types.h b/arch/arm/include/asm/pgtable-2level-types.h
>> index 650e793f41429..02052cef9437a 100644
>> --- a/arch/arm/include/asm/pgtable-2level-types.h
>> +++ b/arch/arm/include/asm/pgtable-2level-types.h
>> @@ -25,7 +25,7 @@ typedef struct { pteval_t pgprot; } pgprot_t;
>>  
>>  #define pte_val(x)      ((x).pte)
>>  #define pmd_val(x)      ((x).pmd)
>> -#define pgd_val(x)	((x).pgd[0])
>> +#define pgd_val(x)      ((x).pgd[0])
>>  #define pgprot_val(x)   ((x).pgprot)
>>  
>>  #define __pte(x)        ((pte_t) { (x) } )
>> @@ -36,14 +36,21 @@ typedef struct { pteval_t pgprot; } pgprot_t;
>>  /*
>>   * .. while these make it easier on the compiler
>>   */
>> +typedef u64 pgdval_t;
>> +
>>  typedef pteval_t pte_t;
>>  typedef pmdval_t pmd_t;
>> -typedef pmdval_t pgd_t[2];
>> +typedef pgdval_t pgd_t;
>>  typedef pteval_t pgprot_t;
>>  
>>  #define pte_val(x)      (x)
>>  #define pmd_val(x)      (x)
>> -#define pgd_val(x)	((x)[0])
>> +
>> +static inline pmdval_t pgd_val(pgd_t pgd)
>> +{
>> +	return (*(pmdval_t (*)[2])&pgd)[0];
> 
> Ugh. This isn't correct C code. It only works because the kernel passes
> -fno-strict-aliasing.

So it is correct C code? ;)

> I would recommend either forcing a struct here, or
> using a u64 with bitmasks/shifts.

I had that, but little vs. big endian was annoying.

-- 
Cheers,

David