Re: [PATCH 09/11] target/mips: Always enable CONFIG_SEMIHOSTING

[Philippe Mathieu-Daudé]

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