| Message ID | 20200316120049.11225-10-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel: Allow targets to use Kconfig, disable semihosting by default | expand |
On 3/16/20 5:00 AM, Philippe Mathieu-Daudé wrote: > On MIPS, the semihosting feature is always required on user-space. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > default-configs/mips-linux-user-common.mak | 4 ++++ > default-configs/mips-linux-user.mak | 2 ++ > default-configs/mips64-linux-user.mak | 2 ++ > default-configs/mips64el-linux-user.mak | 2 ++ > default-configs/mipsel-linux-user.mak | 2 ++ > default-configs/mipsn32-linux-user.mak | 2 ++ > default-configs/mipsn32el-linux-user.mak | 2 ++ > 7 files changed, 16 insertions(+) > create mode 100644 default-configs/mips-linux-user-common.mak Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 16/03/20 13:00, Philippe Mathieu-Daudé wrote: > diff --git a/default-configs/mips-linux-user-common.mak b/default-configs/mips-linux-user-common.mak > new file mode 100644 > index 0000000000..04947706e8 > --- /dev/null > +++ b/default-configs/mips-linux-user-common.mak > @@ -0,0 +1,4 @@ > +# Common mips*-linux-user CONFIG defines > + > +# CONFIG_SEMIHOSTING is always required on this architecture > +CONFIG_SEMIHOSTING=y If it is always required, it should be select-ed. Same for patch 10. Paolo
Hi Paolo, TLDR "how can I select a arch-specific feature?" On 3/18/20 11:31 AM, Paolo Bonzini wrote: > On 16/03/20 13:00, Philippe Mathieu-Daudé wrote: >> diff --git a/default-configs/mips-linux-user-common.mak b/default-configs/mips-linux-user-common.mak >> new file mode 100644 >> index 0000000000..04947706e8 >> --- /dev/null >> +++ b/default-configs/mips-linux-user-common.mak >> @@ -0,0 +1,4 @@ >> +# Common mips*-linux-user CONFIG defines >> + >> +# CONFIG_SEMIHOSTING is always required on this architecture >> +CONFIG_SEMIHOSTING=y > > If it is always required, it should be select-ed. I'm not sure how to do that... Currently we have in hw/semihosting/Kconfig: config SEMIHOSTING bool depends on TCG default n The only per-target generic entry point is minikconf command line. 1/ The less ugly option might be to add an optional target-devices.mak: -- >8 -- diff --git a/Makefile.target b/Makefile.target --- a/Makefile.target +++ b/Makefile.target @@ -5,6 +5,7 @@ BUILD_DIR?=$(CURDIR)/.. include ../config-host.mak include config-target.mak include $(SRC_PATH)/rules.mak +-include $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/target-devices.mak ifdef CONFIG_SOFTMMU include config-devices.mak diff --git a/target/arm/target-devices.mak b/target/arm/target-devices.mak new file mode 100644 --- /dev/null +++ b/target/arm/target-devices.mak @@ -0,0 +1 @@ +CONFIG_SEMIHOSTING=y --- 2/ I can have ./configure adding in config-devices.mak: -- >8 -- @@ -7778,6 +7778,7 @@ echo "# Automatically generated by configure - do not modify" > $config_target_m bflt="no" mttcg="no" +target_require_semihosting="no" interp_prefix1=$(echo "$interp_prefix" | sed "s/%M/$target_name/g") gdb_xml_files="" @@ -7806,6 +7807,7 @@ case "$target_name" in TARGET_SYSTBL_ABI=common,oabi bflt="yes" mttcg="yes" + target_require_semihosting="yes" gdb_xml_files="arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" ;; aarch64|aarch64_be) @@ -7813,6 +7815,7 @@ case "$target_name" in TARGET_BASE_ARCH=arm bflt="yes" mttcg="yes" + target_require_semihosting="yes" gdb_xml_files="aarch64-core.xml aarch64-fpu.xml arm-core.xml arm-vfp.xml arm-vfp3.xml arm-neon.xml" ;; cris) @@ -8031,7 +8034,10 @@ fi if test "$target_bsd_user" = "yes" ; then echo "CONFIG_BSD_USER=y" >> $config_target_mak fi +if test "$target_require_semihosting" = "yes" ; then + echo "$target/config-devices.mak: CONFIG_SEMIHOSTING=y" >> $config_host_mak +fi --- 3/ Or force MINIKCONF_ARGS: --- if test "$target_require_semihosting" = "yes" ; then echo "MINIKCONF_ARGS += CONFIG_SEMIHOSTING=y" >> $config_target_mak fi --- Also note for ARM all configs require it, but for MIPS we only want it for user-mode. With 1/ this can be done as: -- >8 -- diff --git a/target/mips/target-devices.mak b/target/mips/target-devices.mak new file mode 100644 --- /dev/null +++ b/target/mips/target-devices.mak @@ -0,0 +1,3 @@ +ifndef CONFIG_SOFTMMU +CONFIG_SEMIHOSTING=y +endif --- > > Same for patch 10. > > Paolo >
On 23/04/20 11:13, Philippe Mathieu-Daudé wrote: >> Let's then: >> >> 1) allow multiply-defined variables (just remove the "if" from >> do_declaration) > Apparently not needed with 2) Since "config SEMIHOSTING" is in hw/semihosting/Kconfig, it should be needed no? >> 2) do >> >> config SEMIHOSTING >> default y if TCG >> >> in target/arm/Kconfig. > You rock! >
On 23/04/20 10:43, Philippe Mathieu-Daudé wrote: >> >> You can add CONFIG_SEMIHOSTING=y directly in the Kconfig file. > > I didn't know because it is not documented and no examples, but I the > code is here: > > # assignment_var: ID (starting with "CONFIG_") > def parse_assignment_var(self): > if self.tok == TOK_ID: > val = self.val > if not val.startswith("CONFIG_"): > raise KconfigParserError(self, > 'Expected identifier starting with > "CONFIG_"', TOK_NONE) > self.get_token() > return self.data.do_var(val[7:]) > else: > raise KconfigParserError(self, 'Expected identifier') > > # assignment: var EQUAL y_or_n > def parse_assignment(self): > var = self.parse_assignment_var() > if self.tok != TOK_EQUAL: > raise KconfigParserError(self, 'Expected "="') > self.get_token() > self.data.do_assignment(var, self.parse_y_or_n()) > > Thanks! Well yeah, it's a bit of a hack and simply the simplest way to implement it. If it turns out that there are other ways to achieve what you need, it's better. Paolo
diff --git a/default-configs/mips-linux-user-common.mak b/default-configs/mips-linux-user-common.mak new file mode 100644 index 0000000000..04947706e8 --- /dev/null +++ b/default-configs/mips-linux-user-common.mak @@ -0,0 +1,4 @@ +# Common mips*-linux-user CONFIG defines + +# CONFIG_SEMIHOSTING is always required on this architecture +CONFIG_SEMIHOSTING=y diff --git a/default-configs/mips-linux-user.mak b/default-configs/mips-linux-user.mak index 31df57021e..c606e12444 100644 --- a/default-configs/mips-linux-user.mak +++ b/default-configs/mips-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mips-linux-user + +include mips-linux-user-common.mak diff --git a/default-configs/mips64-linux-user.mak b/default-configs/mips64-linux-user.mak index 1598bfcf7d..81e16ac2eb 100644 --- a/default-configs/mips64-linux-user.mak +++ b/default-configs/mips64-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mips64-linux-user + +include mips-linux-user-common.mak diff --git a/default-configs/mips64el-linux-user.mak b/default-configs/mips64el-linux-user.mak index 629f084086..6399af3fd5 100644 --- a/default-configs/mips64el-linux-user.mak +++ b/default-configs/mips64el-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mips64el-linux-user + +include mips-linux-user-common.mak diff --git a/default-configs/mipsel-linux-user.mak b/default-configs/mipsel-linux-user.mak index 4d0e4afb69..4a27d30b45 100644 --- a/default-configs/mipsel-linux-user.mak +++ b/default-configs/mipsel-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mipsel-linux-user + +include mips-linux-user-common.mak diff --git a/default-configs/mipsn32-linux-user.mak b/default-configs/mipsn32-linux-user.mak index 5b97919794..f3ac967463 100644 --- a/default-configs/mipsn32-linux-user.mak +++ b/default-configs/mipsn32-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mipsn32-linux-user + +include mips-linux-user-common.mak diff --git a/default-configs/mipsn32el-linux-user.mak b/default-configs/mipsn32el-linux-user.mak index d6367ff987..63fe4de4fd 100644 --- a/default-configs/mipsn32el-linux-user.mak +++ b/default-configs/mipsn32el-linux-user.mak @@ -1 +1,3 @@ # Default configuration for mipsn32el-linux-user + +include mips-linux-user-common.mak
On MIPS, the semihosting feature is always required on user-space. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- default-configs/mips-linux-user-common.mak | 4 ++++ default-configs/mips-linux-user.mak | 2 ++ default-configs/mips64-linux-user.mak | 2 ++ default-configs/mips64el-linux-user.mak | 2 ++ default-configs/mipsel-linux-user.mak | 2 ++ default-configs/mipsn32-linux-user.mak | 2 ++ default-configs/mipsn32el-linux-user.mak | 2 ++ 7 files changed, 16 insertions(+) create mode 100644 default-configs/mips-linux-user-common.mak