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/03/29 15:00:11 UTC
[GitHub] [incubator-nuttx] no1wudi opened a new pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
no1wudi opened a new pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905
## Summary
Move FPU context relative opeartions to exception common
## Impact
RISCV with FPU
## Testing
QEMU
--
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 a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r837597940
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -147,6 +155,10 @@ exception_common:
REGLOAD s0, REG_INT_CTX(sp) /* restore mstatus */
csrw mstatus, s0
+#ifdef CONFIG_ARCH_FPU
Review comment:
should we restore before mstatus?
--
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 merged pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905
--
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] no1wudi commented on a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r838040654
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -147,6 +155,10 @@ exception_common:
REGLOAD s0, REG_INT_CTX(sp) /* restore mstatus */
csrw mstatus, s0
+#ifdef CONFIG_ARCH_FPU
Review comment:
OK, FPU restore would not touch the mstatus, but it's closer to current mainline (restore FPU context first)
--
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] no1wudi commented on a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r839127053
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -96,6 +98,11 @@ exception_common:
addi s0, sp, XCPTCONTEXT_SIZE
REGSTORE s0, REG_X2(sp) /* original SP */
+#ifdef CONFIG_ARCH_FPU
+ mv a0, sp
+ jal x1, riscv_savefpu
+#endif
Review comment:
Done
--
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 a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r839101963
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -96,6 +98,11 @@ exception_common:
addi s0, sp, XCPTCONTEXT_SIZE
REGSTORE s0, REG_X2(sp) /* original SP */
+#ifdef CONFIG_ARCH_FPU
+ mv a0, sp
+ jal x1, riscv_savefpu
+#endif
Review comment:
But why not move mcause just before line 112?
--
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] pussuw commented on a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
pussuw commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r838408788
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -147,6 +155,10 @@ exception_common:
REGLOAD s0, REG_INT_CTX(sp) /* restore mstatus */
csrw mstatus, s0
+#ifdef CONFIG_ARCH_FPU
Review comment:
Ordering should be:
1. riscv_restorefpu() <- this modifies the context, not the status register first
2. restore mstatus <- restore status register with the updated 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] no1wudi commented on a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r839092683
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -96,6 +98,11 @@ exception_common:
addi s0, sp, XCPTCONTEXT_SIZE
REGSTORE s0, REG_X2(sp) /* original SP */
+#ifdef CONFIG_ARCH_FPU
+ mv a0, sp
+ jal x1, riscv_savefpu
+#endif
Review comment:
It's better to keep current location since a0 in line 111 was occupied by mcause.
--
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] no1wudi commented on a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r838582926
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -147,6 +155,10 @@ exception_common:
REGLOAD s0, REG_INT_CTX(sp) /* restore mstatus */
csrw mstatus, s0
+#ifdef CONFIG_ARCH_FPU
Review comment:
Yes, the latest change restore the fpu context first and then restore mstatus.
--
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 a change in pull request #5905: arch/risc-v: Store/Restore FPU register in exception_common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5905:
URL: https://github.com/apache/incubator-nuttx/pull/5905#discussion_r838810701
##########
File path: arch/risc-v/src/common/riscv_exception_common.S
##########
@@ -96,6 +98,11 @@ exception_common:
addi s0, sp, XCPTCONTEXT_SIZE
REGSTORE s0, REG_X2(sp) /* original SP */
+#ifdef CONFIG_ARCH_FPU
+ mv a0, sp
+ jal x1, riscv_savefpu
+#endif
Review comment:
let's move after line 111 to mirror the restore order.
--
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