You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/01/17 11:04:52 UTC
[GitHub] [incubator-nuttx] zhuyanlinzyl opened a new pull request #5248: xtensa: some fixes in interrupt handler
zhuyanlinzyl opened a new pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248
## Summary
Five patches include:
xtensa: fix svccall enter error
xtensa: add svcall handler
xtensa: move a3 save in handler instead of _xtensa_context_save
arch:xtensa_panic: use right interrupt pointer in xtensa_panic
arch:xtensa:vectors:fix bugs in a0 save
## Impac
No, bug fix.
## Testing
Testing on xtensa EVB borad
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786694561
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, a2, (4 * REG_A3)
Review comment:
```suggestion
s32i a3, a2, (4 * REG_A3)
```
nit: double whitespace, also on the other similar changes in this same file.
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -415,6 +416,7 @@ _xtensa_level2_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, a2, (4 * REG_A3)
Review comment:
```suggestion
s32i a3, a2, (4 * REG_A3)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787858313
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
Looking at this line again https://github.com/apache/incubator-nuttx/blob/13d35276427e9eefd290b3f32c0cdc42986842b9/arch/xtensa/src/common/xtensa_user_handler.S#L372
I see it's not actually restoring A3 at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787547613
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
For syscall, A3 gets save here (of course after we apply your fix :)):https://github.com/apache/incubator-nuttx/blob/13d35276427e9eefd290b3f32c0cdc42986842b9/arch/xtensa/src/common/xtensa_user_handler.S#L350
then restored here: https://github.com/apache/incubator-nuttx/blob/13d35276427e9eefd290b3f32c0cdc42986842b9/arch/xtensa/src/common/xtensa_user_handler.S#L372
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove A3 save in _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 `
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, **but it is a wrong value,** . as it is used in syscal_handler before `_xtensa_save_context`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#issuecomment-1014473411
Hi @zhuyanlinzyl did you test BUILD_PROTECTED ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#issuecomment-1015276609
@acassis this PR contain the minor but critical fix we found in our product. But the key block issue is here: https://github.com/apache/incubator-nuttx/pull/5261. Let's merge this one, and focus on that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787312125
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
after
`
rsr a2, EPC_1 /* a2 = PC of 'syscall' */
addi a3, a2, 3 /* Increment PC */
rsr a0, LEND /* Skip if PC != LEND */
bne a3, a0, 1f
rsr a0, LCOUNT /* Skip if LCOUNT == 0 */
beqz a0, 1f
addi a0, a0, -1 /* Decrement LCOUNT */
rsr a3, LBEG /* Set PC = LBEG */
wsr a0, LCOUNT /* Save the new LCOUNT */
`
the A3 register was used, we must not save it in _xtensa_context_save, as it is a newer value.
@gustavonihei @Ouss4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786750932
##########
File path: arch/xtensa/src/common/xtensa_panic.S
##########
@@ -127,8 +127,13 @@ _xtensa_panic:
*/
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
call0 _xtensa_context_save /* Save full register state */
+ /* Dispatch the sycall as with other interrupts. */
Review comment:
```suggestion
/* Dispatch the syscall as with other interrupts. */
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove A3 save in _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 `
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, **but save a wrong value,** . As it is already used in syscal_handler before `_xtensa_save_context`
When restore from stack,
**It will restore the wrong value.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove A3 in _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 /* Increment PC */`
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, but it is a wrong value, as it is used in syscal_handler before `_xtensa_save_context`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786756480
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
Curious question: why do we need to save `A3` here? It is already saved as part of `_xtensa_context_save`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#issuecomment-1015024713
hi, @acassis.
No, I only test BUILD_FLAT.
And also arch xtensa do not support BULD_PROTECTED now.
All kernel and user code run in user mode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787310262
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
Yes, The reason I have explained in commit messages:
In _xtensa_syscall_handler, A3 register was saved and used before call _xtensa_context_save
So _xtensa_context_save must not save again, as it is corrutped.
I also think this method it not good enough,
Do you has a perfect method to solve my problem>
@Ouss4 @gustavonihei
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786706372
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, a2, (4 * REG_A3)
Review comment:
By the way, since the `SP` regsiter is still unchanged at this point, isn't it more intuitive to use it as the base address?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 merged pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove A3 save in _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 `
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, **but it is a wrong value,** . As it is already used in syscal_handler before `_xtensa_save_context`
When restore from stack,
**It will restore the wrong value.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786694561
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, a2, (4 * REG_A3)
Review comment:
```suggestion
s32i a3, a2, (4 * REG_A3)
```
nit: double whitespace
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl edited a comment on pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl edited a comment on pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#issuecomment-1015024713
hi, @acassis.
No, I only test BUILD_FLAT.
And also arch Xtensa do not support BULD_PROTECTED now.
All kernel and user code run in user mode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787865829
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
@zhuyanlinzyl I think your solution is correct. Thanks for sticking around.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r786718533
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, a2, (4 * REG_A3)
Review comment:
Yes, I think so. Please review again. @gustavonihei Thansk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787059481
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
I don't see a reason to save A3 here before calling `_xtensa_save_context`. This is only valid for `_xtensa_syscall_handler`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787545577
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
@zhuyanlinzyl we are only referring to this commit https://github.com/apache/incubator-nuttx/pull/5248/commits/b89df7c42320498bef087be0dab53efc936c8823. The other one (https://github.com/apache/incubator-nuttx/pull/5248/commits/30a6df4238f9453b836adc86dac7f8d0fa241360) is good and fixes a bug in the syscall handler.
The syscall handler saves A3, uses it as scratch register then restores it. After that `_xtensa_save_context` saves it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove `A3 save` from _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 `
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, **but save a wrong value,** . As it is already used in syscal_handler before `_xtensa_save_context`
When restore from stack,
**It will restore the wrong value.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787043847
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
The commit message says that this is similar to `_xtensa_syscall_handler`, it's actually not similar. In `_xtensa_syscall_handler` we use A3 (and A2) as scratch registers before calling `_xtensa_save_context`, so they are saved.
~Another thing to note regarding `_xtensa_syscall_handler` is that it never restores the first saved A3, but just calls `_xtensa_restore_context` which restores a scratch version of A3 only.~. It actually does restore the correct version before calling `_xtensa_save_context`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787043847
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
The commit message says that this is similar to `_xtensa_syscall_handler`, it's actually not similar. In `_xtensa_syscall_handler` we use A3 (and A2) as scratch registers before calling `_xtensa_save_context`, so they are saved.
Another thing to note regarding `_xtensa_syscall_handler` is that it never restores the first saved A3, but just calls `_xtensa_restore_context` which restores a scratch version of A3 only.
I think when @zhuyanlinzyl removed the A3 save from `_xtensa_save_context` it fixes the issue he had, but I believe we need to organise this a bit more. I think we should fix how `_xtensa_syscall_handler` restores A3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] zhuyanlinzyl commented on a change in pull request #5248: xtensa: some fixes in interrupt handler
Posted by GitBox <gi...@apache.org>.
zhuyanlinzyl commented on a change in pull request #5248:
URL: https://github.com/apache/incubator-nuttx/pull/5248#discussion_r787670525
##########
File path: arch/xtensa/src/common/xtensa_int_handlers.S
##########
@@ -312,6 +312,7 @@ _xtensa_level1_handler:
s32i a2, sp, (4 * REG_A2)
mov a2, sp /* Address of state save on stack */
+ s32i a3, sp, (4 * REG_A3)
Review comment:
hi @Ouss4
I also referring this commit:
https://github.com/apache/incubator-nuttx/commit/b89df7c42320498bef087be0dab53efc936c8823
The reason why I remove A3 save in _xtensa_context_save is:
In syscall handler, we save a3. But we reused in
` addi a3, a2, 3 `
If in _xtensa_save_context
` s32i a3, a2, (4 * REG_A3) `
It save a3 again, **but it is a wrong value,** . As it is used in syscal_handler before `_xtensa_save_context`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org