Re: [Xen-devel] [PATCH v3 3/3] arm: traps: handle SMC32 in check_conditional_instr()

[Julien Grall <julien.grall@xxxxxxx>]

Hi Volodymyr,

On 14/08/17 18:15, Volodymyr Babchuk wrote:
> On ARMv8 architecture we need to ensure that conditional check was passed
> for a trapped SMC instruction that originates from AArch32 state
> (ARM DDI 0487B.a page D7-2271).
> Thus, we should not skip it while checking HSR.EC value.
>
> For this type of exception special coding of HSR.ISS is used. There is
> additional flag (CCKNOWNPASS) to be checked before performing standard
> handling of CCVALID and COND fields.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> ---
>  xen/arch/arm/traps.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index eae2212..39791fc 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1716,8 +1716,24 @@ static int check_conditional_instr(struct cpu_user_regs 
> *regs,
>      unsigned long cpsr, cpsr_cond;
>      int cond;
>
> +    /*
> +     * SMC32 instruction case is special. Under SMC32 we mean SMC
> +     * instruction on ARMv7 or SMC instruction originating from
> +     * AArch32 state on ARMv8.
> +     * On ARMv7 it will be trapped only if it passed condition check
> +     * (ARM DDI 0406C.c page B3-1431), but we need to check conditon
s/conditon/condition/

> +     * flags on ARMv8 (ARM DDI 0487B.a page D7-2271).
> +     * Ecoding for HSR.ISS on ARMv8 is backwards compatible with ARMv7:
s/Ecoding/Encoding/

> +     * HSR.ISS is defined as UNK/SBZP on ARMv7 which means, that it
> +     * will be read as 0. This includes CCKNOWNPASS field.
> +     * If CCKNOWNPASS == 0 then this was an uncoditional instruction or
s/uncoditional/unconditional/

> +     * it have passed conditional check (ARM DDI 0487B.a page D7-2272).
s/have/has/

> +     */
> +    if (hsr.ec == HSR_EC_SMC32 && hsr.smc32.ccknownpass == 0)
Coding style:

if ( ... )

> +        return 1;
> +
>      /* Unconditional Exception classes */
> -    if ( hsr.ec == HSR_EC_UNKNOWN || hsr.ec >= 0x10 )
> +    if ( hsr.ec == HSR_EC_UNKNOWN || (hsr.ec >= 0x10 && hsr.ec != 
> HSR_EC_SMC32))
Ditto.

>          return 1;
>
>      /* Check for valid condition in hsr */
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel