Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on()

[Niek Linnenbank]

Hello Peter,

In the previous version of this patch series I included the fix for setting CP10,CP11 bits

in arm_set_cpu_on(), which is now in master (0c7f8c43daf65560). While that worked, I did not

realize that setting those bits require rebuilding the flags. Philippe reported this [1] initially,

later on during review we discussed [2] and attempted to correct it [3].

Could you please have a short look at this? Right now I don't see anymore

issues, but I'm just not very familiar with this area of the code.

Regards,

Niek

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg01920.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg02784.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg02785.html

On Tue, Dec 17, 2019 at 12:36 AM Niek Linnenbank <address@hidden> wrote:

After setting CP15 bits in arm_set_cpu_on() the cached hflags must
be rebuild to reflect the changed processor state. Without rebuilding,
the cached hflags would be inconsistent until the next call to
arm_rebuild_hflags(). When QEMU is compiled with debugging enabled
(--enable-debug), this problem is captured shortly after the first
call to arm_set_cpu_on() for CPUs running in ARM 32-bit non-secure mode:

  qemu-system-arm: target/arm/helper.c:11359: cpu_get_tb_cpu_state:
  Assertion `flags == rebuild_hflags_internal(env)' failed.
  Aborted (core dumped)

Fixes: 0c7f8c43daf65
Signed-off-by: Niek Linnenbank <address@hidden>
---
 target/arm/arm-powerctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
index b064513d44..b75f813b40 100644
--- a/target/arm/arm-powerctl.c
+++ b/target/arm/arm-powerctl.c
@@ -127,6 +127,9 @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state,
         target_cpu->env.regs[0] = info->context_id;
     }

+    /* CP15 update requires rebuilding hflags */
+    arm_rebuild_hflags(&target_cpu->env);
+
     /* Start the new CPU at the requested address */
     cpu_set_pc(target_cpu_state, info->entry);

--
2.17.1

--

Niek Linnenbank