Re: [Qemu-devel] [PULL 27/27] target-arm: Correctly handle 'sub pc, pc, 1' for ARMv6

[Alex Bennée]

Peter Maydell <address@hidden> writes:

> In the ARM v6 architecture, 'sub pc, pc, 1' is not an interworking
> branch, so the computed new value is written to r15 as a normal
> value. The architecture says that in this case, bits [1:0] of
> the value written must be ignored if we are in ARM mode (or
> bit [0] ignored if in Thumb mode); this is a change from the
> ARMv4/v5 specification that behaviour is UNPREDICTABLE.
> Use the correct mask on the PC value when doing a non-interworking
> store to PC.
>
> A popular library used on RaspberryPi uses this instruction
> as part of a trick to determine whether it is running on
> ARMv6 or ARMv7, and we were mishandling the sequence.
>
> Fixes bug: https://bugs.launchpad.net/bugs/1625295
>
> Reported-by: <address@hidden>
> Signed-off-by: Peter Maydell <address@hidden>
> Message-id: address@hidden

I'm not sure how but this seems to have regressed my ARMv7 test images
(currently Linux 4.7.7). With this change I see the guest spinning in
the vectors table. If I comment out the change it boots.

I'll dig some more but as this affects store_reg are there any cases
when writing to the PC with offset bits would be correct?

> ---
>  target-arm/translate.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 693d4bc..8df24bf 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -180,7 +180,12 @@ static inline TCGv_i32 load_reg(DisasContext *s, int reg)
>  static void store_reg(DisasContext *s, int reg, TCGv_i32 var)
>  {
>      if (reg == 15) {
> -        tcg_gen_andi_i32(var, var, ~1);
> +        /* In Thumb mode, we must ignore bit 0.
> +         * In ARM mode, for ARMv4 and ARMv5, it is UNPREDICTABLE if bits 
> [1:0]
> +         * are not 0b00, but for ARMv6 and above, we must ignore bits [1:0].
> +         * We choose to ignore [1:0] in ARM mode for all architecture 
> versions.
> +         */
> +        tcg_gen_andi_i32(var, var, s->thumb ? ~1 : ~3);
>          s->is_jmp = DISAS_JUMP;
>      }
>      tcg_gen_mov_i32(cpu_R[reg], var);


--
Alex Bennée