[Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

Amusingly Miroslav and myself hit this issue at the same time.

Currently there is no way to pass a CONFIG_X to sources in target/,
except via a Makefile rule (and filling with stubs).

Paolo says this is on purpose, CONFIG_X selectors are meant for
devices and we try to avoid having config-devices.mak in
config-target.h.

Some know (arch-specific) limitations are:

- MIPS ITU is accessed by coprocessor instr (ISA feature)
- MIPS timer is accessed by coprocessor instr (ISA feature)
- MIPS semihosting (ISA feature?)
- ARM semihosting (ISA feature?)
- ARMv7 NVIC (device)

This series attempt to fix this the most trivial way, adding
stubs for unreachable code.

Philippe Mathieu-Daudé (2):
  target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled
  target/mips: Add stubs to build with CONFIG_SEMIHOSTING disabled

 target/arm/Makefile.objs      |  3 ++-
 target/arm/arm-semi-stubs.c   | 21 +++++++++++++++++++++
 target/mips/Makefile.objs     |  3 ++-
 target/mips/mips-semi-stubs.c | 23 +++++++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 target/arm/arm-semi-stubs.c
 create mode 100644 target/mips/mips-semi-stubs.c

-- 
2.20.1

[Qemu-devel] [RFC PATCH 1/2] target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

If a distribution wants to build QEMU without semihosting support,
it currently gets this build failure:

  $ ./configure --target-list=aarch64-softmmu --without-default-devices
  $ sed -i s/CONFIG_SEMIHOSTING=y/CONFIG_SEMIHOSTING=n/ default-configs/arm-softmmu.mak
  $ make subdir-aarch64-softmmu
  [...]
    LINK    aarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: target/arm/arm-semi.o: in function `do_arm_semihosting':
  ./target/arm/arm-semi.c:321: undefined reference to `qemu_semihosting_console_out'
  /usr/bin/ld: ./target/arm/arm-semi.c:318: undefined reference to `qemu_semihosting_console_out'
  collect2: error: ld returned 1 exit status
  make[1]: *** [Makefile:204: qemu-system-aarch64] Error 1
  make: *** [Makefile:472: subdir-aarch64-softmmu] Error 2

Fix it by providing a stub when semihosting is disabled.

Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 target/arm/Makefile.objs    |  3 ++-
 target/arm/arm-semi-stubs.c | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 target/arm/arm-semi-stubs.c

diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
index 6bdcc65c2c..39b02b1fa4 100644
--- a/target/arm/Makefile.objs
+++ b/target/arm/Makefile.objs
@@ -1,4 +1,5 @@
-obj-y += arm-semi.o
+obj-$(CONFIG_SEMIHOSTING) += arm-semi.o
+obj-$(call lnot,$(CONFIG_SEMIHOSTING)) += arm-semi-stubs.o
 obj-$(CONFIG_SOFTMMU) += machine.o psci.o arch_dump.o monitor.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
diff --git a/target/arm/arm-semi-stubs.c b/target/arm/arm-semi-stubs.c
new file mode 100644
index 0000000000..a91ecbd9d5
--- /dev/null
+++ b/target/arm/arm-semi-stubs.c
@@ -0,0 +1,21 @@
+/*
+ *  Arm "Angel" semihosting stubs
+ *
+ * Copyright (c) 2019 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+target_ulong do_arm_semihosting(CPUARMState *env)
+{
+    g_assert_not_reached();
+}
-- 
2.20.1

[Qemu-devel] [RFC PATCH 2/2] target/mips: Add stubs to build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

If a distribution wants to build QEMU without semihosting support,
it currently gets this build failure:

  $ ./configure --target-list=mips64el-softmmu --without-default-devices
  $ sed -i s/CONFIG_SEMIHOSTING=y/CONFIG_SEMIHOSTING=n/ default-configs/mips-softmmu-common.mak
  $ make subdir-mips64el-softmmu
  [...]
    LINK    mips64el-softmmu/qemu-system-mips64el
  /usr/bin/ld: target/mips/mips-semi.o: in function `helper_do_semihosting':
  ./target/mips/mips-semi.c:335: undefined reference to `qemu_semihosting_log_out'
  /usr/bin/ld: ./target/mips/mips-semi.c:338: undefined reference to `qemu_semihosting_log_out'
  collect2: error: ld returned 1 exit status
  make[1]: *** [Makefile:204: qemu-system-mips64el] Error 1
  make: *** [Makefile:472: subdir-mips64el-softmmu] Error 2

Fix it by providing a stub when semihosting is disabled.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
checkpatch ERROR: externs should be avoided in .c files
---
 target/mips/Makefile.objs     |  3 ++-
 target/mips/mips-semi-stubs.c | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 target/mips/mips-semi-stubs.c

diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
index 3448ad5e19..3f4d566a35 100644
--- a/target/mips/Makefile.objs
+++ b/target/mips/Makefile.objs
@@ -1,5 +1,6 @@
 obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
 obj-y += gdbstub.o msa_helper.o
-obj-$(CONFIG_SOFTMMU) += mips-semi.o
+obj-$(CONFIG_SEMIHOSTING) += mips-semi.o
+obj-$(call lnot,$(CONFIG_SEMIHOSTING)) += mips-semi-stubs.o
 obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
 obj-$(CONFIG_KVM) += kvm.o
diff --git a/target/mips/mips-semi-stubs.c b/target/mips/mips-semi-stubs.c
new file mode 100644
index 0000000000..f5b13b9153
--- /dev/null
+++ b/target/mips/mips-semi-stubs.c
@@ -0,0 +1,23 @@
+/*
+ * Unified Hosting Interface syscalls stubs
+ *
+ * Copyright (c) 2019 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+extern void helper_do_semihosting(CPUMIPSState *env);
+
+void helper_do_semihosting(CPUMIPSState *env)
+{
+    g_assert_not_reached();
+}
-- 
2.20.1

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190531154735.20809-1-philmd@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled
Type: series
Message-id: 20190531154735.20809-1-philmd@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190531154735.20809-1-philmd@redhat.com -> patchew/20190531154735.20809-1-philmd@redhat.com
Switched to a new branch 'test'
303fbc811f target/mips: Add stubs to build with CONFIG_SEMIHOSTING disabled
aad1bd85e1 target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled

=== OUTPUT BEGIN ===
1/2 Checking commit aad1bd85e1e5 (target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#40: 
new file mode 100644

total: 0 errors, 1 warnings, 27 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/2 Checking commit 303fbc811f24 (target/mips: Add stubs to build with CONFIG_SEMIHOSTING disabled)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#40: 
new file mode 100644

ERROR: externs should be avoided in .c files
#62: FILE: target/mips/mips-semi-stubs.c:18:
+extern void helper_do_semihosting(CPUMIPSState *env);

total: 1 errors, 1 warnings, 30 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190531154735.20809-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Peter Maydell]

On Fri, 31 May 2019 at 16:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Amusingly Miroslav and myself hit this issue at the same time.
>
> Currently there is no way to pass a CONFIG_X to sources in target/,
> except via a Makefile rule (and filling with stubs).
>
> Paolo says this is on purpose, CONFIG_X selectors are meant for
> devices and we try to avoid having config-devices.mak in
> config-target.h.

...but some things in target/ are devices (like the Arm CPUs,
which inherit from TYPE_DEVICE).

Is there a way we can have a Kconfig fragment that expresses
"if you asked for an Arm CPU then this should 'select SEMIHOSTING'" ?

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

On 5/31/19 6:21 PM, Peter Maydell wrote:
> On Fri, 31 May 2019 at 16:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Amusingly Miroslav and myself hit this issue at the same time.
>>
>> Currently there is no way to pass a CONFIG_X to sources in target/,
>> except via a Makefile rule (and filling with stubs).
>>
>> Paolo says this is on purpose, CONFIG_X selectors are meant for
>> devices and we try to avoid having config-devices.mak in
>> config-target.h.
> 
> ...but some things in target/ are devices (like the Arm CPUs,
> which inherit from TYPE_DEVICE).
> 
> Is there a way we can have a Kconfig fragment that expresses
> "if you asked for an Arm CPU then this should 'select SEMIHOSTING'" ?

Yes, but inversely, we can also deselect a feature, and all features
that requires it get deselected. My guess is Miroslav is building a
KVM-only QEMU, but upstream does not allow to build ARM without TCG.

I'll see what happened to Samuel series "Support disabling TCG on ARM"
and see if it can be salvaged:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html

I suppose in this thread:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05641.html
you refer to this series (not yet merged):
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03137.html

I'll try to figure what "KVM injection of interrupts" is.

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Miroslav Rezanina]

----- Original Message -----
> From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
> To: "Peter Maydell" <peter.maydell@linaro.org>
> Cc: "QEMU Developers" <qemu-devel@nongnu.org>, "Paolo Bonzini" <pbonzini@redhat.com>, "Miroslav Rezanina"
> <mrezanin@redhat.com>, "Richard Henderson" <richard.henderson@linaro.org>, "Aleksandar Rikalo"
> <arikalo@wavecomp.com>, "qemu-arm" <qemu-arm@nongnu.org>, "Aleksandar Markovic" <amarkovic@wavecomp.com>, "Aurelien
> Jarno" <aurelien@aurel32.net>, "Alex Bennée" <alex.bennee@linaro.org>, "Samuel Ortiz" <sameo@linux.intel.com>, "Rob
> Bradford" <robert.bradford@intel.com>
> Sent: Friday, May 31, 2019 6:40:30 PM
> Subject: Re: [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled
> 
> On 5/31/19 6:21 PM, Peter Maydell wrote:
> > On Fri, 31 May 2019 at 16:47, Philippe Mathieu-Daudé <philmd@redhat.com>
> > wrote:
> >>
> >> Amusingly Miroslav and myself hit this issue at the same time.
> >>
> >> Currently there is no way to pass a CONFIG_X to sources in target/,
> >> except via a Makefile rule (and filling with stubs).
> >>
> >> Paolo says this is on purpose, CONFIG_X selectors are meant for
> >> devices and we try to avoid having config-devices.mak in
> >> config-target.h.
> > 
> > ...but some things in target/ are devices (like the Arm CPUs,
> > which inherit from TYPE_DEVICE).
> > 
> > Is there a way we can have a Kconfig fragment that expresses
> > "if you asked for an Arm CPU then this should 'select SEMIHOSTING'" ?
> 
> Yes, but inversely, we can also deselect a feature, and all features
> that requires it get deselected. My guess is Miroslav is building a
> KVM-only QEMU, but upstream does not allow to build ARM without TCG.
> 
> I'll see what happened to Samuel series "Support disabling TCG on ARM"
> and see if it can be salvaged:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html
> 
> I suppose in this thread:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05641.html
> you refer to this series (not yet merged):
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03137.html
> 
> I'll try to figure what "KVM injection of interrupts" is.
> 

What about CONFIG_ARM_VIRT - can we use it to introduce dependency on
CONFIG_SEMIHOSTING or is there valid scenario of qemu build with CONFIG_ARM_VIRT
enabled and CONFIG_SEMIHOSTING disabled?

Mirek
-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Peter Maydell]

On Fri, 31 May 2019 at 17:54, Miroslav Rezanina <mrezanin@redhat.com> wrote:
> What about CONFIG_ARM_VIRT - can we use it to introduce dependency on
> CONFIG_SEMIHOSTING or is there valid scenario of qemu build with CONFIG_ARM_VIRT
> enabled and CONFIG_SEMIHOSTING disabled?

Semihosting is a feature that works on all Arm CPUs
regardless of which machine model you're using (or whether
you're using softmmu or linux-user), so I think
the machine's Kconfig fragment is the wrong place to try
to pull it in.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Peter Maydell]

On Fri, 31 May 2019 at 17:40, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> I'll see what happened to Samuel series "Support disabling TCG on ARM"
> and see if it can be salvaged:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02451.html

That would certainly be useful.

> I suppose in this thread:
> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05641.html
> you refer to this series (not yet merged):
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03137.html
>
> I'll try to figure what "KVM injection of interrupts" is.

This is about some (rare) cases where userspace QEMU in a KVM
setup determines that it needs to deliver an exception to the
guest, and does that by adjusting the CPU state appropriately
(changing PC, PSTATE, ESR_EL1, etc etc) before telling KVM
to KVM_RUN again. Currently we only need that for some places
where we're doing debugging of the guest, but there is that
pending patchset that would also like to do it in case of
a detected hardware memory error. (Most of the time when the
guest takes an exception it's because the host kernel/KVM
have determined that it's necessary and done the relevant
messing with the guest CPU state.)

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Alex Bennée]

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

> On Fri, 31 May 2019 at 17:54, Miroslav Rezanina <mrezanin@redhat.com> wrote:
>> What about CONFIG_ARM_VIRT - can we use it to introduce dependency on
>> CONFIG_SEMIHOSTING or is there valid scenario of qemu build with CONFIG_ARM_VIRT
>> enabled and CONFIG_SEMIHOSTING disabled?
>
> Semihosting is a feature that works on all Arm CPUs
> regardless of which machine model you're using (or whether
> you're using softmmu or linux-user), so I think
> the machine's Kconfig fragment is the wrong place to try
> to pull it in.

Although amusingly it doesn't work in kvm but perhaps it should?

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Peter Maydell]

On Sat, 1 Jun 2019 at 10:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Semihosting is a feature that works on all Arm CPUs
> > regardless of which machine model you're using (or whether
> > you're using softmmu or linux-user), so I think
> > the machine's Kconfig fragment is the wrong place to try
> > to pull it in.
>
> Although amusingly it doesn't work in kvm but perhaps it should?

It would be nice if it did, but the problem IIRC is that semihosting
hooks either SVC or HLT instructions, and inside KVM both of
those go to EL1, ie to the guest, and can't be trapped to KVM.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Paolo Bonzini]

On 31/05/19 18:54, Miroslav Rezanina wrote:
> What about CONFIG_ARM_VIRT - can we use it to introduce dependency on
> CONFIG_SEMIHOSTING or is there valid scenario of qemu build with CONFIG_ARM_VIRT
> enabled and CONFIG_SEMIHOSTING disabled?

If you are not really going to use TCG, disabling SEMIHOSTING makes sense.

I think Philippe's patch are the right way to do it.

Perhaps CONFIG_SEMIHOSTING should be made "default y" and added as
"#CONFIG_SEMIHOSTING=n" to default-configs/, but that's just cosmetic.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

On 6/3/19 10:11 AM, Paolo Bonzini wrote:
> On 31/05/19 18:54, Miroslav Rezanina wrote:
>> What about CONFIG_ARM_VIRT - can we use it to introduce dependency on
>> CONFIG_SEMIHOSTING or is there valid scenario of qemu build with CONFIG_ARM_VIRT
>> enabled and CONFIG_SEMIHOSTING disabled?
> 
> If you are not really going to use TCG, disabling SEMIHOSTING makes sense.
> 
> I think Philippe's patch are the right way to do it.
> 
> Perhaps CONFIG_SEMIHOSTING should be made "default y" and added as
> "#CONFIG_SEMIHOSTING=n" to default-configs/, but that's just cosmetic.

But then it is compiled/linked on target that don't care...

Oh, but this is also true currently:

$ fgrep -r qemu_semihosting_log_out
Binary file ppc-softmmu/qemu-system-ppc matches
...

What about:

  config SEMIHOSTING
      bool
      default n
      depends on !KVM

and keep specific targets using SEMIHOSTING=y

Using "default y" or "default y if !KVM" we have to add SEMIHOSTING=n on
all targets that don't care, which seems an incorrect use of Kconfig.

Aleksandar: Can we use SEMIHOSTING on KVM MIPS?

For ARM Peter said:

"semihosting hooks either SVC or HLT instructions, and inside
 KVM both of those go to EL1, ie to the guest, and can't be
 trapped to KVM."

Thanks,

Phil.

Re: [Qemu-devel] [RFC PATCH 0/2] target: Build with CONFIG_SEMIHOSTING disabled

[Aleksandar Markovic]

> Aleksandar: Can we use SEMIHOSTING on KVM MIPS?
>

You can assume the answer is no, we can't. But James Hogan, who maintains
MIPS KVM, may have different view, and his answer would override mine.

Yours,
Aleksandar

> For ARM Peter said:
>
> "semihosting hooks either SVC or HLT instructions, and inside
>  KVM both of those go to EL1, ie to the guest, and can't be
>  trapped to KVM."
>
> Thanks,
>
> Phil.
>

Re: [Qemu-devel] [RFC PATCH 1/2] target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> If a distribution wants to build QEMU without semihosting support,
> it currently gets this build failure:
>
>   $ ./configure --target-list=aarch64-softmmu --without-default-devices
>   $ sed -i s/CONFIG_SEMIHOSTING=y/CONFIG_SEMIHOSTING=n/default-configs/arm-softmmu.mak

I'm still not convinced we should be adding support for stuff being done
outside the normal build process... that said...

>   $ make subdir-aarch64-softmmu
>   [...]
>     LINK    aarch64-softmmu/qemu-system-aarch64
>   /usr/bin/ld: target/arm/arm-semi.o: in function `do_arm_semihosting':
>   ./target/arm/arm-semi.c:321: undefined reference to `qemu_semihosting_console_out'
>   /usr/bin/ld: ./target/arm/arm-semi.c:318: undefined reference to `qemu_semihosting_console_out'
>   collect2: error: ld returned 1 exit status
>   make[1]: *** [Makefile:204: qemu-system-aarch64] Error 1
>   make: *** [Makefile:472: subdir-aarch64-softmmu] Error 2
>
> Fix it by providing a stub when semihosting is disabled.
>
> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  target/arm/Makefile.objs    |  3 ++-
>  target/arm/arm-semi-stubs.c | 21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 target/arm/arm-semi-stubs.c
>
> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
> index 6bdcc65c2c..39b02b1fa4 100644
> --- a/target/arm/Makefile.objs
> +++ b/target/arm/Makefile.objs
> @@ -1,4 +1,5 @@
> -obj-y += arm-semi.o
> +obj-$(CONFIG_SEMIHOSTING) += arm-semi.o
> +obj-$(call lnot,$(CONFIG_SEMIHOSTING)) += arm-semi-stubs.o
>  obj-$(CONFIG_SOFTMMU) += machine.o psci.o arch_dump.o monitor.o
>  obj-$(CONFIG_KVM) += kvm.o
>  obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
> diff --git a/target/arm/arm-semi-stubs.c b/target/arm/arm-semi-stubs.c
> new file mode 100644
> index 0000000000..a91ecbd9d5
> --- /dev/null
> +++ b/target/arm/arm-semi-stubs.c
> @@ -0,0 +1,21 @@
> +/*
> + *  Arm "Angel" semihosting stubs
> + *
> + * Copyright (c) 2019 Red Hat, Inc.
> + *
> + * Author:
> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +
> +target_ulong do_arm_semihosting(CPUARMState *env)
> +{
> +    g_assert_not_reached();
> +}

Could this not just be added to arm-semi.c in an

  #ifndef CONFIG_SEMIHOSTING
  target_ulong do_arm_semihosting(CPUARMState *env)
  {
      g_assert_not_reached();
  }
  #else
  ... rest of arm-semi.c....
  #endif


--
Alex Bennée

Re: [Qemu-devel] [RFC PATCH 1/2] target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

On 6/17/19 5:19 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> If a distribution wants to build QEMU without semihosting support,
>> it currently gets this build failure:
>>
>>   $ ./configure --target-list=aarch64-softmmu --without-default-devices
>>   $ sed -i s/CONFIG_SEMIHOSTING=y/CONFIG_SEMIHOSTING=n/default-configs/arm-softmmu.mak
> 
> I'm still not convinced we should be adding support for stuff being done
> outside the normal build process...

If the "KVM/TCG split" series is accepted, then we can add a
--disable-tcg job in the list of "normal builds" and we don't need this
particular patch.

> that said...
> 
>>   $ make subdir-aarch64-softmmu
>>   [...]
>>     LINK    aarch64-softmmu/qemu-system-aarch64
>>   /usr/bin/ld: target/arm/arm-semi.o: in function `do_arm_semihosting':
>>   ./target/arm/arm-semi.c:321: undefined reference to `qemu_semihosting_console_out'
>>   /usr/bin/ld: ./target/arm/arm-semi.c:318: undefined reference to `qemu_semihosting_console_out'
>>   collect2: error: ld returned 1 exit status
>>   make[1]: *** [Makefile:204: qemu-system-aarch64] Error 1
>>   make: *** [Makefile:472: subdir-aarch64-softmmu] Error 2
>>
>> Fix it by providing a stub when semihosting is disabled.
>>
>> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  target/arm/Makefile.objs    |  3 ++-
>>  target/arm/arm-semi-stubs.c | 21 +++++++++++++++++++++
>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>  create mode 100644 target/arm/arm-semi-stubs.c
>>
>> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
>> index 6bdcc65c2c..39b02b1fa4 100644
>> --- a/target/arm/Makefile.objs
>> +++ b/target/arm/Makefile.objs
>> @@ -1,4 +1,5 @@
>> -obj-y += arm-semi.o
>> +obj-$(CONFIG_SEMIHOSTING) += arm-semi.o
>> +obj-$(call lnot,$(CONFIG_SEMIHOSTING)) += arm-semi-stubs.o
>>  obj-$(CONFIG_SOFTMMU) += machine.o psci.o arch_dump.o monitor.o
>>  obj-$(CONFIG_KVM) += kvm.o
>>  obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
>> diff --git a/target/arm/arm-semi-stubs.c b/target/arm/arm-semi-stubs.c
>> new file mode 100644
>> index 0000000000..a91ecbd9d5
>> --- /dev/null
>> +++ b/target/arm/arm-semi-stubs.c
>> @@ -0,0 +1,21 @@
>> +/*
>> + *  Arm "Angel" semihosting stubs
>> + *
>> + * Copyright (c) 2019 Red Hat, Inc.
>> + *
>> + * Author:
>> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +
>> +target_ulong do_arm_semihosting(CPUARMState *env)
>> +{
>> +    g_assert_not_reached();
>> +}
> 
> Could this not just be added to arm-semi.c in an
> 
>   #ifndef CONFIG_SEMIHOSTING
>   target_ulong do_arm_semihosting(CPUARMState *env)
>   {
>       g_assert_not_reached();
>   }
>   #else
>   ... rest of arm-semi.c....
>   #endif

OK, thanks for your reviews!

Phil.

Re: [Qemu-devel] [RFC PATCH 1/2] target/arm: Add stubs to build with CONFIG_SEMIHOSTING disabled

[Philippe Mathieu-Daudé]

On 6/17/19 5:33 PM, Philippe Mathieu-Daudé wrote:
> On 6/17/19 5:19 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> If a distribution wants to build QEMU without semihosting support,
>>> it currently gets this build failure:
>>>
>>>   $ ./configure --target-list=aarch64-softmmu --without-default-devices
>>>   $ sed -i s/CONFIG_SEMIHOSTING=y/CONFIG_SEMIHOSTING=n/default-configs/arm-softmmu.mak
>>
>> I'm still not convinced we should be adding support for stuff being done
>> outside the normal build process...
> 
> If the "KVM/TCG split" series is accepted, then we can add a
> --disable-tcg job in the list of "normal builds" and we don't need this
> particular patch.
> 
>> that said...
>>
>>>   $ make subdir-aarch64-softmmu
>>>   [...]
>>>     LINK    aarch64-softmmu/qemu-system-aarch64
>>>   /usr/bin/ld: target/arm/arm-semi.o: in function `do_arm_semihosting':
>>>   ./target/arm/arm-semi.c:321: undefined reference to `qemu_semihosting_console_out'
>>>   /usr/bin/ld: ./target/arm/arm-semi.c:318: undefined reference to `qemu_semihosting_console_out'
>>>   collect2: error: ld returned 1 exit status
>>>   make[1]: *** [Makefile:204: qemu-system-aarch64] Error 1
>>>   make: *** [Makefile:472: subdir-aarch64-softmmu] Error 2
>>>
>>> Fix it by providing a stub when semihosting is disabled.
>>>
>>> Reported-by: Miroslav Rezanina <mrezanin@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  target/arm/Makefile.objs    |  3 ++-
>>>  target/arm/arm-semi-stubs.c | 21 +++++++++++++++++++++
>>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>>  create mode 100644 target/arm/arm-semi-stubs.c
>>>
>>> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
>>> index 6bdcc65c2c..39b02b1fa4 100644
>>> --- a/target/arm/Makefile.objs
>>> +++ b/target/arm/Makefile.objs
>>> @@ -1,4 +1,5 @@
>>> -obj-y += arm-semi.o
>>> +obj-$(CONFIG_SEMIHOSTING) += arm-semi.o
>>> +obj-$(call lnot,$(CONFIG_SEMIHOSTING)) += arm-semi-stubs.o
>>>  obj-$(CONFIG_SOFTMMU) += machine.o psci.o arch_dump.o monitor.o
>>>  obj-$(CONFIG_KVM) += kvm.o
>>>  obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
>>> diff --git a/target/arm/arm-semi-stubs.c b/target/arm/arm-semi-stubs.c
>>> new file mode 100644
>>> index 0000000000..a91ecbd9d5
>>> --- /dev/null
>>> +++ b/target/arm/arm-semi-stubs.c
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + *  Arm "Angel" semihosting stubs
>>> + *
>>> + * Copyright (c) 2019 Red Hat, Inc.
>>> + *
>>> + * Author:
>>> + *   Philippe Mathieu-Daudé <philmd@redhat.com>
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "cpu.h"
>>> +
>>> +target_ulong do_arm_semihosting(CPUARMState *env)
>>> +{
>>> +    g_assert_not_reached();
>>> +}
>>
>> Could this not just be added to arm-semi.c in an
>>
>>   #ifndef CONFIG_SEMIHOSTING
>>   target_ulong do_arm_semihosting(CPUARMState *env)
>>   {
>>       g_assert_not_reached();
>>   }
>>   #else
>>   ... rest of arm-semi.c....
>>   #endif

Note that CONFIG_SEMIHOSTING is a Make definition, not a CPP one,
and it doesn't look straight forward to get it added to config-target.h...