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/05/20 07:05:17 UTC
[GitHub] [incubator-nuttx] pussuw opened a new pull request, #6303: MPFS: Implement S-mode head and start function
pussuw opened a new pull request, #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303
- Remove S-mode initializations from the M-mode head file, they are not
required
- Writing mstatus->tvm from S-mode will result in illegal instruction
## Summary
Adds proper start function for S-mode
## Impact
MPFS only
## Testing
icicle:knsh
--
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] pkarashchenko commented on a diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r878855898
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * arch/risc-v/src/mpfs/mpfs_shead.S
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/irq.h>
+
+#include "chip.h"
+#include "mpfs_memorymap.h"
+#include "riscv_internal.h"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ /* Imported symbols */
+
+ .extern __trap_vec
+
+ .section .text
+ .global __start
+
+/****************************************************************************
+ * Name: __start
+ *
+ * Description:
+ * Supervisor mode start function.
+ *
+ * Input Parameters:
+ * a0 - hartid
+ *
+ ****************************************************************************/
+
+__start:
+
+ /* Disable all interrupts in sie */
+
+ csrw sie, zero
+ csrw sip, zero
+
+ /* Set the S-mode trap vector */
+
+ la t0, __trap_vec
+ csrw stvec, t0
+
+ /* Clear sscratch */
+
+ csrw sscratch, zero
+ csrw scause, zero
+ csrw sepc, zero
+
+ /* initialize global pointer, global data */
+
+.option push
+.option norelax
+ la gp, __global_pointer$
+.option pop
+
+ /* Make sure the writes to CSR stick before continuing */
+
+ fence
+
+ /* Set stack pointer and jump to start */
+
+ la sp, MPFS_IDLESTACK_TOP
+ j __mpfs_start
Review Comment:
do we need to follow same calling strategy as in M-mode?
I mean for M-mode I see
```
jal __mpfs_start
/* We shouldn't return from __mpfs_start
* in case of return, loop forever. nop's added so can be seen in debugger
*/
1:
nop
nop
j 1b
```
but here we expect that `__mpfs_start` never returns.
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877856797
##########
arch/risc-v/src/mpfs/mpfs_head.S:
##########
@@ -100,30 +100,17 @@ __start:
csrr a0, mhartid
beqz a0, .skip_e51
- /* Clear sscratch if u54 */
-
- csrw sscratch, zero
Review Comment:
one simple question: why not use #ifdef/#else/#endif to separate M-mode or S-mode specific code?
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877898130
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
Review Comment:
Fixed
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/irq.h>
+
+#include "chip.h"
+#include "mpfs_memorymap.h"
+#include "riscv_internal.h"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ /* Imported symbols */
+
+ .extern __trap_vec
+
+ .section .text
+ .global __start_s
+
+/****************************************************************************
+ * Name: __start_s
+ *
+ * Description:
+ * Supervisor mode start function.
+ *
+ * Input Parameters:
+ * a0 - hartid
+ *
+ ****************************************************************************/
+
+__start_s:
Review Comment:
Fixed
--
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] pkarashchenko merged pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877893640
##########
arch/risc-v/src/mpfs/mpfs_head.S:
##########
@@ -100,30 +100,17 @@ __start:
csrr a0, mhartid
beqz a0, .skip_e51
- /* Clear sscratch if u54 */
-
- csrw sscratch, zero
Review Comment:
I like having separate files, the mpfs_head.S file is the M-mode head file, mpfs_shead.S is the S-mode head file. To me this makes sense.
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877891686
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
Review Comment:
Wish my editor did not append the root folder name automatically there...
--
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 pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#issuecomment-1133252289
I think the CI error is still not my doing
====================================================================================
Skipping: sim/sotest32
Configuration/Tool: sim/sotest32
------------------------------------------------------------------------------------
Cleaning...
Error: The operation was canceled.
--
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] pkarashchenko commented on a diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877864955
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
Review Comment:
```suggestion
* arch/risc-v/src/mpfs/mpfs_shead.S
```
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/irq.h>
+
+#include "chip.h"
+#include "mpfs_memorymap.h"
+#include "riscv_internal.h"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ /* Imported symbols */
+
+ .extern __trap_vec
+
+ .section .text
+ .global __start_s
+
+/****************************************************************************
+ * Name: __start_s
+ *
+ * Description:
+ * Supervisor mode start function.
+ *
+ * Input Parameters:
+ * a0 - hartid
+ *
+ ****************************************************************************/
+
+__start_s:
Review Comment:
Why it is `__start_s` and not `__start`? I think that Makefile still has `$(LD) --entry=__start`
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r877891436
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * /nuttx-bare/arch/risc-v/src/mpfs/mpfs_shead.S
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/irq.h>
+
+#include "chip.h"
+#include "mpfs_memorymap.h"
+#include "riscv_internal.h"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ /* Imported symbols */
+
+ .extern __trap_vec
+
+ .section .text
+ .global __start_s
+
+/****************************************************************************
+ * Name: __start_s
+ *
+ * Description:
+ * Supervisor mode start function.
+ *
+ * Input Parameters:
+ * a0 - hartid
+ *
+ ****************************************************************************/
+
+__start_s:
Review Comment:
This is an error. I have __start defined in my own "miniSBI" which is the M-mode entry point, and this is why I needed a different name here. I need to change this.
--
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 pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#issuecomment-1132802121
I don't think the CI error is of my making:
==> Downloading https://ghcr.io/v2/homebrew/core/hiredis/manifests/1.0.2
curl: (22) The requested URL returned error: 500
Error: ccache: Failed to download resource "hiredis_bottle_manifest"
Download failed: https://ghcr.io/v2/homebrew/core/hiredis/manifests/1.0.2
Can e.g. @pkarashchenko restart the CI please ?
--
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 diff in pull request #6303: MPFS: Implement S-mode head and start function
Posted by GitBox <gi...@apache.org>.
pussuw commented on code in PR #6303:
URL: https://github.com/apache/incubator-nuttx/pull/6303#discussion_r879051413
##########
arch/risc-v/src/mpfs/mpfs_shead.S:
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * arch/risc-v/src/mpfs/mpfs_shead.S
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/irq.h>
+
+#include "chip.h"
+#include "mpfs_memorymap.h"
+#include "riscv_internal.h"
+
+/****************************************************************************
+ * Public Symbols
+ ****************************************************************************/
+
+ /* Imported symbols */
+
+ .extern __trap_vec
+
+ .section .text
+ .global __start
+
+/****************************************************************************
+ * Name: __start
+ *
+ * Description:
+ * Supervisor mode start function.
+ *
+ * Input Parameters:
+ * a0 - hartid
+ *
+ ****************************************************************************/
+
+__start:
+
+ /* Disable all interrupts in sie */
+
+ csrw sie, zero
+ csrw sip, zero
+
+ /* Set the S-mode trap vector */
+
+ la t0, __trap_vec
+ csrw stvec, t0
+
+ /* Clear sscratch */
+
+ csrw sscratch, zero
+ csrw scause, zero
+ csrw sepc, zero
+
+ /* initialize global pointer, global data */
+
+.option push
+.option norelax
+ la gp, __global_pointer$
+.option pop
+
+ /* Make sure the writes to CSR stick before continuing */
+
+ fence
+
+ /* Set stack pointer and jump to start */
+
+ la sp, MPFS_IDLESTACK_TOP
+ j __mpfs_start
Review Comment:
I did not write the original code. The start function can, and must not return. The jump-and-link and the nop loop is unnecessary
--
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