riscv: Add semihosting support [v10]

[PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Keith Packard via 4 years, 11 months ago

This commit renames two files which provide ARM semihosting support so
that they can be shared by other architectures:

 1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
 2. linux-user/arm/semihost.c -> linux-user/semihost.c

The build system was modified to reflect this change, but the contents
of the two files are unchanged.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
 hw/semihosting/meson.build                            | 2 ++
 linux-user/arm/meson.build                            | 3 ---
 linux-user/meson.build                                | 2 ++
 linux-user/{arm => }/semihost.c                       | 0
 target/arm/meson.build                                | 2 --
 6 files changed, 4 insertions(+), 5 deletions(-)
 rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
 rename linux-user/{arm => }/semihost.c (100%)

diff --git a/target/arm/arm-semi.c b/hw/semihosting/common-semi.c
similarity index 100%
rename from target/arm/arm-semi.c
rename to hw/semihosting/common-semi.c
diff --git a/hw/semihosting/meson.build b/hw/semihosting/meson.build
index f40ac574c4..fbd2841e59 100644
--- a/hw/semihosting/meson.build
+++ b/hw/semihosting/meson.build
@@ -2,3 +2,5 @@ specific_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files(
   'config.c',
   'console.c',
 ))
+
+specific_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
diff --git a/linux-user/arm/meson.build b/linux-user/arm/meson.build
index 432984b58e..5a93c925cf 100644
--- a/linux-user/arm/meson.build
+++ b/linux-user/arm/meson.build
@@ -1,6 +1,3 @@
-linux_user_ss.add(when: 'TARGET_AARCH64', if_true: files('semihost.c'))
-linux_user_ss.add(when: 'TARGET_ARM', if_true: files('semihost.c'))
-
 subdir('nwfpe')
 
 syscall_nr_generators += {
diff --git a/linux-user/meson.build b/linux-user/meson.build
index 2b94e4ba24..2fdd12cee5 100644
--- a/linux-user/meson.build
+++ b/linux-user/meson.build
@@ -17,6 +17,8 @@ linux_user_ss.add(rt)
 linux_user_ss.add(when: 'TARGET_HAS_BFLT', if_true: files('flatload.c'))
 linux_user_ss.add(when: 'TARGET_I386', if_true: files('vm86.c'))
 
+linux_user_ss.add(when: 'TARGET_AARCH64', if_true: files('semihost.c'))
+linux_user_ss.add(when: 'TARGET_ARM', if_true: files('semihost.c'))
 
 syscall_nr_generators = {}
 
diff --git a/linux-user/arm/semihost.c b/linux-user/semihost.c
similarity index 100%
rename from linux-user/arm/semihost.c
rename to linux-user/semihost.c
diff --git a/target/arm/meson.build b/target/arm/meson.build
index f5de2a77b8..15b936c101 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -32,8 +32,6 @@ arm_ss.add(files(
 ))
 arm_ss.add(zlib)
 
-arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))
-
 arm_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
-- 
2.28.0

Re: [PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Alistair Francis 4 years, 11 months ago

On Mon, Oct 26, 2020 at 2:34 PM Keith Packard via <qemu-devel@nongnu.org> wrote:
>
> This commit renames two files which provide ARM semihosting support so
> that they can be shared by other architectures:
>
>  1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
>  2. linux-user/arm/semihost.c -> linux-user/semihost.c
>
> The build system was modified to reflect this change, but the contents
> of the two files are unchanged.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/arm/arm-semi.c => hw/semihosting/common-semi.c | 0
>  hw/semihosting/meson.build                            | 2 ++
>  linux-user/arm/meson.build                            | 3 ---
>  linux-user/meson.build                                | 2 ++
>  linux-user/{arm => }/semihost.c                       | 0
>  target/arm/meson.build                                | 2 --
>  6 files changed, 4 insertions(+), 5 deletions(-)
>  rename target/arm/arm-semi.c => hw/semihosting/common-semi.c (100%)
>  rename linux-user/{arm => }/semihost.c (100%)
>
> diff --git a/target/arm/arm-semi.c b/hw/semihosting/common-semi.c
> similarity index 100%
> rename from target/arm/arm-semi.c
> rename to hw/semihosting/common-semi.c
> diff --git a/hw/semihosting/meson.build b/hw/semihosting/meson.build
> index f40ac574c4..fbd2841e59 100644
> --- a/hw/semihosting/meson.build
> +++ b/hw/semihosting/meson.build
> @@ -2,3 +2,5 @@ specific_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files(
>    'config.c',
>    'console.c',
>  ))
> +
> +specific_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
> diff --git a/linux-user/arm/meson.build b/linux-user/arm/meson.build
> index 432984b58e..5a93c925cf 100644
> --- a/linux-user/arm/meson.build
> +++ b/linux-user/arm/meson.build
> @@ -1,6 +1,3 @@
> -linux_user_ss.add(when: 'TARGET_AARCH64', if_true: files('semihost.c'))
> -linux_user_ss.add(when: 'TARGET_ARM', if_true: files('semihost.c'))
> -
>  subdir('nwfpe')
>
>  syscall_nr_generators += {
> diff --git a/linux-user/meson.build b/linux-user/meson.build
> index 2b94e4ba24..2fdd12cee5 100644
> --- a/linux-user/meson.build
> +++ b/linux-user/meson.build
> @@ -17,6 +17,8 @@ linux_user_ss.add(rt)
>  linux_user_ss.add(when: 'TARGET_HAS_BFLT', if_true: files('flatload.c'))
>  linux_user_ss.add(when: 'TARGET_I386', if_true: files('vm86.c'))
>
> +linux_user_ss.add(when: 'TARGET_AARCH64', if_true: files('semihost.c'))
> +linux_user_ss.add(when: 'TARGET_ARM', if_true: files('semihost.c'))
>
>  syscall_nr_generators = {}
>
> diff --git a/linux-user/arm/semihost.c b/linux-user/semihost.c
> similarity index 100%
> rename from linux-user/arm/semihost.c
> rename to linux-user/semihost.c
> diff --git a/target/arm/meson.build b/target/arm/meson.build
> index f5de2a77b8..15b936c101 100644
> --- a/target/arm/meson.build
> +++ b/target/arm/meson.build
> @@ -32,8 +32,6 @@ arm_ss.add(files(
>  ))
>  arm_ss.add(zlib)
>
> -arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))
> -
>  arm_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
>
>  arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
> --
> 2.28.0
>
>

Re: [PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Peter Maydell 4 years, 11 months ago

On Mon, 26 Oct 2020 at 21:29, Keith Packard <keithp@keithp.com> wrote:
>
> This commit renames two files which provide ARM semihosting support so
> that they can be shared by other architectures:
>
>  1. target/arm/arm-semi.c     -> hw/semihosting/common-semi.c
>  2. linux-user/arm/semihost.c -> linux-user/semihost.c
>
> The build system was modified to reflect this change, but the contents
> of the two files are unchanged.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
> diff --git a/target/arm/arm-semi.c b/hw/semihosting/common-semi.c
> similarity index 100%
> rename from target/arm/arm-semi.c
> rename to hw/semihosting/common-semi.c
> diff --git a/hw/semihosting/meson.build b/hw/semihosting/meson.build
> index f40ac574c4..fbd2841e59 100644
> --- a/hw/semihosting/meson.build
> +++ b/hw/semihosting/meson.build
> @@ -2,3 +2,5 @@ specific_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files(
>    'config.c',
>    'console.c',
>  ))
> +
> +specific_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))

I think this adds this file to the compilation for all TCG targets;
you only want it for targets which have Arm-semihosting-ABI compatible
semihosting. (Various other targets either don't have semihosting
or have their own ABI.)

thanks
-- PMM

Re: [PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Keith Packard via 4 years, 11 months ago

Peter Maydell <peter.maydell@linaro.org> writes:

>> +specific_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
>
> I think this adds this file to the compilation for all TCG targets;
> you only want it for targets which have Arm-semihosting-ABI compatible
> semihosting. (Various other targets either don't have semihosting
> or have their own ABI.)

Right. These should have been target-specific source sets so that the
file was only added to those targets:

        arm_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
        riscv_ss.add(files('common-semi.c'))

This appears to work in my testing (building arm, risc-v and x86_64
configs).

-- 
-keith

Re: [PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Peter Maydell 4 years, 11 months ago

On Tue, 27 Oct 2020 at 21:56, Keith Packard <keithp@keithp.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> >> +specific_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
> >
> > I think this adds this file to the compilation for all TCG targets;
> > you only want it for targets which have Arm-semihosting-ABI compatible
> > semihosting. (Various other targets either don't have semihosting
> > or have their own ABI.)
>
> Right. These should have been target-specific source sets so that the
> file was only added to those targets:
>
>         arm_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
>         riscv_ss.add(files('common-semi.c'))
>
> This appears to work in my testing (building arm, risc-v and x86_64
> configs).

I'm not a kconfig expert but it might be preferable to have
a new CONFIG_ for arm-semihosting-ABI which the relevant
targets enable. Somebody else may be able to advise.

thanks
-- PMM

Re: [PATCH 1/4] semihosting: Move ARM semihosting code to shared directories

Posted by Keith Packard via 4 years, 11 months ago

Peter Maydell <peter.maydell@linaro.org> writes:

>>         arm_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))
>>         riscv_ss.add(files('common-semi.c'))
>>
>> This appears to work in my testing (building arm, risc-v and x86_64
>> configs).
>
> I'm not a kconfig expert but it might be preferable to have
> a new CONFIG_ for arm-semihosting-ABI which the relevant
> targets enable. Somebody else may be able to advise.

The change above makes this do exactly what the old code did -- add this
file to the arm_ss sourceset whenever CONFIG_TCG is true. arm_ss is
only used to build ARM targets, so this file gets added only for those
targets. Here's what the patch to target/arm/meson.build and
hw/semihosting/meson.build looks like now.

diff --git a/target/arm/meson.build b/target/arm/meson.build
index f5de2a77b8..15b936c101 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -32,8 +32,6 @@ arm_ss.add(files(
 ))
 arm_ss.add(zlib)
 
-arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))
-
 arm_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c', 'kvm64.c'), if_false: files('kvm-stub.c'))
 
 arm_ss.add(when: 'TARGET_AARCH64', if_true: files(

diff --git a/hw/semihosting/meson.build b/hw/semihosting/meson.build
index f40ac574c4..26538e81e7 100644
--- a/hw/semihosting/meson.build
+++ b/hw/semihosting/meson.build
@@ -2,3 +2,5 @@ specific_ss.add(when: 'CONFIG_SEMIHOSTING', if_true: files(
   'config.c',
   'console.c',
 ))
+
+arm_ss.add(when: 'CONFIG_TCG', if_true: files ('common-semi.c'))

-- 
-keith