[PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

Thomas Huth posted 8 patches 2 months, 2 weeks ago
[PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"
Posted by Thomas Huth 2 months, 2 weeks ago
This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.

Using "imply" instead of "select" is causing a build failure:

 /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'

Thus revert to fix the build.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/riscv/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index c332616d36..5f30df22f2 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
 config RISCV32
     bool
-    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
 
 config RISCV64
     bool
-    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
-- 
2.46.0
Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"
Posted by Peter Maydell 2 months, 2 weeks ago
On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>
> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>
> Using "imply" instead of "select" is causing a build failure:
>
>  /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'
>
> Thus revert to fix the build.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  target/riscv/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
> index c332616d36..5f30df22f2 100644
> --- a/target/riscv/Kconfig
> +++ b/target/riscv/Kconfig
> @@ -1,9 +1,9 @@
>  config RISCV32
>      bool
> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>      select DEVICE_TREE # needed by boot.c
>
>  config RISCV64
>      bool
> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>      select DEVICE_TREE # needed by boot.c

This will break the intended "we don't need semihosting if
this is a KVM-only compile", though. Can we fix the
build problem use see with
 "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

?

-- PMM
Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
On 5/9/24 21:53, Peter Maydell wrote:
> On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>>
>> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>>
>> Using "imply" instead of "select" is causing a build failure:

(please mention ./configure arguments besides --without-default-devices)

>>
>>   /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'
>>
>> Thus revert to fix the build.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/riscv/Kconfig | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
>> index c332616d36..5f30df22f2 100644
>> --- a/target/riscv/Kconfig
>> +++ b/target/riscv/Kconfig
>> @@ -1,9 +1,9 @@
>>   config RISCV32
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
>>
>>   config RISCV64
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
> 
> This will break the intended "we don't need semihosting if
> this is a KVM-only compile", though. Can we fix the
> build problem use see with
>   "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

We had this discussion with Paolo in
https://lore.kernel.org/qemu-devel/CABgObfbvjG9bBgCwM-kL+YhjhMw1qLnQdQToCEkKW+V3trskoA@mail.gmail.com/
Not sure this is as easy as it looks...

I feel the riscv part could be fixed by a respin of:
https://lore.kernel.org/qemu-devel/20230711121453.59138-1-philmd@linaro.org/
where semihosting is restricted to TCG and isn't an issue
anymore for other accelerators such KVM.

Let me have a try.
Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"
Posted by Thomas Huth 2 months, 2 weeks ago
On 05/09/2024 21.53, Peter Maydell wrote:
> On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>>
>> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>>
>> Using "imply" instead of "select" is causing a build failure:
>>
>>   /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'
>>
>> Thus revert to fix the build.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/riscv/Kconfig | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
>> index c332616d36..5f30df22f2 100644
>> --- a/target/riscv/Kconfig
>> +++ b/target/riscv/Kconfig
>> @@ -1,9 +1,9 @@
>>   config RISCV32
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
>>
>>   config RISCV64
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
> 
> This will break the intended "we don't need semihosting if
> this is a KVM-only compile", though. Can we fix the
> build problem use see with
>   "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

I haven't tried, but I assume that this will produce the same linking issues 
when TCG is disabled. Maybe best if we fix it in the code, see the patch 
suggested here:

  https://lore.kernel.org/qemu-devel/20240906080928.710051-1-thuth@redhat.com/

  Thomas