Samraksh / eMote

eMote OS -- Multiple Ports (using .NET MF v4.3)
0 stars 0 forks source link

.NOW: VT and sleep #242

Closed Nathan-Stohs closed 9 years ago

Nathan-Stohs commented 9 years ago

Testing sleep mode, looks like there is an issue with the Virtual Timer and sleep mode.

Test branch is mfSleep (off of current master)

I have a test app C# that counts on the LCD and UART (Bill's actually). When loaded with sleep mode, the counter stops unless I apply an external interrupt (such as a UART input).

In infer we are stuck in sleep mode. I infer this happens because the VT is not properly setting timer interrupts and so we go into sleep without our intended wake-up source.

With sleep disabled, I assume we don't have this problem because we are continuously returning to the VT in the main OS loop.

ChrisAtSamraksh commented 9 years ago

Can you retest with the C# user timer fix? d53ac3a97fb3f1a22d055c52e86b5eef0933eb50

Nathan-Stohs commented 9 years ago

Still not working. Note that I made some changes recently to allow sleep mode during a debug session.

ChrisAtSamraksh commented 9 years ago

I never specifically looked at sleep mode, so I was just hoping it might be fixed by this checkin. I will have to look into it in more detail.

Nathan-Stohs commented 9 years ago

This probably should be on my plate to at least diagnose further, but I'm busy for the next few days.

Technically I don't think it actually has anything to do with sleeping itself. The violation is that when CPU_Sleep() is called there should be a timer interrupt set and there isn't. In my example, it should be set from a C# user timer firing every second. if this is demonstrably not true, just kick it back to me.

Mike has also pointed out some problems in the way that IRQs are handled; fundamental mistakes were made in the initial port. Very possible this is related.

MichaelAtSamraksh commented 9 years ago

Slogging through GlobalLock led me back here to preach lock and sleep feature interaction. Does the unplugged alarm clock ever ring? The power driver is supposed to turn on interrupts before sleeping (if there do not exist any pending interrupts). The CMSIS intrinsic __irq_disable and its asm slang "cpsid i" call just set PRIMASK so non-masking interrupts and faults are still enabled. So yes, the alarm clock could ring, just not purposefully. The power driver is supposed to enable interrupts for at least the events flags passed in Execution.cpp's DebuggerLoop and WaitForActivity functions, as well as responding to managed code requests to set the wakeup events. Does the unset alarm clock ever ring? See g_CLR_HW_Hardware.m_wakeupEvents. The flag SYSTEM_EVENT_FLAG_DEBUGGER_ACTIVITY is usually passed, along with the CLR setting SYSTEM_EVENT_FLAG_SYSTEM_TIMER when the CLR decides the sleep timeout (which should also be set by the Power driver before going to sleep if conditions are met). Disclaimer: I haven't tested all of that up into a working state yet.

Nathan-Stohs commented 9 years ago

The power driver is supposed to turn on interrupts before sleeping (if there do not exist any pending interrupts). The CMSIS intrinsic __irq_disable and its asm slang "cpsid i" call just set PRIMASK so non-masking interrupts and faults are still enabled. So yes, the alarm clock could ring, just not purposefully.

I don't follow. Enabling or Disabling interrupts is strictly a function of masking their effect on the CPU, not their actual generation, i.e. I claim that cpsid i and cpsie i have no effect at all on wake-up. What matters is the NVIC configuration and the IT configuration of the peripheral itself. So the power driver does not have any particular requirements with respect interrupt masking that another driver wouldn't (e.g. don't break it).

The power driver is supposed to enable interrupts for at least the events flags passed in Execution.cpp's DebuggerLoop and WaitForActivity functions, as well as responding to managed code requests to set the wakeup events.

We wake-up on all events for initialized peripherals, so the "for at least" condition is met. The wakeEvents parameter was a half-baked concept that even the official MF ports from Microsoft ignores. Only the SH72* ports use it and even then only in a trivial way.

Our practice now is that if a peripheral is configured as an interrupt source, then it is a valid wake-up source. Frankly, my real problem here is that I don't trust the PAL to know what it's doing enough to improve on that, which I suspect is why its mostly not implemented elsewhere as well. That isn't to say I can't imagine scenarios where it could be used, but rather that there are other obstacles.

(had more ranting about wakeEvents here but I'll shut up now).

Nathan-Stohs commented 9 years ago

I looked into this further on current master (482bb9f613a4aea08abf9cb0676e3f119484aaac)

It looks like a problem with the VT code that sets interrupts or how it interacts with C# user timers or whatever mechanism Thread.sleep() uses. Does not seem to be a problem with sleep or interrupts or locking etc.

I set up a simple counter with C# using Thread.sleep() and flipped on light sleep mode to test. I set a breakpoint in STM32F10x_AdvancedTimer::SetCompare() which is the HAL call for setting the timer interrupt just below the VT PAL layer.

Result was:

Breakpoint 1, STM32F10x_AdvancedTimer::SetCompare (this=0x20002208 <g_STM32F10x_AdvancedTimer>, counterCorrection=4325102017, compareValue=4264879867, scType=SET_COMPARE_TIMER)

The important part of that is counterCorrection=4325102017, compareValue=4264879867. Both values appear to be garbage. So the timer isn't actually getting set with anything remotely correct.

Nathan-Stohs commented 9 years ago

Some or all of this is related to #245 . At least the counterCorrection being wrong part. Not sure how a bad compareValue is getting computed.

Nathan-Stohs commented 9 years ago

Seems to be a combination of two things. First, the bug in #245 is hugely advancing the clock.

Given that, the second more important bug in the following VT code:

void Time_Driver::SetCompareValue( UINT64 compareTicks )
{
    //CPU_GPIO_SetPinState((GPIO_PIN) 52, TRUE);
    //CPU_GPIO_SetPinState((GPIO_PIN) 52, FALSE);

    UINT32 compareTimeInMicroSecs = 0;

    /*if(compareTicks >= 0xFFFFFFFF)
    {
        compareTimeInMicroSecs = 0xFFFFFFFF;
        //hal_printf("if compareTimeInMicroSecs: %d \r\n", compareTimeInMicroSecs-1);
    }
    else{
        compareTimeInMicroSecs = CPU_TicksToMicroseconds((UINT32)compareTicks, 1);
        //hal_printf("else compareTimeInMicroSecs: %d \r\n", compareTimeInMicroSecs-1);
    }*/

    //if(compareTicks <= 0x25ED097B)
    //if(compareTicks <= 0x19999999)
    if(compareTicks < 0xFFFFFFFF)
    {
        //compareTimeInMicSecs = CPU_TicksToTime((UINT32)compareTicks, 1);
#ifdef PLATFORM_ARM_EmoteDotNow
        compareTimeInMicroSecs = (UINT32)(CPU_TicksToMicroseconds(compareTicks, 1));
#else
        //compareTimeInMicroSecs = ((UINT32)compareTicks) * 4 / 27;
        compareTimeInMicroSecs = (UINT32)(CPU_TicksToMicroseconds(compareTicks, 1));
#endif
    }
    else
        compareTimeInMicroSecs = 0xFFFFFFFF;

    if(VirtTimer_SetTimer(VIRT_TIMER_EVENTS, 0, compareTimeInMicroSecs, TRUE, TRUE, SetCompareHandler) == TimerReserved)
    {
        VirtTimer_Change(VIRT_TIMER_EVENTS, 0, compareTimeInMicroSecs, TRUE);
    }

    VirtTimer_Start( VIRT_TIMER_EVENTS );

    /*if(VirtTimer_SetTimer(VIRT_TIMER_EVENTS, 0, compareTicks, TRUE, TRUE, SetCompareHandler) == TimerReserved)
    {
        //VirtTimer_Change(VIRT_TIMER_EVENTS, 0, compareTicks, TRUE);
    }

    VirtTimer_Start( VIRT_TIMER_EVENTS );
    //VirtTimer_Stop( VIRT_TIMER_EVENTS );
    CPU_Timer_SetCompare(1, compareTicks);*/
}

The check if(compareTicks < 0xFFFFFFFF) is always failing and so the code doesn't attempt to set the timer with a real value.

Not sure why this works without sleep enabled... only guess is that the main OS busy-wait loop was manually catching user timers. And native VT timers might be OK.

ChrisAtSamraksh commented 9 years ago

The VT timers have a max value of 0xFFFFFFFF. Wrap-arounds are kept track of and so we can give an appropriate time, but the VT code was never written to have native timers be more than 0xFFFFFFFF.

C# user timers use a different system and can be set for much longer times.

If you attempt to set a wake-up timer that is larger than 0xFFFFFFFF (which is 8 minutes 56 seconds at 8MHz) then it will be set to the max value and you should have the timer fire after 8 minutes and 56 seconds.

Nathan-Stohs commented 9 years ago
ChrisAtSamraksh commented 9 years ago

C# user timers and MF timers do not go through the virtual timer code at all. They use 64-bit times and the only problem I know about is that about every 11 minutes they can jump 5 billion counts. Ananth found the cause of the problem but hasn't fixed it yet.

Nathan-Stohs commented 9 years ago

The VT sits on top of all hardware timers. Any timer in the system (native, C#, MF, whatever) must use the VT. There just isn't any other timer. All calls to Time are redirected to the VT and the VT handles system time.

I'll be a little more specific about why I say this. There are actually two drivers here, one is the VT and the other is the eMote_Time PAL layer which is Samraksh specific and is really a sister component of the VT. Basically you can think of eMote_Time as a shim / glue layer to interface system time to the VT.

The ONLY ways that MF PAL knows to set a timer is either void HAL_Time_SetCompare( UINT64 CompareValue ) or void HAL_Time_SetCompare_Completion(UINT64 CompareValue). Both of these call g_Time_Driver.SetCompareValue(CompareValue); which itself eventually calls VirtTimer_Start( VIRT_TIMER_EVENTS );

System time is also handled by the VT via struct Time_Driver (written by us) which contains static UINT64 bigCounter; which is the master tick counter and is updated by void TimeHandler(void *arg) which is a call back defined from... VirtTimer_SetTimer(VIRT_TIMER_TIME, 0, maxTicks, FALSE, TRUE, TimeHandler)

So in short, although there are two timing sub-systems in play, virtualized hardware timers and virtualized system time, both are in reality the VT. In the case of native timers, you use the VT directly. In the case of system time (including C# user timers) you use the eMote_Time layer which itself uses the VT. So either way, its the VT.

ChrisAtSamraksh commented 9 years ago

Virtual timers are a queue of various native timers and use the VirtualTimer.cpp code. The radio and HAL_COMPLETIONs found in completions.cpp use these virtual timers. They set an interrupt to occur at a specific timer count for the next soonest timer in the queue.

C# user timers do not use the virtual timer queue code at all. In MicroFrameworkPK_v4_3\CLR\Core\Execution.cpp the MF m_currentMachineTime variable is updated with CLR_RT_ExecutionEngine::UpdateTime() which gets our time with Time_GetMachineTime() which calls g_TimeDriver.GetMachineTime() which calls HAL_Time_CurrentTime() which calls CPU_TicksToTime( HAL_Time_CurrentTicks(Timer) ) which calls g_Time_Driver.CurrentTicks() which calls VirtTimer_GetTicks which calls CPU_Timer_CurrentTicks which calls g_STM32F10x_AdvancedTimer.Get64Counter()

There is some simplification that can be made here but basically all that is just getting the current time and checks to see if a user timer needs to fire. There are no set compares or interrupts involved here.

So, by virtual timers I refer to the virtual timer queue that uses setcompare interrupts up to a 32-bit timer value and the C# user timers that use the 64-bit system time.

Nathan-Stohs commented 9 years ago

There are no set compares or interrupts involved here.

You are only looking at the execution loop when it is polling to determine if anything is on the ready queue. All C# timers must set an interrupt. To say otherwise is to say that power management is impossible because we can't sleep without an interrupt.

Before The execution loop runs, an interrupt for the next task on the queue (e.g. a user timer) is set from: void HAL_COMPLETION::WaitForInterrupts( UINT64 Expire, UINT32 sleepLevel, UINT64 wakeEvents ) or sometimes void HAL_COMPLETION::EnqueueTicks( UINT64 EventTimeTicks )

Then once we wake up from that we enter CLR_RT_ExecutionEngine

Do a Thread.sleep(1000) in a demo app, set some breakpoints, and you will see it's corrosponding SetCompare()

(the above is why sleep is currently broken, because the values get corrupted part-way through, and why it works without sleep is because we are constantly checking the ready queue as a busy-wait)

MichaelAtSamraksh commented 9 years ago

<MAGIC>

https://github.com/Samraksh/MF/wiki/Timer-Architecture

</ MAGIC>

postscriptum: Chris, a correction: HAL_Time_CurrentTime() --> g_Time_Driver.CurrentTime() --> Time_Driver::CurrentTime() Your comment missed the big ol' #if 0 at line 739. This is exactly why I was just preaching about dead code over the weekend...

Srsly, Let's stop documenting architecture in issue threads. I'm guilty of it too. Let's stop this nonsense.

ChrisAtSamraksh commented 9 years ago

Fixed in a769ef95f5ac45d8813ba6d5e56d365df1b6219d