Re: [Qemu-devel] [PATCH] target-arm: Don't decode old cp15 WFI instructions on v7 cores

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Feb 25, 2011 at 03:04:12PM +0000, Peter Maydell wrote:
> In v7 of the ARM architecture, WFI (wait for interrupt) is a first-class
> instruction, but in previous versions this functionality was provided
> via a cp15 coprocessor register. Add correct feature checks to the
> decoding of the cp15 WFI instructions so that they behave correctly
> for newer cores. In particular, the old 0,c7,c8,2 encoding used on
> ARM940 has been reused for VA-to-PA translation in v6 and v7.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This patch stands alone as a fix to target-arm; it's a prerequisite
> for Adam's VA->PA translation patch, because otherwise attempting a
> user-read translation will get you a WFI instead...
> 
>  target-arm/translate.c |   35 ++++++++++++++++++++++++++++++-----
>  1 files changed, 30 insertions(+), 5 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index dbd958b..baa1256 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -2538,13 +2538,38 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>      if (IS_USER(s) && !cp15_user_ok(insn)) {
>          return 1;
>      }
> -    if ((insn & 0x0fff0fff) == 0x0e070f90
> -        || (insn & 0x0fff0fff) == 0x0e070f58) {
> -        /* Wait for interrupt.  */
> -        gen_set_pc_im(s->pc);
> -        s->is_jmp = DISAS_WFI;
> +
> +    /* Pre-v7 versions of the architecture implemented WFI via coprocessor
> +     * instructions rather than a separate instruction.
> +     */
> +    if ((insn & 0x0fff0fff) == 0x0e070f90) {
> +        /* 0,c7,c0,4: Standard v6 WFI (also used in some pre-v6 cores).
> +         * In v7, this must NOP.
> +         */
> +        if (!arm_feature(env, ARM_FEATURE_V7)) {
> +            /* Wait for interrupt.  */
> +            gen_set_pc_im(s->pc);
> +            s->is_jmp = DISAS_WFI;
> +        }
>          return 0;
>      }
> +
> +    if ((insn & 0x0fff0fff) == 0x0e070f58) {
> +        /* 0,c7,c8,2: Not all pre-v6 cores implemented this WFI,
> +         * so this is slightly over-broad.
> +         */
> +        if (!arm_feature(env, ARM_FEATURE_V6)) {
> +            /* Wait for interrupt.  */
> +            gen_set_pc_im(s->pc);
> +            s->is_jmp = DISAS_WFI;
> +            return 0;
> +        }
> +        /* Otherwise fall through to handle via helper function.
> +         * In particular, on v7 and some v6 cores this is one of
> +         * the VA-PA registers.
> +         */
> +    }
> +
>      rd = (insn >> 12) & 0xf;
>  
>      if (cp15_tls_load_store(env, s, insn, rd))
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net