[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 09/08/2017 20:44, Volodymyr Babchuk wrote:
On ARMv8 architecture SMC instruction from aarch32 state can be conditional.

s/aarch32/AArch32/. But SMC instruction are conditional even on ARMv7. The main difference is the ARMv8 architecture allows conditional SMCs to be trapped even if they failed their condition check.

(ARM DDI 0487A.k page D7-1949)
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 check before perfoming standard handling

s/perfoming/performing/

of CCVALID and COND fields.

Because we can't distinguish ARMv7 from aarch32 state, we will always

See my comment on patch #2.

check CCKNOWNPASS field. On ARMv7 it will be read as 0
(ARM DDI 0406C.c page B3-1431), so there will be no problem.

Field are usually made RES{0,1} to allow later revision of the architecture to use them and not break previous version. There are now ARMv8.1, ARMv8.2, ARMv8.3. It does not mean we have 3 new modes. It just add more features on top of the earlier revision.


Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
---

- Added references to the ARM manuals.
- Moved code from under #ifdef CONFIG_ARM_64
- Added comment with rationale for this

---
 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..384219d 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 very special. Under SMC32 we mean
+     * SMC instruction on ARMv7 or SMC instruction originating from
+     * aarch32 state on ARMv8.
+     * On ARMv7 it is uncoditional (ARM DDI 0406C.c page B3-1431),
+     * but is conditional on ARMv8 (ARM DDI 0487A.k page D7-1949).
+     * Problem is that we can't determine if XEN is running on
+     * ARMv7 on ARMv8 in aarch32 mode. Fortunately, while running

See my comment on top.

+     * on ARMv7 all conditonal bits of HSR will read as 0
+     * (ARM DDI 0406C.c page B3-1431).
+     * This includes CCKNOWNPASS bit of HSR. Thereby we will be okay,
+     * if we'll check that bit.
+     */
+    if (hsr.ec == HSR_EC_SMC32 && hsr.smc32.ccknownpass == 0)
+        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))
         return 1;

     /* Check for valid condition in hsr */


Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.