thermosphere: fix ptimer time freezing (again)

thermosphere/src/core_ctx.h

@@ -38,11 +38,11 @@ typedef struct CoreCtx {
 
     // Timer stuff
     u64 totalTimeInHypervisor;          // @0x40. cntvoff_el2 is updated to that value.
-    u64 emulPtimerOffsetThen;           // @0x48. When setting cntp_cval_el0 and on interrupt
+    u64 emulPtimerCval;                 // @0x48. When setting cntp_cval_el0 and on interrupt
 } CoreCtx;
 
 static_assert(offsetof(CoreCtx, executedFunctionSync) == 0x3C, "Wrong definition for CoreCtx");
-static_assert(offsetof(CoreCtx, emulPtimerOffsetThen) == 0x48, "Wrong definition for CoreCtx");
+static_assert(offsetof(CoreCtx, emulPtimerCval) == 0x48, "Wrong definition for CoreCtx");
 
 extern CoreCtx g_coreCtxs[4];
 register CoreCtx *currentCoreCtx asm("x18");

thermosphere/src/exceptions.h

@@ -17,6 +17,7 @@
 #pragma once
 #include <assert.h>
 #include "utils.h"
+#include "core_ctx.h"
 
 typedef struct ExceptionStackFrame {
     u64 x[31]; // x0 .. x30

thermosphere/src/guest_timers.h

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2019 Atmosphère-NX
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "exceptions.h"
+#include "sysreg.h"
+
+static inline u64 computeCntvct(const ExceptionStackFrame *frame)
+{
+    return frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
+}
+
+static inline void writeEmulatedPhysicalCompareValue(ExceptionStackFrame *frame, u64 val)
+{
+    // We lied about the value of cntpct, so we need to compute the time delta
+    // the guest actually intended to use...
+    u64 vct = computeCntvct(frame);
+    currentCoreCtx->emulPtimerCval = val;
+    SET_SYSREG(cntp_cval_el0, frame->cntpct_el0 + (val - vct));
+}

thermosphere/src/irq.c

@@ -19,6 +19,7 @@
 #include "debug_log.h"
 #include "vgic.h"
 #include "timer.h"
+#include "guest_timers.h"
 #include "transport_interface.h"
 
 IrqManager g_irqManager = {0};
@@ -101,15 +102,15 @@ static void initGic(void)
 
 static inline bool checkRescheduleEmulatedPtimer(ExceptionStackFrame *frame)
 {
-    // Evaluate if the timer really has expired in the PoV of the guest kernel. If not, reschedule (add missed time delta) it & exit early
+    // Evaluate if the timer has really expired in the PoV of the guest kernel.
-    u64 cval = GET_SYSREG(cntp_cval_el0);
+    // If not, reschedule (add missed time delta) it & exit early
-    u64 vct  = frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
+    u64 cval = currentCoreCtx->emulPtimerCval;
+    u64 vct  = computeCntvct(frame);
+
     if (cval > vct) {
         // It has not: reschedule the timer
-        u64 offsetNow = GET_SYSREG(cntvoff_el2);
+        // Note: this isn't 100% precise esp. on QEMU so it may take a few tries...
-        SET_SYSREG(cntp_cval_el0, cval + (offsetNow - currentCoreCtx->emulPtimerOffsetThen));
+        writeEmulatedPhysicalCompareValue(frame, cval);
-        currentCoreCtx->emulPtimerOffsetThen = offsetNow;
-
         return false;
     }
 
@@ -117,22 +118,17 @@ static inline bool checkRescheduleEmulatedPtimer(ExceptionStackFrame *frame)
 }
 
 
-static inline bool checkGuestTimerInterrupts(ExceptionStackFrame *frame, bool isLowerEl, u16 irqId)
+static inline bool checkGuestTimerInterrupts(ExceptionStackFrame *frame, u16 irqId)
 {
-    if (irqId != TIMER_IRQID(NS_VIRT_TIMER) && irqId != TIMER_IRQID(NS_PHYS_TIMER)) {
-        return true;
-    }
-
     // A thing that might have happened is losing the race vs disabling the guest interrupts
     // Another thing is that the virtual timer might have fired before us updating voff when executing a top half?
-    if (!isLowerEl) {
+    if (irqId == TIMER_IRQID(NS_VIRT_TIMER)) {
-        return false;
-    } else if (irqId == TIMER_IRQID(NS_VIRT_TIMER)) {
         u64 cval = GET_SYSREG(cntp_cval_el0);
-        u64 vct  = frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
+        return cval <= computeCntvct(frame);
-        return cval <= vct;
+    } else if (irqId == TIMER_IRQID(NS_PHYS_TIMER)) {
-    } else {
         return checkRescheduleEmulatedPtimer(frame);
+    } else {
+        return true;
     }
 }
 
@@ -207,6 +203,7 @@ bool irqIsGuest(u16 id)
 
 void handleIrqException(ExceptionStackFrame *frame, bool isLowerEl, bool isA32)
 {
+    (void)isLowerEl;
     (void)isA32;
     volatile ArmGicV2Controller *gicc = g_irqManager.gic.gicc;
 
@@ -220,7 +217,7 @@ void handleIrqException(ExceptionStackFrame *frame, bool isLowerEl, bool isA32)
     if (irqId == GIC_IRQID_SPURIOUS) {
         // Spurious interrupt received
         return;
-    } else if (!checkGuestTimerInterrupts(frame, isLowerEl, irqId)) {
+    } else if (!checkGuestTimerInterrupts(frame, irqId)) {
         // Deactivate the interrupt, return early
         gicc->eoir = iar;
         gicc->dir  = iar;

thermosphere/src/sysreg_traps.c

@@ -15,9 +15,7 @@
  */
 
 #include "sysreg_traps.h"
-#include "sysreg.h"
+#include "guest_timers.h"
-#include "arm.h"
-#include "debug_log.h"
 #include "software_breakpoints.h"
 
 static inline u64 doSystemRegisterRead(const ExceptionStackFrame *frame, u32 normalizedIss)
@@ -27,23 +25,22 @@ static inline u64 doSystemRegisterRead(const ExceptionStackFrame *frame, u32 nor
     u64 val;
     switch (normalizedIss) {
         case ENCODE_SYSREG_ISS(CNTPCT_EL0): {
-            u64 vct  = frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
+            u64 vct = computeCntvct(frame);
             val = vct;
             break;
         }
         case ENCODE_SYSREG_ISS(CNTP_TVAL_EL0): {
             u64 vct  = frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
-            val = (GET_SYSREG(cntp_cval_el0) - vct) & 0xFFFFFFFF;
+            u64 cval = currentCoreCtx->emulPtimerCval;
+            val = (cval - vct) & 0xFFFFFFFF;
             break;
         }
         case ENCODE_SYSREG_ISS(CNTP_CTL_EL0): {
-            // Passthrough
+            val = frame->cntp_ctl_el0;
-            val = GET_SYSREG(cntp_ctl_el0);
             break;
         }
         case ENCODE_SYSREG_ISS(CNTP_CVAL_EL0): {
-            // Passthrough
+            val = currentCoreCtx->emulPtimerCval;
-            val = GET_SYSREG(cntp_cval_el0);
             break;
         }
 
@@ -58,30 +55,23 @@ static inline u64 doSystemRegisterRead(const ExceptionStackFrame *frame, u32 nor
     return val;
 }
 
-static inline void writeEmulatedPhysicalCompareValue(u64 val)
+static inline void doSystemRegisterWrite(ExceptionStackFrame *frame, u32 normalizedIss, u64 val)
-{
-    currentCoreCtx->emulPtimerOffsetThen = currentCoreCtx->totalTimeInHypervisor;
-    SET_SYSREG(cntp_cval_el0, val);
-}
-
-static inline void doSystemRegisterWrite(const ExceptionStackFrame *frame, u32 normalizedIss, u64 val)
 {
     // See https://developer.arm.com/architectures/learn-the-architecture/generic-timer/single-page
 
     switch (normalizedIss) {
         case ENCODE_SYSREG_ISS(CNTP_TVAL_EL0): {
             // Sign-extend
-            u64 vct  = frame->cntpct_el0 - currentCoreCtx->totalTimeInHypervisor;
+            u64 vct = computeCntvct(frame);
-            writeEmulatedPhysicalCompareValue(vct + (u64)(s32)val);
+            writeEmulatedPhysicalCompareValue(frame, vct + (u64)(s32)val);
             break;
         }
         case ENCODE_SYSREG_ISS(CNTP_CTL_EL0): {
-            // Passthrough
+            frame->cntp_ctl_el0 = val;
-            SET_SYSREG(cntp_ctl_el0, val);
             break;
         }
         case ENCODE_SYSREG_ISS(CNTP_CVAL_EL0): {
-            writeEmulatedPhysicalCompareValue(val);
+            writeEmulatedPhysicalCompareValue(frame, val);
             break;
         }