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/07 15:23:04 UTC
[GitHub] [incubator-nuttx] no1wudi opened a new pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
no1wudi opened a new pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192
## Summary
Replace all register width relative data type (uint32_t/uint64_t) with uintptr_t.
## Impact
Refactor only.
## Testing
CI
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780528189
##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -184,8 +173,8 @@ uint32_t riscv_get_newintctx(void);
void riscv_savefpu(uint64_t *regs);
void riscv_restorefpu(const uint64_t *regs);
#else /* !CONFIG_ARCH_RV64 */
-void riscv_savefpu(uint32_t *regs);
-void riscv_restorefpu(const uint32_t *regs);
+void riscv_savefpu(uintptr_t *regs);
+void riscv_restorefpu(const uintptr_t *regs);
Review comment:
Eliminate `CONFIG_ARCH_RV64` and use only
```
void riscv_savefpu(uintptr_t *regs);
void riscv_restorefpu(const uintptr_t *regs);
```
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780742208
##########
File path: arch/risc-v/src/bl602/bl602_start.c
##########
@@ -82,7 +82,7 @@ static struct boot2_partition_table_s g_boot2_partition_table unused_data;
* Public Data
****************************************************************************/
-uint32_t g_idle_topstack = 0;
+uintptr_t g_idle_topstack = 0;
Review comment:
```suggestion
uintptr_t g_idle_topstack;
```
Assigned to 0 by C standard.
##########
File path: arch/risc-v/src/bl602/bl602_irq_dispatch.c
##########
@@ -67,7 +67,7 @@ void *bl602_dispatch_irq(uint32_t vector, uint32_t *regs)
if (BL602_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
Why this is changed?
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
Why this is changed?
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780702620
##########
File path: arch/risc-v/src/common/riscv_doirq.c
##########
@@ -111,18 +111,18 @@ uint32_t *up_doirq(int irq, uint32_t *regs)
#endif
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780529819
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
The same for `vector` and `irq`
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780884485
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
Do you have the test result?
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780626882
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -105,21 +105,6 @@ exception_common:
sw x30, 30*4(sp) /* t5 */
sw x31, 31*4(sp) /* t6 */
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- csrr x28, 0x7b0
- csrr x29, 0x7b1
- csrr x30, 0x7b2
- sw x28, 33*4(sp)
- sw x29, 34*4(sp)
- sw x30, 35*4(sp)
- csrr x28, 0x7b4
- csrr x29, 0x7b5
- csrr x30, 0x7b6
- sw x28, 36*4(sp)
- sw x29, 37*4(sp)
- sw x30, 38*4(sp)
-#endif
-
Review comment:
It's only works with RV32M1 chips with customized toolchain,
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r782072589
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
Should be relatively simple by:
```
#define STR_INSTR(n) #n
#define INSTR(n) STR_INSTR(n)
#ifdef CONFIG_ARCH_RV32
# define REGLOAD lw
# define REGSTORE sw
#else
# define REGLOAD ld
# define REGSTORE sd
#endif
```
and then
```
static void dispatch_syscall(void)
{
asm volatile
(
" addi sp, sp, -4\n" /* Create a stack frame to hold ra */
" " INSTR(REGSTORE) " ra, 0(sp)\n" /* Save ra in the stack frame */
" la t0, g_stublookup\n" /* t0=The base of the stub lookup table */
" slli a0, a0, 3\n" /* a0=Offset for the stub lookup table */
" add t0, t0, a0\n" /* t0=The address in the table */
" " INSTR(REGLOAD) " t0, 0(t0)\n" /* t0=The address of the stub for this syscall */
" jalr ra, t0\n" /* Call the stub (modifies ra) */
" " INSTR(REGLOAD) " ra, 0(sp)\n" /* Restore ra */
" addi sp, sp, 4\n" /* Destroy the stack frame */
" mv a2, a0\n" /* a2=Save return value in a0 */
" li a0, 3\n" /* a0=SYS_syscall_return (3) */
" ecall" /* Return from the syscall */
);
}
```
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
Should be relatively simple by:
```
#define STR_INSTR(n) #n
#define INSTR(n) STR_INSTR(n)
#ifdef CONFIG_ARCH_RV32
# define REGLOAD lw
# define REGSTORE sw
#else
# define REGLOAD ld
# define REGSTORE sd
#endif
```
and then
```
static void dispatch_syscall(void)
{
asm volatile
(
" addi sp, sp, -4\n" /* Create a stack frame to hold ra */
" " INSTR(REGSTORE) " ra, 0(sp)\n" /* Save ra in the stack frame */
" la t0, g_stublookup\n" /* t0=The base of the stub lookup table */
" slli a0, a0, 3\n" /* a0=Offset for the stub lookup table */
" add t0, t0, a0\n" /* t0=The address in the table */
" " INSTR(REGLOAD) " t0, 0(t0)\n" /* t0=The address of the stub for this syscall */
" jalr ra, t0\n" /* Call the stub (modifies ra) */
" " INSTR(REGLOAD) " ra, 0(sp)\n" /* Restore ra */
" addi sp, sp, 4\n" /* Destroy the stack frame */
" mv a2, a0\n" /* a2=Save return value in a0 */
" li a0, 3\n" /* a0=SYS_syscall_return (3) */
" ecall" /* Return from the syscall */
);
}
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780624976
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -121,7 +121,13 @@
#define INT_XCPT_REGS 33
#endif
-#define INT_XCPT_SIZE (8 * INT_XCPT_REGS)
+#ifdef CONFIG_ARCH_RV32
+#define INT_REG_SIZE 4
+#else
+#define INT_REG_SIZE 8
+#endif
Review comment:
Can't do this since uintptr_t is invalid in assembly.
##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -37,6 +37,18 @@
#include "riscv_internal.h"
#include "riscv_arch.h"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "%08"PRIxPTR
Review comment:
Done
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Yes, all of these can be replaced by uintptr_t
##########
File path: arch/risc-v/src/common/riscv_fault.c
##########
@@ -54,61 +66,61 @@
*
****************************************************************************/
-void up_fault(int irq, uint64_t *regs)
+void up_fault(int irq, uintptr_t *regs)
{
CURRENT_REGS = regs;
- _alert("EPC:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n",
Review comment:
Done
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -105,21 +105,6 @@ exception_common:
sw x30, 30*4(sp) /* t5 */
sw x31, 31*4(sp) /* t6 */
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- csrr x28, 0x7b0
- csrr x29, 0x7b1
- csrr x30, 0x7b2
- sw x28, 33*4(sp)
- sw x29, 34*4(sp)
- sw x30, 35*4(sp)
- csrr x28, 0x7b4
- csrr x29, 0x7b5
- csrr x30, 0x7b6
- sw x28, 36*4(sp)
- sw x29, 37*4(sp)
- sw x30, 38*4(sp)
-#endif
-
Review comment:
It's only works with RV32M1 chips with customized toolchain,
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743635
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
I find that ecall instruction is on 4 byte align address now, so we must update the epc adjustment.
I'll find a better way for it.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780742474
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
Review comment:
QPFPU is defined in original file but not actually used.
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780527694
##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -171,8 +160,8 @@ void riscv_ack_irq(int irq);
void riscv_copystate(uint64_t *dest, uint64_t *src);
void riscv_copyfullstate(uint64_t *dest, uint64_t *src);
#else
-void riscv_copystate(uint32_t *dest, uint32_t *src);
-void riscv_copyfullstate(uint32_t *dest, uint32_t *src);
+void riscv_copystate(uintptr_t *dest, uintptr_t *src);
+void riscv_copyfullstate(uintptr_t *dest, uintptr_t *src);
#endif
Review comment:
```suggestion
void riscv_copystate(uintptr_t *dest, uintptr_t *src);
void riscv_copyfullstate(uintptr_t *dest, uintptr_t *src);
```
eliminate `#ifdef CONFIG_ARCH_RV64`
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780389520
##########
File path: arch/risc-v/src/common/riscv_fault.c
##########
@@ -54,61 +66,61 @@
*
****************************************************************************/
-void up_fault(int irq, uint64_t *regs)
+void up_fault(int irq, uintptr_t *regs)
{
CURRENT_REGS = regs;
- _alert("EPC:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n",
Review comment:
Maybe move `"0"` to `PRIxREG`?
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Maybe can switch to `uintptr_t` here at well? Or it is not possible?
##########
File path: arch/risc-v/src/k210/k210_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ extern void up_fault(int irq, uint64_t *regs);
* k210_dispatch_irq
****************************************************************************/
-void *k210_dispatch_irq(uint64_t vector, uint64_t *regs)
+void *k210_dispatch_irq(uint64_t vector, uintptr_t *regs)
Review comment:
Can we go with `uintptr_t vector`?
##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -37,6 +37,18 @@
#include "riscv_internal.h"
#include "riscv_arch.h"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "%08"PRIxPTR
Review comment:
I recommend to leave `"%"` in format string and move here to
```suggestion
# define PRIxREG "08" PRIxPTR
```
Same in other places
##########
File path: arch/risc-v/src/litex/litex_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ volatile uint32_t * g_current_regs;
* litex_dispatch_irq
****************************************************************************/
-void *litex_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *litex_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Can we go with `uintprt_t *mepc`?
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
Review comment:
In 32 bit variant of include we had different vales for FPU registers.
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
Review comment:
32-bit variant had
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE 2
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE 4
#else
# define FPU_REG_SIZE 1
#endif
```
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
or
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# define FPU_REG_SIZE (1 / REG_SIZE)
#endif
#if FPU_REG_SIZE == 0
# error not supported !!!
#endif
```
##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -171,8 +160,8 @@ void riscv_ack_irq(int irq);
void riscv_copystate(uint64_t *dest, uint64_t *src);
void riscv_copyfullstate(uint64_t *dest, uint64_t *src);
#else
-void riscv_copystate(uint32_t *dest, uint32_t *src);
-void riscv_copyfullstate(uint32_t *dest, uint32_t *src);
+void riscv_copystate(uintptr_t *dest, uintptr_t *src);
+void riscv_copyfullstate(uintptr_t *dest, uintptr_t *src);
#endif
Review comment:
```suggestion
void riscv_copystate(uintptr_t *dest, uintptr_t *src);
void riscv_copyfullstate(uintptr_t *dest, uintptr_t *src);
```
eliminate `#ifdef CONFIG_ARCH_RV64`
##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -184,8 +173,8 @@ uint32_t riscv_get_newintctx(void);
void riscv_savefpu(uint64_t *regs);
void riscv_restorefpu(const uint64_t *regs);
#else /* !CONFIG_ARCH_RV64 */
-void riscv_savefpu(uint32_t *regs);
-void riscv_restorefpu(const uint32_t *regs);
+void riscv_savefpu(uintptr_t *regs);
+void riscv_restorefpu(const uintptr_t *regs);
Review comment:
Eliminate `CONFIG_ARCH_RV64` and use only
```
void riscv_savefpu(uintptr_t *regs);
void riscv_restorefpu(const uintptr_t *regs);
```
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -455,7 +455,7 @@ int riscv_swint(int irq, void *context, void *arg)
rtcb->flags |= TCB_FLAG_SYSCALL;
#else
- svcerr("ERROR: Bad SYS call: %" PRId64 "\n", regs[REG_A0]);
+ svcerr("ERROR: Bad SYS call: %" PRIdPTR "\n", regs[REG_A0]);
Review comment:
Maybe can switch to `PRIxREG` here.
Also rework `static void riscv_registerdump(const uint64_t *regs)` to use `uintptr_t`.
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
The same for `vector` and `irq`
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -154,21 +139,6 @@ exception_common:
lw s0, 32*4(sp) /* restore mstatus */
csrw mstatus, s0
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- lw x28, 36*4(sp)
- lw x29, 37*4(sp)
- lw x30, 38*4(sp)
- csrrw x0, 0x7b4, x28
- csrrw x0, 0x7b5, x29
- csrrw x0, 0x7b6, x30
- lw x28, 33*4(sp)
- lw x29, 34*4(sp)
- lw x30, 35*4(sp)
- csrrw x0, 0x7b0, x28
- csrrw x0, 0x7b1, x29
- csrrw x0, 0x7b2, x30
-#endif
Review comment:
Why this is removed?
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -105,21 +105,6 @@ exception_common:
sw x30, 30*4(sp) /* t5 */
sw x31, 31*4(sp) /* t6 */
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- csrr x28, 0x7b0
- csrr x29, 0x7b1
- csrr x30, 0x7b2
- sw x28, 33*4(sp)
- sw x29, 34*4(sp)
- sw x30, 35*4(sp)
- csrr x28, 0x7b4
- csrr x29, 0x7b5
- csrr x30, 0x7b6
- sw x28, 36*4(sp)
- sw x29, 37*4(sp)
- sw x30, 38*4(sp)
-#endif
-
Review comment:
Why this is removed?
##########
File path: arch/risc-v/src/rv32m1/rv32m1_irq_dispatch.c
##########
@@ -53,11 +53,11 @@ volatile uint32_t * g_current_regs;
****************************************************************************/
LOCATE_ITCM
-void *rv32m1_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *rv32m1_dispatch_irq(uintptr_t vector, uintptr_t *regs)
{
int vec = vector & 0x1f;
int irq = (vector >> 27) + vec;
Review comment:
```suggestion
uintptr_t vec = vector & 0x1f;
uintptr_t irq = (vector >> 27) + vec;
```
##########
File path: arch/risc-v/src/mpfs/mpfs_head.S
##########
@@ -24,7 +24,7 @@
#include <nuttx/config.h>
#include <arch/csr.h>
-#include <arch/rv64gc/irq.h>
+#include <arch/common/irq.h>
Review comment:
```suggestion
#include <arch/irq.h>
```
##########
File path: arch/risc-v/src/rv32m1/rv32m1_head.S
##########
@@ -23,7 +23,7 @@
****************************************************************************/
#include <nuttx/config.h>
-#include <arch/rv32im/irq.h>
+#include <arch/common/irq.h>
Review comment:
```suggestion
#include <arch/irq.h>
```
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780701661
##########
File path: arch/risc-v/src/litex/litex_irq_dispatch.c
##########
@@ -114,13 +114,13 @@ void *litex_dispatch_irq(uint32_t vector, uint32_t *regs)
#endif
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
- g_current_regs = NULL;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
I think cast is not needed
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/mpfs/mpfs_irq.c
##########
@@ -41,7 +41,7 @@
* Public Data
****************************************************************************/
-volatile uint64_t *g_current_regs[1];
+volatile uint64_t *g_current_regs;
Review comment:
I think this should be reverted to keep the same style with other files.
```suggestion
volatile uintptr_t *g_current_regs[1];
```
##########
File path: arch/risc-v/src/common/riscv_doirq.c
##########
@@ -111,18 +111,18 @@ uint32_t *up_doirq(int irq, uint32_t *regs)
#endif
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
```suggestion
uintptr_t *mepc = regs;
```
##########
File path: arch/risc-v/src/litex/litex_head.S
##########
@@ -23,7 +23,7 @@
****************************************************************************/
#include <nuttx/config.h>
-#include <arch/rv32im/irq.h>
+#include <arch/common/irq.h>
Review comment:
```suggestion
#include <arch/irq.h>
```
##########
File path: arch/risc-v/src/k210/k210_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ extern void up_fault(int irq, uint64_t *regs);
* k210_dispatch_irq
****************************************************************************/
-void *k210_dispatch_irq(uint64_t vector, uint64_t *regs)
+void *k210_dispatch_irq(uint64_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> (27 + 32)) | (vector & 0xf);
Review comment:
```suggestion
uintptr_t irq = (vector >> (27 + 32)) | (vector & 0xf);
```
##########
File path: arch/risc-v/src/litex/litex_irq_dispatch.c
##########
@@ -41,7 +41,7 @@
* Public Data
****************************************************************************/
-volatile uint32_t * g_current_regs;
+volatile uintptr_t *g_current_regs;
Review comment:
```suggestion
volatile uintptr_t *g_current_regs[1];
```
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -111,13 +111,13 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
#endif
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
- g_current_regs = NULL;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/k210/k210_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ extern void up_fault(int irq, uint64_t *regs);
* k210_dispatch_irq
****************************************************************************/
-void *k210_dispatch_irq(uint64_t vector, uint64_t *regs)
+void *k210_dispatch_irq(uint64_t vector, uintptr_t *regs)
Review comment:
```suggestion
void *k210_dispatch_irq(uintptr_t vector, uintptr_t *regs)
```
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
Review comment:
```suggestion
uintptr_t irq = (vector >> 27) | (vector & 0xf);
```
##########
File path: arch/risc-v/src/k210/k210_irq_dispatch.c
##########
@@ -120,7 +120,7 @@ void *k210_dispatch_irq(uint64_t vector, uint64_t *regs)
* switch occurred during interrupt processing.
*/
- regs = (uint64_t *)CURRENT_REGS;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
Review comment:
```suggestion
void *fe310_dispatch_irq(uintptr_t vector, uintptr_t *regs)
```
##########
File path: arch/risc-v/src/bl602/bl602_irq_dispatch.c
##########
@@ -121,13 +121,13 @@ void *bl602_dispatch_irq(uint32_t vector, uint32_t *regs)
#endif /* CONFIG_SUPPRESS_INTERRUPTS */
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
- g_current_regs = NULL;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_irq.c
##########
@@ -436,8 +436,8 @@ IRAM_ATTR uint32_t *esp32c3_dispatch_irq(uint32_t mcause, uint32_t *regs)
}
}
- regs = (uint32_t *)g_current_regs;
- g_current_regs = NULL;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
```suggestion
regs = CURRENT_REGS;
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_irq.c
##########
@@ -390,14 +390,14 @@ IRAM_ATTR uint32_t *esp32c3_dispatch_irq(uint32_t mcause, uint32_t *regs)
}
else
{
- /* Check "g_current_regs" only in interrupt or ecall */
+ /* Check "CURRENT_REGS" only in interrupt or ecall */
- DEBUGASSERT(g_current_regs == NULL);
+ DEBUGASSERT(CURRENT_REGS == NULL);
}
- g_current_regs = regs;
+ CURRENT_REGS = (uintptr_t *)regs;
Review comment:
```suggestion
CURRENT_REGS = regs;
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780866814
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
We can test it with FE310 and qemu-rv32 on 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] no1wudi commented on a change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780760601
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
Not only 4 or 8 but load/store instruction should also different (sd/ld vs sw/lw)
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780977319
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
Review comment:
```suggestion
#define INT_REG_SIZE 4
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780811347
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
Review comment:
remove two space before 4/8 for alignment
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
Review comment:
But, there is code support QPFPU:
https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/common/riscv_fpu.S#L55-L64
it's better to add ARCH_QFPU to Kconfig instead.
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
Yes, but we can define:
```
#ifdef CONFIG_ARCH_RV32
# define REGLOAD lw
# define REGSTORE sw
#else
# define REGLOAD ld
# define REGSTORE sd
#endif
```
like what it done in https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/common/riscv_fpu.S.
Maybe, we can move the macro to up_internal.h
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780524344
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780626571
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Yes, all of these can be replaced by uintptr_t
##########
File path: arch/risc-v/src/common/riscv_fault.c
##########
@@ -54,61 +66,61 @@
*
****************************************************************************/
-void up_fault(int irq, uint64_t *regs)
+void up_fault(int irq, uintptr_t *regs)
{
CURRENT_REGS = regs;
- _alert("EPC:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n",
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] pkarashchenko commented on a change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r782059779
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
Review comment:
```suggestion
# if defined(CONFIG_ARCH_DPFPU)
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
Review comment:
```suggestion
# define INT_REG_SIZE 8
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#if defined(CONFIG_ARCH_QPFPU)
Review comment:
```suggestion
# if defined(CONFIG_ARCH_QPFPU)
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
Review comment:
```suggestion
# define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
Review comment:
```suggestion
# define INT_REG_SIZE 4
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
Review comment:
```suggestion
# define INT_XCPT_REGS 33
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
Review comment:
```suggestion
# elif defined(CONFIG_ARCH_FPU)
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
Review comment:
```suggestion
# elif defined(CONFIG_ARCH_QPFPU)
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#if defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 2
+#else
+# define FPU_REG_SIZE 1
+#endif
Review comment:
```suggestion
# endif
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#if defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 2
+#else
Review comment:
```suggestion
# else
```
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
Review comment:
```suggestion
# endif
```
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780524344
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
or
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# define FPU_REG_SIZE (1 / REG_SIZE)
#endif
#if FPU_REG_SIZE == 0
# error not supported !!!
#endif
```
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780634166
##########
File path: arch/risc-v/src/mpfs/mpfs_head.S
##########
@@ -24,7 +24,7 @@
#include <nuttx/config.h>
#include <arch/csr.h>
-#include <arch/rv64gc/irq.h>
+#include <arch/common/irq.h>
Review comment:
```suggestion
#include <arch/irq.h>
```
##########
File path: arch/risc-v/src/rv32m1/rv32m1_head.S
##########
@@ -23,7 +23,7 @@
****************************************************************************/
#include <nuttx/config.h>
-#include <arch/rv32im/irq.h>
+#include <arch/common/irq.h>
Review comment:
```suggestion
#include <arch/irq.h>
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780673700
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
Review comment:
Yes, and CONFIG_ARCH_QPFPU had been removed since there are no actual support for qpfpu
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780977216
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
Review comment:
```suggestion
#define INT_REG_SIZE 8
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743676
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#define FPU_REG_SIZE 1
Review comment:
That means FPU in thread/task context will consume 64bit per register like DFPU, may waste some memory (33 x 4 bytes) but can simplify the logic.
I guess it's not a problem for 64bit platform.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743635
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
ECALL instructin is alwasy 4 byte in spec, , so we must update the epc adjustment.
+ 2 in my previous PR is wrong but works with some unknown reason.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743660
##########
File path: arch/risc-v/src/bl602/bl602_irq_dispatch.c
##########
@@ -67,7 +67,7 @@ void *bl602_dispatch_irq(uint32_t vector, uint32_t *regs)
if (BL602_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
See above.
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780531230
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -154,21 +139,6 @@ exception_common:
lw s0, 32*4(sp) /* restore mstatus */
csrw mstatus, s0
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- lw x28, 36*4(sp)
- lw x29, 37*4(sp)
- lw x30, 38*4(sp)
- csrrw x0, 0x7b4, x28
- csrrw x0, 0x7b5, x29
- csrrw x0, 0x7b6, x30
- lw x28, 33*4(sp)
- lw x29, 34*4(sp)
- lw x30, 35*4(sp)
- csrrw x0, 0x7b0, x28
- csrrw x0, 0x7b1, x29
- csrrw x0, 0x7b2, x30
-#endif
Review comment:
Why this is removed?
##########
File path: arch/risc-v/src/qemu-rv32/qemu_rv32_head.S
##########
@@ -105,21 +105,6 @@ exception_common:
sw x30, 30*4(sp) /* t5 */
sw x31, 31*4(sp) /* t6 */
-#if defined(INT_XCPT_REGS) && INT_XCPT_REGS >= 39
- csrr x28, 0x7b0
- csrr x29, 0x7b1
- csrr x30, 0x7b2
- sw x28, 33*4(sp)
- sw x29, 34*4(sp)
- sw x30, 35*4(sp)
- csrr x28, 0x7b4
- csrr x29, 0x7b5
- csrr x30, 0x7b6
- sw x28, 36*4(sp)
- sw x29, 37*4(sp)
- sw x30, 38*4(sp)
-#endif
-
Review comment:
Why this is removed?
##########
File path: arch/risc-v/src/rv32m1/rv32m1_irq_dispatch.c
##########
@@ -53,11 +53,11 @@ volatile uint32_t * g_current_regs;
****************************************************************************/
LOCATE_ITCM
-void *rv32m1_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *rv32m1_dispatch_irq(uintptr_t vector, uintptr_t *regs)
{
int vec = vector & 0x1f;
int irq = (vector >> 27) + vec;
Review comment:
```suggestion
uintptr_t vec = vector & 0x1f;
uintptr_t irq = (vector >> 27) + vec;
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780722927
##########
File path: arch/risc-v/src/litex/litex_irq_dispatch.c
##########
@@ -114,13 +114,13 @@ void *litex_dispatch_irq(uint32_t vector, uint32_t *regs)
#endif
/* If a context switch occurred while processing the interrupt then
- * g_current_regs may have change value. If we return any value different
+ * CURRENT_REGS may have change value. If we return any value different
* from the input regs, then the lower level will know that a context
* switch occurred during interrupt processing.
*/
- regs = (uint32_t *)g_current_regs;
- g_current_regs = NULL;
+ regs = (uintptr_t *)CURRENT_REGS;
Review comment:
We should keep this cast to avoid warning "assignment discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers]".
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780521089
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
Review comment:
32-bit variant had
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE 2
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE 4
#else
# define FPU_REG_SIZE 1
#endif
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780346942
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -1,5 +1,5 @@
/****************************************************************************
- * arch/risc-v/include/rv64gc/irq.h
+ * arch/risc-v/include/common/irq.h
Review comment:
move to the upper level
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -121,7 +121,13 @@
#define INT_XCPT_REGS 33
#endif
-#define INT_XCPT_SIZE (8 * INT_XCPT_REGS)
+#ifdef CONFIG_ARCH_RV32
+#define INT_REG_SIZE 4
+#else
+#define INT_REG_SIZE 8
+#endif
Review comment:
#define INT_REG_SIZE sizeof(uinptr_t)
##########
File path: arch/risc-v/src/k210/k210_irq.c
##########
@@ -46,9 +46,9 @@
* such value for each processor that can receive an interrupt.
*/
-volatile uint64_t *g_current_regs[CONFIG_SMP_NCPUS];
+volatile uintptr_t *g_current_regs[CONFIG_SMP_NCPUS];
Review comment:
let define CURRENT_REG macro like arm to simplify SMP/UP handing
##########
File path: arch/risc-v/src/k210/k210_irq.c
##########
@@ -46,9 +46,9 @@
* such value for each processor that can receive an interrupt.
*/
-volatile uint64_t *g_current_regs[CONFIG_SMP_NCPUS];
+volatile uintptr_t *g_current_regs[CONFIG_SMP_NCPUS];
#else
-volatile uint64_t *g_current_regs[1];
+volatile uintptr_t *g_current_regs;
Review comment:
keep [1] to unify SMP and UP
##########
File path: arch/risc-v/src/qemu-rv32/Make.defs
##########
@@ -25,7 +25,7 @@ HEAD_ASRC = qemu_rv32_head.S
# Specify our general Assembly files
CHIP_ASRCS = qemu_rv32_vectors.S
-CMN_ASRCS += riscv_testset.S
+# CMN_ASRCS += riscv_testset.S
Review comment:
why comment out
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780626530
##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -37,6 +37,18 @@
#include "riscv_internal.h"
#include "riscv_arch.h"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "%08"PRIxPTR
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] no1wudi commented on a change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r781958691
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
I prefer to do this in next PR since it's hard to use macro in inline assembly function, it's better to implement a pure assembly function instead.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780673385
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Removed, REG_SIZE is unused
##########
File path: arch/risc-v/src/common/riscv_internal.h
##########
@@ -171,8 +160,8 @@ void riscv_ack_irq(int irq);
void riscv_copystate(uint64_t *dest, uint64_t *src);
void riscv_copyfullstate(uint64_t *dest, uint64_t *src);
#else
-void riscv_copystate(uint32_t *dest, uint32_t *src);
-void riscv_copyfullstate(uint32_t *dest, uint32_t *src);
+void riscv_copystate(uintptr_t *dest, uintptr_t *src);
+void riscv_copyfullstate(uintptr_t *dest, uintptr_t *src);
#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] no1wudi commented on a change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780624976
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -121,7 +121,13 @@
#define INT_XCPT_REGS 33
#endif
-#define INT_XCPT_SIZE (8 * INT_XCPT_REGS)
+#ifdef CONFIG_ARCH_RV32
+#define INT_REG_SIZE 4
+#else
+#define INT_REG_SIZE 8
+#endif
Review comment:
Can't do this since uintptr_t is invalid in assembly.
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780389520
##########
File path: arch/risc-v/src/common/riscv_fault.c
##########
@@ -54,61 +66,61 @@
*
****************************************************************************/
-void up_fault(int irq, uint64_t *regs)
+void up_fault(int irq, uintptr_t *regs)
{
CURRENT_REGS = regs;
- _alert("EPC:%016" PRIx64 "\n",
+ _alert("EPC:%0" PRIxREG "\n",
Review comment:
Maybe move `"0"` to `PRIxREG`?
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -52,10 +52,10 @@ volatile uint32_t * g_current_regs;
* fe310_dispatch_irq
****************************************************************************/
-void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *fe310_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Maybe can switch to `uintptr_t` here at well? Or it is not possible?
##########
File path: arch/risc-v/src/k210/k210_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ extern void up_fault(int irq, uint64_t *regs);
* k210_dispatch_irq
****************************************************************************/
-void *k210_dispatch_irq(uint64_t vector, uint64_t *regs)
+void *k210_dispatch_irq(uint64_t vector, uintptr_t *regs)
Review comment:
Can we go with `uintptr_t vector`?
##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -37,6 +37,18 @@
#include "riscv_internal.h"
#include "riscv_arch.h"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "%08"PRIxPTR
Review comment:
I recommend to leave `"%"` in format string and move here to
```suggestion
# define PRIxREG "08" PRIxPTR
```
Same in other places
##########
File path: arch/risc-v/src/litex/litex_irq_dispatch.c
##########
@@ -51,10 +51,10 @@ volatile uint32_t * g_current_regs;
* litex_dispatch_irq
****************************************************************************/
-void *litex_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *litex_dispatch_irq(uint32_t vector, uintptr_t *regs)
{
uint32_t irq = (vector >> 27) | (vector & 0xf);
- uint32_t *mepc = regs;
+ uint32_t *mepc = (uint32_t *)regs;
Review comment:
Can we go with `uintprt_t *mepc`?
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
Review comment:
In 32 bit variant of include we had different vales for FPU registers.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743635
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
ECALL instructin is alwasy 4 byte in spec, , so we must update the epc adjustment.
+ 2 in my previous PR is wrong with works with some unknown reason.
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780524344
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -134,6 +140,14 @@
# error not supported !!!
#endif
+/* REG size */
+
+#ifdef CONFIG_ARCH_RV32
+#define REG_SIZE 1
+#else
+#define REG_SIZE 2
+#endif
Review comment:
Where this is used? Probably the idea was to define this before `#ifdef CONFIG_ARCH_FPU` and have a next code:
```
#if defined(CONFIG_ARCH_DPFPU)
# define FPU_REG_SIZE (2 / REG_SIZE)
#elif defined(CONFIG_ARCH_QPFPU)
# define FPU_REG_SIZE (4 / REG_SIZE)
#else
# if REG_SIZE > 1
# error not supported !!!
# else
# define FPU_REG_SIZE 1
# endif
#endif
```
??
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780529480
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -455,7 +455,7 @@ int riscv_swint(int irq, void *context, void *arg)
rtcb->flags |= TCB_FLAG_SYSCALL;
#else
- svcerr("ERROR: Bad SYS call: %" PRId64 "\n", regs[REG_A0]);
+ svcerr("ERROR: Bad SYS call: %" PRIdPTR "\n", regs[REG_A0]);
Review comment:
Maybe can switch to `PRIxREG` here.
Also rework `static void riscv_registerdump(const uint64_t *regs)` to use `uintptr_t`.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780756114
##########
File path: arch/risc-v/include/mcause.h
##########
@@ -18,8 +18,8 @@
*
****************************************************************************/
-#ifndef __ARCH_RISCV_INCLUDE_RV32IM_MCAUSE_H
-#define __ARCH_RISCV_INCLUDE_RV32IM_MCAUSE_H
+#ifndef __ARCH_RISCV_INCLUDE_MCAUSE_H
+#define __ARCH_RISCV_INCLUDE_MCAUSE_H
Review comment:
update the end of file too
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#define FPU_REG_SIZE 1
Review comment:
add space after #
##########
File path: arch/risc-v/src/common/riscv_assert.c
##########
@@ -106,35 +106,35 @@ static inline void riscv_registerdump(volatile uintptr_t *regs)
{
/* Are user registers available from interrupt processing? */
- _alert("EPC:" PRIxREG "\n", regs[REG_EPC]);
- _alert("A0:" PRIxREG " A1:" PRIxREG "A2:" PRIxREG
- " A3:" PRIxREG "\n",
+ _alert("EPC:%" PRIxREG "\n", regs[REG_EPC]);
Review comment:
should we add space after reach ":"?
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -51,29 +63,34 @@
****************************************************************************/
#ifdef CONFIG_DEBUG_SYSCALL_INFO
-static void riscv_registerdump(const uint64_t *regs)
+static void riscv_registerdump(const uintptr_t *regs)
{
- svcinfo("EPC:%08x\n",
+ svcinfo("EPC:%" PRIxREG "\n",
Review comment:
ditto
##########
File path: arch/risc-v/src/common/riscv_fault.c
##########
@@ -54,61 +66,61 @@
*
****************************************************************************/
-void up_fault(int irq, uint64_t *regs)
+void up_fault(int irq, uintptr_t *regs)
{
CURRENT_REGS = regs;
- _alert("EPC:%016" PRIx64 "\n",
+ _alert("EPC:%" PRIxREG "\n",
Review comment:
ditto
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
can we use macro(4 or 8) here to avoid the code dup?
##########
File path: arch/risc-v/src/common/riscv_schedulesigaction.c
##########
@@ -37,6 +37,18 @@
#include "riscv_internal.h"
#include "riscv_arch.h"
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Format output with register width and hex */
+
+#ifdef CONFIG_ARCH_RV32
+# define PRIxREG "08" PRIxPTR
Review comment:
should we move this macro to up_internal.h to avoid the dup?
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r781153367
##########
File path: arch/risc-v/src/rv32m1/rv32m1_irq_dispatch.c
##########
@@ -53,11 +53,11 @@ volatile uint32_t * g_current_regs;
****************************************************************************/
LOCATE_ITCM
-void *rv32m1_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *rv32m1_dispatch_irq(uintptr_t vector, uintptr_t *regs)
{
int vec = vector & 0x1f;
int irq = (vector >> 27) + vec;
Review comment:
This is still not addressed
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r781226655
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
Review comment:
add two space for alignment
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
Review comment:
intent four space
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
Review comment:
let's order #if by CONFIG_ARCH_QPFPU/CONFIG_ARCH_DPFPU
##########
File path: arch/risc-v/src/rv32m1/rv32m1_irq_dispatch.c
##########
@@ -53,26 +53,26 @@ volatile uint32_t * g_current_regs;
****************************************************************************/
LOCATE_ITCM
-void *rv32m1_dispatch_irq(uint32_t vector, uint32_t *regs)
+void *rv32m1_dispatch_irq(uintptr_t vector, uintptr_t *regs)
{
- int vec = vector & 0x1f;
- int irq = (vector >> 27) + vec;
- uint32_t *mepc = regs;
+ uintptr_t vec = vector & 0x1f;
+ uintptr_t irq = (vector >> 27) + vec;
+ uintptr_t *mepc = regs;
int irqofs = 0;
/* NOTE: In case of ecall, we need to adjust mepc in the context */
if (RV32M1_IRQ_ECALL_M == irq)
{
- *mepc += 2;
+ *mepc += 4;
}
if (RV32M1_IRQ_INTMUX0 <= irq)
{
- uint32_t const chn = irq - RV32M1_IRQ_INTMUX0;
- uint32_t regaddr = RV32M1_INTMUX_CH_BASE(chn) + INTMUX_CH_VEC_OFFSET;
- uint32_t regval = getreg32(regaddr);
+ uintptr_t const chn = irq - RV32M1_IRQ_INTMUX0;
Review comment:
remove const
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,21 +31,457 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_QPFPU)
+# define FPU_REG_SIZE 4
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
Review comment:
intent two space
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r782072589
##########
File path: arch/risc-v/src/common/riscv_swint.c
##########
@@ -116,6 +134,26 @@ static void dispatch_syscall(void)
" ecall" /* Return from the syscall */
);
}
+#else
+static void dispatch_syscall(void)
+{
+ asm volatile
+ (
+ " addi sp, sp, -4\n" /* Create a stack frame to hold ra */
Review comment:
Should be relatively simple by:
```
#define STR_INSTR(n) #n
#define INSTR(n) STR_INSTR(n)
#ifdef CONFIG_ARCH_RV32
# define REGLOAD lw
# define REGSTORE sw
#else
# define REGLOAD ld
# define REGSTORE sd
#endif
```
and then
```
static void dispatch_syscall(void)
{
asm volatile
(
" addi sp, sp, -4\n" /* Create a stack frame to hold ra */
" " INSTR(REGSTORE) " ra, 0(sp)\n" /* Save ra in the stack frame */
" la t0, g_stublookup\n" /* t0=The base of the stub lookup table */
" slli a0, a0, 3\n" /* a0=Offset for the stub lookup table */
" add t0, t0, a0\n" /* t0=The address in the table */
" " INSTR(REGLOAD) " t0, 0(t0)\n" /* t0=The address of the stub for this syscall */
" jalr ra, t0\n" /* Call the stub (modifies ra) */
" " INSTR(REGLOAD) " ra, 0(sp)\n" /* Restore ra */
" addi sp, sp, 4\n" /* Destroy the stack frame */
" mv a2, a0\n" /* a2=Save return value in a0 */
" li a0, 3\n" /* a0=SYS_syscall_return (3) */
" ecall" /* Return from the syscall */
);
}
```
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780346942
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -1,5 +1,5 @@
/****************************************************************************
- * arch/risc-v/include/rv64gc/irq.h
+ * arch/risc-v/include/common/irq.h
Review comment:
move to the upper level
##########
File path: arch/risc-v/include/common/irq.h
##########
@@ -121,7 +121,13 @@
#define INT_XCPT_REGS 33
#endif
-#define INT_XCPT_SIZE (8 * INT_XCPT_REGS)
+#ifdef CONFIG_ARCH_RV32
+#define INT_REG_SIZE 4
+#else
+#define INT_REG_SIZE 8
+#endif
Review comment:
#define INT_REG_SIZE sizeof(uinptr_t)
##########
File path: arch/risc-v/src/k210/k210_irq.c
##########
@@ -46,9 +46,9 @@
* such value for each processor that can receive an interrupt.
*/
-volatile uint64_t *g_current_regs[CONFIG_SMP_NCPUS];
+volatile uintptr_t *g_current_regs[CONFIG_SMP_NCPUS];
Review comment:
let define CURRENT_REG macro like arm to simplify SMP/UP handing
##########
File path: arch/risc-v/src/k210/k210_irq.c
##########
@@ -46,9 +46,9 @@
* such value for each processor that can receive an interrupt.
*/
-volatile uint64_t *g_current_regs[CONFIG_SMP_NCPUS];
+volatile uintptr_t *g_current_regs[CONFIG_SMP_NCPUS];
#else
-volatile uint64_t *g_current_regs[1];
+volatile uintptr_t *g_current_regs;
Review comment:
keep [1] to unify SMP and UP
##########
File path: arch/risc-v/src/qemu-rv32/Make.defs
##########
@@ -25,7 +25,7 @@ HEAD_ASRC = qemu_rv32_head.S
# Specify our general Assembly files
CHIP_ASRCS = qemu_rv32_vectors.S
-CMN_ASRCS += riscv_testset.S
+# CMN_ASRCS += riscv_testset.S
Review comment:
why comment out
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780842652
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
I don't know if this was ever needed for RV32 chips since the ECALL handler does the same thing:
https://github.com/apache/incubator-nuttx/blob/1fd51ccbe244b554386baa78d1922af5dd89d835/arch/risc-v/src/rv32im/riscv_swint.c#L138-L141
For instance, we didn't do that for ESP32-C3, but we couldn't change it for other chips since we don't have boards to test.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743676
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#define FPU_REG_SIZE 1
Review comment:
That means FPU in thread/task context will consume 64bit per register like DFPU, may waste some memory (32 x 4 bytes) but can simplify the logic.
I guess it's not a problem for 64bit platform.
--
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 #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
no1wudi commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780743635
##########
File path: arch/risc-v/src/fe310/fe310_irq_dispatch.c
##########
@@ -72,7 +72,7 @@ void *fe310_dispatch_irq(uint32_t vector, uint32_t *regs)
if (FE310_IRQ_ECALLM == irq)
{
- *mepc += 2;
+ *mepc += 4;
Review comment:
I find that ecall instruction is 4 byte even with C (compress) extension now by disassembly, so we must update the epc adjustment.
--
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 change in pull request #5192: arch/risc-v: Merge rv32im and rv64gc into common
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5192:
URL: https://github.com/apache/incubator-nuttx/pull/5192#discussion_r780741621
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
Review comment:
So there is not QFPU case as in the original file?
##########
File path: arch/risc-v/include/irq.h
##########
@@ -31,20 +31,450 @@
/* Include chip-specific IRQ definitions (including IRQ numbers) */
+#include <nuttx/config.h>
+#include <arch/types.h>
+
+#ifndef __ASSEMBLY__
#include <stdint.h>
#include <nuttx/irq.h>
#include <arch/csr.h>
#include <arch/chip/irq.h>
+#endif
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Configuration ************************************************************/
+
+/* If this is a kernel build, how many nested system calls should we
+ * support?
+ */
+
+#ifndef CONFIG_SYS_NNEST
+# define CONFIG_SYS_NNEST 2
+#endif
+
+/* Processor PC */
+
+#define REG_EPC_NDX 0
+
+/* General pupose registers
+ * $0: Zero register does not need to be saved
+ * $1: ra (return address)
+ */
+
+#define REG_X1_NDX 1
+
+/* $2: Stack POinter
+ * $3: Global Pointer
+ * $4: Thread Pointer
+ */
+
+#define REG_X2_NDX 2
+#define REG_X3_NDX 3
+#define REG_X4_NDX 4
+
+/* $5-$7 = t0-t3: Temporary registers */
+
+#define REG_X5_NDX 5
+#define REG_X6_NDX 6
+#define REG_X7_NDX 7
+
+/* $8: s0 / fp Frame pointer */
+
+#define REG_X8_NDX 8
+
+/* $89 s1 Saved register */
+
+#define REG_X9_NDX 9
+
+/* $10-$17 = a0-a7: Argument registers */
+
+#define REG_X10_NDX 10
+#define REG_X11_NDX 11
+#define REG_X12_NDX 12
+#define REG_X13_NDX 13
+#define REG_X14_NDX 14
+#define REG_X15_NDX 15
+#define REG_X16_NDX 16
+#define REG_X17_NDX 17
+
+/* $18-$27 = s2-s11: Saved registers */
+
+#define REG_X18_NDX 18
+#define REG_X19_NDX 19
+#define REG_X20_NDX 20
+#define REG_X21_NDX 21
+#define REG_X22_NDX 22
+#define REG_X23_NDX 23
+#define REG_X24_NDX 24
+#define REG_X25_NDX 25
+#define REG_X26_NDX 26
+#define REG_X27_NDX 27
+
+/* $28-31 = t3-t6: Temporary (Volatile) registers */
+
+#define REG_X28_NDX 28
+#define REG_X29_NDX 29
+#define REG_X30_NDX 30
+#define REG_X31_NDX 31
+
+/* Interrupt Context register */
+
+#define REG_INT_CTX_NDX 32
+
+#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+ #define INT_XCPT_REGS (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#else
+ #define INT_XCPT_REGS 33
+#endif
+
+#ifdef CONFIG_ARCH_RV32
+ #define INT_REG_SIZE 4
+#else
+ #define INT_REG_SIZE 8
+#endif
+
+#define INT_XCPT_SIZE (INT_REG_SIZE * INT_XCPT_REGS)
+
+#ifdef CONFIG_ARCH_RV32
+#if defined(CONFIG_ARCH_DPFPU)
+# define FPU_REG_SIZE 2
+#elif defined(CONFIG_ARCH_FPU)
+# define FPU_REG_SIZE 1
+#endif
+#else
+#define FPU_REG_SIZE 1
Review comment:
```suggestion
# define FPU_REG_SIZE 1
```
So 64 bit case does not have QFPU?
Is the DFPU the only case for 64bit case?
--
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