Like other architectures, use the common cmd_vdso_check to make sure of
that.

Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 arch/riscv/kernel/vdso/Makefile | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index 9b517fe1b8a8..dd4877f25928 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -49,7 +49,7 @@ $(obj)/vdso.o: $(obj)/vdso.so
 
 # link rule for the .so file, .lds has to be first
 $(obj)/vdso.so.dbg: $(obj)/vdso.lds $(obj-vdso) FORCE
-	$(call if_changed,vdsold)
+	$(call if_changed,vdsold_and_check)
 LDFLAGS_vdso.so.dbg = -shared -S -soname=linux-vdso.so.1 \
 	--build-id=sha1 --hash-style=both --eh-frame-hdr
 
@@ -69,7 +69,8 @@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE
 # actual build commands
 # The DSO images are built using a special linker script
 # Make sure only to export the intended __vdso_xxx symbol offsets.
-quiet_cmd_vdsold = VDSOLD  $@
-      cmd_vdsold = $(LD) $(ld_flags) -T $(filter-out FORCE,$^) -o $@.tmp && \
+quiet_cmd_vdsold_and_check = VDSOLD  $@
+      cmd_vdsold_and_check = $(LD) $(ld_flags) -T $(filter-out FORCE,$^) -o $@.tmp && \
                    $(OBJCOPY) $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
-                   rm $@.tmp
+                   rm $@.tmp && \
+                   $(cmd_vdso_check)
-- 
2.39.2

Hey Alex,

Alexandre Ghiti <alexghiti@rivosinc.com> writes:

> Like other architectures, use the common cmd_vdso_check to make sure of
> that.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Looking in PW, this fails on rv32? Is that a false negative?

Other than that,
Reviewed-by: Björn Töpel <bjorn@rivosinc.com>

Hi Björn,

On 03/07/2024 14:41, Björn Töpel wrote:
> Hey Alex,
>
> Alexandre Ghiti <alexghiti@rivosinc.com> writes:
>
>> Like other architectures, use the common cmd_vdso_check to make sure of
>> that.
>>
>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Looking in PW, this fails on rv32? Is that a false negative?


I had missed that, thanks.

So it's not a false positive, the check this patch adds actually reveals 
an issue: gcc seems to be smart enough to insert a memset() in 
riscv_vdso_get_cpus(). And this call to memset() inserts a relocation. 
Weirdly, only on rv32.

I managed to fix this by adding:

ccflags-y += -fno-tree-loop-distribute-patterns

I'll send this fix soon.

Thanks,


>
> Other than that,
> Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Hi Alexandre,

26.03.2024 14:38, Alexandre Ghiti wrote:
> 
> Like other architectures, use the common cmd_vdso_check to make sure of
> that.
> 
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>  arch/riscv/kernel/vdso/Makefile | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
> index 9b517fe1b8a8..dd4877f25928 100644
> --- a/arch/riscv/kernel/vdso/Makefile
> +++ b/arch/riscv/kernel/vdso/Makefile
> @@ -49,7 +49,7 @@ $(obj)/vdso.o: $(obj)/vdso.so
> 
>  # link rule for the .so file, .lds has to be first
>  $(obj)/vdso.so.dbg: $(obj)/vdso.lds $(obj-vdso) FORCE
> -       $(call if_changed,vdsold)
> +       $(call if_changed,vdsold_and_check)
>  LDFLAGS_vdso.so.dbg = -shared -S -soname=linux-vdso.so.1 \
>         --build-id=sha1 --hash-style=both --eh-frame-hdr
> 
> @@ -69,7 +69,8 @@ include/generated/vdso-offsets.h: $(obj)/vdso.so.dbg FORCE
>  # actual build commands
>  # The DSO images are built using a special linker script
>  # Make sure only to export the intended __vdso_xxx symbol offsets.
> -quiet_cmd_vdsold = VDSOLD  $@
> -      cmd_vdsold = $(LD) $(ld_flags) -T $(filter-out FORCE,$^) -o $@.tmp && \
> +quiet_cmd_vdsold_and_check = VDSOLD  $@
> +      cmd_vdsold_and_check = $(LD) $(ld_flags) -T $(filter-out FORCE,$^) -o $@.tmp && \
>                     $(OBJCOPY) $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
> -                   rm $@.tmp
> +                   rm $@.tmp && \
> +                   $(cmd_vdso_check)

For some reason I thought that this check is in the common vdso code and just missed hwprobe relocation...

Tested-by: Vladimir Isaev <vladimir.isaev@syntacore.com>

> --
> 2.39.2
> 

Thank you,
Vladimir Isaev