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

Pedro Falcato <[email protected]>
Newsgroups dev.linux.lists.loongarch,org.infradead.lists.kvm-riscv,org.infradead.lists.linux-riscv,org.kernel.vger.linux-arch,org.kernel.vger.linux-csky,org.kernel.vger.linux-kernel,org.kernel.vger.linux-m68k,org.kernel.vger.linux-mips,org.kernel.vger.linux-openrisc,org.kvack.linux-mm
Message-ID <alYL7B0HAvmLVHGU@pedro-suse>
On Mon, Jul 13, 2026 at 02:55:41PM +0100, Yeoreum Yun wrote:
> From: "David Hildenbrand (Arm)" <[email protected]>
> 
> 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) <[email protected]>
> Signed-off-by: Yeoreum Yun <[email protected]>
> ---
>  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. I would recommend either forcing a struct here, or
using a u64 with bitmasks/shifts.

-- 
Pedro
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.