Received-SPF: pass (google.com: domain of linux-kernel+bounces-189283-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Sat, 25 May 2024 06:54:46 +0200
From: Oscar Salvador <osalvador@suse.de>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jason Gunthorpe <jgg@nvidia.com>, Peter Xu <peterx@redhat.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [RFC PATCH v2 15/20] powerpc/85xx: Switch to 64 bits PGD
Message-ID: <ZlFvFk6QmyD-ieS5@localhost.localdomain>
References: <cover.1715971869.git.christophe.leroy@csgroup.eu>
 <a1d92dd7c390672c163ce0611600dde8cb0eaab4.1715971869.git.christophe.leroy@csgroup.eu>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a1d92dd7c390672c163ce0611600dde8cb0eaab4.1715971869.git.christophe.leroy@csgroup.eu>

On Fri, May 17, 2024 at 09:00:09PM +0200, Christophe Leroy wrote:
> In order to allow leaf PMD entries, switch the PGD to 64 bits entries.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

I do not quite understand this change.
Are not powerE500 and power85xx two different things?
You are changing making it 64 for PPC_E500_64bits, but you are updating head_85xx.
Are they sharing this code?

Also, we would benefit from a slightly bigger changelog, explaining why
do we need this change in some more detail.

 
> diff --git a/arch/powerpc/include/asm/pgtable-types.h b/arch/powerpc/include/asm/pgtable-types.h
> index 082c85cc09b1..db965d98e0ae 100644
> --- a/arch/powerpc/include/asm/pgtable-types.h
> +++ b/arch/powerpc/include/asm/pgtable-types.h
> @@ -49,7 +49,11 @@ static inline unsigned long pud_val(pud_t x)
>  #endif /* CONFIG_PPC64 */
>  
>  /* PGD level */
> +#if defined(CONFIG_PPC_E500) && defined(CONFIG_PTE_64BIT)
> +typedef struct { unsigned long long pgd; } pgd_t;
> +#else
>  typedef struct { unsigned long pgd; } pgd_t;
> +#endif
>  #define __pgd(x)	((pgd_t) { (x) })
>  static inline unsigned long pgd_val(pgd_t x)
>  {
> diff --git a/arch/powerpc/kernel/head_85xx.S b/arch/powerpc/kernel/head_85xx.S
> index 39724ff5ae1f..a305244afc9f 100644
> --- a/arch/powerpc/kernel/head_85xx.S
> +++ b/arch/powerpc/kernel/head_85xx.S
> @@ -307,8 +307,9 @@ set_ivor:
>  #ifdef CONFIG_PTE_64BIT
>  #ifdef CONFIG_HUGETLB_PAGE
>  #define FIND_PTE	\
> -	rlwinm	r12, r10, 13, 19, 29;	/* Compute pgdir/pmd offset */	\
> -	lwzx	r11, r12, r11;		/* Get pgd/pmd entry */		\
> +	rlwinm	r12, r10, 14, 18, 28;	/* Compute pgdir/pmd offset */	\
> +	add	r12, r11, r12;

You add the offset to pgdir? 

> +	lwz	r11, 4(r12);		/* Get pgd/pmd entry */		\

What is i offset 4?


-- 
Oscar Salvador
SUSE Labs