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

Re: [PATCH v11 3/3] xen/arm64: io: Handle data abort due to cache maintenance instructions


  • To: Julien Grall <julien@xxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>
  • Date: Tue, 22 Mar 2022 12:06:58 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=xen.org smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=9reWh0nBqe487DWaQDuoaXi6yMWC3UHF40vALmPnpRo=; b=K4EUBSAmn01VatkzLe+QPZnZMuHqsyo3SKh7cGb3DgZ2K4j+QSIXeXuL+kHDc5PATajEsXjnV+Y1BJ26debOmqEDuWM79YQ521ddggPurQkco8uFSmQBEX5bE4dbp9XUJ7WyPOTlmpB1+bPpMbkOArffvtvERX+HpP0R/sL3CTs1oeQIMQvtnqver0h4fXuJ7T1ugxQDEE+dvk4ZyMrbtiH52YrorrO1VnnkrCqIL6pZlNwBGrVdaEyvX02xafyBuq4BKiCIRQs28YkQCDKxREB5g8/j7Wjcx96FXlemLnn9jPCpc0qldvxzcJxJ23UYy57bWBL9DutcOBSDW+7Y8w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=N/obHxIqCpteJ7J9dkCIfGKj5fqHFLRTSDR3llYGSsi3Olf5g1adV3PS8OlBIJlyTB0i3tXH50rt9O4tuQ7s+tLHkIJeBDsFaJ3HE3mlw/n93w8Ap2FCNXQqg3nEbX+DAwXErsN6EIADD2QLJKBB43Z04MMjVmOv0tSRE10hNotG0n8/Csy2Rz794Gb8Xx5mSy3eK7N+pLaIYEomfbM9Ekdzc+zFAFMrD7C2Esl8biljPuvBVOzlhegwIKj46Rqciw+kyKkVX+5KFN1+PNDfudr95wzeQ/jYVHi72PtDFA7PuDSghq8SU9FsWn8JrpAtaHzkWx1oZYouVvXKZSHT4w==
  • Cc: <sstabellini@xxxxxxxxxx>, <stefanos@xxxxxxxxxx>, <Volodymyr_Babchuk@xxxxxxxx>, <bertrand.marquis@xxxxxxx>, <andrew.cooper3@xxxxxxxxxx>, <george.dunlap@xxxxxxxxxx>, <jbeulich@xxxxxxxx>, <wl@xxxxxxx>, <paul@xxxxxxx>, <roger.pau@xxxxxxxxxx>
  • Delivery-date: Tue, 22 Mar 2022 12:07:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 18/03/2022 18:26, Julien Grall wrote:
Hi Ayan,
Hi Julien,

On 17/03/2022 14:00, Ayan Kumar Halder wrote:
diff --git a/xen/arch/arm/include/asm/mmio.h b/xen/arch/arm/include/asm/mmio.h
index ca259a79c2..79e64d9af8 100644
--- a/xen/arch/arm/include/asm/mmio.h
+++ b/xen/arch/arm/include/asm/mmio.h
@@ -35,6 +35,7 @@ enum instr_decode_state
       * instruction.
       */
      INSTR_LDR_STR_POSTINDEXING,
+    INSTR_CACHE,                    /* Cache Maintenance instr */
  };
    typedef struct
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 6f458ee7fd..26c716b4a5 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -139,6 +139,17 @@ void try_decode_instruction(const struct cpu_user_regs *regs,
          return;
      }
  +    /*
+     * When the data abort is caused due to cache maintenance, Xen should check +     * if the address belongs to an emulated MMIO region or not. The behavior
+     * will differ accordingly.
+     */
+    if ( info->dabt.cache )
+    {
+        info->dabt_instr.state = INSTR_CACHE;
+        return;
+    }
+
      /*
       * Armv8 processor does not provide a valid syndrome for decoding some        * instructions. So in order to process these instructions, Xen must @@ -177,6 +188,13 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
          return rc;
      }
  +    /*
+     * When the data abort is caused due to cache maintenance and the address +     * belongs to an emulated region, Xen should ignore this instruction.
+     */
+    if ( info->dabt_instr.state == INSTR_CACHE )

Reading the Arm Arm, the ISS should be invalid for cache instructions. So, I think the check at the beginning of try_handle_mmio() would prevent us to reach this check.

Can you check that cache instructions on emulated region will effectively be ignored?

Yes, you are correct.

I tested with the following (dis)assembly snippet :-

0x3001000 is the base address of GIC Distributor base.

    __asm__ __volatile__("ldr x1, =0x3001000");
    40000ca8:   58000301    ldr x1, 40000d08 <main+0x70>
    __asm __volatile__("DC CVAU, x1");
    40000cac:   d50b7b21    dc  cvau, x1

This resulting in hitting the assertion :-

(XEN) Assertion 'unreachable' failed at arch/arm/io.c:178

I dumped the registers as follows, to determine that the fault is caused by the instruction at 40000cac.

HSR=0x00000092000147  regs->pc = 0x40000cac info.gpa = 0x3001000


So, my patch needs to be modified as follows:-

@@ -172,7 +173,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,

     ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL);

-    if ( !info->dabt.valid )
+    if ( !(info->dabt.valid || (info->dabt_instr.state == INSTR_CACHE)) )
     {
         ASSERT_UNREACHABLE();
         return IO_ABORT;

I will send a v12 patch with this change.

- Ayan


Cheers,




 


Rackspace

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