Re: [PULL 15/21] linux-user/riscv: Add vdso

[Vineet Gupta]

On 1/12/24 13:35, Richard Henderson wrote:
> On 1/12/24 08:49, Vineet Gupta wrote:
>> Hi Richard, Alistair
>>
>> On 10/30/23 14:17, Richard Henderson wrote:
>>> diff --git a/linux-user/riscv/Makefile.vdso b/linux-user/riscv/Makefile.vdso
>>> new file mode 100644
>>> index 0000000000..2c257dbfda
>>> --- /dev/null
>>> +++ b/linux-user/riscv/Makefile.vdso
>>> @@ -0,0 +1,15 @@
>>> +include $(BUILD_DIR)/tests/tcg/riscv64-linux-user/config-target.mak
>>> +
>>> +SUBDIR = $(SRC_PATH)/linux-user/riscv
>>> +VPATH += $(SUBDIR)
>>> +
>>> +all: $(SUBDIR)/vdso-32.so $(SUBDIR)/vdso-64.so
>>> +
>>> +LDFLAGS = -nostdlib -shared -fpic -Wl,-h,linux-vdso.so.1 
>>> -Wl,--build-id=sha1 \
>>> +     -Wl,--hash-style=both -Wl,-T,$(SUBDIR)/vdso.ld
>>> +
>>> +$(SUBDIR)/vdso-32.so: vdso.S vdso.ld vdso-asmoffset.h
>>> +   $(CC) -o $@ $(LDFLAGS) -mabi=ilp32d -march=rv32g $<
>>> +
>>> +$(SUBDIR)/vdso-64.so: vdso.S vdso.ld vdso-asmoffset.h
>>> +   $(CC) -o $@ $(LDFLAGS) -mabi=lp64d -march=rv64g $<
>> So by default qemu ships the vdso binary. How can one rebuild it ?
>>
>>  From skimming the build files it seems following ought to do it
>>      make update-linux-vdso
>>
>> with a prior configure cmd like below with PATH pointing to the cross
>> compiler.
>> ../configure  --target-list=riscv64-linux-user
>> --cross-cc-riscv64=riscv64-unknown-linux-gnu-gcc
> Yes, that should do it.
>
>> But it doesn't, I'm sure we are missing something basis here.
> Do you get an error message?
> Did $(BUILD_DIR)/tests/tcg/riscv64-linux-user/config-target.mak get created 
> properly?

It was indeed, but invoking make wasn't doing anything.

Weirdly enought, after (yet another) clean rebuild the vdso does seem to
build.
So the vdso is subsumed into the qemu binary ? I mean do we need to
rebuild qemu after the vdso rebuild. Something in the build dependency
is off for sure.
Sorry but do u have a cmdline for dummies to rebuild vdso and then qemu.

Thing is something is definitely off with 8.2.

With 2 setups of riscv-gnu-toolchain and everything being same except
for qemu (v8.2 and v8.1.2) we see 4 additional failures in gcc testsuite.
These all pertain to unwinding off signal stack

                 ========= Summary of gcc testsuite =========
| # of unexpected case/ # of unique unexpected case
|          gcc |         g++ |     gfortran |
rv64imafdc_zba_zbb_zbs_zicond/  lp64d/ medlow |  40 /    19 |   23 /     8 |   
72 /    12 |       # v8.2
rv64imafdc_zba_zbb_zbs_zicond/  lp64d/ medlow |  36 /    15 |    7 /     4 |    
  - |             # v8.1.2

FAIL: gcc.dg/cleanup-10.c execution test
FAIL: gcc.dg/cleanup-11.c execution test
FAIL: gcc.dg/cleanup-8.c execution test
FAIL: gcc.dg/cleanup-9.c execution test

Now if only I could rebuild vdso/qemu with the revert of following and
the reg size change.
     2021-07-06 468c1bb5cac9 linux-user/riscv: Add vdso  


>
>> For starters we saw something that seems like a thinko in
>>
>> diff --git a/linux-user/riscv/vdso.S b/linux-user/riscv/vdso.S
>> -#define sizeof_reg     (__riscv_xlen / 4)
>> +#define sizeof_reg     (__riscv_xlen / 8)
> Oops.

Thx for taking care of that.


>> As as aside, we also see that rt_sigreturn in kernel vdso elides the
>> explicit the call frame information. Again we naively don't know if that
>> is required in qemu.
>>
>>      .text
>> ENTRY(__vdso_rt_sigreturn)
>>      .cfi_startproc
>>      .cfi_signal_frame
>>      li a7, __NR_rt_sigreturn
>>      ecall
>>      .cfi_endproc
>> ENDPROC(__vdso_rt_sigreturn)
> Perhaps it's not required, no.  But I'd consider the lack of info from the 
> kernel to be a 
> bug.  Lack of it means places like gcc have to have special cases.