[PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state

Luc Michel posted 1 patch 4 years, 5 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora failed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191101103232.3692818-1-luc.michel@greensocs.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Andrzej Zaborowski <balrogg@gmail.com>
hw/arm/pxa2xx.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by Luc Michel 4 years, 5 months ago
This machine modifies the CPU state when simulating suspend mode. This
commit adds a missing call to arm_rebuild_hflags after those
modifications.

Signed-off-by: Luc Michel <luc.michel@greensocs.com>
---
I came over this missing hflags rebuild while reviewing Edgar's similar
fix in hw/arm/boot.c. I could not find any other place where it would be
missing but I may be wrong.
---
 hw/arm/pxa2xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index cdafde2f76..7982ffbfbe 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -306,6 +306,8 @@ static void pxa2xx_pwrmode_write(CPUARMState *env, const ARMCPRegInfo *ri,
         cpu_physical_memory_write(8, &buffer, 4);
 #endif
 
+        arm_rebuild_hflags(s->cpu->env);
+
         /* Suspend */
         cpu_interrupt(current_cpu, CPU_INTERRUPT_HALT);
 
-- 
2.23.0


Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by no-reply@patchew.org 4 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/20191101103232.3692818-1-luc.michel@greensocs.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/hw/arm/allwinner-a10.o
  CC      aarch64-softmmu/hw/arm/cubieboard.o
/tmp/qemu-test/src/hw/arm/pxa2xx.c: In function 'pxa2xx_pwrmode_write':
/tmp/qemu-test/src/hw/arm/pxa2xx.c:309:9: error: incompatible type for argument 1 of 'arm_rebuild_hflags'
         arm_rebuild_hflags(s->cpu->env);
         ^
In file included from /tmp/qemu-test/src/hw/arm/pxa2xx.c:15:0:
/tmp/qemu-test/src/target/arm/cpu.h:3304:6: note: expected 'struct CPUARMState *' but argument is of type 'CPUARMState'
 void arm_rebuild_hflags(CPUARMState *env);
      ^
make[1]: *** [hw/arm/pxa2xx.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  LINK    x86_64-softmmu/qemu-system-x86_64
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=f71fa5bac4424def845b677aad7f199a', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-f_tlfkqr/src/docker-src.2019-11-01-07.57.40.19897:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=f71fa5bac4424def845b677aad7f199a
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-f_tlfkqr/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m36.680s
user    0m7.186s


The full log is available at
http://patchew.org/logs/20191101103232.3692818-1-luc.michel@greensocs.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by no-reply@patchew.org 4 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/20191101103232.3692818-1-luc.michel@greensocs.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/trace/generated-helpers.o
  CC      aarch64-softmmu/target/arm/translate-sve.o
/tmp/qemu-test/src/hw/arm/pxa2xx.c: In function 'pxa2xx_pwrmode_write':
/tmp/qemu-test/src/hw/arm/pxa2xx.c:309:34: error: incompatible type for argument 1 of 'arm_rebuild_hflags'
         arm_rebuild_hflags(s->cpu->env);
                            ~~~~~~^~~~~
In file included from /tmp/qemu-test/src/hw/arm/pxa2xx.c:15:
/tmp/qemu-test/src/target/arm/cpu.h:3304:38: note: expected 'CPUARMState *' {aka 'struct CPUARMState *'} but argument is of type 'CPUARMState' {aka 'struct CPUARMState'}
 void arm_rebuild_hflags(CPUARMState *env);
                         ~~~~~~~~~~~~~^~~
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: hw/arm/pxa2xx.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:488: aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  GEN     x86_64-softmmu/qemu-system-x86_64.exe
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=3d794fd9df7246238d93d7058a5c15bf', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-7unbyocz/src/docker-src.2019-11-01-08.06.39.18927:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=3d794fd9df7246238d93d7058a5c15bf
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-7unbyocz/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m52.847s
user    0m6.563s


The full log is available at
http://patchew.org/logs/20191101103232.3692818-1-luc.michel@greensocs.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by Peter Maydell 4 years, 5 months ago
On Fri, 1 Nov 2019 at 10:32, Luc Michel <luc.michel@greensocs.com> wrote:
>
> This machine modifies the CPU state when simulating suspend mode. This
> commit adds a missing call to arm_rebuild_hflags after those
> modifications.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> ---
> I came over this missing hflags rebuild while reviewing Edgar's similar
> fix in hw/arm/boot.c. I could not find any other place where it would be
> missing but I may be wrong.

pxa2xx_pwrmode_write() is a cp14 coprocessor register write
function, so I think that the hflags rebuild that is done by
translate.c:disas_coproc_insn() after a cp register write
should already handle this case ?


The other place that might need checking is the PSCI/etc
code for doing CPU power on/off (and other callers to the
power up/down functions like the imx6 power control regs).
Richard, did you look at that code to see if it needed hflags updates?

thanks
-- PMM

Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by Richard Henderson 4 years, 5 months ago
On 11/1/19 11:42 AM, Peter Maydell wrote:
> The other place that might need checking is the PSCI/etc
> code for doing CPU power on/off (and other callers to the
> power up/down functions like the imx6 power control regs).
> Richard, did you look at that code to see if it needed hflags updates?

I had a quick grovel through hw/misc/imx6_src.c and didn't see anything; was
there something more specific you were thinking of?


r~

Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by Peter Maydell 4 years, 5 months ago
On Tue, 5 Nov 2019 at 11:20, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/1/19 11:42 AM, Peter Maydell wrote:
> > The other place that might need checking is the PSCI/etc
> > code for doing CPU power on/off (and other callers to the
> > power up/down functions like the imx6 power control regs).
> > Richard, did you look at that code to see if it needed hflags updates?
>
> I had a quick grovel through hw/misc/imx6_src.c and didn't see anything; was
> there something more specific you were thinking of?

Not anything all that specific, but all the power-on/off
code is basically doing "change the state of this other
CPU by mangling its register state/etc". I forget how much
of this we do in common code and how much in the calling
code that handles the specifics of ACPI/imx6/etc.

thanks
-- PMM

Re: [PATCH] hw/arm/pxa2xx: rebuild hflags cache when modifying CPU state
Posted by Luc Michel 4 years, 5 months ago
On 11/1/19 11:42 AM, Peter Maydell wrote:
> On Fri, 1 Nov 2019 at 10:32, Luc Michel <luc.michel@greensocs.com> wrote:
>>
>> This machine modifies the CPU state when simulating suspend mode. This
>> commit adds a missing call to arm_rebuild_hflags after those
>> modifications.
>>
>> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
>> ---
>> I came over this missing hflags rebuild while reviewing Edgar's similar
>> fix in hw/arm/boot.c. I could not find any other place where it would be
>> missing but I may be wrong.
> 
> pxa2xx_pwrmode_write() is a cp14 coprocessor register write
> function, so I think that the hflags rebuild that is done by
> translate.c:disas_coproc_insn() after a cp register write
> should already handle this case ?
OK. I missed the fact that it was a co-processor write callback.

> 
> 
> The other place that might need checking is the PSCI/etc
> code for doing CPU power on/off (and other callers to the
> power up/down functions like the imx6 power control regs).
> Richard, did you look at that code to see if it needed hflags updates?
> 
> thanks
> -- PMM
>