MCU-PLUS-SDK-AM243X: "warning: call to function could clobber interruptee's VFP registers; consider using the `interrupt_save_fp` attribute to prevent this behavior [-Wextra]"

Hi Felix,

We have assigned this to our MCU Plus SDK team, will review internally and get back to you.

Regards

Karan

Hey Tushar, 
Yes, I thought of that. But I was curious why this is not already done in the SDK, since it complains about functions that are called which are inside the SDK and we did not plan to modify those functions.
So since the sdk compiles and I've seen no complaints, I thought that maybe I am doing something wrong? We are compiling our overwriting-part as c++-files and the sdk as c-files of course, so is there a difference for this issue?

best regards

Felix

no, we are not doing that. 
As I mentioned, this warning comes up, when I do no changes to the code at all, just copy the source-file HwiP_armv7r_handlers_freertos.c into our code-base, so it will overwrite the sdk-implementation. Of cours,e it's also compiled with c++-linkage, but extern "C" does its job here. 

So with no modification of the code at all, it will complain about the call to GET_SPSR() which I mentioned in the entry post. That is a function provided by the SDK. we do no floating point operations or anything. 

I want to know if I am doing something wrong, since the compilation of the sdk does not produce those warnings, but the exact same TI-code compiled in our code-base does, even if it does not use our code-base at all. 

I don't want to modify the SDK and add this attribute to the functions GET_SPSR() and so on, since this means we will need to modify it all the time with every update. that's not our goal. 

So is there an issue in the SDK, or is it me, that misses something out here?

Hi Felix,

Felix Heil said:

Of cours,e it's also compiled with c++-linkage, but extern "C" does its job here

We have not tested our SDK with C++ linkage, All our libraries are built using C. Please refer below image.

When you remove the C++ linkage, Are you seeing the above warnings?

Also what all C++ flags have you used during build process?

Regards,

Tushar

So I tried also just copying the .c-file, so it should be available via C-linkage. But the same warnings were issued. 
The flags we use are:

-Wno-gnu-variable-sized-type-not-at-end
-mcpu=cortex-r5
-mfloat-abi=hard
-mfpu=vfpv3-d16
-Wno-error=ti-macros
-Wno-unused-function
-Wno-invalid-command-line-argument
-fno-rtti
-ffunction-sections
-fdata-sections
-mno-unaligned-access
-Werror=return-type
-Wall
-Wextra
-Og
-g
-fdiagnostics-color=always
-mlittle-endian
-Wno-c99-designator
-Wno-extern-c-compat
-Wno-c++11-narrowing
-Wno-reorder-init-list
-Wno-register
-Wno-writable-strings
-Wno-enum-compare
-Wno-reserved-user-defined-literal
-Wno-unused-const-variable
-Wno-vla-cxx-extension
-std=c++14

Can you try once after removing the below flags from the build process?

-mfloat-abi=hard
-mfpu=vfpv3-d16

After removing the above rebuild the libraries and example. Please let us know if this works.

tried it, but still those messages occur. just one example:

HwiP_armv7r_handlers_nortos.cpp:156:28: warning: call to function could clobber interruptee's VFP registers; consider using the `interrupt_save_fp` attribute to prevent this behavior [-Wextra]
156 |         uint32_t isPulse = HwiP_isPulse(intNum);
HwiP_armv7r_vim.h:143:37: note: 'HwiP_isPulse' declared here
143 | static inline uint32_t HWI_SECTION HwiP_isPulse(uint32_t intNum) 

Can you please provide the steps to replicate the same behavior on our end?

The current solution is to use the attribute "interrupt_save_fp" to function definition to avoid the warnings.

Sorry for the late reply.

So I tried it by just adding a .cpp-file with the redefinitions to the hello_worls-example and ti gives me exactly those issues:

File is added here:

content of the file:

/*
 *  Copyright (C) 2018-2023 Texas Instruments Incorporated
 *
 *  Redistribution and use in source and binary forms, with or without
 *  modification, are permitted provided that the following conditions
 *  are met:
 *
 *    Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 *
 *    Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the
 *    distribution.
 *
 *    Neither the name of Texas Instruments Incorporated nor the names of
 *    its contributors may be used to endorse or promote products derived
 *    from this software without specific prior written permission.
 *
 *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
 *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
 *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
 *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 *  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
#include <kernel/dpl/HwiP.h>
#include <kernel/nortos/dpl/r5/HwiP_armv7r_vim.h>
#include <drivers/hw_include/csl_types.h>
#include <kernel/dpl/DebugP.h>
#ifdef __cplusplus
extern "C" {
#endif
void __attribute__((interrupt("SWI"), section(".text.hwi"))) HwiP_svc_handler(void);
void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_data_abort_handler_c(volatile uint32_t var);
void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_prefetch_abort_handler_c(volatile uint32_t var);
void __attribute__((interrupt("UNDEF"), section(".text.hwi"),weak)) HwiP_undefined_handler_c(volatile uint32_t var);
void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_user_data_abort_handler_c(DFSR dfsr,ADFSR adfsr,volatile uint32_t DFAR,volatile uint32_t ADDRESS,volatile uint32_t SPSR);
void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_user_prefetch_abort_handler_c(IFSR ifsr,AIFSR aifsr,volatile uint32_t IFAR,volatile uint32_t ADDRESS,volatile uint32_t SPSR);
void __attribute__((interrupt("UNDEF"), section(".text.hwi"),weak)) HwiP_user_undefined_handler_c(volatile uint32_t ADDRESS,volatile uint32_t SPSR);

volatile uint32_t GET_DFSR(void);
volatile uint32_t GET_ADFSR(void);
volatile uint32_t GET_DFAR(void);
volatile uint32_t GET_IFSR(void);
volatile uint32_t GET_AIFSR(void);
volatile uint32_t GET_IFAR(void);
volatile uint32_t GET_SPSR(void);
volatile uint32_t GET_LR(void);

#ifdef __cplusplus
}
#endif

/* compile flag to enable or disable interrupt nesting */
#define HWIP_NESTED_INTERRUPTS_IRQ_ENABLE

/* Save FPU context, used in FIQ Handler */
static inline  void Hwip_save_fpu_context(void)
{
    __asm__ __volatile__ ( "FMRX  R0, FPSCR"  "\n\t": : : "memory");
    __asm__ __volatile__ ( "VPUSH {D0-D15}"  "\n\t": : : "memory");
    __asm__ __volatile__ ( "PUSH  {R0}"  "\n\t": : : "memory");
}

/* Restore FPU context, used in FIQ Handler */
static inline  void Hwip_restore_fpu_context(void)
{
    __asm__ __volatile__ ( "POP   {R0}"  "\n\t": : : "memory");
    __asm__ __volatile__ ( "VPOP  {D0-D15}"  "\n\t": : : "memory");
    __asm__ __volatile__ ( "VMSR  FPSCR, R0"  "\n\t": : : "memory");
}


/* IRQ handler starts execution in HwiP_irq_handler, defined in
HwiP_armv7r_handlers_nortos_asm.S
 * After some initial assembly logic it then branches to this function.
 * After exiting this function it does some more assembly before exiting
 */
void __attribute__((section(".text.hwi"))) HwiP_irq_handler_c(void)
{
    int32_t status;
    uint32_t intNum;

    #ifndef HWIP_VIM_VIC_ENABLE

    /* Read to force prioritization logic to take effect, in non-VIC mode */
    HwiP_getIRQVecAddr();
    #endif

    status = HwiP_getIRQ(&intNum);
    if(status==SystemP_SUCCESS)
    {
        uint32_t isPulse = HwiP_isPulse(intNum);
        HwiP_FxnCallback isr;
        void *args;

        if(isPulse!=0U)
        {
            HwiP_clearInt(intNum);
        }

        isr = gHwiCtrl.isr[intNum];
        args = gHwiCtrl.isrArgs[intNum];

        #ifdef HWIP_NESTED_INTERRUPTS_IRQ_ENABLE
        /* allow nesting of interrupts */
        HwiP_enable();
        #endif

        if(isr!=NULL)
        {
            isr(args);
        }

        /* disallow nesting of interrupts */
        (void) HwiP_disable();

        if(isPulse==0U)
        {
            HwiP_clearInt(intNum);
        }
        HwiP_ackIRQ(intNum);
    }
    else
    {
        /* spurious interrupt */
        gHwiCtrl.spuriousIRQCount++;
        HwiP_ackIRQ(0);
    }
}

void __attribute__((interrupt("FIQ"), section(".text.hwi"))) HwiP_fiq_handler(void)
{
    int32_t status;
    uint32_t intNum;
    volatile uint32_t dummy;

    #ifdef EN_SAVE_RESTORE_FPU_CONTEXT
    Hwip_save_fpu_context();
    #endif

    /* Read to force prioritization logic to take effect */
    dummy = HwiP_getFIQVecAddr();
    (void)dummy;

    status = HwiP_getFIQ(&intNum);
    if(status==SystemP_SUCCESS)
    {
        uint32_t isPulse = HwiP_isPulse(intNum);
        HwiP_FxnCallback isr;
        void *args;

        if(isPulse!=0U)
        {
            HwiP_clearInt(intNum);
        }

        isr = gHwiCtrl.isr[intNum];
        args = gHwiCtrl.isrArgs[intNum];

        #if 0   /* FIQ interrupt nesting not supported */
        /* allow nesting of interrupts */
        HwiP_enableFIQ();
        #endif

        if(isr!=NULL)
        {
            isr(args);
        }

        /* disallow nesting of interrupts */
        (void) HwiP_disableFIQ();

        if(isPulse==0U)
        {
            HwiP_clearInt(intNum);
        }
        HwiP_ackFIQ(intNum);
    }
    else
    {
        /* spurious interrupt */
        gHwiCtrl.spuriousFIQCount++;
        HwiP_ackFIQ(0);
    }

    #ifdef EN_SAVE_RESTORE_FPU_CONTEXT
    Hwip_restore_fpu_context();
    #endif
}

void __attribute__((interrupt("UNDEF"), section(".text.hwi"))) HwiP_reserved_handler(void)
{
    volatile uint32_t loop = 1;
    while(loop!=0U)
    {
        ;
    }
}

void __attribute__((interrupt("UNDEF"), section(".text.hwi"))) HwiP_user_undefined_handler_c(volatile uint32_t ADDRESS,volatile uint32_t SPSR){

    volatile uint32_t loop = 1;
    while(loop != 0U){ ; }
}

void __attribute__((interrupt("UNDEF"), section(".text.hwi"))) HwiP_undefined_handler_c(volatile uint32_t SP)
{

    typedef struct {
        volatile uint32_t SPSR;
        /* DFSR register */
        volatile uint32_t ADDRESS;
        /* Instruction causing the exception*/
    }UNDEF_REG;

    UNDEF_REG abort_regs;
    abort_regs.SPSR=GET_SPSR();
    abort_regs.ADDRESS=*(&(SP)+10);

    HwiP_user_undefined_handler_c(abort_regs.ADDRESS,abort_regs.SPSR);

}

void __attribute__((interrupt("SWI"), section(".text.hwi"))) HwiP_svc_handler(void)
{
    volatile uint32_t loop = 1;
    while(loop!=0U)
    {
        ;
    }

}

void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_user_prefetch_abort_handler_c(IFSR ifsr,AIFSR aifsr,volatile uint32_t IFAR,volatile uint32_t ADDRESS,volatile uint32_t SPSR){

    volatile uint32_t loop = 1;
    while(loop != 0U){ ; }
}

void __attribute__((interrupt("ABORT"), section(".text.hwi"))) HwiP_prefetch_abort_handler_c(volatile uint32_t SP)
{

    typedef struct {
        volatile uint32_t IFSR;
        /* IFSR register */
        volatile uint32_t AIFSR;
        /* AIFSR register */
        volatile uint32_t IFAR;
        /* IFAR register */
        volatile uint32_t ADDRESS;
        /* Instruction causing the exception*/
        volatile uint32_t SPSR;
        /* SPSR register*/
    }PREFETCH_ABORT_REG;

    PREFETCH_ABORT_REG abort_regs;

    /*Extract register values through functions coded in ASM*/
    abort_regs.AIFSR=GET_AIFSR();
    abort_regs.IFAR=GET_IFAR();
    abort_regs.IFSR=GET_IFSR();
    abort_regs.ADDRESS=*(&(SP)+14);
    abort_regs.SPSR=GET_SPSR();

    /*Extract contents of IFSR register
    1. status: indicates the type of fault generated
    2. sd: distinguishes between an AXI Decode or Slave error on an external abort.
    This bit is only valid for external aborts. For all other aborts types of abort,
    this bit is set to zero*/
    IFSR ifsr;
    ifsr.status=(abort_regs.IFSR & 0xF) |((abort_regs.IFSR>>10 & 0x1)<<4);
    ifsr.sd=(abort_regs.IFSR>>12) & 0x1;

    /*Extract contents of AIFSR register
    1. index: returns the index value for the access giving the error
    2. side_ext: value returned in this field indicates the source of the error
    3. recoverable_error:  value returned in this field indicates if the error is recoverable
        (0=Unrecoverable error, 1=Recoverable Error)
    4. cacheway: value returned in this field indicates the cache way or ways in which the error occurred*/
    AIFSR aifsr;
    aifsr.index=(abort_regs.AIFSR>>5) & 0x1FF;
    aifsr.side_ext=((abort_regs.AIFSR>>22) & 0x3) | ((abort_regs.AIFSR>>20 & 0x1)<<2);
    aifsr.recoverable_error=(abort_regs.AIFSR>>21) & 0x1;
    aifsr.cacheway=(abort_regs.AIFSR>>24) & 0xF;

    HwiP_user_prefetch_abort_handler_c(ifsr,aifsr,abort_regs.IFAR,abort_regs.ADDRESS,abort_regs.SPSR);

}

void __attribute__((interrupt("ABORT"), section(".text.hwi"),weak)) HwiP_user_data_abort_handler_c(DFSR dfsr,ADFSR adfsr,volatile uint32_t DFAR,volatile uint32_t ADDRESS,volatile uint32_t SPSR){

    /* in debug builds we will break to identify the source of the abort */
    volatile uint32_t loop = 1;
    while(loop != 0U){ ; }
}

void __attribute__((interrupt("ABORT"), section(".text.hwi"))) HwiP_data_abort_handler_c(volatile uint32_t SP)
{

    typedef struct {
        volatile uint32_t DFSR;
        /* DFSR register */
        volatile uint32_t ADFSR;
        /* ADFSR register */
        volatile uint32_t DFAR;
        /* DFAR register */
        volatile uint32_t ADDRESS;
        /* Instruction causing the exception*/
        volatile uint32_t SPSR;
        /* SPSR register*/
    }DATA_ABORT_REG;

    /*Extract register values through functions coded in ASM*/
    DATA_ABORT_REG abort_regs;
    abort_regs.ADFSR=GET_ADFSR();
    abort_regs.DFAR=GET_DFAR();
    abort_regs.DFSR=GET_DFSR();
    abort_regs.ADDRESS=*(&(SP)+15);
    abort_regs.SPSR=GET_SPSR();

    /*Extract contents of DFSR register
    1. status: indicates the type of fault generated
    2. sd: distinguishes between an AXI Decode or Slave error on an external abort.
    This bit is only valid for external aborts. For all other aborts types of abort,
    this bit is set to zero
    3. rw:  Indicates whether a read or write access caused an abort
        (0=read abort; 1=write abort)*/
    DFSR dfsr;
    dfsr.status=(abort_regs.DFSR & 0xF) |((abort_regs.DFSR>>10 & 0x1)<<4);
    dfsr.rw=(abort_regs.DFSR>>11) & 0x1;
    dfsr.sd=(abort_regs.DFSR>>12) & 0x1;

    /*Extract contents of ADFSR register
    1. index: returns the index value for the access giving the error
    2. side_ext: value returned in this field indicates the source of the error
    3. recoverable_error:  value returned in this field indicates if the error is recoverable
        (0=Unrecoverable error, 1=Recoverable Error)
    4. cacheway: value returned in this field indicates the cache way or ways in which the error occurred*/
    ADFSR adfsr;
    adfsr.index=(abort_regs.ADFSR>>5) & 0x1FF;
    adfsr.side_ext=((abort_regs.ADFSR>>22) & 0x3) | ((abort_regs.ADFSR>>20 & 0x1)<<2);
    adfsr.recoverable_error=(abort_regs.ADFSR>>21) & 0x1;
    adfsr.cacheway=(abort_regs.ADFSR>>24) & 0xF;

    HwiP_user_data_abort_handler_c(dfsr,adfsr,abort_regs.DFAR,abort_regs.ADDRESS,abort_regs.SPSR);

}

Best regards, 
Felix

Hi Felix,

Apologies for the delay here. I am able to replicate the issue with the method suggested above.

I checked the project and SDK, all the compiler options and flags are identical for both project and SDK but the issue still persist.

I am still debugging, the current workaround is already mentioned  here.  

Regards,

Tushar