Skip to contentSkip to conversation

OpenSBI debug console support

PATCH Superseded Anup Patel 1 participant 10 messages Feb 10, 2023

lore
Share & tools
lore

Anup Patel

author
#0 lore raw

v6 cover OpenSBI debug console support

The latest proposal of SBI debug console extension can be found at:
https://lists.riscv.org/g/tech-prs/message/96

This series implements SBI debug console support in OpenSBI as-per above draft proposal.

To test these patches corresponding Linux patches can be found in riscv_sbi_dbcn_v1 branch at: https://github.com/avpatel/linux.git

These patches can also be found in riscv_sbi_dbcn_v6 branch at:
https://github.com/avpatel/opensbi.git

Changes since v5:
- Rebased on latest OpenSBI sources
- Updated Reviewed-by tags
- Addressed Drew's comments in all patches
- Updated SBI debug console implementation as-per latest proposal where console write() is non-blocking.

Changes since v4:
- Rebased on latest OpenSBI sources
- Updated Reviewed-by tags
- Added new PATCH8 to speed-up sbi_printf() using nputs()

Changes since v3:
- Rebased on OpenSBI v1.2 release
- Updated Reviewed-by tags
- Added new PATCH3 for sbi_ngets() function
- Updated SBI debug console implementation in PATCH5 as-per latest proposal having both console write() and read() functions.

Changes since v2:
- Reworked sbi_domain_check_addr_range() in PATCH3 to make it work for overlapping regions.

Changes since v1:
- New PATCH3 to implement sbi_domain_check_addr_range() function
- Added checks in PATCH4 for location and size of string to print
- New PATCH5 to add console_puts() callback in the console device
- New PATCH6 to implement console_puts() for semihosting

Anup Patel (8):
include: Add defines for SBI debug console extension
lib: sbi: Add sbi_nputs() function
lib: sbi: Add sbi_ngets() function
lib: sbi: Add sbi_domain_check_addr_range() function
lib: sbi: Implement SBI debug console extension
lib: sbi: Add console_puts() callback in the console device
lib: utils/serial: Implement console_puts() for semihosting
lib: sbi: Speed-up sbi_printf() and friends using nputs()

 include/sbi/sbi_console.h         |  7 +++
 include/sbi/sbi_domain.h          | 15 ++++++
 include/sbi/sbi_ecall_interface.h |  6 +++
 lib/sbi/Kconfig                   |  4 ++
 lib/sbi/objects.mk                |  3 ++
 lib/sbi/sbi_console.c             | 88 ++++++++++++++++++++++++++++---
 lib/sbi/sbi_domain.c              | 69 ++++++++++++++++++++++++
 lib/sbi/sbi_ecall_dbcn.c          | 72 +++++++++++++++++++++++++
 lib/utils/serial/semihosting.c    | 37 +++++++++++++
 9 files changed, 294 insertions(+), 7 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_dbcn.c
Signature · 1 line
2.34.1
author
#1 lore raw

PATCH v6 1/8 include: Add defines for SBI debug console extension

We add SBI debug console extension related defines to the SBI ecall interface header.

---
 include/sbi/sbi_ecall_interface.h | 6 ++++++
 1 file changed, 6 insertions(+)
include/sbi/sbi_ecall_interface.h +6 -0
index a3f2bf4..9d6f474 100644
@@ -29,6 +29,7 @@
 #define SBI_EXT_HSM				0x48534D
 #define SBI_EXT_SRST				0x53525354
 #define SBI_EXT_PMU				0x504D55
+#define SBI_EXT_DBCN				0x4442434E
 
 /* SBI function IDs for BASE extension*/
 #define SBI_EXT_BASE_GET_SPEC_VERSION		0x0
@@ -230,6 +231,11 @@ enum sbi_pmu_ctr_type {
 /* Flags defined for counter stop function */
 #define SBI_PMU_STOP_FLAG_RESET (1 << 0)
 
+/* SBI function IDs for DBCN extension */
+#define SBI_EXT_DBCN_CONSOLE_WRITE		0x0
+#define SBI_EXT_DBCN_CONSOLE_READ		0x1
+#define SBI_EXT_DBCN_CONSOLE_WRITE_BYTE		0x2
+
 /* SBI base specification related macros */
 #define SBI_SPEC_VERSION_MAJOR_OFFSET		24
 #define SBI_SPEC_VERSION_MAJOR_MASK		0x7f
Signature · 1 line
2.34.1
author
#2 lore raw

PATCH v6 2/8 lib: sbi: Add sbi_nputs() function

In reply to Anup Patel

We add new sbi_nputs() which help us print a fixed number of characters from a physical memory location.

---
 include/sbi/sbi_console.h |  2 ++
 lib/sbi/sbi_console.c     | 12 ++++++++++++
 2 files changed, 14 insertions(+)
include/sbi/sbi_console.h +2 -0
index e15b55d..dd6a905 100644
@@ -33,6 +33,8 @@ void sbi_putc(char ch);
 
 void sbi_puts(const char *str);
 
+unsigned long sbi_nputs(const char *str, unsigned long len);
+
 void sbi_gets(char *s, int maxwidth, char endchar);
 
 int __printf(2, 3) sbi_sprintf(char *out, const char *format, ...);
lib/sbi/sbi_console.c +12 -0
index bb6e1ef..2f83a26 100644
@@ -51,6 +51,18 @@ void sbi_puts(const char *str)
 	spin_unlock(&console_out_lock);
 }
 
+unsigned long sbi_nputs(const char *str, unsigned long len)
+{
+	unsigned long i;
+
+	spin_lock(&console_out_lock);
+	for (i = 0; i < len; i++)
+		sbi_putc(str[i]);
+	spin_unlock(&console_out_lock);
+
+	return i;
+}
+
 void sbi_gets(char *s, int maxwidth, char endchar)
 {
 	int ch;
Signature · 1 line
2.34.1
author
#3 lore raw

PATCH v6 3/8 lib: sbi: Add sbi_ngets() function

In reply to Anup Patel

We add new sbi_ngets() which help us read characters into a physical memory location.

---
 include/sbi/sbi_console.h |  2 ++
 lib/sbi/sbi_console.c     | 15 +++++++++++++++
 2 files changed, 17 insertions(+)
include/sbi/sbi_console.h +2 -0
index dd6a905..3c1f5e5 100644
@@ -37,6 +37,8 @@ unsigned long sbi_nputs(const char *str, unsigned long len);
 
 void sbi_gets(char *s, int maxwidth, char endchar);
 
+unsigned long sbi_ngets(char *str, unsigned long len);
+
 int __printf(2, 3) sbi_sprintf(char *out, const char *format, ...);
 
 int __printf(3, 4) sbi_snprintf(char *out, u32 out_sz, const char *format, ...);
lib/sbi/sbi_console.c +15 -0
index 2f83a26..c734bc0 100644
@@ -76,6 +76,21 @@ void sbi_gets(char *s, int maxwidth, char endchar)
 	*retval = '\0';
 }
 
+unsigned long sbi_ngets(char *str, unsigned long len)
+{
+	int ch;
+	unsigned long i;
+
+	for (i = 0; i < len; i++) {
+		ch = sbi_getc();
+		if (ch < 0)
+			break;
+		str[i] = ch;
+	}
+
+	return i;
+}
+
 #define PAD_RIGHT 1
 #define PAD_ZERO 2
 #define PAD_ALTERNATE 4
Signature · 1 line
2.34.1
author
#4 lore raw

PATCH v6 4/8 lib: sbi: Add sbi_domain_check_addr_range() function

In reply to Anup Patel

We add sbi_domain_check_addr_range() helper function to check whether a given address range is accessible under a particular domain.

---
 include/sbi/sbi_domain.h | 15 +++++++++
 lib/sbi/sbi_domain.c     | 69 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
include/sbi/sbi_domain.h +15 -0
index bbb3eff..ab1a944 100644
@@ -196,6 +196,21 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom,
 			   unsigned long addr, unsigned long mode,
 			   unsigned long access_flags);
 
+/**
+ * Check whether we can access specified address range for given mode and
+ * memory region flags under a domain
+ * @param dom pointer to domain
+ * @param addr the start of the address range to be checked
+ * @param size the size of the address range to be checked
+ * @param mode the privilege mode of access
+ * @param access_flags bitmask of domain access types (enum sbi_domain_access)
+ * @return TRUE if access allowed otherwise FALSE
+ */
+bool sbi_domain_check_addr_range(const struct sbi_domain *dom,
+				 unsigned long addr, unsigned long size,
+				 unsigned long mode,
+				 unsigned long access_flags);
+
 /** Dump domain details on the console */
 void sbi_domain_dump(const struct sbi_domain *dom, const char *suffix);
 
lib/sbi/sbi_domain.c +69 -0

81 lines not on this page · raw message

Signature · 1 line
2.34.1
author
#5 lore raw

PATCH v6 5/8 lib: sbi: Implement SBI debug console extension

In reply to Anup Patel

We implement SBI debug console extension as one of the replacement SBI extensions. This extension is only available when OpenSBI platform provides a console device to generic library.

---
 lib/sbi/Kconfig          |  4 +++
 lib/sbi/objects.mk       |  3 ++
 lib/sbi/sbi_ecall_dbcn.c | 72 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 lib/sbi/sbi_ecall_dbcn.c
lib/sbi/Kconfig +4 -0
index df74bba..ef6728b 100644
@@ -26,6 +26,10 @@ config SBI_ECALL_PMU
 	bool "Performance Monitoring Unit extension"
 	default y
 
+config SBI_ECALL_DBCN
+	bool "Debug Console extension"
+	default y
+
 config SBI_ECALL_LEGACY
 	bool "SBI v0.1 legacy extensions"
 	default y
lib/sbi/objects.mk +3 -0
index c774ebb..319f38d 100644
@@ -37,6 +37,9 @@ libsbi-objs-$(CONFIG_SBI_ECALL_SRST) += sbi_ecall_srst.o
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_PMU) += ecall_pmu
 libsbi-objs-$(CONFIG_SBI_ECALL_PMU) += sbi_ecall_pmu.o
 
+carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_DBCN) += ecall_dbcn
+libsbi-objs-$(CONFIG_SBI_ECALL_DBCN) += sbi_ecall_dbcn.o
+
 carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_LEGACY) += ecall_legacy
 libsbi-objs-$(CONFIG_SBI_ECALL_LEGACY) += sbi_ecall_legacy.o
 
lib/sbi/sbi_ecall_dbcn.c new file +72 -0

72 lines not on this page · raw message

Signature · 1 line
2.34.1
author
#6 lore raw

PATCH v6 6/8 lib: sbi: Add console_puts() callback in the console device

In reply to Anup Patel

We add console_puts() callback in the console device which allows console drivers (such as semihosting) to implement a specialized way to output character string.

---
 include/sbi/sbi_console.h |  3 +++
 lib/sbi/sbi_console.c     | 29 ++++++++++++++++++++++-------
 2 files changed, 25 insertions(+), 7 deletions(-)
include/sbi/sbi_console.h +3 -0
index 3c1f5e5..0979765 100644
@@ -19,6 +19,9 @@ struct sbi_console_device {
 	/** Write a character to the console output */
 	void (*console_putc)(char ch);
 
+	/** Write a character string to the console output */
+	unsigned long (*console_puts)(const char *str, unsigned long len);
+
 	/** Read a character from the console input */
 	int (*console_getc)(void);
 };
lib/sbi/sbi_console.c +22 -7
index c734bc0..8a5d9de 100644
@@ -12,6 +12,7 @@
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_platform.h>
 #include <sbi/sbi_scratch.h>
+#include <sbi/sbi_string.h>
 
 static const struct sbi_console_device *console_dev = NULL;
 static spinlock_t console_out_lock	       = SPIN_LOCK_INITIALIZER;
@@ -43,24 +44,38 @@ void sbi_putc(char ch)
 
 void sbi_puts(const char *str)
 {
+	unsigned long p, len;
+
 	spin_lock(&console_out_lock);
-	while (*str) {
-		sbi_putc(*str);
-		str++;
+	if (console_dev && console_dev->console_puts) {
+		p = 0;
+		len = sbi_strlen(str);
+		while (p < len)
+			p += console_dev->console_puts(&str[p], len - p);
+	} else {
+		while (*str) {
+			sbi_putc(*str);
+			str++;
+		}
 	}
 	spin_unlock(&console_out_lock);
 }
 
 unsigned long sbi_nputs(const char *str, unsigned long len)
 {
-	unsigned long i;
+	unsigned long i, ret;
 
 	spin_lock(&console_out_lock);
-	for (i = 0; i < len; i++)
-		sbi_putc(str[i]);
+	if (console_dev && console_dev->console_puts) {
+		ret = console_dev->console_puts(str, len);
+	} else {
+		for (i = 0; i < len; i++)
+			sbi_putc(str[i]);
+		ret = len;
+	}
 	spin_unlock(&console_out_lock);
 
-	return i;
+	return ret;
 }
 
 void sbi_gets(char *s, int maxwidth, char endchar)
Signature · 1 line
2.34.1
author
#7 lore raw

PATCH v6 7/8 lib: utils/serial: Implement console_puts() for semihosting

In reply to Anup Patel

We implement console_puts() for semihosting serial driver to speed-up semihosting based prints.

---
 lib/utils/serial/semihosting.c | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
lib/utils/serial/semihosting.c +37 -0
index 86fa296..ce65887 100644
@@ -15,6 +15,7 @@
 
 #define SYSOPEN     0x01
 #define SYSWRITEC   0x03
+#define SYSWRITE    0x05
 #define SYSREAD     0x06
 #define SYSREADC    0x07
 #define SYSERRNO	0x13
@@ -93,6 +94,7 @@ static int semihosting_errno(void)
 }
 
 static int semihosting_infd = SBI_ENODEV;
+static int semihosting_outfd = SBI_ENODEV;
 
 static long semihosting_open(const char *fname, enum semihosting_open_mode mode)
 {
@@ -141,6 +143,21 @@ static long semihosting_read(long fd, void *memp, size_t len)
 	return len - ret;
 }
 
+static long semihosting_write(long fd, const void *memp, size_t len)
+{
+	long ret;
+	struct semihosting_rdwr_s write;
+
+	write.fd = fd;
+	write.memp = (void *)memp;
+	write.len = len;
+
+	ret = semihosting_trap(SYSWRITE, &write);
+	if (ret < 0)
+		return semihosting_errno();
+	return len - ret;
+}
+
 /* clang-format on */
 
 static void semihosting_putc(char ch)
@@ -148,6 +165,24 @@ static void semihosting_putc(char ch)
 	semihosting_trap(SYSWRITEC, &ch);
 }
 
+static unsigned long semihosting_puts(const char *str, unsigned long len)
+{
+	char ch;
+	long ret;
+	unsigned long i;
+
+	if (semihosting_outfd < 0) {
+		for (i = 0; i < len; i++) {
+			ch = str[i];
+			semihosting_trap(SYSWRITEC, &ch);
+		}
+		ret = len;
+	} else
+		ret = semihosting_write(semihosting_outfd, str, len);
+
+	return (ret < 0) ? 0 : ret;
+}
+
 static int semihosting_getc(void)
 {
 	char ch = 0;
@@ -165,12 +200,14 @@ static int semihosting_getc(void)
 static struct sbi_console_device semihosting_console = {
 	.name = "semihosting",
 	.console_putc = semihosting_putc,
+	.console_puts = semihosting_puts,
 	.console_getc = semihosting_getc
 };
 
 int semihosting_init(void)
 {
 	semihosting_infd = semihosting_open(":tt", MODE_READ);
+	semihosting_outfd = semihosting_open(":tt", MODE_WRITE);
 
 	sbi_console_set_device(&semihosting_console);
 
Signature · 1 line
2.34.1
author
#8 lore raw

PATCH v6 8/8 lib: sbi: Speed-up sbi_printf() and friends using nputs()

In reply to Anup Patel

The sbi_printf() is slow for semihosting because it prints one character at a time. To speed-up sbi_printf() for semihosting, we use a temporary buffer and nputs().

---
 lib/sbi/sbi_console.c | 76 ++++++++++++++++++++++++++++++-------------
 1 file changed, 54 insertions(+), 22 deletions(-)
lib/sbi/sbi_console.c +54 -22
index 8a5d9de..f3ac003 100644
@@ -14,7 +14,11 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+#define CONSOLE_TBUF_MAX 256
+
 static const struct sbi_console_device *console_dev = NULL;
+static char console_tbuf[CONSOLE_TBUF_MAX];
+static u32 console_tbuf_len;
 static spinlock_t console_out_lock	       = SPIN_LOCK_INITIALIZER;
 
 bool sbi_isprintable(char c)
@@ -42,37 +46,44 @@ void sbi_putc(char ch)
 	}
 }
 
-void sbi_puts(const char *str)
+static unsigned long nputs(const char *str, unsigned long len)
 {
-	unsigned long p, len;
+	unsigned long i, ret;
 
-	spin_lock(&console_out_lock);
 	if (console_dev && console_dev->console_puts) {
-		p = 0;
-		len = sbi_strlen(str);
-		while (p < len)
-			p += console_dev->console_puts(&str[p], len - p);
+		ret = console_dev->console_puts(str, len);
 	} else {
-		while (*str) {
-			sbi_putc(*str);
-			str++;
-		}
+		for (i = 0; i < len; i++)
+			sbi_putc(str[i]);
+		ret = len;
 	}
+
+	return ret;
+}
+
+static void nputs_all(const char *str, unsigned long len)
+{
+	unsigned long p = 0;
+
+	while (p < len)
+		p += nputs(&str[p], len - p);
+}
+
+void sbi_puts(const char *str)
+{
+	unsigned long len = sbi_strlen(str);
+
+	spin_lock(&console_out_lock);
+	nputs_all(str, len);
 	spin_unlock(&console_out_lock);
 }
 
 unsigned long sbi_nputs(const char *str, unsigned long len)
 {
-	unsigned long i, ret;
+	unsigned long ret;
 
 	spin_lock(&console_out_lock);
-	if (console_dev && console_dev->console_puts) {
-		ret = console_dev->console_puts(str, len);
-	} else {
-		for (i = 0; i < len; i++)
-			sbi_putc(str[i]);
-		ret = len;
-	}
+	ret = nputs(str, len);
 	spin_unlock(&console_out_lock);
 
 	return ret;
@@ -225,12 +236,30 @@ static int printi(char **out, u32 *out_len, long long i, int b, int sg,
 
 static int print(char **out, u32 *out_len, const char *format, va_list args)
 {
-	int width, flags;
-	int pc = 0;
-	char scr[2];
+	int width, flags, pc = 0;
+	char scr[2], *tout;
+	bool use_tbuf = (!out) ? true : false;
 	unsigned long long tmp;
 
+	/*
+	 * The console_tbuf is protected by console_out_lock and
+	 * print() is always called with console_out_lock held
+	 * when out == NULL.
+	 */
+	if (use_tbuf) {
+		console_tbuf_len = CONSOLE_TBUF_MAX;
+		tout = console_tbuf;
+		out = &tout;
+		out_len = &console_tbuf_len;
+	}
+
 	for (; *format != 0; ++format) {
+		if (use_tbuf && !console_tbuf_len) {
+			nputs_all(console_tbuf, CONSOLE_TBUF_MAX);
+			console_tbuf_len = CONSOLE_TBUF_MAX;
+			tout = console_tbuf;
+		}
+
 		if (*format == '%') {
 			++format;
 			width = flags = 0;
@@ -356,6 +385,9 @@ literal:
 		}
 	}
 
+	if (use_tbuf && console_tbuf_len < CONSOLE_TBUF_MAX)
+		nputs_all(console_tbuf, CONSOLE_TBUF_MAX - console_tbuf_len);
+
 	return pc;
 }
 
Signature · 1 line
2.34.1
author Applied
#9 lore raw

v6 cover OpenSBI debug console support

In reply to Anup Patel
Quoted 42 lines · Anup Patel

The latest proposal of SBI debug console extension can be found at:
https://lists.riscv.org/g/tech-prs/message/96

This series implements SBI debug console support in OpenSBI as-per above draft proposal.

To test these patches corresponding Linux patches can be found in riscv_sbi_dbcn_v1 branch at: https://github.com/avpatel/linux.git

These patches can also be found in riscv_sbi_dbcn_v6 branch at:
https://github.com/avpatel/opensbi.git

Changes since v5:
- Rebased on latest OpenSBI sources
- Updated Reviewed-by tags
- Addressed Drew's comments in all patches
- Updated SBI debug console implementation as-per latest proposal where console write() is non-blocking.

Changes since v4:
- Rebased on latest OpenSBI sources
- Updated Reviewed-by tags
- Added new PATCH8 to speed-up sbi_printf() using nputs()

Changes since v3:
- Rebased on OpenSBI v1.2 release
- Updated Reviewed-by tags
- Added new PATCH3 for sbi_ngets() function
- Updated SBI debug console implementation in PATCH5 as-per latest proposal having both console write() and read() functions.

Changes since v2:
- Reworked sbi_domain_check_addr_range() in PATCH3 to make it work for overlapping regions.

Changes since v1:
- New PATCH3 to implement sbi_domain_check_addr_range() function
- Added checks in PATCH4 for location and size of string to print
- New PATCH5 to add console_puts() callback in the console device
- New PATCH6 to implement console_puts() for semihosting

Quoted 9 lines

Anup Patel (8):
include: Add defines for SBI debug console extension
lib: sbi: Add sbi_nputs() function
lib: sbi: Add sbi_ngets() function
lib: sbi: Add sbi_domain_check_addr_range() function
lib: sbi: Implement SBI debug console extension
lib: sbi: Add console_puts() callback in the console device
lib: utils/serial: Implement console_puts() for semihosting
lib: sbi: Speed-up sbi_printf() and friends using nputs()

Applied this series to the riscv/opensbi repo.

Regards,
Anup

Quoted 16 lines
 include/sbi/sbi_console.h         |  7 +++
 include/sbi/sbi_domain.h          | 15 ++++++
 include/sbi/sbi_ecall_interface.h |  6 +++
 lib/sbi/Kconfig                   |  4 ++
 lib/sbi/objects.mk                |  3 ++
 lib/sbi/sbi_console.c             | 88 ++++++++++++++++++++++++++++---
 lib/sbi/sbi_domain.c              | 69 ++++++++++++++++++++++++
 lib/sbi/sbi_ecall_dbcn.c          | 72 +++++++++++++++++++++++++
 lib/utils/serial/semihosting.c    | 37 +++++++++++++
 9 files changed, 294 insertions(+), 7 deletions(-)
 create mode 100644 lib/sbi/sbi_ecall_dbcn.c

--
2.34.1