Re: [PULL 35/37] target/arm: Do hflags rebuild in cpsr_write()

[Philippe Mathieu-Daudé]

On 8/26/21 7:03 PM, Peter Maydell wrote:
> Currently we rely on all the callsites of cpsr_write() to rebuild the
> cached hflags if they change one of the CPSR bits which we use as a
> TB flag and cache in hflags.  This is a bit awkward when we want to
> change the set of CPSR bits that we cache, because it means we need
> to re-audit all the cpsr_write() callsites to see which flags they
> are writing and whether they now need to rebuild the hflags.
> 
> Switch instead to making cpsr_write() call arm_rebuild_hflags()
> itself if one of the bits being changed is a cached bit.
> 
> We don't do the rebuild for the CPSRWriteRaw write type, because that
> kind of write is generally doing something special anyway.  For the
> CPSRWriteRaw callsites in the KVM code and inbound migration we
> definitely don't want to recalculate the hflags; the callsites in
> boot.c and arm-powerctl.c have to do a rebuild-hflags call themselves
> anyway because of other CPU state changes they make.
> 
> This allows us to drop explicit arm_rebuild_hflags() calls in a
> couple of places where the only reason we needed to call it was the
> CPSR write.

Just noticing this patch, nice.

> This fixes a bug where we were incorrectly failing to rebuild hflags
> in the code path for a gdbstub write to CPSR, which meant that you
> could make QEMU assert by breaking into a running guest, altering the
> CPSR to change the value of, for example, CPSR.E, and then
> continuing.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-id: 20210817201843.3829-1-peter.maydell@linaro.org
> ---
>  target/arm/cpu.h        | 10 ++++++++--
>  linux-user/arm/signal.c |  2 --
>  target/arm/helper.c     |  5 +++++
>  3 files changed, 13 insertions(+), 4 deletions(-)