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

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

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

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.

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

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

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

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

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

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.

> 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.
>

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