[PATCH] aarch64: implement minimal PL031 support

[Waldemar Kozaczuk]

This patch provides basic implementation of a subset of
functionality of the PrimeCell Real Time Clock (PL031) device
that is enough to capture boot time as number of seconds since epoch.
Please note that this implementation does not handle any host
clock drift and provides current time functionality that is only
accurate up to a second.

This patch effectively fixes 2 unit tests - tst-time.cc and
tst-timerfd.cc.

Fixes #1091

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
Makefile | 1 +
arch/aarch64/arch-dtb.cc | 19 +++++++++++++
arch/aarch64/arch-dtb.hh | 7 +++++
arch/aarch64/arm-clock.cc | 18 +++++++++++--
drivers/pl031.cc | 56 +++++++++++++++++++++++++++++++++++++++
drivers/pl031.hh | 50 ++++++++++++++++++++++++++++++++++
scripts/test.py | 4 +--
7 files changed, 150 insertions(+), 5 deletions(-)
create mode 100644 drivers/pl031.cc
create mode 100644 drivers/pl031.hh

diff --git a/Makefile b/Makefile
index 833803cf..f33b1cdb 100644
--- a/Makefile
+++ b/Makefile
@@ -835,6 +835,7 @@ endif # x64
ifeq ($(arch),aarch64)
drivers += drivers/mmio-isa-serial.o
drivers += drivers/pl011.o
+drivers += drivers/pl031.o
drivers += drivers/cadence-uart.o
drivers += drivers/xenconsole.o
drivers += drivers/virtio.o
diff --git a/arch/aarch64/arch-dtb.cc b/arch/aarch64/arch-dtb.cc
index 24ac9267..63db8b00 100644
--- a/arch/aarch64/arch-dtb.cc
+++ b/arch/aarch64/arch-dtb.cc
@@ -190,6 +190,25 @@ u64 dtb_get_uart(int *irqid)
return retval;
}

+u64 dtb_get_rtc()
+{
+ u64 retval;
+ int node; size_t len;
+
+ if (!dtb)
+ return 0;
+
+ node = fdt_node_offset_by_compatible(dtb, -1, "arm,pl031");
+ if (node < 0)
+ return 0;
+
+ len = dtb_get_reg(node, &retval);
+ if (!len)
+ return 0;
+
+ return retval;
+}
+
u64 dtb_get_mmio_serial_console(int *irqid)
{
int node;
diff --git a/arch/aarch64/arch-dtb.hh b/arch/aarch64/arch-dtb.hh
index 0aebb4b3..10e1c859 100644
--- a/arch/aarch64/arch-dtb.hh
+++ b/arch/aarch64/arch-dtb.hh
@@ -49,6 +49,13 @@ size_t dtb_get_phys_memory(u64 *addr);
*/
u64 dtb_get_uart(int *irqid);

+/* u64 dtb_get_rtc()
+ *
+ * return the base address of the RTC (PL031)
+ * device or returns zero on failure.
+ */
+u64 dtb_get_rtc();
+
/* u64 dtb_get_mmio_serial_console(int *irqid)
*
* return the base address of the serial console and writes the
diff --git a/arch/aarch64/arm-clock.cc b/arch/aarch64/arm-clock.cc
index c1f4b277..7d11e454 100644
--- a/arch/aarch64/arm-clock.cc
+++ b/arch/aarch64/arm-clock.cc
@@ -7,6 +7,7 @@

#include "drivers/clockevent.hh"
#include "drivers/clock.hh"
+#include "drivers/pl031.hh"
#include "arm-clock.hh"
#include <osv/interrupt.hh>
#include "exceptions.hh"
@@ -36,6 +37,8 @@ protected:
u32 freq_hz; /* frequency in Hz (updates per second) */

friend class arm_clock_events;
+private:
+ u64 _boot_time;
};

arm_clock::arm_clock() {
@@ -49,6 +52,13 @@ arm_clock::arm_clock() {
#if CONF_logger_debug
debug_early_u64("arm_clock(): frequency read as ", freq_hz);
#endif
+ u64 rtc_address = dtb_get_rtc();
+ if (rtc_address) {
+ pl031 rtc(rtc_address);
+ _boot_time = rtc.wallclock_ns();
+ } else {
+ _boot_time = 0;
+ }
}

static __attribute__((constructor(init_prio::clock))) void setup_arm_clock()
@@ -70,12 +80,16 @@ s64 arm_clock::uptime()

s64 arm_clock::time()
{
- return uptime();
+ //Given that _boot_time came from RTC which gives only 1 second
+ //precision the result time() is only accurate up to a second
+ //On top of this, this implementation does not account for any host
+ //clock drifts
+ return _boot_time + uptime();
}

s64 arm_clock::boot_time()
{
- return uptime();
+ return _boot_time;
}

u64 arm_clock::processor_to_nano(u64 ticks)
diff --git a/drivers/pl031.cc b/drivers/pl031.cc
new file mode 100644
index 00000000..b81b38b9
--- /dev/null
+++ b/drivers/pl031.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2021 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include "pl031.hh"
+#include <osv/mmu.hh>
+
+/* spec: see PrimeCell Real Time Clock (PL031) Technical Reference Manual.
+ * implemented according to Revision: r1p3.
+ * See https://static.docs.arm.com/ddi0224/c/real_time_clock_pl031_r1p3_technical_reference_manual_DDI0224C.pdf
+ * Please note that this a minimal implementation of a subset of the
+ * functionality which is enough to read the current time in seconds
+ * at the boot time.
+ */
+
+#define rtc_reg(ADDR,REG_OFFSET) (*(volatile u32 *)(ADDR+REG_OFFSET))
+
+pl031::pl031(u64 address)
+{
+ _address = address;
+ mmu::linear_map((void *)_address, _address, mmu::page_size, mmu::page_size,
+ mmu::mattr::dev);
+}
+
+pl031::~pl031()
+{
+ mmu::munmap((void*)_address, mmu::page_size);
+}
+
+uint64_t pl031::wallclock_ns()
+{
+ // Let us read various identificatin registers to
+ // verify that it is indeed a valid PL031 device
+ if( rtc_reg(_address, RTCPeriphID0) == RTCPeriphID0_val &&
+ rtc_reg(_address, RTCPeriphID1) == RTCPeriphID1_val &&
+ (rtc_reg(_address, RTCPeriphID2) & RTCPeriphID2_mask) == RTCPeriphID2_val &&
+ rtc_reg(_address, RTCPeriphID3) == RTCPeriphID3_val &&
+ rtc_reg(_address, RTCPCellID0) == RTCPCellID0_val &&
+ rtc_reg(_address, RTCPCellID1) == RTCPCellID1_val &&
+ rtc_reg(_address, RTCPCellID2) == RTCPCellID2_val &&
+ rtc_reg(_address, RTCPCellID3) == RTCPCellID3_val) {
+ // Read value of RTC which is number of seconds since epoch
+ // representing current time
+ u64 epoch_time_in_seconds = rtc_reg(_address, RTCDR);
+#if CONF_logger_debug
+ debug_early_u64("pl031::wallclock_ns(): RTC seconds since epoch read as ", epoch_time_in_seconds);
+#endif
+ return epoch_time_in_seconds * 1000000000;
+ } else {
+ debug_early("pl031: could no detect!");
+ return 0;
+ }
+}
diff --git a/drivers/pl031.hh b/drivers/pl031.hh
new file mode 100644
index 00000000..7b594ff4
--- /dev/null
+++ b/drivers/pl031.hh
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2021 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#ifndef OSV_PL031_HH
+#define OSV_PL031_HH
+
+#include <osv/types.h>
+
+#define RTCDR 0x000 /* data */
+#define RTCMR 0x004 /* match */
+#define RTCLR 0x008 /* load */
+#define RTCCR 0x00c /* control */
+#define RTCIMSC 0x010 /* interrupt mask set or clear */
+#define RTCRIS 0x014 /* raw interrupt status */
+#define RTCMIS 0x018 /* masked interrupt status */
+#define RTCICR 0x01c /* interrupt clear */
+#define RTCPeriphID0 0xfe0 /* peripheral ID bits [7:0] */
+#define RTCPeriphID1 0xfe4 /* peripheral ID bits [15:8] */
+#define RTCPeriphID2 0xfe8 /* peripheral ID bits [23:16] */
+#define RTCPeriphID3 0xfec /* peripheral ID bits [31:24] */
+#define RTCPCellID0 0xff0 /* PrimeCell ID bits [7:0] */
+#define RTCPCellID1 0xff4 /* PrimeCell ID bits [7:0] */
+#define RTCPCellID2 0xff8 /* PrimeCell ID bits [7:0] */
+#define RTCPCellID3 0xffc /* PrimeCell ID bits [7:0] */
+
+#define RTCPeriphID0_val 0x31
+#define RTCPeriphID1_val 0x10
+#define RTCPeriphID2_mask 0x0f
+#define RTCPeriphID2_val 0x04
+#define RTCPeriphID3_val 0x00
+
+#define RTCPCellID0_val 0x0d
+#define RTCPCellID1_val 0xf0
+#define RTCPCellID2_val 0x05
+#define RTCPCellID3_val 0xb1
+
+class pl031 {
+public:
+ pl031(u64 address);
+ ~pl031();
+ uint64_t wallclock_ns();
+private:
+ u64 _address;
+};
+
+#endif //OSV_PL031_HH
diff --git a/scripts/test.py b/scripts/test.py
index 8e3882af..f0c12d6c 100755
--- a/scripts/test.py
+++ b/scripts/test.py
@@ -31,7 +31,7 @@ firecracker_disabled_list= [
"tcp_close_without_reading_on_qemu"
]

-#At this point there are 120 out of 131 unit tests that pass on aarch64.
+#At this point there are 122 out of 131 unit tests that pass on aarch64.
#The remaining ones are disabled below until we fix various
#issues that prevent those tests from passing.
aarch64_disabled_list= [
@@ -46,8 +46,6 @@ aarch64_disabled_list= [
#Please see comments on the right side for more details
"tst-elf-permissions.so", # Infinite page fault
"tst-mmap.so", # Infinite page fault
- "tst-time.so", # One assertion fails - 'tst-time.cc(70): fatal error: in "time_time": critical check (static_cast<time_t>(0)) != (t1) has failed'
- "tst-timerfd.so", # Some assertions fail - 'SUMMARY: 212 tests, 10 failures'
#These tests fail due to some other shortcomings in the test scripts
"tracing_smoke_test",
"tcp_close_without_reading_on_fc",
--
2.30.2

[Nadav Har'El]

Looks good to me, but I'm not at all an expert on these ARM devices.

I only left minor comments below.

I think it will be good - using the name of this variable or comments - stress that this variable has nanosecond units (as in boot_time() in drivers/clock.hh). There are two many other ways to understand this.

 };

 arm_clock::arm_clock() {
@@ -49,6 +52,13 @@ arm_clock::arm_clock() {
 #if CONF_logger_debug
     debug_early_u64("arm_clock(): frequency read as ", freq_hz);
 #endif
+    u64 rtc_address = dtb_get_rtc();
+    if (rtc_address) {
+        pl031 rtc(rtc_address);
+       _boot_time = rtc.wallclock_ns();
+    } else {
+       _boot_time = 0;
+    }
 }

 static __attribute__((constructor(init_prio::clock))) void setup_arm_clock()
@@ -70,12 +80,16 @@ s64 arm_clock::uptime()

 s64 arm_clock::time()
 {
-    return uptime();
+    //Given that _boot_time came from RTC which gives only 1 second
+    //precision the result time() is only accurate up to a second

Is this 1-second precision a limitation of this pl031, or we're not yet using it properly?

+    //On top of this, this implementation does not account for any host
+    //clock drifts
+    return _boot_time + uptime();
 }

 s64 arm_clock::boot_time()
 {
-    return uptime();
+    return _boot_time;
 }

 u64 arm_clock::processor_to_nano(u64 ticks)
diff --git a/drivers/pl031.cc b/drivers/pl031.cc

I think at some point we should reconsider the drivers/ directory.

E.g., why is arm-clock.hh in arch/aarch64 but the new pl031.hh in drivers?

Perhaps we should move the ARM-specific drivers to arch/aarch64, or another

option is to create drivers/aarch64 where we put ARM-only drivers (and similarly

have other subdirectories in drivers? Like x86, xen, kvm, etc.

Another goal is for x86 builds to exclude ARM-specific drivers, and vice versa.

But we can reconsider this later.

 

new file mode 100644
index 00000000..b81b38b9
--- /dev/null
+++ b/drivers/pl031.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2021 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include "pl031.hh"
+#include <osv/mmu.hh>
+
+/* spec: see PrimeCell Real Time Clock (PL031) Technical Reference Manual.

Maybe you should mention that this is an ARM thing?

The more stuff we have in drivers/, the less obvious it becomes which systems these drivers are relevant for.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20210319153359.69109-1-jwkozaczuk%40gmail.com.

[Waldek Kozaczuk]

Agreed. 

 };

 arm_clock::arm_clock() {
@@ -49,6 +52,13 @@ arm_clock::arm_clock() {
 #if CONF_logger_debug
     debug_early_u64("arm_clock(): frequency read as ", freq_hz);
 #endif
+    u64 rtc_address = dtb_get_rtc();
+    if (rtc_address) {
+        pl031 rtc(rtc_address);
+       _boot_time = rtc.wallclock_ns();
+    } else {
+       _boot_time = 0;
+    }
 }

 static __attribute__((constructor(init_prio::clock))) void setup_arm_clock()
@@ -70,12 +80,16 @@ s64 arm_clock::uptime()

 s64 arm_clock::time()
 {
-    return uptime();
+    //Given that _boot_time came from RTC which gives only 1 second
+    //precision the result time() is only accurate up to a second

Is this 1-second precision a limitation of this pl031, or we're not yet using it properly?

As I understand the spec it is the limitation of  pl031.

+    //On top of this, this implementation does not account for any host
+    //clock drifts
+    return _boot_time + uptime();
 }

 s64 arm_clock::boot_time()
 {
-    return uptime();
+    return _boot_time;
 }

 u64 arm_clock::processor_to_nano(u64 ticks)
diff --git a/drivers/pl031.cc b/drivers/pl031.cc

I think at some point we should reconsider the drivers/ directory.

E.g., why is arm-clock.hh in arch/aarch64 but the new pl031.hh in drivers?

Perhaps we should move the ARM-specific drivers to arch/aarch64, or another

option is to create drivers/aarch64 where we put ARM-only drivers (and similarly

have other subdirectories in drivers? Like x86, xen, kvm, etc.

Another goal is for x86 builds to exclude ARM-specific drivers, and vice versa.

But we can reconsider this later.

Good idea. 

[Waldemar Kozaczuk]

This patch provides basic implementation of a subset of
functionality of the PrimeCell Real Time Clock (PL031) device
that is enough to capture boot time as number of seconds since epoch.

Please note that this implementation does not handle any host
clock drift and provides current time functionality that is only
accurate up to a second.

This patch effectively fixes 2 unit tests - tst-time.cc and
tst-timerfd.cc.

Fixes #1091

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
Makefile | 1 +
arch/aarch64/arch-dtb.cc | 19 +++++++++++++
arch/aarch64/arch-dtb.hh | 7 +++++

arch/aarch64/arm-clock.cc | 22 +++++++++++++--

drivers/pl031.cc | 56 +++++++++++++++++++++++++++++++++++++++
drivers/pl031.hh | 50 ++++++++++++++++++++++++++++++++++
scripts/test.py | 4 +--

7 files changed, 154 insertions(+), 5 deletions(-)

index 20c35528..534726ad 100644

--- a/arch/aarch64/arm-clock.cc
+++ b/arch/aarch64/arm-clock.cc
@@ -7,6 +7,7 @@

#include "drivers/clockevent.hh"
#include "drivers/clock.hh"
+#include "drivers/pl031.hh"
#include "arm-clock.hh"
#include <osv/interrupt.hh>
#include "exceptions.hh"
@@ -36,6 +37,8 @@ protected:
u32 freq_hz; /* frequency in Hz (updates per second) */

friend class arm_clock_events;
+private:

+ u64 _boot_time_in_ns;
};

arm_clock::arm_clock() {
@@ -49,6 +52,13 @@ arm_clock::arm_clock() {
#if CONF_logger_debug
debug_early_u64("arm_clock(): frequency read as ", freq_hz);
#endif
+ u64 rtc_address = dtb_get_rtc();
+ if (rtc_address) {
+ pl031 rtc(rtc_address);

+ _boot_time_in_ns = rtc.wallclock_ns();
+ } else {
+ _boot_time_in_ns = 0;

+ }
}

static __attribute__((constructor(init_prio::clock))) void setup_arm_clock()

@@ -61,6 +71,10 @@ static __attribute__((constructor(init_prio::clock))) void setup_arm_clock()

s64 arm_clock::uptime()
{
+ //Please note we read CNTVCT cpu system register which provides
+ //the accross-system consistent value of the virtual system counter.
+ //This means that uptime() should return the exact same value
+ //on each cpu.
u64 cntvct;
asm volatile ("isb; mrs %0, cntvct_el0; isb; " : "=r"(cntvct) :: "memory");

@@ -70,12 +84,16 @@ s64 arm_clock::uptime()

s64 arm_clock::time()
{
- return uptime();

+ //Given that _boot_time_in_ns came from RTC which gives only 1 second
+ //precision the resulting time() value is only accurate up to a second

+ //On top of this, this implementation does not account for any host
+ //clock drifts

+ return _boot_time_in_ns + uptime();

}

s64 arm_clock::boot_time()
{
- return uptime();

+ return _boot_time_in_ns;

}

u64 arm_clock::processor_to_nano(u64 ticks)
diff --git a/drivers/pl031.cc b/drivers/pl031.cc

new file mode 100644
index 00000000..b81b38b9
--- /dev/null
+++ b/drivers/pl031.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2021 Waldemar Kozaczuk
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include "pl031.hh"
+#include <osv/mmu.hh>
+
+/* spec: see PrimeCell Real Time Clock (PL031) Technical Reference Manual.

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

aarch64: implement minimal PL031 support

This patch provides basic implementation of a subset of
functionality of the PrimeCell Real Time Clock (PL031) device
that is enough to capture boot time as number of seconds since epoch.

Please note that this implementation does not handle any host
clock drift and provides current time functionality that is only
accurate up to a second.

This patch effectively fixes 2 unit tests - tst-time.cc and
tst-timerfd.cc.

Fixes #1091

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/Makefile b/Makefile

--- a/Makefile
+++ b/Makefile
@@ -835,6 +835,7 @@ endif # x64
ifeq ($(arch),aarch64)
drivers += drivers/mmio-isa-serial.o
drivers += drivers/pl011.o
+drivers += drivers/pl031.o
drivers += drivers/cadence-uart.o
drivers += drivers/xenconsole.o
drivers += drivers/virtio.o
diff --git a/arch/aarch64/arch-dtb.cc b/arch/aarch64/arch-dtb.cc

--- a/arch/aarch64/arch-dtb.hh
+++ b/arch/aarch64/arch-dtb.hh
@@ -49,6 +49,13 @@ size_t dtb_get_phys_memory(u64 *addr);
*/
u64 dtb_get_uart(int *irqid);

+/* u64 dtb_get_rtc()
+ *
+ * return the base address of the RTC (PL031)
+ * device or returns zero on failure.
+ */
+u64 dtb_get_rtc();
+
/* u64 dtb_get_mmio_serial_console(int *irqid)
*
* return the base address of the serial console and writes the
diff --git a/arch/aarch64/arm-clock.cc b/arch/aarch64/arm-clock.cc

--- a/arch/aarch64/arm-clock.cc
+++ b/arch/aarch64/arm-clock.cc
@@ -7,6 +7,7 @@

#include "drivers/clockevent.hh"
#include "drivers/clock.hh"
+#include "drivers/pl031.hh"
#include "arm-clock.hh"
#include <osv/interrupt.hh>
#include "exceptions.hh"

@@ -36,6 +37,8 @@ class arm_clock : public clock {

--- a/drivers/pl031.cc

--- a/drivers/pl031.hh

--- a/scripts/test.py
+++ b/scripts/test.py
@@ -31,7 +31,7 @@

"tcp_close_without_reading_on_qemu"
]

-#At this point there are 120 out of 131 unit tests that pass on aarch64.
+#At this point there are 122 out of 131 unit tests that pass on aarch64.
#The remaining ones are disabled below until we fix various
#issues that prevent those tests from passing.
aarch64_disabled_list= [
@@ -46,8 +46,6 @@