Cc'ing Richard : this is one for you I think... (surely we need to rebuild the hflags from scratch when we power up a CPU anyway?)
thanks -- PMM On Mon, 16 Dec 2019 at 23:44, Niek Linnenbank <nieklinnenb...@gmail.com> wrote: > > 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 <nieklinnenb...@gmail.com> > 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 <nieklinnenb...@gmail.com> >> --- >> 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