Re: [Qemu-arm] [PATCH v2 35/68] target/arm: Convert CPS (privileged)

[Peter Maydell]

On Mon, 19 Aug 2019 at 22:38, Richard Henderson
<address@hidden> wrote:
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate.c       | 87 +++++++++++++++---------------------
>  target/arm/a32-uncond.decode |  3 ++
>  target/arm/t32.decode        |  3 ++
>  3 files changed, 42 insertions(+), 51 deletions(-)
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index 18c268e712..354ad77fe6 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -44,6 +44,7 @@
>  &bfi             !extern rd rn lsb msb
>  &sat             !extern rd rn satimm imm sh
>  &pkh             !extern rd rn rm imm tb
> +&cps             !extern mode imod M A I F
>
>  # Data-processing (register)
>
> @@ -340,6 +341,8 @@ CLZ              1111 1010 1011 ---- 1111 .... 1000 ....  
>     @rdm
>      SMC          1111 0111 1111 imm:4 1000 0000 0000 0000     &i
>      HVC          1111 0111 1110 ....  1000 .... .... ....     \
>                   &i imm=%imm16_16_0
> +    CPS          1111 0011 1010 1111 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \
> +                 &cps

In T32 the CPS insn overlaps with the hint space (hint insns have
bits [10:8] all-zeroes, whereas all the valid CPS insns have either
M set or one of the imod bits set) -- why doesn't it need to be
in the same insn group as the hints? If we're going to put it
separated in the .decode file from the insns it overlaps with
I guess a comment to that effect would help so it doesn't get
inadvertently reordered with them.

CPS shouldn't exist at all for M-profile, but the legacy decoder
got this wrong too, so we should put that on the todo list for
fixing later (along, maybe, with UNDEFing on some of the
unpredictable combinations of M/imod/etc for A profile?).

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM