| Message ID | 633ced21788a3abf5079c9a191794616bb1ad351.1674226563.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISCV basic exception handling implementation | expand |
On 20/01/2023 2:59 pm, Oleksii Kurochko wrote: > Add ability to print hex number. > It might be useful to print register value as debug information > in BUG(), WARN(), etc... > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> I think it would be better to get s(n)printf() working than to take these. We're going to need to get it working soon anyway, and will be much easier than doing the full printk() infrastructure. ~Andrew
On 20.01.2023 15:59, Oleksii Kurochko wrote: > Add ability to print hex number. > It might be useful to print register value as debug information > in BUG(), WARN(), etc... > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Orthogonal to Andrew's reply (following which I think would be best) a couple of comments which may be applicable elsewhere as well: > --- a/xen/arch/riscv/early_printk.c > +++ b/xen/arch/riscv/early_printk.c > @@ -43,3 +43,42 @@ void early_printk(const char *str) > str++; > } > } > + > +static void reverse(char *s, int length) Please can you get things const-correct (const char *s) and signedness- correct (unsigned int length) from the beginning. We're converting other code as we touch it, but this is extremely slow going and hence would better be avoided in the first place in new code. > +{ > + int c; > + char *begin, *end, temp; > + > + begin = s; > + end = s + length - 1; > + > + for ( c = 0; c < length/2; c++ ) Style: Blanks around binary operators. > + { > + temp = *end; > + *end = *begin; > + *begin = temp; > + > + begin++; > + end--; > + } > +} > + > +void early_printk_hnum(const register_t reg_val) Likely this function wants to be __init? (All functions that can be should also be made so.) With that, reverse() then would also want to become __init. As to the const here vs the remark further up: In cases like this one we typically don't use const. You're free to keep it of course, but I think it should at least be purged from the declaration (and maybe also the stub). > +{ > + char hex[] = "0123456789ABCDEF"; static const char __initconst? > + char buf[17] = {0}; > + > + register_t num = reg_val; > + unsigned int count = 0; > + > + for ( count = 0; num != 0; count++, num >>= 4 ) > + buf[count] = hex[num & 0x0000000f]; Just 0xf? Jan
On Fri, 2023-01-20 at 15:39 +0000, Andrew Cooper wrote: > On 20/01/2023 2:59 pm, Oleksii Kurochko wrote: > > Add ability to print hex number. > > It might be useful to print register value as debug information > > in BUG(), WARN(), etc... > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > I think it would be better to get s(n)printf() working than to take > these. We're going to need to get it working soon anyway, and will > be > much easier than doing the full printk() infrastructure. > Agree here. I re-watched the patch and I do not use this function now at all. (it looks like it was needed only for my personal debug stuff) This patch can be dropped now. > ~Andrew
diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c index 6f590e712b..876d022dd6 100644 --- a/xen/arch/riscv/early_printk.c +++ b/xen/arch/riscv/early_printk.c @@ -43,3 +43,42 @@ void early_printk(const char *str) str++; } } + +static void reverse(char *s, int length) +{ + int c; + char *begin, *end, temp; + + begin = s; + end = s + length - 1; + + for ( c = 0; c < length/2; c++ ) + { + temp = *end; + *end = *begin; + *begin = temp; + + begin++; + end--; + } +} + +void early_printk_hnum(const register_t reg_val) +{ + char hex[] = "0123456789ABCDEF"; + char buf[17] = {0}; + + register_t num = reg_val; + unsigned int count = 0; + + for ( count = 0; num != 0; count++, num >>= 4 ) + buf[count] = hex[num & 0x0000000f]; + + buf[count] = '\0'; + + reverse(buf, count); + + early_printk("0x"); + early_printk(buf); + early_printk("\n"); +} diff --git a/xen/arch/riscv/include/asm/early_printk.h b/xen/arch/riscv/include/asm/early_printk.h index 05106e160d..f6d7580eb0 100644 --- a/xen/arch/riscv/include/asm/early_printk.h +++ b/xen/arch/riscv/include/asm/early_printk.h @@ -5,8 +5,10 @@ #ifdef CONFIG_EARLY_PRINTK void early_printk(const char *str); +void early_printk_hnum(const register_t reg_val); #else static inline void early_printk(const char *s) {}; +static inline void early_printk_hnum(const register_t reg_val) {}; #endif #endif /* __EARLY_PRINTK_H__ */
Add ability to print hex number. It might be useful to print register value as debug information in BUG(), WARN(), etc... Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/early_printk.c | 39 +++++++++++++++++++++++ xen/arch/riscv/include/asm/early_printk.h | 2 ++ 2 files changed, 41 insertions(+)