OpenSBI debug console support
v2 cover OpenSBI debug console support
The latest draft-v4 proposal of SBI debug console can be found here:
https://lists.riscv.org/g/tech-unixplatformspec/message/1815
This series implements SBI debug console support in OpenSBI as-per above draft proposal.
These patches can also be found in riscv_sbi_dbcn_v2 branch
at: https://github.com/avpatel/opensbi.git
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 (6):
include: Add defines for SBI debug console extension
lib: sbi: Add sbi_nputs() 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
include/sbi/sbi_console.h | 5 +++ include/sbi/sbi_domain.h | 15 ++++++++ include/sbi/sbi_ecall_interface.h | 4 ++ lib/sbi/Kconfig | 4 ++ lib/sbi/objects.mk | 3 ++ lib/sbi/sbi_console.c | 25 +++++++++++-- lib/sbi/sbi_domain.c | 47 +++++++++++++++++++++++ lib/sbi/sbi_ecall_dbcn.c | 62 +++++++++++++++++++++++++++++++ lib/utils/serial/semihosting.c | 33 ++++++++++++++++ 9 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 lib/sbi/sbi_ecall_dbcn.c
Signature · 1 line
2.34.1
PATCH v2 1/6 include: Add defines for SBI debug console extension
We add SBI debug console extension related defines/enum to the SBI ecall interface header.
--- include/sbi/sbi_ecall_interface.h | 4 ++++ 1 file changed, 4 insertions(+)
include/sbi/sbi_ecall_interface.h +4 -0
@@ -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,9 @@ 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_PUTS 0x0 +/* 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
PATCH v2 2/6 lib: sbi: Add sbi_nputs() function
In reply to Anup PatelWe 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 | 10 ++++++++++ 2 files changed, 12 insertions(+)
include/sbi/sbi_console.h +2 -0
@@ -33,6 +33,8 @@ void sbi_putc(char ch);void sbi_puts (const char *str); +void 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 +10 -0
@@ -51,6 +51,16 @@ void sbi_puts(const char *str)spin_unlock (&console_out_lock); } +void 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); +} +void sbi_gets (char *s,int maxwidth,char endchar) {int ch;
Signature · 1 line
2.34.1
PATCH v2 3/6 lib: sbi: Add sbi_domain_check_addr_range() function
In reply to Anup PatelWe 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 | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+)
include/sbi/sbi_domain.h +15 -0
@@ -154,6 +154,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 +47 -0
59 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v2 3/6 lib: sbi: Add sbi_domain_check_addr_range() function
? 2022-11-23???? 16:27 +0530?Anup Patel???
Quoted 81 lines
We add
sbi_domain_check_addr_range()helper function to check whether a given address range is accessible under a particular domain.Signed-off-by Anup Patel <[email protected]>---
?include/sbi/sbi_domain.h | 15 +++++++++++++ ?lib/sbi/sbi_domain.c???? | 47 ++++++++++++++++++++++++++++++++++++++++ ?2 files changed, 62 insertions(+)include/sbi/sbi_domain.h +0
-0@@ -154,6 +154,21 @@ bool sbi_domain_check_addr(const struct sbi_domain *dom, ?????????????????????????? unsigned long addr, unsigned long mode, ?????????????????????????? unsigned long access_flags);?
include/sbi/sbi_domain.h +15
-0+/** +* 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 */
?voidsbi_domain_dump(const struct sbi_domain *dom, const char *suffix); ?lib/sbi/sbi_domain.c +0
-0@@ -195,6 +195,24 @@ static bool is_region_before(const struct sbi_domain_memregion *regA, ????????return FALSE; ?}?
lib/sbi/sbi_domain.c +18
-0+static const struct sbi_domain_memregion *find_region( +???????????????????????????????????????????????const struct sbi_domain *dom, +???????????????????????????????????????????????unsigned long addr) +{ +???????unsigned long rstart, rend; +???????struct sbi_domain_memregion *reg; + +???????sbi_domain_for_each_memregion(dom, reg) { +???????????????rstart = reg->base; +???????????????rend = (reg->order < __riscv_xlen) ? +???????????????????????rstart + ((1UL << reg->order) - 1) : -1UL; +???????????????if (rstart <= addr && addr <= rend) +???????????????????????return reg; +???????} + +???????return NULL; +} +?static int
sanitize_domain(const struct sbi_platform *plat, ?????????????????????????? struct sbi_domain *dom)?{lib/sbi/sbi_domain.c +0
-0@@ -303,6 +321,35 @@ static int sanitize_domain(const struct sbi_platform *plat, ????????return 0; ?}?
lib/sbi/sbi_domain.c +12
-0+bool sbi_domain_check_addr_range (const struct sbi_domain *dom, +??????????????????????????????? unsigned long addr, unsigned long size, +??????????????????????????????? unsigned long mode, +??????????????????????????????? unsigned long access_flags) +{ +???????unsigned long pos = 0; +???????const struct sbi_domain_memregion *reg; + +???????if (!dom) +???????????????return FALSE; + +???????while (pos < size) {
lib/sbi/sbi_domain.c
Quoted 10 lines
lib/sbi/sbi_domain.c +10
-0+???????????????reg = find_region(dom, addr + pos); +???????????????if (!reg) +???????????????????????return FALSE; + +???????????????if (!sbi_domain_check_addr(dom, addr + pos, +????????????????????????????????????????? mode, access_flags)) +???????????????????????return FALSE; + +???????????????if (reg->order < __riscv_xlen) +???????????????????????pos += 1UL << reg->order;
addr+pos not always point to the beginning of the region, so the size of the region cannot be added directly.
suggestion:
pos = reg->base + (1 << reg->order) - addr
Regards,
Xiang W
Quoted 14 lines
+???????????????else
+???????????????????????break;
+???????}lib/sbi/sbi_domain.c +4
-0+ +???????return TRUE; +} +?void
sbi_domain_dump(const struct sbi_domain *dom, const char *suffix)?{
????????u32 i, k;Signature · 1 line
2.34.1
PATCH v2 3/6 lib: sbi: Add sbi_domain_check_addr_range() function
Quoted 96 lines · Xiang W
? 2022-11-23???? 16:27 +0530?Anup Patel???
Quoted 81 lines
We add
sbi_domain_check_addr_range()helper function to check whether a given address range is accessible under a particular domain.Signed-off-by Anup Patel <[email protected]>--- include/sbi/sbi_domain.h | 15 +++++++++++++ lib/sbi/sbi_domain.c | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+)include/sbi/sbi_domain.h +15
-0@@ -154,6 +154,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 +30
-0@@ -195,6 +195,24 @@ static bool is_region_before(const struct sbi_domain_memregion *regA,return FALSE; } +static const struct sbi_domain_memregion *find_region ( +const struct sbi_domain *dom, +unsigned long addr) +{ +unsigned long rstart, rend; +struct sbi_domain_memregion *reg; + +sbi_domain_for_each_memregion (dom, reg) { + rstart = reg->base; + rend = (reg->order < __riscv_xlen) ? + rstart + ((1UL << reg->order) -1 ) :-1UL ; +if (rstart <= addr && addr <= rend) +return reg; + } + +return NULL ; +} +static int sanitize_domain (const struct sbi_platform *plat,struct sbi_domain *dom) {@@ -303,6 +321,35 @@ static int sanitize_domain(const struct sbi_platform *plat,return 0 ; } +bool sbi_domain_check_addr_range (const struct sbi_domain *dom, +unsigned long addr,unsigned long size, +unsigned long mode, +unsigned long access_flags) +{ +unsigned long pos =0 ; +const struct sbi_domain_memregion *reg; + +if (!dom) +return FALSE; + +while (pos < size) {lib/sbi/sbi_domain.c static int sanitize_domain(const struct sbi_platform *plat, · line 345
Quoted 10 lines
lib/sbi/sbi_domain.c +10
-0+ reg =find_region (dom, addr + pos); +if (!reg) +return FALSE; + +if (!sbi_domain_check_addr (dom, addr + pos, + mode, access_flags)) +return FALSE; + +if (reg->order < __riscv_xlen) + pos +=1UL << reg->order;addr+pos not always point to the beginning of the region, so the size of the region cannot be added directly.
Quoted 2 lines
suggestion:
pos = reg->base + (1 << reg->order) - addr
This loop seems to be broken for overlapping regions.
What you are suggesting will also not work for overlapping regions.
Let me rethink.
Regards,
Anup
Quoted 24 lines
Regards,
Xiang WQuoted 11 lines
lib/sbi/sbi_domain.c +7
-1+else +break ; + } + +return TRUE; +} +void sbi_domain_dump (const struct sbi_domain *dom,const char *suffix) {u32 i, k; --Quoted 3 lines
2.34.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
PATCH v2 4/6 lib: sbi: Implement SBI debug console extension
In reply to Anup PatelWe 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 | 62 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 lib/sbi/sbi_ecall_dbcn.c
lib/sbi/Kconfig +4 -0
@@ -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
@@ -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 +62 -0
62 lines not on this page · raw message
Signature · 1 line
2.34.1
PATCH v2 5/6 lib: sbi: Add console_puts() callback in the console device
In reply to Anup PatelWe 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 | 19 ++++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-)
include/sbi/sbi_console.h +3 -0
@@ -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 */ +void (*console_puts)(const char *str,unsigned long len); +/** Read a character from the console input */ int (*console_getc)(void ); };
lib/sbi/sbi_console.c +14 -5
@@ -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 ;
@@ -44,9 +45,13 @@ void sbi_putc(char ch)void sbi_puts (const char *str) {spin_lock (&console_out_lock); -while (*str) { -sbi_putc (*str); - str++; +if (console_dev && console_dev->console_puts) { + console_dev->console_puts (str,sbi_strlen (str)); + }else { +while (*str) { +sbi_putc (*str); + str++; + } }spin_unlock (&console_out_lock); }
@@ -56,8 +61,12 @@ void 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]); +if (console_dev && console_dev->console_puts) { + console_dev->console_puts (str, len); + }else { +for (i =0 ; i < len; i++) +sbi_putc (str[i]); + }spin_unlock (&console_out_lock); }
Signature · 1 line
2.34.1
PATCH v2 6/6 lib: utils/serial: Implement console_puts() for semihosting
In reply to Anup PatelWe implement console_puts() for semihosting serial driver to speed-up semihosting based prints.
--- lib/utils/serial/semihosting.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
lib/utils/serial/semihosting.c +33 -0
@@ -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,20 @@ static void semihosting_putc(char ch)semihosting_trap (SYSWRITEC , &ch); } +static void semihosting_puts (const char *str,unsigned long len) +{ +char ch; +unsigned long i; + +if (semihosting_outfd <0 ) { +for (i =0 ; i < len; i++) { + ch = str[i]; +semihosting_trap (SYSWRITEC , &ch); + } + }else +semihosting_write (semihosting_outfd, str, len); +} +static int semihosting_getc (void ) {char ch =0 ;
@@ -165,12 +196,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