[v2,2/2] arch: arm64: always set IL=1 when injecting an abort exception

Commit Message

Volodymyr Babchuk Feb. 13, 2025, 3:37 p.m. UTC

ARM Architecture Reference Manual states that IL field of ESR_EL1
register should be 1 in some cases, and all these cases are covered by
inject_abt64_exception()

Section D24.2.40, page D24-7337 of ARM DDI 0487L:

  IL, bit [25]
  Instruction Length for synchronous exceptions. Possible values of this bit are:

  [...]

  0b1 - 32-bit instruction trapped.
  This value is also used when the exception is one of the following:
  [...]
   - An Instruction Abort exception.
   - A Data Abort exception for which the value of the ISV bit is 0.
  [...]

inject_abt64_exception() function injects either Instruction Abort or
Data Abort exception. In both cases, ISS is 0, which means that ISV
bit is 0 as well. Thus, IL must be set to 1 unconditionally.

To align code with the specification, set .len field to 1 in
inject_abt64_exception() and remove unneeded third parameter.

Signed-off-by: Volodymyr Babchuk <[e-mail]>

Changes in v2:
 - Introduced in v2
---
 xen/arch/arm/traps.c | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

Comments

Michal Orzel Feb. 14, 2025, 8:50 a.m. UTC | #1

On 13/02/2025 16:37, Volodymyr Babchuk wrote:
> 
> 
> ARM Architecture Reference Manual states that IL field of ESR_EL1
> register should be 1 in some cases, and all these cases are covered by
> inject_abt64_exception()
> 
> Section D24.2.40, page D24-7337 of ARM DDI 0487L:
> 
>   IL, bit [25]
>   Instruction Length for synchronous exceptions. Possible values of this bit are:
> 
>   [...]
> 
>   0b1 - 32-bit instruction trapped.
>   This value is also used when the exception is one of the following:
>   [...]
>    - An Instruction Abort exception.
>    - A Data Abort exception for which the value of the ISV bit is 0.
>   [...]
> 
> inject_abt64_exception() function injects either Instruction Abort or
> Data Abort exception. In both cases, ISS is 0, which means that ISV
> bit is 0 as well. Thus, IL must be set to 1 unconditionally.
> 
> To align code with the specification, set .len field to 1 in
> inject_abt64_exception() and remove unneeded third parameter.
> 
> Signed-off-by: Volodymyr Babchuk <[e-mail]>
Reviewed-by: Michal Orzel <[e-mail]>

~Michal

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 5338d5c033..3071c38768 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -559,13 +559,12 @@  void inject_undef64_exception(struct cpu_user_regs *regs)
 /* Inject an abort exception into a 64 bit guest */
 static void inject_abt64_exception(struct cpu_user_regs *regs,
                                    int prefetch,
-                                   register_t addr,
-                                   int instr_len)
+                                   register_t addr)
 {
     vaddr_t handler;
     union hsr esr = {
         .iss = 0,
-        .len = instr_len,
+        .len = 1,
     };
 
     if ( regs_mode_is_user(regs) )
@@ -591,17 +590,15 @@  static void inject_abt64_exception(struct cpu_user_regs *regs,
 }
 
 static void inject_dabt64_exception(struct cpu_user_regs *regs,
-                                   register_t addr,
-                                   int instr_len)
+                                    register_t addr)
 {
-    inject_abt64_exception(regs, 0, addr, instr_len);
+    inject_abt64_exception(regs, 0, addr);
 }
 
 static void inject_iabt64_exception(struct cpu_user_regs *regs,
-                                   register_t addr,
-                                   int instr_len)
+                                    register_t addr)
 {
-    inject_abt64_exception(regs, 1, addr, instr_len);
+    inject_abt64_exception(regs, 1, addr);
 }
 
 #endif
@@ -617,26 +614,24 @@  void inject_undef_exception(struct cpu_user_regs *regs)
 }
 
 static void inject_iabt_exception(struct cpu_user_regs *regs,
-                                  register_t addr,
-                                  int instr_len)
+                                  register_t addr)
 {
         if ( is_32bit_domain(current->domain) )
             inject_pabt32_exception(regs, addr);
 #ifdef CONFIG_ARM_64
         else
-            inject_iabt64_exception(regs, addr, instr_len);
+            inject_iabt64_exception(regs, addr);
 #endif
 }
 
 static void inject_dabt_exception(struct cpu_user_regs *regs,
-                                  register_t addr,
-                                  int instr_len)
+                                  register_t addr)
 {
         if ( is_32bit_domain(current->domain) )
             inject_dabt32_exception(regs, addr);
 #ifdef CONFIG_ARM_64
         else
-            inject_dabt64_exception(regs, addr, instr_len);
+            inject_dabt64_exception(regs, addr);
 #endif
 }
 
@@ -1965,9 +1960,9 @@  inject_abt:
              "HSR=%#"PRIregister" pc=%#"PRIregister" gva=%#"PRIvaddr" gpa=%#"PRIpaddr"\n",
              hsr.bits, regs->pc, gva, gpa);
     if ( is_data )
-        inject_dabt_exception(regs, gva, hsr.len);
+        inject_dabt_exception(regs, gva);
     else
-        inject_iabt_exception(regs, gva, hsr.len);
+        inject_iabt_exception(regs, gva);
 }
 
 static inline bool needs_ssbd_flip(struct vcpu *v)