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

[Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>]

Hi Julien,

On 28.07.17 23:37, Julien Grall wrote:
> Hi,
>
> On 07/28/2017 08:43 PM, Volodymyr Babchuk wrote:
> > On ARMv8 architecture SMC instruction in aarch32 state can be 
> > conditional.
> version + paragraph please.
>
> Also, ARMv8 supports both AArch32 and AArch64. As I said in my answer on 
> "arm: smccc: handle SMCs/HVCs according to SMCCC" ([1]), This field 
> exists for both architecture. I really don't want to tie the 32-bit port 
> to ARMv7. We should be able to use ARMv8 too.
Not sure if I got this.

My ARM 7 ARM (ARM DDI 0406C.c ID051414 page B3-1431) say following:

"SMC instructions cannot be trapped if they fail their condition code 
check.
Therefore, the syndrome information for this exception does not include
conditionality information."

ARMv8 ARM (ARM DDI 0487A.k ID092916) says that SMC from aarch32 state can
be conditional and my patch checks this. But SMC from aarch64 state is
unconditional, so there are nothing to check. At least, when looking at
ISS encoding, i see imm16 field and RES0 field. No conditional flags.

> > 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 to check before perfoming standart handling of CCVALID
> performing standard
>
> > and COND fields.
> >
> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > ---
> >   xen/arch/arm/traps.c | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index eae2212..6a21763 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -1717,8 +1717,20 @@ static int check_conditional_instr(struct 
> > cpu_user_regs *regs,
> >       int cond;
> >       /* Unconditional Exception classes */
> > +#ifdef CONFIG_ARM_32
> >       if ( hsr.ec == HSR_EC_UNKNOWN || hsr.ec >= 0x10 )
> >           return 1;
> > +#else
> > +    if ( hsr.ec == HSR_EC_UNKNOWN || (hsr.ec >= 0x10 && hsr.ec != 
> > HSR_EC_SMC32))
> > +        return 1;
> > +
> > +    /*
> > +     * Special case for SMC32: we need to check CCKNOWNPASS before
> > +     * checking CCVALID
> Missing full stop.
>
> > +     */
> > +    if (hsr.ec == HSR_EC_SMC32 && hsr.cond.ccknownpass == 0)
> > +        return 1;
> > +#endif
> >       /* Check for valid condition in hsr */
> >       cond = hsr.cond.ccvalid ? hsr.cond.cc : -1;
> Cheers,
>
> [1] https://lists.xen.org/archives/html/xen-devel/2017-07/msg01671.html";
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel