diff mbox

[Xen-devel,RFC,05/22] xen/arm: traps: Move MMIO emulation code in a separate helper

Message ID 1469717505-8026-6-git-send-email-julien.grall@arm.com
State Superseded
Headers show

Commit Message

Julien Grall July 28, 2016, 2:51 p.m. UTC
Currently, a stage-2 fault translation will likely access an emulated
region. All the checks are pre-sanitity check for MMIO emulation.

A follow-up patch will handle a new case that could lead to a stage-2
translation. To improve the clarity of the code and the changes, the
current implementation is move in a separate helper.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c | 58 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 25 deletions(-)

Comments

Julien Grall Aug. 31, 2016, 10:36 a.m. UTC | #1
Hi Stefano,

On 16/08/16 01:49, Stefano Stabellini wrote:
> On Thu, 28 Jul 2016, Julien Grall wrote:
>>  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>>                                       const union hsr hsr)
>>  {
>> @@ -2487,40 +2519,16 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>>          break;
>>      }
>>      case FSC_FLT_TRANS:
>> -        if ( dabt.s1ptw )
>> -            goto bad_data_abort;
>> -
>> -        /* XXX: Decode the instruction if ISS is not valid */
>> -        if ( !dabt.valid )
>> -            goto bad_data_abort;
>> -
>> -        /*
>> -         * Erratum 766422: Thumb store translation fault to Hypervisor may
>> -         * not have correct HSR Rt value.
>> -         */
>> -        if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
>> -             dabt.write )
>> -        {
>> -            rc = decode_instruction(regs, &info.dabt);
>> -            if ( rc )
>> -            {
>> -                gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
>> -                goto bad_data_abort;
>> -            }
>> -        }
>> -
>> -        if ( handle_mmio(&info) )
>> +        if ( try_handle_mmio(regs, &info) )
>>          {
>>              advance_pc(regs, hsr);
>>              return;
>>          }
>> -        break;
>
> I would keep this break, because we don't want to print the warning
> below in all error cases, such as !dabt.valid.

It was meant to be kept, I removed it by error.

> Aside from the break removal:
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thank you!

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46e0663..b46284c 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2444,6 +2444,38 @@  static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     inject_iabt_exception(regs, gva, hsr.len);
 }
 
+static bool_t try_handle_mmio(struct cpu_user_regs *regs,
+                              mmio_info_t *info)
+{
+    const struct hsr_dabt dabt = info->dabt;
+    int rc;
+
+    /* stage-1 page table should never live in an emulated MMIO region */
+    if ( dabt.s1ptw )
+        return 0;
+
+    /* All the instructions used on emulated MMIO region should be valid */
+    if ( !dabt.valid )
+        return 0;
+
+    /*
+     * Erratum 766422: Thumb store translation fault to Hypervisor may
+     * not have correct HSR Rt value.
+     */
+    if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
+         dabt.write )
+    {
+        rc = decode_instruction(regs, &info->dabt);
+        if ( rc )
+        {
+            gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
+            return 0;
+        }
+    }
+
+    return !!handle_mmio(info);
+}
+
 static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
                                      const union hsr hsr)
 {
@@ -2487,40 +2519,16 @@  static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
         break;
     }
     case FSC_FLT_TRANS:
-        if ( dabt.s1ptw )
-            goto bad_data_abort;
-
-        /* XXX: Decode the instruction if ISS is not valid */
-        if ( !dabt.valid )
-            goto bad_data_abort;
-
-        /*
-         * Erratum 766422: Thumb store translation fault to Hypervisor may
-         * not have correct HSR Rt value.
-         */
-        if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
-             dabt.write )
-        {
-            rc = decode_instruction(regs, &info.dabt);
-            if ( rc )
-            {
-                gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
-                goto bad_data_abort;
-            }
-        }
-
-        if ( handle_mmio(&info) )
+        if ( try_handle_mmio(regs, &info) )
         {
             advance_pc(regs, hsr);
             return;
         }
-        break;
     default:
         gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
                 hsr.bits, dabt.dfsc);
     }
 
-bad_data_abort:
     gdprintk(XENLOG_DEBUG, "HSR=0x%x pc=%#"PRIregister" gva=%#"PRIvaddr
              " gpa=%#"PRIpaddr"\n", hsr.bits, regs->pc, info.gva, info.gpa);
     inject_dabt_exception(regs, info.gva, hsr.len);