Called out possibly missing fix below.  Beside it, I'm convinced this
change is beneficial, other than maybe for readability.

The change does not account for uses of the affected calls in
target-arm/translate.c.  As well, the change has somewhat a ripple effect
as certain code expects to receive or return a 32-bit value.

Unless there is a functional benefit, my suggestion would be to omit this
change or submit it separately as it is not really related or required for
your  EL2/EL3 changes.

Greg


On 9 June 2014 10:04, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
>
> Avoids the explicit 16bit mask. No functional change.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> ---
>  target-arm/internals.h     | 14 +++++++-------
>  target-arm/translate-a64.c |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 08fa697..707643e 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -199,23 +199,23 @@ static inline uint32_t syn_uncategorized(void)
>      return (EC_UNCATEGORIZED << ARM_EL_EC_SHIFT) | ARM_EL_IL;
>  }
>
> -static inline uint32_t syn_aa64_svc(uint32_t imm16)
> +static inline uint32_t syn_aa64_svc(uint16_t imm16)
>  {
> -    return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 &
> 0xffff);
> +    return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
>  }
>
> -static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
> +static inline uint32_t syn_aa32_svc(uint16_t imm16, bool is_thumb)
>  {
> -    return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
> +    return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | imm16
>          | (is_thumb ? 0 : ARM_EL_IL);
>  }
>
> -static inline uint32_t syn_aa64_bkpt(uint32_t imm16)
> +static inline uint32_t syn_aa64_bkpt(uint16_t imm16)
>  {
> -    return (EC_AA64_BKPT << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 &
> 0xffff);
> +    return (EC_AA64_BKPT << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
>  }
>
> -static inline uint32_t syn_aa32_bkpt(uint32_t imm16, bool is_thumb)
> +static inline uint32_t syn_aa32_bkpt(uint16_t imm16, bool is_thumb)
>  {
>      return (EC_AA32_BKPT << ARM_EL_EC_SHIFT) | (imm16 & 0xffff)
>          | (is_thumb ? 0 : ARM_EL_IL);
>

Looks like you forgot to remove the mask on this one.


> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index a9c4633..3589898 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1433,7 +1433,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>  {
>      int opc = extract32(insn, 21, 3);
>      int op2_ll = extract32(insn, 0, 5);
> -    int imm16 = extract32(insn, 5, 16);
> +    uint16_t imm16 = extract32(insn, 5, 16);
>
>      switch (opc) {
>      case 0:
> --
> 1.8.3.2
>
>