[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: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxxxxx>
  • Date: Tue, 22 Mar 2022 12:38:35 +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=P8wG/Vl/qyGUk8DmqyojureUN+2ILdmPNHrKtT4MShc=; b=AxjDAJM9lt1twLsfiPgT5Gpocc/aaoG+OvRNKjpCncQ5xJZmLsYk9m9EPpZcDXDkqmrniHTNDoy92qIlzDLDfGoBiXl4fo2GoQzJ1lgb+N+6d6B9YQj4YtotwLBxIsJ+UbDDUUrU9+Ri050pFubc0lZ2+4c+l/gZV4fd0ufMO6hFjp0Yr/XdYDU+s0JoSK0/MFN8Rj6NAiAQLM2gJKnRJu0EcYv1yaLnlLNWWkTGbNjkpmkm6iuNKmeFBL66/cNbDetk4iVmp1eeUxhnKP6hjdRqUUU92RH5TpjT6NLaxBXrcofiMaQ00zr+pQ8N/z32Fay7g5D9SVmol7O+w96IzQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=O+rZDg3MzcqjAdgiBRNuxS37K75U+erLa9thaRImzAbWmULenquW6rH/mIY4PnmEBqxAHWF//tzVP7iswm6E1RspPWvgHfmF03leRhuFZn84t8AZ0gRXLBKgbzbKLu/4/sddarK0ONc1vNocCWFh9vFhvHyQ5cJiBqKfCwe9WidV7Wl5ZJS74bMCebgXs4hZHT6qmsTTEH7WOt8mPgC0JfUYZBxrDe4gBcHJsrOIWmvvGP50+QSe0uMxob8Eq5hVYdS2Rl0fmjk23nIGawo2wx1NBFUQPL3ccl5gCEwxZQfv+Ytet2q1zvaG1dlQs6Dyu5B4WpeTB9mj8g7bYVugPw==
  • 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:39:04 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 22/03/2022 12:06, Ayan Kumar Halder wrote:

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)) )

Actually this is not needed.

The following change is sufficient :-

@@ -146,7 +146,9 @@ void try_decode_instruction(const struct cpu_user_regs *regs,
      */
     if ( info->dabt.cache )
     {
         info->dabt_instr.state = INSTR_CACHE;
+        info->dabt.valid = 1;
         return;
     }

"info->dabt.valid == 1" means the instruction is valid or decoded successfully (this holds true for INSTR_CACHE as well).




 


Rackspace

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