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

Andrew Jones
2023-02-24T14:18:36Z

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