Re: [PATCH] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Thomas Huth <thuth@redhat.com>]

On 06/09/2024 10.58, Peter Maydell wrote:
> On Fri, 6 Sept 2024 at 09:09, Thomas Huth <thuth@redhat.com> wrote:
>>
>> When QEMU has been configured with "--without-default-devices", the build
>> is currently failing with:
>>
>>   /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
>>    in function `riscv_cpu_do_interrupt':
>>   .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
>>    reference to `do_common_semihosting'
>>
>> Avoid calling into do_common_semihosting() if the corresponding Kconfig
>> switch has not been set.
> 
> This would be inconsistent with Arm, where you always
> get semihosting if you're using TCG. (For KVM, semihosting
> is up to the kernel to provide, which is why we don't
> want the code in that case.)
> 
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/riscv/cpu_helper.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 395a1d9140..c7a6569e2d 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -34,6 +34,10 @@
>>   #include "debug.h"
>>   #include "tcg/oversized-guest.h"
>>
>> +#ifndef CONFIG_USER_ONLY
>> +#include CONFIG_DEVICES
>> +#endif
>> +
>>   int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
>>   {
>>   #ifdef CONFIG_USER_ONLY
>> @@ -1674,10 +1678,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>>       if (!async) {
>>           /* set tval to badaddr for traps with address information */
>>           switch (cause) {
>> +#ifdef CONFIG_ARM_COMPATIBLE_SEMIHOSTING
>>           case RISCV_EXCP_SEMIHOST:
>>               do_common_semihosting(cs);
>>               env->pc += 4;
>>               return;
>> +#endif
>>           case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
>>           case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
>>           case RISCV_EXCP_LOAD_ADDR_MIS:
> 
> If you do want to do this thhen this isn't sufficient, because
> you would also need to change the code that generates the
> RISCV_EXCP_SEMIHOST exception so that it instead generates
> the "behave as if we don't have semihosting and the
> semihosting-trap instruction sequence were executed "normally".
> But I think the best thing is to use "select if TCG" in the Kconfig.

Ok, but I think we then still need a #ifdef CONFIG_TCG here, otherwise 
linking will fail for KVM-only builds?

  Thomas