lib: Don't return any invalid error from SBI ecall

We should only return valid error codes from SBI ecalls as defined by the RISC-V SBI spec. To achieve this: 1. We use SBI_Exxxx defines for OpenSBI internal errors with error values starting from -1000 2. We use SBI_ERR_xxxx defines for errors defined by SBI spec 3. We map some of the SBI_Exxxx defines to SBI_ERR_xxxx defines which are semantically same 4. We throw a error print and force return error code to SBI_ERR_FAILED in sbi_ecall_handler() if we see an invalid error code being returned to S-mode Signed-off-by: Anup Patel <[email protected]> Reviewed-by: Atish Patra <[email protected]>
riscv-software-src · Jun 20, 2020 · 2314101 · 2314101
1 parent 9bd5f8f
commit 2314101
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 20 deletions.
diff --git a/include/sbi/sbi_ecall_interface.h b/include/sbi/sbi_ecall_interface.h
@@ -13,7 +13,7 @@
 /* clang-format off */
 
 /* SBI Extension IDs */
-#define	SBI_EXT_0_1_SET_TIMER			0x0
+#define SBI_EXT_0_1_SET_TIMER			0x0
 #define SBI_EXT_0_1_CONSOLE_PUTCHAR		0x1
 #define SBI_EXT_0_1_CONSOLE_GETCHAR		0x2
 #define SBI_EXT_0_1_CLEAR_IPI			0x3
@@ -69,6 +69,18 @@
 #define SBI_EXT_VENDOR_END			0x09FFFFFF
 #define SBI_EXT_FIRMWARE_START			0x0A000000
 #define SBI_EXT_FIRMWARE_END			0x0AFFFFFF
+
+/* SBI return error codes */
+#define SBI_SUCCESS				0
+#define SBI_ERR_FAILED				-1
+#define SBI_ERR_NOT_SUPPORTED			-2
+#define SBI_ERR_INVALID_PARAM			-3
+#define SBI_ERR_DENIED				-4
+#define SBI_ERR_INVALID_ADDRESS			-5
+#define SBI_ERR_ALREADY_AVAILABLE		-6
+
+#define SBI_LAST_ERR				SBI_ERR_ALREADY_AVAILABLE
+
 /* clang-format on */
 
 #endif
diff --git a/include/sbi/sbi_error.h b/include/sbi/sbi_error.h
@@ -10,25 +10,28 @@
 #ifndef __SBI_ERROR_H__
 #define __SBI_ERROR_H__
 
+#include <sbi/sbi_ecall_interface.h>
+
 /* clang-format off */
 
 #define SBI_OK			0
-#define SBI_EFAIL		-1
-#define SBI_ENOTSUPP		-2
-#define SBI_EINVAL		-3
-#define SBI_DENIED		-4
-#define SBI_INVALID_ADDR	-5
-#define SBI_ENODEV		-6
-#define SBI_ENOSYS		-7
-#define SBI_ETIMEDOUT		-8
-#define SBI_EIO			-9
-#define SBI_EILL		-10
-#define SBI_ENOSPC		-11
-#define SBI_ENOMEM		-12
-#define SBI_ETRAP		-13
-#define SBI_EUNKNOWN		-14
-#define SBI_ENOENT		-15
-#define SBI_EALREADY_STARTED	-16
+#define SBI_EFAIL		SBI_ERR_FAILED
+#define SBI_ENOTSUPP		SBI_ERR_NOT_SUPPORTED
+#define SBI_EINVAL		SBI_ERR_INVALID_PARAM
+#define SBI_EDENIED		SBI_ERR_DENIED
+#define SBI_EINVALID_ADDR	SBI_ERR_INVALID_ADDRESS
+#define SBI_EALREADY		SBI_ERR_ALREADY_AVAILABLE
+
+#define SBI_ENODEV		-1000
+#define SBI_ENOSYS		-1001
+#define SBI_ETIMEDOUT		-1002
+#define SBI_EIO			-1003
+#define SBI_EILL		-1004
+#define SBI_ENOSPC		-1005
+#define SBI_ENOMEM		-1006
+#define SBI_ETRAP		-1007
+#define SBI_EUNKNOWN		-1008
+#define SBI_ENOENT		-1009
 
 /* clang-format on */
 

diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
@@ -7,6 +7,7 @@
  *   Anup Patel <[email protected]>
  */
 
+#include <sbi/sbi_console.h>
 #include <sbi/sbi_ecall.h>
 #include <sbi/sbi_ecall_interface.h>
 #include <sbi/sbi_error.h>
@@ -124,6 +125,13 @@ int sbi_ecall_handler(struct sbi_trap_regs *regs)
 		trap.epc = regs->mepc;
 		sbi_trap_redirect(regs, &trap);
 	} else {
+		if (ret < SBI_LAST_ERR) {
+			sbi_printf("%s: Invalid error %d for ext=0x%lx "
+				   "func=0x%lx\n", __func__, ret,
+				   extension_id, func_id);
+			ret = SBI_ERR_FAILED;
+		}
+
 		/*
 		 * This function should return non-zero value only in case of
 		 * fatal error. However, there is no good way to distinguish

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
@@ -200,7 +200,7 @@ int sbi_hart_pmp_check_addr(struct sbi_scratch *scratch, unsigned long addr,
 			continue;
 		if (tempaddr <= addr && addr <= tempaddr + size)
 			if (!(prot & attr))
-				return SBI_INVALID_ADDR;
+				return SBI_EINVALID_ADDR;
 	}
 
 	return SBI_OK;

diff --git a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c
@@ -219,7 +219,7 @@ int sbi_hsm_hart_start(struct sbi_scratch *scratch, u32 hartid,
 	hstate = atomic_cmpxchg(&hdata->state, SBI_HART_STOPPED,
 				SBI_HART_STARTING);
 	if (hstate == SBI_HART_STARTED)
-		return SBI_EALREADY_STARTED;
+		return SBI_EALREADY;
 
 	/**
 	 * if a hart is already transition to start or stop, another start call
@@ -263,7 +263,7 @@ int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow)
 	if (oldstate != SBI_HART_STARTED) {
 		sbi_printf("%s: ERR: The hart is in invalid state [%u]\n",
 			   __func__, oldstate);
-		return SBI_DENIED;
+		return SBI_EDENIED;
 	}
 
 	if (exitnow)