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

Re: [Minios-devel] [UNIKRAFT PATCHv5 26/46] plat/common: Add counter workaround for Cortex-A73 erratum 858921





On 13.08.2018 11:03, Julien Grall wrote:
Hi Wei,

On 10/08/18 08:08, Wei Chen wrote:
The errata #858921 describes that Cortex-A73 (r0p0 - r0p2)
counter read can return a wrong value when the counter crosses
a 32bit boundary, but newer Cortex-A73 are not affected.

The workaround involves performing the read twice, compare
bit[32] of the two read values. If bit[32] is different,
keep the first value, otherwise keep the second value.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
  arch/arm/arm64/Config.uk |  9 +++++++++
  plat/common/arm/time.c   | 20 ++++++++++++++++++++
  2 files changed, 29 insertions(+)

diff --git a/arch/arm/arm64/Config.uk b/arch/arm/arm64/Config.uk
index 7797516..07bc8ec 100644
--- a/arch/arm/arm64/Config.uk
+++ b/arch/arm/arm64/Config.uk
@@ -46,3 +46,12 @@ config MARCH_ARM64_CORTEXA75
          Compile for Armv8.2 Cortex-A75 (and compatible) CPUs
  endchoice
+
+config ARM64_ERRATUM_858921
+    bool "Workaround for Cortex-A73 erratum 858921"
+    default n
+    depends on MARCH_ARM64_CORTEXA73

I don't think this is correct here. MARCH_ARM64_CORTEXA73 is about how the code was optimized for a given processor. It would still be possible to run a code compiled with generic option on Cortex-A73.

So you want to at least drop the depends on here and possibly default y for CORTEX_A73 and GENERIC.

Cheers,

I am okay with Juliens suggestion. I agree that it is safer to have the erratum enabled on default for people that do not know if their SOC is affected or not. We may need to think about the MARCH_ARM64_NATIVE and the other cases where the could would run on A73. Probably these should also go to the `depends on` list?



+    help
+      This option enables a workaround for Cortex-A73 (r0p0 - r0p2),
+      whose counter may return a wrong value when the counter crosses
+      a 32-bit boundary. The newer Cortex-A73 are not affected.
diff --git a/plat/common/arm/time.c b/plat/common/arm/time.c
index 4d955f3..1829938 100644
--- a/plat/common/arm/time.c
+++ b/plat/common/arm/time.c
@@ -60,10 +60,30 @@ static inline uint64_t get_counter_frequency(void)
      return SYSREG_READ(cntfrq_el0);
  }
+#ifdef CONFIG_ARM64_ERRATUM_858921
+/*
+ * The errata #858921 describes that Cortex-A73 (r0p0 - r0p2) counter
+ * read can return a wrong value when the counter crosses a 32bit boundary.
+ * But newer Cortex-A73 are not affected.
+ *
+ * The workaround involves performing the read twice, compare bit[32] of
+ * the two read values. If bit[32] is different, keep the first value,
+ * otherwise keep the second value.
+ */
+static uint64_t read_virtual_count(void)
+{
+    uint64_t val_1st, val_2nd;
+
+    val_1st = SYSREG_READ(cntvct_el0);
+    val_2nd = SYSREG_READ(cntvct_el0);
+    return (((val_1st ^ val_2nd) >> 32) & 1) ? val_1st : val_2nd;
+}
+#else
  static inline uint64_t read_virtual_count(void)
  {
      return SYSREG_READ(cntvct_el0);
  }
+#endif
  /*
   * monotonic_clock(): returns # of nanoseconds passed since



_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

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