Received-SPF: pass (google.com: domain of linux-kernel+bounces-75940-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Michael Ellerman <mpe@ellerman.id.au>
To: Christophe Leroy <christophe.leroy@csgroup.eu>, Nicholas Piggin
 <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>,
 linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 1/2] powerpc: Refactor __kernel_map_pages()
In-Reply-To: <3656d47c53bff577739dac536dbae31fff52f6d8.1708078640.git.christophe.leroy@csgroup.eu>
References: <3656d47c53bff577739dac536dbae31fff52f6d8.1708078640.git.christophe.leroy@csgroup.eu>
Date: Thu, 22 Feb 2024 16:32:31 +1100
Message-ID: <8734tlawsw.fsf@mail.lhotse>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> __kernel_map_pages() is almost identical for PPC32 and RADIX.
>
> Refactor it.
>
> On PPC32 it is not needed for KFENCE, but to keep it simple
> just make it similar to PPC64.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/book3s/64/pgtable.h | 10 ----------
>  arch/powerpc/include/asm/book3s/64/radix.h   |  2 --
>  arch/powerpc/mm/book3s64/radix_pgtable.c     | 14 --------------
>  arch/powerpc/mm/pageattr.c                   | 19 +++++++++++++++++++
>  arch/powerpc/mm/pgtable_32.c                 | 15 ---------------
>  5 files changed, 19 insertions(+), 41 deletions(-)
>
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> index 421db7c4f2a4..16b8d20d6ca8 100644
> --- a/arch/powerpc/mm/pageattr.c
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -101,3 +101,22 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
>  	return apply_to_existing_page_range(&init_mm, start, size,
>  					    change_page_attr, (void *)action);
>  }
> +
> +#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_KFENCE)
> +#ifdef CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC
> +void __kernel_map_pages(struct page *page, int numpages, int enable)
> +{
> +	unsigned long addr = (unsigned long)page_address(page);
> +
> +	if (PageHighMem(page))
> +		return;
> +
> +	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled())
> +		hash__kernel_map_pages(page, numpages, enable);
> +	else if (enable)
> +		set_memory_p(addr, numpages);
> +	else
> +		set_memory_np(addr, numpages);
> +}

This doesn't build on 32-bit, eg. ppc32_allmodconfig:

./arch/powerpc/mm/pageattr.c: In function '__kernel_map_pages':
./arch/powerpc/mm/pageattr.c:116:23: error: implicit declaration of function 'hash__kernel_map_pages' [-Werror=implicit-function-declaration]
  116 |                 err = hash__kernel_map_pages(page, numpages, enable);
      |                       ^~~~~~~~~~~~~~~~~~~~~~

I couldn't see a nice way to get around it, so ended up with:

void __kernel_map_pages(struct page *page, int numpages, int enable)
{
	int err;
	unsigned long addr = (unsigned long)page_address(page);

	if (PageHighMem(page))
		return;

#ifdef CONFIG_PPC_BOOK3S_64
	if (!radix_enabled())
		err = hash__kernel_map_pages(page, numpages, enable);
	else
#endif
	if (enable)
		err = set_memory_p(addr, numpages);
	else
		err = set_memory_np(addr, numpages);


cheers