Thread (17 messages) flat view 17 messages, 3 authors, 2021-08-14
STALE1852d

Revision v2 of 8 in this series.

Revisions (8)
  1. v1 [diff vs current]
  2. v2 [diff vs current]
  3. v2 current
  4. v2 [diff vs current]
  5. v2 [diff vs current]
  6. v2 [diff vs current]
  7. v2 [diff vs current]
  8. v2 [diff vs current]

[PATCH v2 5/5] lib: utils/timer: Simplify MTIMER synchronization

From: Atish Patra <hidden>
Date: 2021-08-11 21:07:30

On Sat, Aug 7, 2021 at 6:34 AM Anup Patel [off-list ref] wrote:
quoted hunk ↗ jump to hunk
We simplify MTIMER synchronization as follows:

1) Detect MTIMER devices with unique (or non-shared) MTIME
   register at boot-time
2) Select first MTIMER device with no associated HART as our
   reference MTIMER device
3) Only synchronize MTIMER devices with unique (or non-shared)
   MTIME register using reference MTIMER device
4) Directly update the MTIME register at time of synchronization
   because MTIME is a read/write register.

Signed-off-by: Anup Patel <redacted>
---
 include/sbi_utils/timer/aclint_mtimer.h |  7 ++-
 lib/utils/timer/aclint_mtimer.c         | 67 +++++++++++++++----------
 lib/utils/timer/fdt_timer_mtimer.c      | 39 ++++++++++++--
 3 files changed, 80 insertions(+), 33 deletions(-)
diff --git a/include/sbi_utils/timer/aclint_mtimer.h b/include/sbi_utils/timer/aclint_mtimer.h
index fdc46cd..a9fe445 100644
--- a/include/sbi_utils/timer/aclint_mtimer.h
+++ b/include/sbi_utils/timer/aclint_mtimer.h
@@ -31,14 +31,19 @@ struct aclint_mtimer_data {
        u32 first_hartid;
        u32 hart_count;
        bool has_64bit_mmio;
+       bool has_shared_mtime;
        /* Private details (initialized and used by ACLINT MTIMER library) */
        struct aclint_mtimer_data *time_delta_reference;
        unsigned long time_delta_computed;
-       u64 time_delta;
        u64 (*time_rd)(volatile u64 *addr);
        void (*time_wr)(bool timecmp, u64 value, volatile u64 *addr);
 };

+void aclint_mtimer_sync(struct aclint_mtimer_data *mt);
+
+void aclint_mtimer_set_reference(struct aclint_mtimer_data *mt,
+                                struct aclint_mtimer_data *ref);
+
 int aclint_mtimer_warm_init(void);

 int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
diff --git a/lib/utils/timer/aclint_mtimer.c b/lib/utils/timer/aclint_mtimer.c
index 0e4e846..d612b12 100644
--- a/lib/utils/timer/aclint_mtimer.c
+++ b/lib/utils/timer/aclint_mtimer.c
@@ -57,7 +57,7 @@ static u64 mtimer_value(void)
        u64 *time_val = (void *)mt->mtime_addr;

        /* Read MTIMER Time Value */
-       return mt->time_rd(time_val) + mt->time_delta;
+       return mt->time_rd(time_val);
 }

 static void mtimer_event_stop(void)
@@ -77,7 +77,7 @@ static void mtimer_event_start(u64 next_event)
        u64 *time_cmp = (void *)mt->mtimecmp_addr;

        /* Program MTIMER Time Compare */
-       mt->time_wr(true, next_event - mt->time_delta,
+       mt->time_wr(true, next_event,
                    &time_cmp[target_hart - mt->first_hartid]);
 }
@@ -88,36 +88,51 @@ static struct sbi_timer_device mtimer = {
        .timer_event_stop = mtimer_event_stop
 };

+void aclint_mtimer_sync(struct aclint_mtimer_data *mt)
+{
+       u64 v1, v2, mv, delta;
+       u64 *mt_time_val, *ref_time_val;
+       struct aclint_mtimer_data *reference;
+
+       /* Sync-up non-shared MTIME if reference is available */
+       if (mt->has_shared_mtime || !mt->time_delta_reference)
+               return;
+
+       reference = mt->time_delta_reference;
+       mt_time_val = (void *)mt->mtime_addr;
+       ref_time_val = (void *)reference->mtime_addr;
+       if (!atomic_raw_xchg_ulong(&mt->time_delta_computed, 1)) {
+               v1 = mt->time_rd(mt_time_val);
+               mv = reference->time_rd(ref_time_val);
+               v2 = mt->time_rd(mt_time_val);
+               delta = mv - ((v1 / 2) + (v2 / 2));
+               mt->time_wr(false, mt->time_rd(mt_time_val) + delta,
+                           mt_time_val);
+       }
+
+}
+
+void aclint_mtimer_set_reference(struct aclint_mtimer_data *mt,
+                                struct aclint_mtimer_data *ref)
+{
+       if (!mt || !ref || mt == ref)
+               return;
+
+       mt->time_delta_reference = ref;
+       mt->time_delta_computed = 0;
+}
+
 int aclint_mtimer_warm_init(void)
 {
-       u64 v1, v2, mv;
+       u64 *mt_time_cmp;
        u32 target_hart = current_hartid();
-       struct aclint_mtimer_data *reference;
-       u64 *mt_time_val, *mt_time_cmp, *ref_time_val;
        struct aclint_mtimer_data *mt = mtimer_hartid2data[target_hart];

        if (!mt)
                return SBI_ENODEV;

-       /*
-        * Compute delta if reference available
-        *
-        * We deliberately compute time_delta in warm init so that time_delta
-        * is computed on a HART which is going to use given MTIMER. We use
-        * atomic flag timer_delta_computed to ensure that only one HART does
-        * time_delta computation.
-        */
-       if (mt->time_delta_reference) {
-               reference = mt->time_delta_reference;
-               mt_time_val = (void *)mt->mtime_addr;
-               ref_time_val = (void *)reference->mtime_addr;
-               if (!atomic_raw_xchg_ulong(&mt->time_delta_computed, 1)) {
-                       v1 = mt->time_rd(mt_time_val);
-                       mv = reference->time_rd(ref_time_val);
-                       v2 = mt->time_rd(mt_time_val);
-                       mt->time_delta = mv - ((v1 / 2) + (v2 / 2));
-               }
-       }
+       /* Sync-up MTIME register */
+       aclint_mtimer_sync(mt);

        /* Clear Time Compare */
        mt_time_cmp = (void *)mt->mtimecmp_addr;
@@ -173,9 +188,7 @@ int aclint_mtimer_cold_init(struct aclint_mtimer_data *mt,
                return SBI_EINVAL;

        /* Initialize private data */
-       mt->time_delta_reference = reference;
-       mt->time_delta_computed = 0;
-       mt->time_delta = 0;
+       aclint_mtimer_set_reference(mt, reference);
        mt->time_rd = mtimer_time_rd32;
        mt->time_wr = mtimer_time_wr32;
diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c
index 15a36ed..26b5a2a 100644
--- a/lib/utils/timer/fdt_timer_mtimer.c
+++ b/lib/utils/timer/fdt_timer_mtimer.c
@@ -17,19 +17,18 @@

 static unsigned long mtimer_count = 0;
 static struct aclint_mtimer_data mtimer[MTIMER_MAX_NR];
+static struct aclint_mtimer_data *mt_reference = NULL;

 static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                                  const struct fdt_match *match)
 {
-       int rc;
+       int i, rc;
        unsigned long offset, addr[2], size[2];
-       struct aclint_mtimer_data *mt, *mtmaster = NULL;
+       struct aclint_mtimer_data *mt;

        if (MTIMER_MAX_NR <= mtimer_count)
                return SBI_ENOSPC;
        mt = &mtimer[mtimer_count];
-       if (0 < mtimer_count)
-               mtmaster = &mtimer[0];

        rc = fdt_parse_aclint_node(fdt, nodeoff, true,
                                   &addr[0], &size[0], &addr[1], &size[1],
@@ -37,6 +36,7 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
        if (rc)
                return rc;
        mt->has_64bit_mmio = true;
+       mt->has_shared_mtime = false;

        if (match->data) { /* SiFive CLINT */
                /* Set CLINT addresses */
@@ -63,10 +63,39 @@ static int timer_mtimer_cold_init(void *fdt, int nodeoff,
                        mt->has_64bit_mmio = false;
        }

-       rc = aclint_mtimer_cold_init(mt, mtmaster);
+       /* Check if MTIMER device has shared MTIME address */
+       mt->has_shared_mtime = false;
+       for (i = 0; i < mtimer_count; i++) {
+               if (mtimer[i].mtime_addr == mt->mtime_addr) {
+                       mt->has_shared_mtime = true;
+                       break;
+               }
+       }
+
+       /* Initialize the MTIMER device */
+       rc = aclint_mtimer_cold_init(mt, mt_reference);
        if (rc)
                return rc;

+       /*
+        * Select first MTIMER device with no associated HARTs as
+        * our reference MTIMER device
+        */
+       if (!mt->hart_count && !mt_reference) {
+               mt_reference = mt;
+               /*
+                * Set reference for already propbed MTIMER devices
+                * with non-shared MTIME
+                */
+               for (i = 0; i < mtimer_count; i++)
+                       if (!mtimer[i].has_shared_mtime)
+                               aclint_mtimer_set_reference(&mtimer[i], mt);
+       }
+
Is there a reason this is done after cold_init ?
+       /* Explicitly sync-up MTIMER devices not associated with any HARTs */
+       if (!mt->hart_count)
+               aclint_mtimer_sync(mt);
+
        mtimer_count++;
        return 0;
 }
--
2.25.1


--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

--
Regards,
Atish
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help