fix[RL78 Port] incorrect register image for pvParameters in FAR mode (#1316) #1317
Conversation
|
@KeitaKashima What do you think about this change? |
|
@aggarg san
My team will check the code. Could you please have some time to check it in about one or two weeks? |
|
@KeitaKashima Of course. Take your time. Thank you for your help with the review! |
|
@aggarg @TakeoTakahashi2020 and @ShunichiroNakamura of my team members, will check and answer the result. |
|
@KeitaKashima No worries at all! Please take your time. |
| * sizeof( StackType_t ) == 2 and sizeof( StackType_t * ) == 2. */ | ||
|
|
||
| #if __DATA_MODEL__ == __DATA_MODEL_FAR__ | ||
| #if __DATA_MODEL__ == __DATA_MODEL_FAR__ |
There was a problem hiding this comment.
Regarding the line 109 (*pulLocal = ( uint32_t ) pvParameters;).
When using __DATA_MODEL_FAR__, is saving the prvParameters to the stack to maintain compatibility with IAR compiler V1 and V2?
If yes, that’s fine, because I understand saving to the A:DE registers is dummy operation in the V1, and saving to the stack is dummy operation in the V2. According to the IAR compiler's calling convention, it seems that the V1 passes the prvParameters through the stack whereas the V2 passes it through the A:DE registers.
If you prefer to refine the code, I propose switching the location where the prvParameters is saved by using the predefined macro __CC_V1__ or __CC_V2__ (or else).
There was a problem hiding this comment.
Thank you for the review and clarification. I was not aware of the V1 vs V2 calling convention difference when I first submitted the patch — my intent was only to fix the issue I had in FAR model. The stack write remained as legacy from the original code. Based on your feedback I’ve updated the patch to explicitly branch on CC_V2 so that only the relevant write is kept. This way the fix still solves the FAR bug and is now aligned with both V1 and V2.
portable/IAR/RL78/port.c
Outdated
| { | ||
| uint32_t * pulLocal; | ||
|
|
||
| /* With large code and large data sizeof( StackType_t ) == 2, and |
There was a problem hiding this comment.
Strictly speaking, sizeof( StackType_t * ) is affected by __DATA_MODEL__, not __CODE_MODEL__. So, I propose updating comments (*1):
- "With large code and large data" -> "With large data"
- "With small code and small data" -> "With small data"
Alternatively, I propose clearly explaining effects of __CODE_MODEL__ and __DATA_MODEL__ for users (*2). This means adding a comment that the size of argument pxCode of pxPortInitialiseStack() is 2 or 4 bytes depending on __CODE_MODEL__.
*1: I guess the reason for mentioning both of code and data was that __CODE_MODEL__/__DATA_MODEL__ combinations were used only near/near and far/far in the test environments.
*2: The sizes of arguments for pxPortInitialiseStack() are shown below:
| Code model | Data model | StackType_t *pxTopOfStack | TaskFunction_t pxCode | void *pvParameters |
|---|---|---|---|---|
| near | near | 2bytes | 2bytes | 2bytes |
| near | far | 4bytes | 2bytes | 4bytes |
| far | near | 2bytes | 4bytes | 2bytes |
| far | far | 4bytes | 4bytes | 4bytes |
There was a problem hiding this comment.
I’ve updated the comment to reflect that sizeof(StackType_t*) depends on DATA_MODEL (“With large data” / “With small data”).
|
@ShunichiroNakamura and @TakeoTakahashi2020, thank you for reviewing the code. @sauleThsQuin Would you please address the comments? |
…FreeRTOS#1316) In the RL78 FAR data model, pxPortInitialiseStack() did not initialize the register image for the task parameter (pvParameters) correctly. A:DE registers were saved with dummy values instead of the actual pointer. Effect: on first context restore the function prologue read a corrupted parameter. NEAR builds were not affected. This patch aligns the FAR path with the calling convention and compiler version: - IAR V2: pass pvParameters via registers → DE = low 16 bits, A = high 8 - IAR V1 (fallback): keep legacy stack write Also keeps the original stack-frame layout and updates the comment to reflect that pointer sizes depend on __DATA_MODEL__. Result: tasks in FAR receive the correct parameter at entry; NEAR remains unchanged.
…reeRTOS#1314) * cortex-M ports: Clarify hardware-saved exception frame size variable - Rename ulStackFrameSize to ulHardwareSavedExceptionFrameSize to reflect the hardware-saved exception frame (8 or 26 words based on FPU/lazy stacking). - Add comments explaining standard vs extended frames. - Apply across Cortex-M ports. - No functional change, improves readability. Signed-off-by: Ahmed Ismail <[email protected]> Signed-off-by: Gaurav Aggarwal <[email protected]> * kernel-checker-script: Modify Arm copyright header regex Arm's copyright header regex is modified with the following: * Accept both single year copyright headers (e.g., "2024") and year range copyright headers (e.g., "2024-2025"). * Accept both single-line copyright header and also multi-line header. * Add the escape backslash to accept only literal dot not any character. Signed-off-by: Gaurav Aggarwal <[email protected]> Signed-off-by: Ahmed Ismail <[email protected]> --------- Signed-off-by: Ahmed Ismail <[email protected]> Signed-off-by: Gaurav Aggarwal <[email protected]>
sauleThsQuin
force-pushed
the
fix/rl78-port-far-param-init
branch
from
59b9ade to
8138738
Compare
|
Title
fix[RL78 Port] incorrect register image for pvParameters in FAR mode (#1316)
Description
In the RL78 FAR data model, pxPortInitialiseStack() did not correctly
initialize the register image in the task stack for the parameter (pvParameters).
The saved A:DE registers were filled with dummy values instead of the actual pointer value.
As a result, when the first context restore occurred, the compiler's function prologue read a corrupted parameter from the register image.
This only affected FAR builds; NEAR model was not impacted.
This patch updates the RL78 FAR path so that the register image matches the RL78 calling convention:
DE = low 16 bits of pvParameters
A = high 8 bits of pvParameters (via AX register image, X = 0)
With this change, tasks in FAR model receive the correct parameter at entry, while NEAR model remains unchanged.
Test Steps
Before fix: value corrupted (dummy registers A:DE = 0x1111/0xDEDE).
After fix: value matches the pointer passed to xTaskCreate.
Confirm no regression with NEAR model build.
Checklist
I have tested my changes. No regression in existing tests.
I have modified and/or added unit-tests to cover the code changes in this Pull Request.
Related Issue
#1316