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

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

Posted by Thomas Huth 2 months, 2 weeks ago

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.

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:
-- 
2.46.0

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

Posted by Peter Maydell 2 months, 2 weeks ago

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.

thanks
-- PMM

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

Posted by Thomas Huth 2 months, 2 weeks ago

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

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

Posted by Peter Maydell 2 months, 2 weeks ago

On Fri, 6 Sept 2024 at 10:30, Thomas Huth <thuth@redhat.com> wrote:
>
> 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.)

> > 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?

Yes, I expect so -- the equivalent code in target/arm is
wrapped in #ifdef CONFIG_TCG.

-- PMM