MPC5748G with OS tick lower than 1ms (100us)

[alexvinchev]

Hi,
I'm running ChibiOS 2.6.9.
I've tried to run my board at OS tick speed shorter than 1ms. When I do so, I'm going to core trap "Machine Check Interrupt (IVOR1)".
OS goes there from the _port_switch function, which in my case is:

Code: Select all

_port_switch:
    e_subi      r1, r1, 0x50                    ; Size of the intctx structure.
    se_mflr     r0
    e_stw       r0, 84(r1)                      ; LR into the caller frame.
    mfcr        r0
    e_stw       r0, 0(r1)                       ; CR.
    e_stmw      r14, 4(r1)                      ; GPR14...GPR31.

    e_stw       r1, 12(r4)                      ; Store swapped-out stack.
    e_lwz       r1, 12(r3)                      ; Load swapped-in stack.

    e_lmw       r14, 4(r1)                      ; GPR14...GPR31.
    e_lwz       r0, 0(r1)                       ; CR.
    mtcr        r0
    e_lwz       r0, 84(r1)                      ; LR from the caller frame.
    mtlr        r0
    e_addi      r1, r1, 80                      ; Size of the intctx structure.
    se_blr


When execution reach e_lmw r14, 4(r1) row, IVOR1 is hit. I saw, that at this point SP (r1) is zero, which cannot be the case, so somewhere, something is wrong. Any idea how to find what is not OK?

As a try to locate the exact problem, I've tried to switch CH_DBG_SYSTEM_STATE_CHECK to TRUE, but in this case very first task creation (chThdCreateStatic) hits chDbgPanic("SV#4") in dbg_check_lock. Any idea why this happens? Obviously, I didn't misplaced chSysLock(), since this is legacy ChibiOS code... What is wrong with my setup?
I could provide full code for evaluation.

[Giovanni]

Hi,

Could you provide a stack trace when the system hits the "SV#4" error? that error is caused by two consecutive chSysLock() (critical zones cannot be nested). Please also verify if the error occurs always in the same place or randomly.

Another thing to check are possible stack overflows.

A minimal project could be useful for debug.

Giovanni

[alexvinchev]

Hi Giovanni,
I've been looking a lot at other ports of ChibiOS and figured out that my port was not good enough. After re-writing of IVOR4 interrupt and PIT ISR, seems that system is more stable (at least was running at CH_FREQUENCY=250kHz for ~4 days/~96hours at full CAN load without causing IVOR1). Pushing CH_FREQUENCY above mentioned frequency could not be done, system is too slow for that and I'm starting to miss PIT interrupts.

After the changes, described above I'm still hitting chDbgPanic, when I switch CH_DBG_SYSTEM_STATE_CHECK to TRUE but this time it is SV#8.
Full source code & temporary files are in the attached archive. At the attached picture, you can see something like call stack.

[Giovanni]

Hi,

SV#8 is a strange error:

Code: Select all

 *            - SV#8, misplaced @p CH_IRQ_PROLOGUE().
 *              - Not called at ISR begin.
 *              - Called from a critical zone.
 *              .


I would look into the implementation of ISR macros in the port layer.

Giovanni

[Giovanni]

Hi,

This is the e200Z implementation in 3.0.0:

Code: Select all

/*
    ChibiOS - Copyright (C) 2006..2015 Giovanni Di Sirio.

    This file is part of ChibiOS.

    ChibiOS is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 3 of the License, or
    (at your option) any later version.

    ChibiOS is distributed in the hope that 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/>.
*/

/**
 * @file    ivor.s
 * @brief   Kernel ISRs.
 *
 * @addtogroup PPC_CORE
 * @{
 */

#if !defined(FALSE) || defined(__DOXYGEN__)
#define FALSE   0
#endif

#if !defined(TRUE) || defined(__DOXYGEN__)
#define TRUE    1
#endif

/*
 * Imports the PPC configuration headers.
 */
#define _FROM_ASM_
#include "chconf.h"
#include "chcore.h"

#if !defined(__DOXYGEN__)
        /*
         * INTC registers address.
         */
        .equ  INTC_IACKR, 0xfff48010
        .equ  INTC_EOIR,  0xfff48018

        .section    .handlers, "ax"

#if PPC_SUPPORTS_DECREMENTER
        /*
         * _IVOR10 handler (Book-E decrementer).
         */
        .align      4
        .globl      _IVOR10
        .type       _IVOR10, @function
_IVOR10:
        /* Saving the external context (port_extctx structure).*/
        stwu        %sp, -80(%sp)
#if PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI
        e_stmvsrrw  8(%sp)                  /* Saves PC, MSR.               */
        e_stmvsprw  16(%sp)                 /* Saves CR, LR, CTR, XER.      */
        e_stmvgprw  32(%sp)                 /* Saves GPR0, GPR3...GPR12.    */
#else /* !(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */
        stw         %r0, 32(%sp)            /* Saves GPR0.                  */
        mfSRR0      %r0
        stw         %r0, 8(%sp)             /* Saves PC.                    */
        mfSRR1      %r0
        stw         %r0, 12(%sp)            /* Saves MSR.                   */
        mfCR        %r0
        stw         %r0, 16(%sp)            /* Saves CR.                    */
        mfLR        %r0
        stw         %r0, 20(%sp)            /* Saves LR.                    */
        mfCTR       %r0
        stw         %r0, 24(%sp)            /* Saves CTR.                   */
        mfXER       %r0
        stw         %r0, 28(%sp)            /* Saves XER.                   */
        stw         %r3, 36(%sp)            /* Saves GPR3...GPR12.          */
        stw         %r4, 40(%sp)
        stw         %r5, 44(%sp)
        stw         %r6, 48(%sp)
        stw         %r7, 52(%sp)
        stw         %r8, 56(%sp)
        stw         %r9, 60(%sp)
        stw         %r10, 64(%sp)
        stw         %r11, 68(%sp)
        stw         %r12, 72(%sp)
#endif /* !(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */

        /* Increasing the SPGR0 register.*/
        mfspr       %r0, 272
        eaddi       %r0, %r0, 1
        mtspr       272, %r0

        /* Reset DIE bit in TSR register.*/
        lis         %r3, 0x0800             /* DIS bit mask.                */
        mtspr       336, %r3                /* TSR register.                */

        /* Restoring pre-IRQ MSR register value.*/
        mfSRR1      %r0
#if !PPC_USE_IRQ_PREEMPTION
        /* No preemption, keeping EE disabled.*/
        se_bclri    %r0, 16                 /* EE = bit 16.                 */
#endif
        mtMSR       %r0

#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_enter_isr
        bl          _dbg_check_lock_from_isr
#endif
        /* System tick handler invocation.*/
        bl          chSysTimerHandlerI
#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_unlock_from_isr
        bl          _dbg_check_leave_isr
#endif

#if PPC_USE_IRQ_PREEMPTION
        /* Prevents preemption again.*/
        wrteei      0
#endif

        /* Jumps to the common IVOR epilogue code.*/
        b           _ivor_exit
#endif /* PPC_SUPPORTS_DECREMENTER */

        /*
         * _IVOR4 handler (Book-E external interrupt).
         */
        .align      4
        .globl      _IVOR4
        .type       _IVOR4, @function
_IVOR4:
        /* Saving the external context (port_extctx structure).*/
        stwu        %sp, -80(%sp)
#if PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI
        e_stmvsrrw  8(%sp)                  /* Saves PC, MSR.               */
        e_stmvsprw  16(%sp)                 /* Saves CR, LR, CTR, XER.      */
        e_stmvgprw  32(%sp)                 /* Saves GPR0, GPR3...GPR12.    */
#else /* !(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */
        stw         %r0, 32(%sp)            /* Saves GPR0.                  */
        mfSRR0      %r0
        stw         %r0, 8(%sp)             /* Saves PC.                    */
        mfSRR1      %r0
        stw         %r0, 12(%sp)            /* Saves MSR.                   */
        mfCR        %r0
        stw         %r0, 16(%sp)            /* Saves CR.                    */
        mfLR        %r0
        stw         %r0, 20(%sp)            /* Saves LR.                    */
        mfCTR       %r0
        stw         %r0, 24(%sp)            /* Saves CTR.                   */
        mfXER       %r0
        stw         %r0, 28(%sp)            /* Saves XER.                   */
        stw         %r3, 36(%sp)            /* Saves GPR3...GPR12.          */
        stw         %r4, 40(%sp)
        stw         %r5, 44(%sp)
        stw         %r6, 48(%sp)
        stw         %r7, 52(%sp)
        stw         %r8, 56(%sp)
        stw         %r9, 60(%sp)
        stw         %r10, 64(%sp)
        stw         %r11, 68(%sp)
        stw         %r12, 72(%sp)
#endif /* !(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */

        /* Increasing the SPGR0 register.*/
        mfspr       %r0, 272
        eaddi       %r0, %r0, 1
        mtspr       272, %r0

        /* Software vector address from the INTC register.*/
        lis         %r3, INTC_IACKR@h
        ori         %r3, %r3, INTC_IACKR@l  /* IACKR register address.      */
        lwz         %r3, 0(%r3)             /* IACKR register value.        */
        lwz         %r3, 0(%r3)
        mtCTR       %r3                     /* Software handler address.    */

        /* Restoring pre-IRQ MSR register value.*/
        mfSRR1      %r0
#if !PPC_USE_IRQ_PREEMPTION
        /* No preemption, keeping EE disabled.*/
        se_bclri    %r0, 16                 /* EE = bit 16.                 */
#endif
        mtMSR       %r0

        /* Exectes the software handler.*/
        bctrl

#if PPC_USE_IRQ_PREEMPTION
        /* Prevents preemption again.*/
        wrteei      0
#endif

        /* Informs the INTC that the interrupt has been served.*/
        mbar        0
        lis         %r3, INTC_EOIR@h
        ori         %r3, %r3, INTC_EOIR@l
        stw         %r3, 0(%r3)             /* Writing any value should do. */

        /* Common IVOR epilogue code, context restore.*/
        .globl      _ivor_exit
_ivor_exit:
        /* Decreasing the SPGR0 register.*/
        mfspr       %r0, 272
        eaddi       %r0, %r0, -1
        mtspr       272, %r0

#if CH_DBG_STATISTICS
        bl          _stats_start_measure_crit_thd
#endif
#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_lock
#endif
        bl          chSchIsPreemptionRequired
        cmpli       cr0, %r3, 0
        beq         cr0, .noresch
        bl          chSchDoReschedule
.noresch:
#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_unlock
#endif
#if CH_DBG_STATISTICS
        bl          _stats_stop_measure_crit_thd
#endif

        /* Restoring the external context.*/
#if PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI
        e_lmvgprw   32(%sp)                 /* Restores GPR0, GPR3...GPR12. */
        e_lmvsprw   16(%sp)                 /* Restores CR, LR, CTR, XER.   */
        e_lmvsrrw   8(%sp)                  /* Restores PC, MSR.            */
#else /*!(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */
        lwz         %r3, 36(%sp)            /* Restores GPR3...GPR12.       */
        lwz         %r4, 40(%sp)
        lwz         %r5, 44(%sp)
        lwz         %r6, 48(%sp)
        lwz         %r7, 52(%sp)
        lwz         %r8, 56(%sp)
        lwz         %r9, 60(%sp)
        lwz         %r10, 64(%sp)
        lwz         %r11, 68(%sp)
        lwz         %r12, 72(%sp)
        lwz         %r0, 8(%sp)
        mtSRR0      %r0                     /* Restores PC.                 */
        lwz         %r0, 12(%sp)
        mtSRR1      %r0                     /* Restores MSR.                */
        lwz         %r0, 16(%sp)
        mtCR        %r0                     /* Restores CR.                 */
        lwz         %r0, 20(%sp)
        mtLR        %r0                     /* Restores LR.                 */
        lwz         %r0, 24(%sp)
        mtCTR       %r0                     /* Restores CTR.                */
        lwz         %r0, 28(%sp)
        mtXER       %r0                     /* Restores XER.                */
        lwz         %r0, 32(%sp)            /* Restores GPR0.               */
#endif /* !(PPC_USE_VLE && PPC_SUPPORTS_VLE_MULTI) */
        addi        %sp, %sp, 80            /* Back to the previous frame.  */
        rfi

#endif /* !defined(__DOXYGEN__) */

/** @} */


Note this part:

Code: Select all

#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_enter_isr
        bl          _dbg_check_lock_from_isr
#endif
        /* System tick handler invocation.*/
        bl          chSysTimerHandlerI
#if CH_DBG_SYSTEM_STATE_CHECK
        bl          _dbg_check_unlock_from_isr
        bl          _dbg_check_leave_isr
#endif


If the macro CH_DBG_SYSTEM_STATE_CHECK is not "seen" by the asm module then the calls to the state checker are not compiled and this could cause that SV#8.

Giovanni

[alexvinchev]

Hi Giovanni,
this part in my code is implemented in hal_lld.c @ 227-245, function CH_IRQ_HANDLER(MPC57XX_PIT00_HANDLER).
It is proven that this code sees the CH_DBG_SYSTEM_STATE_CHECK macro. Macro itself is project defined (PPC-MPC5748G-GHS-C2.gpj@23 :: -DCH_DBG_SYSTEM_STATE_CHECK=TRUE[/b]), not by header file. This definition is seen by both C & ASM files.

Only these checks exist in IVOR4:

Code: Select all

#if CH_DBG_SYSTEM_STATE_CHECK
    e_bl        dbg_check_lock
#endif
    e_bl        chSchIsPreemptionRequired
    e_cmpli     cr0, r3, 0
    e_beq       cr0, ivor4_epilogue_core_0
    e_bl        chSchDoReschedule
    ; --- Code for ChibiOS end ---
ivor4_epilogue_core_0:
#if CH_DBG_SYSTEM_STATE_CHECK
    e_bl        dbg_check_unlock
#endif


My suspicion is that problem is caused, because ALL interrupts are handled through IVOR4 handler, even the OS tick (PIT0 / Vector226). I don't have decrementer in the MPC5748G core. What will be the effect on the OS behavior in such implementation? Is it possible to implement like this, or I should have different handling for OS tick interrupt?

Actual call chain in case of OS tick interrupt is:

Code: Select all

    PIT0ISR trigerred
        IVOR4 call begin
            Save registers to stack
            Prepare ISR call
            CH_IRQ_HANDLER(MPC57XX_PIT00_HANDLER)
                dbg_check_enter_isr();
                dbg_check_lock_from_isr();
                chSysTimerHandlerI();
                dbg_check_unlock_from_isr();
                dbg_check_leave_isr();
                dbg_check_lock();
                chSchIsPreemptionRequired();
                    ? chSchDoReschedule();
                dbg_check_unlock();
            Complete ISR call (update EOIR0)
            dbg_check_lock();
            chSchIsPreemptionRequired();
                ? chSchDoReschedule();
            dbg_check_unlock();
            Restore registers from stack
        IVOR4 call end
 

Maybe it will be good idea to remove rows 237 - 243 from hal.lld.c to avoid extra check for preemption. I.e. to have call sequence like this:

Code: Select all

    PIT0ISR trigerred
        IVOR4 call begin
            Save registers to stack
            Prepare ISR call
            CH_IRQ_HANDLER(MPC57XX_PIT00_HANDLER)
                dbg_check_enter_isr();
                dbg_check_lock_from_isr();
                chSysTimerHandlerI();
                dbg_check_unlock_from_isr();
                dbg_check_leave_isr();
            Complete ISR call (update EOIR0)
            dbg_check_lock();
            chSchIsPreemptionRequired();
                ? chSchDoReschedule();
            dbg_check_unlock();
            Restore registers from stack
        IVOR4 call end
 

[Giovanni]

Hi,

Preemption using PIT through IVOR4 is fine, it is an interrupts like all others, this is what it is done on e200z cores without decrementer.

Giovanni

[alexvinchev]

It seems I'm entering ISR with interrupts disabled, which is insane...

[Giovanni]

Careful, if you don't simulate a dedicated ISR stack it is not advisable to enable ISR preemption, that would mean that each task would have to allocate enough stack space for the whole ISR nesting (hard to estimate and hard to debug in case of overflows caused by nesting).

PA does not support an exception stack like ARM does, it would require a SW handling in the IRQ prologue/epilogue. It is something I planned to add (you can see there is already a nesting counter in IVOR4) but PA is not exactly popular so it is not high priority right now.

Giovanni

[alexvinchev]

I have found the problem.
It was in my porting implementation.
Thread start was:

Code: Select all

; @brief   Start a thread by invoking its work function.
; @details If the work function returns @p chThdExit() is automatically invoked.
.global _port_thread_start
_port_thread_start:
    wrteei      1
    mr          r3, r31                         ; Thread parameter.
    mtctr       r30
    se_bctrl                                    ; Invoke thread function.
    e_bl       chThdExit                       ; Thread termination on exit.


instead of:

Code: Select all

; @brief   Start a thread by invoking its work function.
; @details If the work function returns @p chThdExit() is automatically invoked.
.global _port_thread_start
_port_thread_start:
#if CH_DBG_SYSTEM_STATE_CHECK
    e_bl        dbg_check_unlock
#endif
    wrteei      1
    mr          r3, r31                         ; Thread parameter.
    mtctr       r30
    se_bctrl                                    ; Invoke thread function.
    e_bl       chThdExit                       ; Thread termination on exit.


Now debug works fine and I've found first real problem in my implementation (even this, simplest one) - I was mistakenly using chThdSleepS(TIME_INFINITE);, assuming that this is delay in seconds, not S-class function...

Thanks for your support Giovanni!