Message ID | 20190531154735.20809-1-philmd@redhat.com |
---|---|
Headers | show |
Series | target: Build with CONFIG_SEMIHOSTING disabled | expand |
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
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
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
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. >