Message ID | 20210916153025.1944763-1-richard.henderson@linaro.org |
---|---|
Headers | show |
Series | tcg patch queue | expand |
On Thu, 16 Sept 2021 at 16:30, Richard Henderson <richard.henderson@linaro.org> wrote: > > The following changes since commit 57b6f58c1d0df757c9311496c32d502925056894: > > Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging (2021-09-15 18:55:59 +0100) > > are available in the Git repository at: > > https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210916 > > for you to fetch changes up to 50febfe212f24a9b91b4224d03f653415fddf8e1: > > tcg/mips: Drop special alignment for code_gen_buffer (2021-09-16 09:37:39 -0400) > > ---------------------------------------------------------------- > Restrict cpu_has_work to sysemu, and move to AccelOpsClass. > Move cpu_signal_handler declaration out of target/. > Misc tcg/mips/ cleanups. > This seems to result in a failure on the s390x all-linux-static CI job: https://gitlab.com/qemu-project/qemu/-/jobs/1604251543 due to a core dump running the 'trap' test. The 'check-acceptance' job also hits a timeout on the emcraft_sf2 test: https://gitlab.com/qemu-project/qemu/-/jobs/1604251596 -- PMM
On 9/20/21 12:07, Peter Maydell wrote: > On Thu, 16 Sept 2021 at 16:30, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> The following changes since commit 57b6f58c1d0df757c9311496c32d502925056894: >> >> Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging (2021-09-15 18:55:59 +0100) >> >> are available in the Git repository at: >> >> https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210916 >> >> for you to fetch changes up to 50febfe212f24a9b91b4224d03f653415fddf8e1: >> >> tcg/mips: Drop special alignment for code_gen_buffer (2021-09-16 09:37:39 -0400) >> >> ---------------------------------------------------------------- >> Restrict cpu_has_work to sysemu, and move to AccelOpsClass. >> Move cpu_signal_handler declaration out of target/. >> Misc tcg/mips/ cleanups. >> > > This seems to result in a failure on the s390x all-linux-static > CI job: > https://gitlab.com/qemu-project/qemu/-/jobs/1604251543 > due to a core dump running the 'trap' test. I don't have access to that runner, nor to an Ubuntu based one. I can't reproduce on a RHEL8.5 host. I ran git-bisect, - from 57b6f58c1d0 ("Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging") - to 62e76dc7dab ("Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210916' into staging") using: $ time make -j 2 -C build qemu-s390x run-tcg-tests-s390x-linux-user All steps consistently report: RUN TCG tests for s390x-linux-user BUILD s390x-linux-user guest-tests with cc RUN tests for s390x TEST float_convs on s390x TEST threadcount on s390x SKIPPED float_convs check on s390x because no reference TEST sha1 on s390x TEST linux-test on s390x TEST testthread on s390x SKIPPED signals on s390x because BROKEN awaiting sigframe clean-ups and vdso support TEST test-mmap (default) on s390x TEST float_madds on s390x SKIPPED float_madds check on s390x because no reference TEST hello-s390x on s390x TEST csst on s390x TEST ipm on s390x TEST exrl-trt on s390x TEST exrl-trtr on s390x TEST pack on s390x TEST mvo on s390x TEST mvc on s390x TEST trap on s390x TEST signals-s390x on s390x [ RUN ] Operation exception [ OK ] [ RUN ] Translation exception from stg [ OK ] [ RUN ] Translation exception from mvc [ OK ] [ RUN ] Protection exception from stg [ OK ] [ RUN ] Protection exception from mvc [ OK ] [ PASSED ] SKIPPED gdbstub test sha1 on s390x because need working gdb SKIPPED gdbstub test qxfer-auxv-read on s390x because need working gdb However I note another job timeouted on the same runner: https://gitlab.com/qemu-project/qemu/-/jobs/1603171791 Are we running multiple jobs in parallel in the same runner? > The 'check-acceptance' job also hits a timeout on the emcraft_sf2 > test: > https://gitlab.com/qemu-project/qemu/-/jobs/1604251596 Not looked at this one yet.
On 9/20/21 12:07, Peter Maydell wrote: > On Thu, 16 Sept 2021 at 16:30, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> The following changes since commit 57b6f58c1d0df757c9311496c32d502925056894: >> >> Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging (2021-09-15 18:55:59 +0100) >> >> are available in the Git repository at: >> >> https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210916 >> >> for you to fetch changes up to 50febfe212f24a9b91b4224d03f653415fddf8e1: >> >> tcg/mips: Drop special alignment for code_gen_buffer (2021-09-16 09:37:39 -0400) >> >> ---------------------------------------------------------------- >> Restrict cpu_has_work to sysemu, and move to AccelOpsClass. >> Move cpu_signal_handler declaration out of target/. >> Misc tcg/mips/ cleanups. >> > > The 'check-acceptance' job also hits a timeout on the emcraft_sf2 > test: > https://gitlab.com/qemu-project/qemu/-/jobs/1604251596 cd0d814b4b9b732f11885889070adacf87447751 is the first bad commit accel/tcg: Implement AccelOpsClass::has_work() as stub Add TCG target-specific has_work() handler in TCGCPUOps, and add tcg_cpu_has_work() as AccelOpsClass has_work() implementation. include/hw/core/tcg-cpu-ops.h | 4 ++++ accel/tcg/tcg-accel-ops.c | 12 ++++++++++++ 2 files changed, 16 insertions(+)
On 9/20/21 3:07 AM, Peter Maydell wrote: > This seems to result in a failure on the s390x all-linux-static > CI job: > https://gitlab.com/qemu-project/qemu/-/jobs/1604251543 > due to a core dump running the 'trap' test. Curious. I can't reproduce this on s390x.ci.qemu.org manually. r~
On 9/20/21 15:14, Philippe Mathieu-Daudé wrote: > On 9/20/21 12:07, Peter Maydell wrote: >> On Thu, 16 Sept 2021 at 16:30, Richard Henderson >> <richard.henderson@linaro.org> wrote: >>> >>> The following changes since commit 57b6f58c1d0df757c9311496c32d502925056894: >>> >>> Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging (2021-09-15 18:55:59 +0100) >>> >>> are available in the Git repository at: >>> >>> https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210916 >>> >>> for you to fetch changes up to 50febfe212f24a9b91b4224d03f653415fddf8e1: >>> >>> tcg/mips: Drop special alignment for code_gen_buffer (2021-09-16 09:37:39 -0400) >>> >>> ---------------------------------------------------------------- >>> Restrict cpu_has_work to sysemu, and move to AccelOpsClass. >>> Move cpu_signal_handler declaration out of target/. >>> Misc tcg/mips/ cleanups. >>> >> > >> The 'check-acceptance' job also hits a timeout on the emcraft_sf2 >> test: >> https://gitlab.com/qemu-project/qemu/-/jobs/1604251596 > > cd0d814b4b9b732f11885889070adacf87447751 is the first bad commit > > accel/tcg: Implement AccelOpsClass::has_work() as stub > > Add TCG target-specific has_work() handler in TCGCPUOps, > and add tcg_cpu_has_work() as AccelOpsClass has_work() > implementation. -- >8 -- diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c index ed4ebe735fe..2f96553f6f7 100644 --- a/accel/tcg/tcg-accel-ops.c +++ b/accel/tcg/tcg-accel-ops.c @@ -79,7 +79,7 @@ static bool tcg_cpu_has_work(CPUState *cpu) CPUClass *cc = CPU_GET_CLASS(cpu); if (!cc->tcg_ops->has_work) { - return false; + return true; } return cc->tcg_ops->has_work(cpu); } ---
On 9/20/21 15:52, Philippe Mathieu-Daudé wrote: > On 9/20/21 15:14, Philippe Mathieu-Daudé wrote: >> On 9/20/21 12:07, Peter Maydell wrote: >>> On Thu, 16 Sept 2021 at 16:30, Richard Henderson >>> <richard.henderson@linaro.org> wrote: >>>> >>>> The following changes since commit 57b6f58c1d0df757c9311496c32d502925056894: >>>> >>>> Merge remote-tracking branch 'remotes/hreitz/tags/pull-block-2021-09-15' into staging (2021-09-15 18:55:59 +0100) >>>> >>>> are available in the Git repository at: >>>> >>>> https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20210916 >>>> >>>> for you to fetch changes up to 50febfe212f24a9b91b4224d03f653415fddf8e1: >>>> >>>> tcg/mips: Drop special alignment for code_gen_buffer (2021-09-16 09:37:39 -0400) >>>> >>>> ---------------------------------------------------------------- >>>> Restrict cpu_has_work to sysemu, and move to AccelOpsClass. >>>> Move cpu_signal_handler declaration out of target/. >>>> Misc tcg/mips/ cleanups. >>>> >>> >> >>> The 'check-acceptance' job also hits a timeout on the emcraft_sf2 >>> test: >>> https://gitlab.com/qemu-project/qemu/-/jobs/1604251596 >> >> cd0d814b4b9b732f11885889070adacf87447751 is the first bad commit >> >> accel/tcg: Implement AccelOpsClass::has_work() as stub >> >> Add TCG target-specific has_work() handler in TCGCPUOps, >> and add tcg_cpu_has_work() as AccelOpsClass has_work() >> implementation. > > -- >8 -- > diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c > index ed4ebe735fe..2f96553f6f7 100644 > --- a/accel/tcg/tcg-accel-ops.c > +++ b/accel/tcg/tcg-accel-ops.c > @@ -79,7 +79,7 @@ static bool tcg_cpu_has_work(CPUState *cpu) > CPUClass *cc = CPU_GET_CLASS(cpu); > > if (!cc->tcg_ops->has_work) { > - return false; > + return true; Forget this crap. The missing piece was: -- >8 -- diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index 0d5adccf1a7..da348938407 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -23,6 +23,11 @@ #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG) +static bool arm_v7m_cpu_has_work(CPUState *cs) +{ + return cs->interrupt_request & CPU_INTERRUPT_HARD; +} + static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); @@ -920,6 +925,7 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data) acc->info = data; #ifdef CONFIG_TCG + cc->has_work = arm_v7m_cpu_has_work; cc->tcg_ops = &arm_v7m_tcg_ops; #endif /* CONFIG_TCG */ ---
On Mon, 20 Sept 2021 at 22:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Forget this crap. The missing piece was: > > -- >8 -- > diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c > index 0d5adccf1a7..da348938407 100644 > --- a/target/arm/cpu_tcg.c > +++ b/target/arm/cpu_tcg.c > @@ -23,6 +23,11 @@ > #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) > > #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG) > +static bool arm_v7m_cpu_has_work(CPUState *cs) > +{ > + return cs->interrupt_request & CPU_INTERRUPT_HARD; > +} Is this really all that's needed ? I would have expected at least a check on the power_state. -- PMM
On 9/21/21 11:28, Peter Maydell wrote: > On Mon, 20 Sept 2021 at 22:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Forget this crap. The missing piece was: >> >> -- >8 -- >> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c >> index 0d5adccf1a7..da348938407 100644 >> --- a/target/arm/cpu_tcg.c >> +++ b/target/arm/cpu_tcg.c >> @@ -23,6 +23,11 @@ >> #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) >> >> #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG) >> +static bool arm_v7m_cpu_has_work(CPUState *cs) >> +{ >> + return cs->interrupt_request & CPU_INTERRUPT_HARD; >> +} > > Is this really all that's needed ? I would have expected > at least a check on the power_state. I started reading the PSCI spec this morning and you are right, it doesn't seem restricted to A/R profiles, M profiles also have it.
On Tue, 21 Sept 2021 at 10:41, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > On 9/21/21 11:28, Peter Maydell wrote: > > On Mon, 20 Sept 2021 at 22:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > >> Forget this crap. The missing piece was: > >> > >> -- >8 -- > >> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c > >> index 0d5adccf1a7..da348938407 100644 > >> --- a/target/arm/cpu_tcg.c > >> +++ b/target/arm/cpu_tcg.c > >> @@ -23,6 +23,11 @@ > >> #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) > >> > >> #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG) > >> +static bool arm_v7m_cpu_has_work(CPUState *cs) > >> +{ > >> + return cs->interrupt_request & CPU_INTERRUPT_HARD; > >> +} > > > > Is this really all that's needed ? I would have expected > > at least a check on the power_state. > > I started reading the PSCI spec this morning and you are right, > it doesn't seem restricted to A/R profiles, M profiles also have > it. It's not that we implement PSCI for M profile (which I don't think does exist), it's just that we use cpu->power_state to track "core is powered off or not", and we happen to use the PSCI_ON/PSCI_OFF constant names for that. This is just for historical reasons; we started with PSCI support and then later broadened that into generic "power control" (see arm-powerctl.[ch]), which is a set of functionality that provides the underlying "power on, power off" that is used by both our PSCI emulation and by our emulation of real hardware power-controller devices. The imx and the allwinner SoCs are A-profile devices that provide a power-controller emulation. For M-profile some of the dual-core MPS2 boards set the CPU property start-powered-off to true, which will cause arm_cpu_reset() to set cpu->power_state to PSCI_OFF. They're then powered on by the device code in hw/misc/iotkit-sysctl.c calling arm_set_cpu_on_and_reset() when the guest writes to the "start this core now" register. -- PMM
On Mon, 20 Sept 2021 at 14:19, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 9/20/21 3:07 AM, Peter Maydell wrote: > > This seems to result in a failure on the s390x all-linux-static > > CI job: > > https://gitlab.com/qemu-project/qemu/-/jobs/1604251543 > > due to a core dump running the 'trap' test. > > Curious. I can't reproduce this on s390x.ci.qemu.org manually. I've just seen this on an unrelated merge: https://gitlab.com/qemu-project/qemu/-/jobs/1609218763 so it's presumably an existing intermittent failure :-/ -- PMM