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

[PATCH 2/2] xen/arm: Add defensive barrier in get_cycles for Arm64


  • To: <wei.chen@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <sstabellini@xxxxxxxxxx>, <julien@xxxxxxx>
  • From: Wei Chen <wei.chen@xxxxxxx>
  • Date: Tue, 5 Jan 2021 15:19:46 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 40.67.248.234) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.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-SenderADCheck; bh=3senGGhHMGiySIHILlSB+Z4Hs/Io4eSyk8AgHdDNgVI=; b=ABSd/DgXMlXyQjMGcTFEMMMX8z63AacWBetncdKspz28Q3YZKeeZVTuMZYWQ/kcgLxPc3PbFzicBso09JhxZ0lLJOpQDwhWeZDBhAfRmaWyjvuFA0BpmyDpM2/Yg1/KaIXIwEHnyfg3y7+pcQDKfY0SdqeDpYh8HKH+b5fJIK3wXzAEs5Fl5jBvHE2WEG7UMVDudf8/X+CtDVL4xWA3ajXjk2UqVdOoXbY55JeKMgV2QbZ8Xyj2E2UyY0ZOHwKwovlVDbiqAXAVPnDAR2UPq6uvHV7u2ymh5QW+3s6dx4eB7NFUZQ0tgN3xDCHvcpdw03h5oiBQyRT4Mp2pzGYWNWw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=miFAdZR3rUTVJtwkV3m/iRktQftKU1W95PnXvv4w7+KKEVIWirT+Ev2BooTqu2UGKDiohdWyzxZUUJOngcw1E80FthMcoFMiepqk1TxBdiKzn4KPF3uU3O68a0Egd3OYp/R4/jYKC3ZavHntIkefb65No5xLY1OghFN8A6Ao2oCKM3NpoYO4dgL95YsJIGg2HwCDX8OWezsizCjSutIzufouGxrODfoupZKx0RGoW0DQDqC/7mYOe0FuXdUaTaFfi7CSk9Kk/PrCL8bCAzQOiFBMakEFUIZdg3l8NkPPOBxdElophWsa55kNYNvQ8sKdqgFQXN7Xmab6OmqgmZ9Faw==
  • Cc: <Bertrand.Marquis@xxxxxxx>, <Penny.Zheng@xxxxxxx>, <Jiamei.Xie@xxxxxxx>, <nd@xxxxxxx>
  • Delivery-date: Tue, 05 Jan 2021 07:20:44 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true

As the dicsussion [1] in mailing list. We'd better to have
a barrier after reading CNTPCT in get_cycles. If there is
not any barrier there. When get_cycles being used in some
seqlock critical context in the future, the seqlock can be
speculated potentially.

In order to reduce the impact of new barrier, we perfer to
use enforce order instead of ISB [2].

Currently, enforce order is not applied to arm32 as this is
not done in Linux at the date of this patch. If this is done
in Linux it will need to be also done in Xen.

[1] https://lists.xenproject.org/archives/html/xen-devel/2020-12/msg00181.html
[2] https://lkml.org/lkml/2020/3/13/645

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
 xen/include/asm-arm/time.h | 43 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 5c4529ebb5..3ef4e7cd3d 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -11,9 +11,26 @@
 
 typedef uint64_t cycles_t;
 
-static inline cycles_t get_cycles(void)
+/*
+ * Ensure that reads of the counter are treated the same as memory reads
+ * for the purposes of ordering by subsequent memory barriers.
+ */
+#if defined(CONFIG_ARM_64)
+#define read_cntpct_enforce_ordering(val) do { \
+    u64 tmp, _val = (val);                     \
+                                               \
+    asm volatile(                              \
+    "eor %0, %1, %1\n"                         \
+    "add %0, sp, %0\n"                         \
+    "ldr xzr, [%0]"                            \
+    : "=r" (tmp) : "r" (_val));                \
+} while (0)
+#else
+#define read_cntpct_enforce_ordering(val) do {} while (0)
+#endif
+
+static inline cycles_t read_cntpct_stable(void)
 {
-    isb();
     /*
      * ARM_WORKAROUND_858921: Cortex-A73 (all versions) counter read
      * can return a wrong value when the counter crosses a 32bit boundary.
@@ -36,6 +53,28 @@ static inline cycles_t get_cycles(void)
     }
 }
 
+static inline cycles_t get_cycles(void)
+{
+    cycles_t cnt;
+
+    isb();
+    cnt = read_cntpct_stable();
+
+    /*
+     * If there is not any barrier here. When get_cycles being used in
+     * some seqlock critical context in the future, the seqlock can be
+     * speculated potentially.
+     *
+     * To prevent seqlock from being speculated silently, we add a barrier
+     * here defensively. Normally, we just need an ISB here is enough, but
+     * considering the minimum performance cost. We prefer to use enforce
+     * order here.
+     */
+    read_cntpct_enforce_ordering(cnt);
+
+    return cnt;
+}
+
 /* List of timer's IRQ */
 enum timer_ppi
 {
-- 
2.25.1




 


Rackspace

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