Pierrick Bouvier <pierrick.bouv...@linaro.org> writes:

> Hi Kevin,
>
> On 8/2/25 2:58 AM, Kevin Per wrote:
>> Hi,
>> my goal is to count the number of executed instructions. However, I
>> receive an error when running because an assertion is violated but I
>> haven't figured out how to fix it.
>> The crash:
>>     0.275 **
>>     0.275 ERROR:../tests/tcg/plugins/insn.c:97:vcpu_init: assertion
>>     failed: (count > 0)
>>     0.275 Bail out! ERROR:../tests/tcg/plugins/insn.c:97:vcpu_init:
>>     assertion failed: (count > 0)
>>     0.433 Aborted (core dumped)
>> When I execute the command:
>>     /opt/qemu/bin/qemu-system-riscv32 -plugin
>> /opt/qemu/plugins/plugins/
>>     libinsn.so -L /opt/riscv/riscv64-unknown-elf -nographic -machine
>>     spike -bios /work/main
>> Qemu Version: 9.2.2
>> I also tried to add "-smp cpus=1", however, it did not have an
>> effect.
>> I have made a minimal setup to reproduce the issue with docker. You
>> can find it here https://github.com/kper/
>> qemu_riscv32_crash_counting_instructions/ <https://github.com/kper/
>> qemu_riscv32_crash_counting_instructions/tree/master>.
>> Thank you!
>> Best regards,
>> Kevin Per
>
> It can be reproduced without any additional file with both riscv32/64:
> ./build/qemu-system-riscv32 -plugin ./build/tests/tcg/plugins/libinsn.so
>
> When reading htimedelta register, a RISCV_EXCP_ILLEGAL_INST is
> returned from read_htimedelta (called from riscv_csrrw_debug).
> It seems that riscv always define this register and selectively return
> an illegal instruction on access depending if it's available or not.
>
> @Alex: It seems that we should not do this assert in insn plugin
> (added in 6036b9c "tests/tcg: expand insn test case to exercise
> register API"), because some registers access may be illegal.
> In your opinion, should riscv implement this differently or is it
> correct?

It should implement it differently - its conflating an instruction
access to a register which could be illegal with a debug access via the
plugin/gdbstub which should always work.

It looks like 753e3fe207d (RISC-V: Add debug support for accessing
CSRs.) introduced the concept of env->debugger as some sort of
workaround for this. Maybe that needs expanding?

>
> Kevin, you can safely remove this assert for now in insn.c. It should
> not impact anything else.

Its safe for Kevin to remove it, but I think we should keep the assert
upstream as its purpose is to catch things violating the API
expectations.

>
> Thanks for the bug report,
> Pierrick

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to