| Message ID | 20230503193833.29047-2-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: disable-tcg and without-default-devices fixes | expand |
On 5/3/23 21:38, Fabiano Rosas wrote: > When building --without-default-devices, the semihosting code will not > be available, so check the proper config. Acked-by: Paolo Bonzini <pbonzini@redhat.com> for this change; however, there are two more related issues: 1) you still want to leave out the code if !TCG, because KVM is not able to exit to userspace on semihosting calls as far as I understand 2) I am not sure why CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y appears in config/targets/{arm,riscv32,riscv64}-softmmu/default.mak. Putting things together you also need something like diff --git a/semihosting/Kconfig b/semihosting/Kconfig index eaf3a20ef5b2..671020a33426 100644 --- a/semihosting/Kconfig +++ b/semihosting/Kconfig @@ -4,4 +4,5 @@ config SEMIHOSTING config ARM_COMPATIBLE_SEMIHOSTING bool + default y if (ARM && TCG) || RISCV32 || RISCV64 select SEMIHOSTING diff --git a/configs/devices/arm-softmmu/default.mak b/configs/devices/arm-softmmu/default.mak index 1b49a7830c7e..5e7a17d05bf8 100644 --- a/configs/devices/arm-softmmu/default.mak +++ b/configs/devices/arm-softmmu/default.mak @@ -41,5 +41,4 @@ CONFIG_FSL_IMX25=y CONFIG_FSL_IMX7=y CONFIG_FSL_IMX6UL=y CONFIG_SEMIHOSTING=y -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y CONFIG_ALLWINNER_H3=y diff --git a/configs/devices/riscv32-softmmu/default.mak b/configs/devices/riscv32-softmmu/default.mak index d847bd5692ec..94a236c9c25b 100644 --- a/configs/devices/riscv32-softmmu/default.mak +++ b/configs/devices/riscv32-softmmu/default.mak @@ -3,8 +3,6 @@ # Uncomment the following lines to disable these optional devices: # #CONFIG_PCI_DEVICES=n -CONFIG_SEMIHOSTING=y -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y # Boards: # diff --git a/configs/devices/riscv64-softmmu/default.mak b/configs/devices/riscv64-softmmu/default.mak index bc69301fa4a6..3f6805944849 100644 --- a/configs/devices/riscv64-softmmu/default.mak +++ b/configs/devices/riscv64-softmmu/default.mak @@ -3,8 +3,6 @@ # Uncomment the following lines to disable these optional devices: # #CONFIG_PCI_DEVICES=n -CONFIG_SEMIHOSTING=y -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y # Boards: # Paolo > Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build") > Signed-off-by: Fabiano Rosas<farosas@suse.de> > --- > target/arm/helper.c | 4 ++-- > target/arm/tcg/m_helper.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-)
On Thu, 4 May 2023 at 08:33, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 5/3/23 21:38, Fabiano Rosas wrote: > > When building --without-default-devices, the semihosting code will not > > be available, so check the proper config. I think the changes to the ifdeffery are conceptually fine (only do semihosting if it was configured in), but it sounds like there's a separate problem here. Whether we need semihosting depends on the accelerator (ie "is it TCG or not"), not on what set of devices we're building. So the problem seems to me to be that --without-default-devices is causing the semihosting code not to be built in. > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > > for this change; however, there are two more related issues: > > 1) you still want to leave out the code if !TCG, because KVM is not able > to exit to userspace on semihosting calls as far as I understand > > 2) I am not sure why CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y appears in > config/targets/{arm,riscv32,riscv64}-softmmu/default.mak. Because those are the architectures which have "arm-compatible" semihosting ABIs ? -- PMM
Paolo Bonzini <pbonzini@redhat.com> writes: > On 5/3/23 21:38, Fabiano Rosas wrote: >> When building --without-default-devices, the semihosting code will not >> be available, so check the proper config. > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > > for this change; however, there are two more related issues: > > 1) you still want to leave out the code if !TCG, because KVM is not able > to exit to userspace on semihosting calls as far as I understand Correct. > 2) I am not sure why CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y appears in > config/targets/{arm,riscv32,riscv64}-softmmu/default.mak. I think we need to be clearer in the development guide when things are triggered by config/devices/ and when they are triggered by default rules in Kconfig. After Fabino's series the delta between config/aarch64-softmmu/minimal.mk and config/aarch64-softmmu/default.mk is pretty small. I guess minimal.mk should be renamed to virt.mk, sbsa-ref dropped from it (as you can't run it under KVM) and what controls which devices the virt platform can use? Should it be: CONFIG_PCI_DEVICES=y CONFIG_VIRTIO_PCI=y to ensure we can plug and play all the various VirtIO bits we need for the KVM/Xen use case? > > Putting things together you also need something like > > diff --git a/semihosting/Kconfig b/semihosting/Kconfig > index eaf3a20ef5b2..671020a33426 100644 > --- a/semihosting/Kconfig > +++ b/semihosting/Kconfig > @@ -4,4 +4,5 @@ config SEMIHOSTING > config ARM_COMPATIBLE_SEMIHOSTING > bool > + default y if (ARM && TCG) || RISCV32 || RISCV64 > select SEMIHOSTING > diff --git a/configs/devices/arm-softmmu/default.mak b/configs/devices/arm-softmmu/default.mak > index 1b49a7830c7e..5e7a17d05bf8 100644 > --- a/configs/devices/arm-softmmu/default.mak > +++ b/configs/devices/arm-softmmu/default.mak > @@ -41,5 +41,4 @@ CONFIG_FSL_IMX25=y > CONFIG_FSL_IMX7=y > CONFIG_FSL_IMX6UL=y > CONFIG_SEMIHOSTING=y > -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y > CONFIG_ALLWINNER_H3=y > diff --git a/configs/devices/riscv32-softmmu/default.mak b/configs/devices/riscv32-softmmu/default.mak > index d847bd5692ec..94a236c9c25b 100644 > --- a/configs/devices/riscv32-softmmu/default.mak > +++ b/configs/devices/riscv32-softmmu/default.mak > @@ -3,8 +3,6 @@ > # Uncomment the following lines to disable these optional devices: > # > #CONFIG_PCI_DEVICES=n > -CONFIG_SEMIHOSTING=y > -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y > # Boards: > # > diff --git a/configs/devices/riscv64-softmmu/default.mak b/configs/devices/riscv64-softmmu/default.mak > index bc69301fa4a6..3f6805944849 100644 > --- a/configs/devices/riscv64-softmmu/default.mak > +++ b/configs/devices/riscv64-softmmu/default.mak > @@ -3,8 +3,6 @@ > # Uncomment the following lines to disable these optional devices: > # > #CONFIG_PCI_DEVICES=n > -CONFIG_SEMIHOSTING=y > -CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y > # Boards: > # > > Paolo > >> Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build") >> Signed-off-by: Fabiano Rosas<farosas@suse.de> >> --- >> target/arm/helper.c | 4 ++-- >> target/arm/tcg/m_helper.c | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-)
Il gio 4 mag 2023, 10:59 Peter Maydell <peter.maydell@linaro.org> ha scritto: > On Thu, 4 May 2023 at 08:33, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 5/3/23 21:38, Fabiano Rosas wrote: > > > When building --without-default-devices, the semihosting code will not > > > be available, so check the proper config. > > I think the changes to the ifdeffery are conceptually > fine (only do semihosting if it was configured in), but > it sounds like there's a separate problem here. > Whether we need semihosting depends on the accelerator (ie > "is it TCG or not"), not on what set of devices we're building. > So the problem seems to me to be that --without-default-devices > is causing the semihosting code not to be built in. > > > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > > > > for this change; however, there are two more related issues: > > > > 1) you still want to leave out the code if !TCG, because KVM is not able > > to exit to userspace on semihosting calls as far as I understand > > > > 2) I am not sure why CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y appears in > > config/targets/{arm,riscv32,riscv64}-softmmu/default.mak. > > Because those are the architectures which have > "arm-compatible" semihosting ABIs ? > Yes but is there a reason to do it in configs/ where all the other symbols are boards, or was it just overlooked and a "default y" (as I suggested in the previous reply) or "imply" is better? The latter would be config ARM_COMPATIBLE_SEMIHOSTING bool depends on TCG and under "config ARM/RISCV32/RISCV64" imply ARM_COMPATIBLE_SEMIHOSTING Paolo > > -- PMM > >
Paolo Bonzini <pbonzini@redhat.com> writes: > Il gio 4 mag 2023, 10:59 Peter Maydell <peter.maydell@linaro.org> ha > scritto: > >> On Thu, 4 May 2023 at 08:33, Paolo Bonzini <pbonzini@redhat.com> wrote: >> > >> > On 5/3/23 21:38, Fabiano Rosas wrote: >> > > When building --without-default-devices, the semihosting code will not >> > > be available, so check the proper config. >> >> I think the changes to the ifdeffery are conceptually >> fine (only do semihosting if it was configured in), but >> it sounds like there's a separate problem here. >> Whether we need semihosting depends on the accelerator (ie >> "is it TCG or not"), not on what set of devices we're building. >> So the problem seems to me to be that --without-default-devices >> is causing the semihosting code not to be built in. >> >> > Acked-by: Paolo Bonzini <pbonzini@redhat.com> >> > >> > for this change; however, there are two more related issues: >> > >> > 1) you still want to leave out the code if !TCG, because KVM is not able >> > to exit to userspace on semihosting calls as far as I understand >> > >> > 2) I am not sure why CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y appears in >> > config/targets/{arm,riscv32,riscv64}-softmmu/default.mak. >> >> Because those are the architectures which have >> "arm-compatible" semihosting ABIs ? >> > > Yes but is there a reason to do it in configs/ where all the other symbols > are boards, or was it just overlooked and a "default y" (as I suggested in > the previous reply) or "imply" is better? For arm it has been taken out of configs/ and moved into target/arm/Kconfig: ... # This config exists just so we can make SEMIHOSTING default when TCG # is selected without also changing it for other architectures. config ARM_SEMIHOSTING bool default y if TCG && ARM select ARM_COMPATIBLE_SEMIHOSTING So I guess we'd need a similar change to what you suggested for ARM_V7M: config ARM bool select ARM_V7M if TCG select ARM_COMPATIBLE_SEMIHOSTING if TCG
On 5/4/23 15:01, Fabiano Rosas wrote: > ... > # This config exists just so we can make SEMIHOSTING default when TCG > # is selected without also changing it for other architectures. > config ARM_SEMIHOSTING > bool > default y if TCG && ARM > select ARM_COMPATIBLE_SEMIHOSTING This can be replaced by "imply ARM_COMPATIBLE_SEMIHOSTING if TCG" placed under "config ARM" (and also RISCV32/RISCV64)". Paolo
diff --git a/target/arm/helper.c b/target/arm/helper.c index 2297626bfb..24bb7efb34 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -10972,7 +10972,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) * We only see semihosting exceptions in TCG only as they are not * trapped to the hypervisor in KVM. */ -#ifdef CONFIG_TCG +#ifdef CONFIG_SEMIHOSTING static void tcg_handle_semihosting(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); @@ -11033,7 +11033,7 @@ void arm_cpu_do_interrupt(CPUState *cs) * that caused the exception, not the target exception level, so * must be handled here. */ -#ifdef CONFIG_TCG +#ifdef CONFIG_SEMIHOSTING if (cs->exception_index == EXCP_SEMIHOST) { tcg_handle_semihosting(cs); return; diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c index 9758f225d6..4261f1bb1e 100644 --- a/target/arm/tcg/m_helper.c +++ b/target/arm/tcg/m_helper.c @@ -2345,7 +2345,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) qemu_log_mask(CPU_LOG_INT, "...handling as semihosting call 0x%x\n", env->regs[0]); -#ifdef CONFIG_TCG +#ifdef CONFIG_SEMIHOSTING do_common_semihosting(cs); #else g_assert_not_reached();
When building --without-default-devices, the semihosting code will not be available, so check the proper config. Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only build") Signed-off-by: Fabiano Rosas <farosas@suse.de> --- target/arm/helper.c | 4 ++-- target/arm/tcg/m_helper.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)