| Message ID | 1501271035-8592-5-git-send-email-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > 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@epam.com> > --- > 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"
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@epam.com> >> --- >> 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" >
On 08/08/17 21:42, Volodymyr Babchuk wrote: > Hi Julien, Hi Volodymyr, > 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. ARM 32-bit is not only ARMv7, it could also be ARMv8. If you look at Part G describing the AArch32 state, specifically G1-4434, "The ARMv8-A architecture permits, but does not require, this trap to apply to conditional SMC instructions that fail their Condition code check...". Xen ARM 32-bits should be able to boot any 32-bit ARM platform (ARMv7, ARMv8). But your #ifdef belows prevent that. Cheers,
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 + */ + 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;
On ARMv8 architecture SMC instruction in aarch32 state can be conditional. 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 and COND fields. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- xen/arch/arm/traps.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)