Re: [PATCH] Fix exception handler for supporting FPU
Could you please open a ticket on our trac to describe the problem this fixes, and add "closes #." to your patch commit message? Thanks, Gedare On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan wrote: > Patch attached here for ARMv7M Exception Handler. Looks like git send-email > didn't deliver the mail. Something is not quite right with our mail server > here. Avoid this email if patch delivered through git. > > Thanks and Regards, > > Sudarshan > > > > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: obsolete m32r port
Plesae check all obsoletes through users ml. On Thu, Aug 27, 2015 at 5:54 PM, Joel Sherrill wrote: > Hi > > Another one on my chopping block. I don't see the official > not recommended for new designs language but I also don't > seen much hinting an update in five years either. > > http://am.renesas.com/products/mpumcu/m32r/index.jsp > > Anyone got any thoughts about it being a part that is still > being sold and we should support it? > > Again, mark it as deprecated in the 4.11 notes and remove it > on master if no one objects. > > -- > Joel Sherrill, Ph.D. Director of Research & Development > joel.sherr...@oarcorp.comOn-Line Applications Research > Ask me about RTEMS: a free RTOS Huntsville AL 35805 > Support Available(256) 722-9985 > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: cppcheck errors
On Thu, Aug 27, 2015 at 6:38 PM, Joel Sherrill
wrote:
>
>
> On 8/27/2015 4:15 PM, Martin Galvan wrote:
>>
>> On Thu, Aug 27, 2015 at 6:10 PM, Daniel Gutson
>> wrote:
>>>
>>> Maybe we can just provide the list until we provide the fixes? Martín?
>>
>>
>> Gladly. Keep in mind we ran cppcheck only on the modules we use
>> (though some things may've slipped in, like nios):
>
>
> Most of these are worth looking at. Honestly, the ones on the tcp/ip
> stack and other directly imported code aren't going to get touched.
>
>> [c/src/lib/libbsp/shared/umon/umon.h:21]: (error) Invalid number of
>> character ({) when these macros are defined: '__cplusplus'.
>
>
> Spot check shows this is missing the closing magic for extern "C".
> I see a couple more of these below and we should fix though.
>
>> [cpukit/libmisc/dumpbuf/dumpbuf.c:69]: (error) Undefined behavior:
>> Variable 'line_buffer' is used as parameter and destination in
>> s[n]printf().
>> [cpukit/libmisc/dumpbuf/dumpbuf.c:76]: (error) Undefined behavior:
>> Variable 'line_buffer' is used as parameter and destination in
>> s[n]printf().
>
>
> This should be looked at.
>
>> [cpukit/libmisc/stackchk/check.c:285] ->
>> [cpukit/libmisc/stackchk/check.c:294]: (performance) Variable
>> 'pattern_ok' is reassigned a value before the old one has been used.
>
>
> This should be looked at.
>
>> [cpukit/libmisc/stackchk/check.c:255]: (portability) 'pattern_area' is
>> of type 'void *'. When using void pointers in calculations, the
>> behaviour is undefined.
>
>
> Not sure what to do about this. I think this is defined behavior
> for at least GCC.
This is because + and - operators are not defined for void*, since
addr(pointer + k) = addr(pointer) + k * sizeof(*pointer),
but since pointer is void*, sizeof(void) is not defined, that's why it
is non-standard. However, gcc extensions
considers this case as char*, which is exactly the way to make this
portable: just cast it to char* before doing the arithmetics.
>
>> [cpukit/libnetworking/kern/kern_subr.c:93]: (portability) 'cp' is of
>> type 'void *'. When using void pointers in calculations, the behaviour
>> is undefined.
>> [cpukit/libnetworking/kern/uipc_socket2.c:616]: (error) Uninitialized
>> variable: o
>
>
> Imported code.
At the end it goes to the same binary, no matter where it came from,
so IMHO it may be worth
to send a patch to where this comes from.
>
>> [cpukit/libnetworking/lib/ftpfs.c:704] ->
>> [cpukit/libnetworking/lib/ftpfs.c:709]: (performance) Variable
>> 'port_socket' is reassigned a value before the old one has been used.
>> [cpukit/libnetworking/lib/tftpDriver.c:503]: (error) Common realloc
>> mistake: 'current' nulled but not freed upon failure
>
>
> Need to be looked at.
>
>> [cpukit/libnetworking/libc/ether_addr.c:72]: (portability) scanf
>> without field width limits can crash with huge input data on some
>> versions of libc.
>> [cpukit/libnetworking/libc/ether_addr.c:94]: (portability) scanf
>> without field width limits can crash with huge input data on some
>> versions of libc.
>> [cpukit/libnetworking/libc/gethostbyht.c:233]: (error) Common realloc
>> mistake: 'hostmap' nulled but not freed upon failure
>> [cpukit/libnetworking/libc/ns_addr.c:112]: (portability) scanf without
>> field width limits can crash with huge input data on some versions of
>> libc.
>> [cpukit/libnetworking/libc/ns_addr.c:120]: (portability) scanf without
>> field width limits can crash with huge input data on some versions of
>> libc.
>> [cpukit/libnetworking/libc/ns_addr.c:128]: (portability) scanf without
>> field width limits can crash with huge input data on some versions of
>> libc.
>> [cpukit/libnetworking/libc/ns_addr.c:137]: (portability) scanf without
>> field width limits can crash with huge input data on some versions of
>> libc.
>
>
> All imported code. maybe the realloc() is worth addressing.
>
>> [cpukit/libnetworking/rtems/rtems_dhcp.c:401]: (error) Common realloc
>> mistake: 'dhcp_hostname' nulled but not freed upon failure
>
>
> This is the only one in our code.
>
>> [cpukit/librpc/src/rpc/netnamer.c:331]: (error) Resource leak: fd
>
>
> Probably should be looked at.
>
>> [cpukit/posix/include/rtems/posix/ptimer.h:33]: (error) Invalid number
>> of character ({) when these macros are defined: '__cplusplus'.
>> [cpukit/rtems/include/rtems/rtems/dpmemimpl.h:116]: (error) Invalid
>> number of character ({) when these macros are defined: '__cplusplus'.
>
>
> Easy.
>
>> [cpukit/score/cpu/h8300/cpu.c:54]: (error) Uninitialized variable:
>> _ccr (si no se inicializa, se hace un #warning pero igual existe el
>> problema)
>
>
> Appears to be confused by conditional or inline asm.
>
>> [cpukit/zlib/gzwrite.c:80]: (error) Uninitialized variable: got
>
>
> Hmm.. but third party code.
>
>> [tools/build/binpatch.c:104]: (error) Resource leak: ifp
>> [tools/build/binpatch.c:63]: (error) Uninitialized variable: off
>> [tools/build/unhex.c:238]: (error) Resource leak: outfp
>
>
> Need to be looked at but these are h
Re: [PATCH] Fix exception handler for supporting FPU
On Fri, Aug 28, 2015 at 12:20 PM, Gedare Bloom wrote: > Could you please open a ticket on our trac to describe the problem > this fixes, and add "closes #." to your patch commit message? Additionally, please clarify which architecture this applies to. I suspect this is for cortex-m4. In any case, please clarify which architectures you tested on. We can analyze and test cortex-m3 if you don't have one. > > Thanks, > Gedare > > On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan > wrote: >> Patch attached here for ARMv7M Exception Handler. Looks like git send-email >> didn't deliver the mail. Something is not quite right with our mail server >> here. Avoid this email if patch delivered through git. >> >> Thanks and Regards, >> >> Sudarshan >> >> >> >> >> ___ >> devel mailing list >> devel@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel -- Daniel F. Gutson Chief Engineering Officer, SPD San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype:dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: ARMv7M Exception Handler
On 2015-08-27 17:06, Joel Sherrill wrote: On 8/27/2015 3:58 PM, Daniel Gutson wrote: On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan wrote: Hey guys, I was working on the exception handler for the CPU hard fault. Managed to register the fatal error user extension to RTEMS and call the user defined function when an exception occurs. But the pointer to the stacked frame didn't have the correct register values(esp. the PC register). So I looked into the assembly code in /cpukit/score/cpu/arm/armv7m-exception-default.c, where it decides which stack pointer was used (MSP or PSP) before the exception occurred depending on the error code returned in the Link Register. After carefully examining all the assembly instructions, I guess theres a little bug in the code. The instruction "cmn r2, #3\n" in line 31 basically compares the Link Register(lr) to value 0xFFFD (negative #3, because CMN negates the RHS and compares with LHS) and chooses MSP or PSP in the following IT block. This is pretty cool! But, it will not work if you have the floating-point unit (FPU) enabled in your processor, which is enabled in mine. With FPU enabled, the error code returned is either 0xFFE9 or 0xFFED, for which the above assembly instruction will not work out and MSP will be selected always. Better way to do is to check the 2nd bit of the error code to determine which stack pointer was used before the exception happened - "tst lr, #4\n" and change the IT block from "itt ne" to "itt eq" and the "mov" and "add" within this IT block. Have tested this with the above changes and it works. I have sent the patch "0001-Fix-exception-handler-for-supporting-FPU.patch" to the devel mailing list that fixes this problem. :) Nice. Have you tested this without FPU support too? Hi Daniel, Just tested the fix with FPU disabled in my processor and it works fine. This patch can be pushed! Daniel .. when you are happy, we can push it. Should this go on the 4.11 branch and master? If so, it needs a ticket. Joel, I think this fix is vital and should be pushed to master. Will make a ticket for this. Thanks and Regards, Sudarshan -- Sudarshan Rajagopalan sudarshan.rajagopalan at vecna.com www.vecna.com Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: ARMv7M Exception Handler
On Fri, Aug 28, 2015 at 12:33 PM, sudarshan.rajagopalan wrote: > On 2015-08-27 17:06, Joel Sherrill wrote: >> >> On 8/27/2015 3:58 PM, Daniel Gutson wrote: >>> >>> On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan >>> wrote: Hey guys, I was working on the exception handler for the CPU hard fault. Managed to register the fatal error user extension to RTEMS and call the user defined function when an exception occurs. But the pointer to the stacked frame didn't have the correct register values(esp. the PC register). So I looked into the assembly code in /cpukit/score/cpu/arm/armv7m-exception-default.c, where it decides which stack pointer was used (MSP or PSP) before the exception occurred depending on the error code returned in the Link Register. After carefully examining all the assembly instructions, I guess theres a little bug in the code. The instruction "cmn r2, #3\n" in line 31 basically compares the Link Register(lr) to value 0xFFFD (negative #3, because CMN negates the RHS and compares with LHS) and chooses MSP or PSP in the following IT block. This is pretty cool! But, it will not work if you have the floating-point unit (FPU) enabled in your processor, which is enabled in mine. With FPU enabled, the error code returned is either 0xFFE9 or 0xFFED, for which the above assembly instruction will not work out and MSP will be selected always. Better way to do is to check the 2nd bit of the error code to determine which stack pointer was used before the exception happened - "tst lr, #4\n" and change the IT block from "itt ne" to "itt eq" and the "mov" and "add" within this IT block. Have tested this with the above changes and it works. I have sent the patch "0001-Fix-exception-handler-for-supporting-FPU.patch" to the devel mailing list that fixes this problem. :) >>> >>> >>> Nice. Have you tested this without FPU support too? > > > Hi Daniel, > > Just tested the fix with FPU disabled in my processor and it works fine. > This patch can be pushed! As I mentioned before, please mention what architecture you tested (what use cases) and which you didn't. > >> Daniel .. when you are happy, we can push it. >> >> Should this go on the 4.11 branch and master? If so, it needs a ticket. > > > Joel, I think this fix is vital and should be pushed to master. Will make a > ticket for this. > > Thanks and Regards, > > Sudarshan > -- > Sudarshan Rajagopalan > sudarshan.rajagopalan at vecna.com > www.vecna.com > >> Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel >>> >>> >>> >>> > -- Daniel F. Gutson Chief Engineering Officer, SPD San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype:dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On 2015-08-28 11:30, Daniel Gutson wrote: On Fri, Aug 28, 2015 at 12:20 PM, Gedare Bloom wrote: Could you please open a ticket on our trac to describe the problem this fixes, and add "closes #." to your patch commit message? Hi Gedare, Sure will do! Additionally, please clarify which architecture this applies to. I suspect this is for cortex-m4. In any case, please clarify which architectures you tested on. We can analyze and test cortex-m3 if you don't have one. Daniel, I've tested it on Cortex-M4 but this patch should also apply to Cortex-M3 or all ARMv7M based processor (including M7). But please feel free to test it on Cortex-M3 since I don't have it with me now. Am also attaching the links to ARM Cortex-M3, M4 and M7 Exception entry and return referenece documents for a quick reference, where the table gives all the possible exception error codes returned and which SP to pick depending on the error code. The only tricky part would be error code: 0xFFE1. But the asm "tst.w lr, #4" with lr=0xFFE1 will result into zero and will set the Zero flag, which will make the ITT EQ block execute, which will choose MSP, which it should for the return code 0xFFE1 (or even 0xFFF1) in all M3, M4 and M7. But please feel free to test it on real M3 an M7 processor. I have tested it by manually giving all the return error codes for M3 and M7 to LR and and it chooses the right SP value. ARM Reference Docs - Exception Entry and Return: C-3: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Babefdjc.html C-4: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0553a/Babefdjc.html C-7: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0646a/Babefdjc.html Thanks and Regards, Sudarshan Thanks, Gedare On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan wrote: Patch attached here for ARMv7M Exception Handler. Looks like git send-email didn't deliver the mail. Something is not quite right with our mail server here. Avoid this email if patch delivered through git. Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: ARMv7M Exception Handler
On 2015-08-28 11:37, Daniel Gutson wrote: On Fri, Aug 28, 2015 at 12:33 PM, sudarshan.rajagopalan wrote: On 2015-08-27 17:06, Joel Sherrill wrote: On 8/27/2015 3:58 PM, Daniel Gutson wrote: On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan wrote: Hey guys, I was working on the exception handler for the CPU hard fault. Managed to register the fatal error user extension to RTEMS and call the user defined function when an exception occurs. But the pointer to the stacked frame didn't have the correct register values(esp. the PC register). So I looked into the assembly code in /cpukit/score/cpu/arm/armv7m-exception-default.c, where it decides which stack pointer was used (MSP or PSP) before the exception occurred depending on the error code returned in the Link Register. After carefully examining all the assembly instructions, I guess theres a little bug in the code. The instruction "cmn r2, #3\n" in line 31 basically compares the Link Register(lr) to value 0xFFFD (negative #3, because CMN negates the RHS and compares with LHS) and chooses MSP or PSP in the following IT block. This is pretty cool! But, it will not work if you have the floating-point unit (FPU) enabled in your processor, which is enabled in mine. With FPU enabled, the error code returned is either 0xFFE9 or 0xFFED, for which the above assembly instruction will not work out and MSP will be selected always. Better way to do is to check the 2nd bit of the error code to determine which stack pointer was used before the exception happened - "tst lr, #4\n" and change the IT block from "itt ne" to "itt eq" and the "mov" and "add" within this IT block. Have tested this with the above changes and it works. I have sent the patch "0001-Fix-exception-handler-for-supporting-FPU.patch" to the devel mailing list that fixes this problem. :) Nice. Have you tested this without FPU support too? Hi Daniel, Just tested the fix with FPU disabled in my processor and it works fine. This patch can be pushed! As I mentioned before, please mention what architecture you tested (what use cases) and which you didn't. Hi Daniel, I have replied to the same question in the thread containing the patch. Daniel .. when you are happy, we can push it. Should this go on the 4.11 branch and master? If so, it needs a ticket. Joel, I think this fix is vital and should be pushed to master. Will make a ticket for this. Thanks and Regards, Sudarshan -- Sudarshan Rajagopalan sudarshan.rajagopalan at vecna.com www.vecna.com Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On 2015-08-28 12:18, sudarshan.rajagopalan wrote: On 2015-08-28 11:30, Daniel Gutson wrote: On Fri, Aug 28, 2015 at 12:20 PM, Gedare Bloom wrote: Could you please open a ticket on our trac to describe the problem this fixes, and add "closes #." to your patch commit message? Hi Gedare, Sure will do! Additionally, please clarify which architecture this applies to. I suspect this is for cortex-m4. In any case, please clarify which architectures you tested on. We can analyze and test cortex-m3 if you don't have one. Daniel, I've tested it on Cortex-M4 but this patch should also apply to Cortex-M3 or all ARMv7M based processor (including M7). But please feel free to test it on Cortex-M3 since I don't have it with me now. Am also attaching the links to ARM Cortex-M3, M4 and M7 Exception entry and return referenece documents for a quick reference, where the table gives all the possible exception error codes returned and which SP to pick depending on the error code. The only tricky part would be error code: 0xFFE1. But the asm "tst.w lr, #4" with lr=0xFFE1 will result into zero and will set the Zero flag, which will make the ITT EQ block execute, which will choose MSP, which it should for the return code 0xFFE1 (or even 0xFFF1) in all M3, M4 and M7. But please feel free to test it on real M3 an M7 processor. I have tested it by manually giving all the return error codes for M3 and M7 to LR and and it chooses the right SP value. ARM Reference Docs - Exception Entry and Return: C-3: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Babefdjc.html C-4: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0553a/Babefdjc.html C-7: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0646a/Babefdjc.html Thanks and Regards, Sudarshan Ok, so we tested the fix on Cortex-M7 processor. The error codes for C-4 and C-7 are all pretty much the same. This leaves with C-3, which I believe should work too, becasue non-FPU error codes are same for C-3, C-4 and C-7. Thanks, Sudarshan Thanks, Gedare On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan wrote: Patch attached here for ARMv7M Exception Handler. Looks like git send-email didn't deliver the mail. Something is not quite right with our mail server here. Avoid this email if patch delivered through git. Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On Fri, Aug 28, 2015 at 2:31 PM, sudarshan.rajagopalan wrote: > On 2015-08-28 12:18, sudarshan.rajagopalan wrote: >> >> On 2015-08-28 11:30, Daniel Gutson wrote: >>> >>> On Fri, Aug 28, 2015 at 12:20 PM, Gedare Bloom wrote: Could you please open a ticket on our trac to describe the problem this fixes, and add "closes #." to your patch commit message? >> >> >> Hi Gedare, Sure will do! >> >>> >>> Additionally, please clarify which architecture this applies to. I >>> suspect this is for cortex-m4. >>> In any case, please clarify which architectures you tested on. We can >>> analyze and test cortex-m3 if you don't have one. >> >> >> Daniel, I've tested it on Cortex-M4 but this patch should also apply >> to Cortex-M3 or all ARMv7M based processor (including M7). But please >> feel free to test it on Cortex-M3 since I don't have it with me now. >> >> Am also attaching the links to ARM Cortex-M3, M4 and M7 Exception >> entry and return referenece documents for a quick reference, where the >> table gives all the possible exception error codes returned and which >> SP to pick depending on the error code. The only tricky part would be >> error code: 0xFFE1. But the asm "tst.w lr, #4" with lr=0xFFE1 >> will result into zero and will set the Zero flag, which will make the >> ITT EQ block execute, which will choose MSP, which it should for the >> return code 0xFFE1 (or even 0xFFF1) in all M3, M4 and M7. But >> please feel free to test it on real M3 an M7 processor. I have tested >> it by manually giving all the return error codes for M3 and M7 to LR >> and and it chooses the right SP value. >> >> ARM Reference Docs - Exception Entry and Return: >> C-3: >> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Babefdjc.html >> C-4: >> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0553a/Babefdjc.html >> C-7: >> >> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0646a/Babefdjc.html >> >> Thanks and Regards, >> Sudarshan > > > > Ok, so we tested the fix on Cortex-M7 processor. The error codes for C-4 and > C-7 are all pretty much the same. This leaves with C-3, which I believe > should work too, becasue non-FPU error codes are same for C-3, C-4 and C-7. Hi Sudarshan, ok thanks. We will comment on Monday since we have a release today. Good job. Daniel. > > Thanks, > > Sudarshan >> >> >>> Thanks, Gedare On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan wrote: > > Patch attached here for ARMv7M Exception Handler. Looks like git > send-email > didn't deliver the mail. Something is not quite right with our mail > server > here. Avoid this email if patch delivered through git. > > Thanks and Regards, > > Sudarshan > > > > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel >> >> >> ___ >> devel mailing list >> devel@rtems.org >> http://lists.rtems.org/mailman/listinfo/devel > > -- Daniel F. Gutson Chief Engineering Officer, SPD San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype:dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On 2015-08-28 13:44, Daniel Gutson wrote: On Fri, Aug 28, 2015 at 2:31 PM, sudarshan.rajagopalan wrote: On 2015-08-28 12:18, sudarshan.rajagopalan wrote: On 2015-08-28 11:30, Daniel Gutson wrote: On Fri, Aug 28, 2015 at 12:20 PM, Gedare Bloom wrote: Could you please open a ticket on our trac to describe the problem this fixes, and add "closes #." to your patch commit message? Hi Gedare, Sure will do! Additionally, please clarify which architecture this applies to. I suspect this is for cortex-m4. In any case, please clarify which architectures you tested on. We can analyze and test cortex-m3 if you don't have one. Daniel, I've tested it on Cortex-M4 but this patch should also apply to Cortex-M3 or all ARMv7M based processor (including M7). But please feel free to test it on Cortex-M3 since I don't have it with me now. Am also attaching the links to ARM Cortex-M3, M4 and M7 Exception entry and return referenece documents for a quick reference, where the table gives all the possible exception error codes returned and which SP to pick depending on the error code. The only tricky part would be error code: 0xFFE1. But the asm "tst.w lr, #4" with lr=0xFFE1 will result into zero and will set the Zero flag, which will make the ITT EQ block execute, which will choose MSP, which it should for the return code 0xFFE1 (or even 0xFFF1) in all M3, M4 and M7. But please feel free to test it on real M3 an M7 processor. I have tested it by manually giving all the return error codes for M3 and M7 to LR and and it chooses the right SP value. ARM Reference Docs - Exception Entry and Return: C-3: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0552a/Babefdjc.html C-4: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0553a/Babefdjc.html C-7: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0646a/Babefdjc.html Thanks and Regards, Sudarshan Ok, so we tested the fix on Cortex-M7 processor. The error codes for C-4 and C-7 are all pretty much the same. This leaves with C-3, which I believe should work too, becasue non-FPU error codes are same for C-3, C-4 and C-7. Hi Sudarshan, ok thanks. We will comment on Monday since we have a release today. Good job. Daniel. Sure, Daniel. This requires diving into the ARM architecture a little bit. Will catch up on Monday. Thanks, Sudarshan Thanks, Sudarshan Thanks, Gedare On Thu, Aug 27, 2015 at 4:33 PM, sudarshan.rajagopalan wrote: Patch attached here for ARMv7M Exception Handler. Looks like git send-email didn't deliver the mail. Something is not quite right with our mail server here. Avoid this email if patch delivered through git. Thanks and Regards, Sudarshan ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan
wrote:
> The instruction "cmn r2, #3\n" in line 31 basically compares the Link
> Register(lr) to value 0xFFFD (negative #3, because CMN negates the RHS
> and compares with LHS) and chooses MSP or PSP in the following IT block.
> This is pretty cool! But, it will not work if you have the floating-point
> unit (FPU) enabled in your processor, which is enabled in mine. With FPU
> enabled, the error code returned is either 0xFFE9 or 0xFFED, for
> which the above assembly instruction will not work out and MSP will be
> selected always.
>
> Better way to do is to check the 2nd bit of the error code to determine
> which stack pointer was used before the exception happened - "tst lr, #4\n"
> and change the IT block from "itt ne" to "itt eq" and the "mov" and "add"
> within this IT block.
Hi Sudarshan! First of all, great catch. It looks like there's indeed a bug
on certain conditions.
While your method is indeed way less convoluted than the clever "compare to -3"
the old code did, I think we can actually make it even simpler
(and save a couple of instructions) by doing the following:
"tst lr, #4\n" /* Check if bit 2 of LR is zero */
"itte eq\n" /* IF bit 2 of LR is zero... */
"mrseq r0, msp\n" /* THEN we were using MSP */
"addeq r0, %[cpufsz]\n" /* THEN revert the previous 'sub sp, %[cpufsz]' */
"mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP */
What do you think?
Also, I must admit it took me quite a while to understand what was going on
there.
If you don't mind, I'd also like to add a brief comment explaining how to
decode LR
after taking an exception. The patch would end up looking like this:
---
cpukit/score/cpu/arm/armv7m-exception-default.c | 27 +
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c
b/cpukit/score/cpu/arm/armv7m-exception-default.c
index e890cdf..d04dbea 100644
--- a/cpukit/score/cpu/arm/armv7m-exception-default.c
+++ b/cpukit/score/cpu/arm/armv7m-exception-default.c
@@ -22,16 +22,27 @@
void __attribute__((naked)) _ARMV7M_Exception_default( void )
{
+/* On exception entry, ARMv7M saves context state onto a stack pointed to
+ * by either MSP or PSP. The value stored in LR indicates whether we were
+ * in Thread or Handler mode, whether we were using the FPU (if any),
+ * and which stack pointer we were using.
+ * In particular, bit 2 of LR will be 0 if we were using MSP.
+ *
+ * For a more detailed explanation, see the Exception Entry Behavior
+ * section of the ARMv7M Architecture Reference Manual.
+ */
+
+/* As we're in Handler mode here, we'll always operate on MSP.
+ * However, we need to store the right SP in our CPU_Exception_frame.
+ */
__asm__ volatile (
-"sub sp, %[cpufsz]\n"
+"sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. */
"stm sp, {r0-r12}\n"
-"mov r2, lr\n"
-"mrs r1, msp\n"
-"mrs r0, psp\n"
-"cmn r2, #3\n"
-"itt ne\n"
-"movne r0, r1\n"
-"addne r0, %[cpufsz]\n"
+"tst lr, #4\n" /* Check if bit 2 of LR is zero. (If so, PSR.Z = 1) */
+"itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */
+"mrseq r0, msp\n" /* THEN we were using MSP. */
+"addeq r0, %[cpufsz]\n" /* THEN, set r0 = old MSP. */
+"mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP. */
"add r2, r0, %[v7mlroff]\n"
"add r1, sp, %[cpulroff]\n"
"ldm r2, {r3-r5}\n"
--
1.9.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH] Fix exception handler for supporting FPU
On 2015-08-28 17:18, Martin Galvan wrote:
On Thu, Aug 27, 2015 at 3:53 PM, sudarshan.rajagopalan
wrote:
The instruction "cmn r2, #3\n" in line 31 basically compares the Link
Register(lr) to value 0xFFFD (negative #3, because CMN negates the
RHS
and compares with LHS) and chooses MSP or PSP in the following IT
block.
This is pretty cool! But, it will not work if you have the
floating-point
unit (FPU) enabled in your processor, which is enabled in mine. With
FPU
enabled, the error code returned is either 0xFFE9 or 0xFFED,
for
which the above assembly instruction will not work out and MSP will be
selected always.
Better way to do is to check the 2nd bit of the error code to
determine
which stack pointer was used before the exception happened - "tst lr,
#4\n"
and change the IT block from "itt ne" to "itt eq" and the "mov" and
"add"
within this IT block.
Hi Sudarshan! First of all, great catch. It looks like there's indeed a
bug
on certain conditions.
While your method is indeed way less convoluted than the clever
"compare to -3"
the old code did, I think we can actually make it even simpler
(and save a couple of instructions) by doing the following:
"tst lr, #4\n" /* Check if bit 2 of LR is zero */
"itte eq\n" /* IF bit 2 of LR is zero... */
"mrseq r0, msp\n" /* THEN we were using MSP */
"addeq r0, %[cpufsz]\n" /* THEN revert the previous 'sub sp,
%[cpufsz]' */
"mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP */
What do you think?
Hi Martin,
Yup, we could definitely save 2 cycles and make it look simpler and
understandable. Thumps up for the comments explaining LR decoding!
Thanks and Regards,
Sudarshan
-
Sudarshan Rajagopalan
sudarshan.rajagopalan at vecna.com
www.vecna.com
Also, I must admit it took me quite a while to understand what was
going on there.
If you don't mind, I'd also like to add a brief comment explaining how
to decode LR
after taking an exception. The patch would end up looking like this:
---
cpukit/score/cpu/arm/armv7m-exception-default.c | 27
+
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c
b/cpukit/score/cpu/arm/armv7m-exception-default.c
index e890cdf..d04dbea 100644
--- a/cpukit/score/cpu/arm/armv7m-exception-default.c
+++ b/cpukit/score/cpu/arm/armv7m-exception-default.c
@@ -22,16 +22,27 @@
void __attribute__((naked)) _ARMV7M_Exception_default( void )
{
+/* On exception entry, ARMv7M saves context state onto a stack
pointed to
+ * by either MSP or PSP. The value stored in LR indicates whether
we were
+ * in Thread or Handler mode, whether we were using the FPU (if
any),
+ * and which stack pointer we were using.
+ * In particular, bit 2 of LR will be 0 if we were using MSP.
+ *
+ * For a more detailed explanation, see the Exception Entry
Behavior
+ * section of the ARMv7M Architecture Reference Manual.
+ */
+
+/* As we're in Handler mode here, we'll always operate on MSP.
+ * However, we need to store the right SP in our
CPU_Exception_frame.
+ */
__asm__ volatile (
-"sub sp, %[cpufsz]\n"
+"sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame.
*/
"stm sp, {r0-r12}\n"
-"mov r2, lr\n"
-"mrs r1, msp\n"
-"mrs r0, psp\n"
-"cmn r2, #3\n"
-"itt ne\n"
-"movne r0, r1\n"
-"addne r0, %[cpufsz]\n"
+"tst lr, #4\n" /* Check if bit 2 of LR is zero. (If so, PSR.Z = 1)
*/
+"itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */
+"mrseq r0, msp\n" /* THEN we were using MSP. */
+"addeq r0, %[cpufsz]\n" /* THEN, set r0 = old MSP. */
+"mrsne r0, psp\n" /* ELSE it's not zero; we were using PSP. */
"add r2, r0, %[v7mlroff]\n"
"add r1, sp, %[cpulroff]\n"
"ldm r2, {r3-r5}\n"
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v2] ARMv7M: Fix exception handler for supporting FPU
On exception entry, _ARMV7M_Exception_default stores the previous Stack Pointer
in a CPU_Exception_frame. The SP can be MSP or PSP, depending on the mode
in which the exception was taken. To know this we must check the value of LR.
Right now the code checks whether it should store MSP or PSP by comparing LR to
-3. However, this doesn't work if we're using an FPU. This patch fixes that
by checking bit 2 of LR, which indicates which SP we were using.
Thanks to Sudarshan Rajagopalan for finding the bug and proposing a first
version of the fix.
---
cpukit/score/cpu/arm/armv7m-exception-default.c | 27 +
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/cpukit/score/cpu/arm/armv7m-exception-default.c
b/cpukit/score/cpu/arm/armv7m-exception-default.c
index e890cdf..d04dbea 100644
--- a/cpukit/score/cpu/arm/armv7m-exception-default.c
+++ b/cpukit/score/cpu/arm/armv7m-exception-default.c
@@ -22,16 +22,27 @@
void __attribute__((naked)) _ARMV7M_Exception_default( void )
{
+/* On exception entry, ARMv7M saves context state onto a stack pointed to
+ * by either MSP or PSP. The value stored in LR indicates whether we were
+ * in Thread or Handler mode, whether we were using the FPU (if any),
+ * and which stack pointer we were using.
+ * In particular, bit 2 of LR will be 0 if we were using MSP.
+ *
+ * For a more detailed explanation, see the Exception Entry Behavior
+ * section of the ARMv7M Architecture Reference Manual.
+ */
+
+/* As we're in Handler mode here, we'll always operate on MSP.
+ * However, we need to store the right SP in our CPU_Exception_frame.
+ */
__asm__ volatile (
-"sub sp, %[cpufsz]\n"
+"sub sp, %[cpufsz]\n" /* Allocate space for a CPU_Exception_frame. */
"stm sp, {r0-r12}\n"
-"mov r2, lr\n"
-"mrs r1, msp\n"
-"mrs r0, psp\n"
-"cmn r2, #3\n"
-"itt ne\n"
-"movne r0, r1\n"
-"addne r0, %[cpufsz]\n"
+"tst lr, #4\n" /* Check if bit 2 of LR is zero. (If so, PSR.Z = 1) */
+"itte eq\n" /* IF bit 2 of LR is zero... (PSR.Z == 1) */
+"mrseq r0, msp\n"/* THEN we were using MSP. */
+"addeq r0, %[cpufsz]\n" /* THEN, set r0 = old MSP. */
+"mrsne r0, psp\n"/* ELSE it's not zero; we were using PSP. */
"add r2, r0, %[v7mlroff]\n"
"add r1, sp, %[cpulroff]\n"
"ldm r2, {r3-r5}\n"
--
1.9.1
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
