Re: [PATCH] RISC-V: Fixup clear_page export when using Zicboz

From: Andrew Jones <ajones@ventanamicro.com>
To: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: linux-riscv@lists.infradead.org,
	Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
Subject: Re: [PATCH] RISC-V: Fixup clear_page export when using  Zicboz
Date: Fri, 24 Feb 2023 15:18:12 +0100	[thread overview]
Message-ID: <20230224141812.nftdkbtt4gbgvnts@orel> (raw)
In-Reply-To: <20230224135844.619054-1-ben.dooks@codethink.co.uk>

On Fri, Feb 24, 2023 at 01:58:44PM +0000, Ben Dooks wrote:
> When the clear_page() via Zicboz is enabled, the module build
> fails as clear_page() is not marked as a ksym entry. Fix this
> by changing the asm code to use <asm-generic/export.h> to add
> the correct export.
> 
> Also remove the weak clear_page() as there's nothing else in
> the build defining this symbol, so just make it the entry when
> the Zicboz is enabled.
> 
> Fixes modpost errors such as this:
> ERROR: modpost: "clear_page" [drivers/gpu/drm/ttm/ttm.ko] undefined!
> 
> Reported-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>  arch/riscv/lib/clear_page.S | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/lib/clear_page.S b/arch/riscv/lib/clear_page.S
> index 7c7fa45b5ab5..4ed5fef52d80 100644
> --- a/arch/riscv/lib/clear_page.S
> +++ b/arch/riscv/lib/clear_page.S
> @@ -6,6 +6,7 @@
>  #include <linux/linkage.h>
>  #include <asm/asm.h>
>  #include <asm/alternative-macros.h>
> +#include <asm-generic/export.h>
>  #include <asm/hwcap.h>
>  #include <asm/insn-def.h>
>  #include <asm/page.h>
> @@ -17,7 +18,7 @@
>  
>  /* void clear_page(void *page) */
>  ENTRY(__clear_page)
> -WEAK(clear_page)
> +SYM_FUNC_START(clear_page)
>  	li	a2, PAGE_SIZE
>  
>  	/*
> @@ -70,4 +71,5 @@ WEAK(clear_page)
>  .Lno_zicboz:
>  	li	a1, 0
>  	tail	__memset
> -END(__clear_page)
> +SYM_FUNC_END(clear_page)
> +EXPORT_SYMBOL(clear_page)
> -- 
> 2.39.1
>

Hi Ben,

This looks good to me. I agree that I shouldn't have made clear_page weak,
but not because there isn't currently anything else defining it, but
because it's not something a subsystem is likely to ever define. Exporting
for modules is also definitely needed.

Is there any reason I shouldn't just squash this into the patch which
introduces clear_page? That patch is still under review, and merging
it broken just to immediately fix it would break bisection for no reason.

Thanks,
drew

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv