| Message ID | 915bd184c6648a1a3bf0ac6a79b5274972bb33dd.1673877778.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | The patch series introduces the following: | expand |
On Mon, Jan 16, 2023 at 04:39:31PM +0200, Oleksii Kurochko wrote: > Because printk() relies on a serial driver (like the ns16550 driver) > and drivers require working virtual memory (ioremap()) there is not > print functionality early in Xen boot. > > The patch introduces the basic stuff of early_printk functionality > which will be enough to print 'hello from C environment". > > Originally early_printk.{c,h} was introduced by Bobby Eshleman > (https://github.com/glg-rv/xen/commit/a3c9916bbdff7ad6030055bbee7e53d1aab71384) > but some functionality was changed: > early_printk() function was changed in comparison with the original as > common isn't being built now so there is no vscnprintf. > > This commit adds early printk implementation built on the putc SBI call. > > As sbi_console_putchar() is already being planned for deprecation > it is used temporarily now and will be removed or reworked after > real uart will be ready. > > Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > Changes in V4: > - Remove "depends on RISCV*" from Kconfig.debug as it is located in > arch specific folder so by default RISCV configs should be ebabled. > - Add "ifdef __riscv_cmodel_medany" to be sure that PC-relative addressing > is used as early_*() functions can be called from head.S with MMU-off and > before relocation (if it would be needed at all for RISC-V) > - fix code style > --- > Changes in V3: > - reorder headers in alphabetical order > - merge changes related to start_xen() function from "[PATCH v2 7/8] > xen/riscv: print hello message from C env" to this patch > - remove unneeded parentheses in definition of STACK_SIZE > --- > Changes in V2: > - introduce STACK_SIZE define. > - use consistent padding between instruction mnemonic and operand(s) > --- > --- > xen/arch/riscv/Kconfig.debug | 6 +++ > xen/arch/riscv/Makefile | 1 + > xen/arch/riscv/early_printk.c | 45 +++++++++++++++++++++++ > xen/arch/riscv/include/asm/early_printk.h | 12 ++++++ > xen/arch/riscv/setup.c | 6 ++- > 5 files changed, 69 insertions(+), 1 deletion(-) > create mode 100644 xen/arch/riscv/early_printk.c > create mode 100644 xen/arch/riscv/include/asm/early_printk.h > > diff --git a/xen/arch/riscv/Kconfig.debug b/xen/arch/riscv/Kconfig.debug > index e69de29bb2..e139e44873 100644 > --- a/xen/arch/riscv/Kconfig.debug > +++ b/xen/arch/riscv/Kconfig.debug > @@ -0,0 +1,6 @@ > +config EARLY_PRINTK > + bool "Enable early printk" > + default DEBUG > + help > + > + Enables early printk debug messages > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > index fd916e1004..1a4f1a6015 100644 > --- a/xen/arch/riscv/Makefile > +++ b/xen/arch/riscv/Makefile > @@ -1,3 +1,4 @@ > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > obj-$(CONFIG_RISCV_64) += riscv64/ > obj-y += sbi.o > obj-y += setup.o > diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c > new file mode 100644 > index 0000000000..6bc29a1942 > --- /dev/null > +++ b/xen/arch/riscv/early_printk.c > @@ -0,0 +1,45 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * RISC-V early printk using SBI > + * > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com> > + */ > +#include <asm/early_printk.h> > +#include <asm/sbi.h> > + > +/* > + * early_*() can be called from head.S with MMU-off. > + * > + * The following requiremets should be honoured for early_*() to > + * work correctly: > + * It should use PC-relative addressing for accessing symbols. > + * To achieve that GCC cmodel=medany should be used. > + */ > +#ifndef __riscv_cmodel_medany > +#error "early_*() can be called from head.S before relocate so it should not use absolute addressing." > +#endif > + > +/* > + * TODO: > + * sbi_console_putchar is already planned for deprecation > + * so it should be reworked to use UART directly. > +*/ > +void early_puts(const char *s, size_t nr) > +{ > + while ( nr-- > 0 ) > + { > + if ( *s == '\n' ) > + sbi_console_putchar('\r'); > + sbi_console_putchar(*s); > + s++; > + } > +} > + > +void early_printk(const char *str) > +{ > + while ( *str ) > + { > + early_puts(str, 1); > + str++; > + } > +} > diff --git a/xen/arch/riscv/include/asm/early_printk.h b/xen/arch/riscv/include/asm/early_printk.h > new file mode 100644 > index 0000000000..05106e160d > --- /dev/null > +++ b/xen/arch/riscv/include/asm/early_printk.h > @@ -0,0 +1,12 @@ > +#ifndef __EARLY_PRINTK_H__ > +#define __EARLY_PRINTK_H__ > + > +#include <xen/early_printk.h> > + > +#ifdef CONFIG_EARLY_PRINTK > +void early_printk(const char *str); > +#else > +static inline void early_printk(const char *s) {}; > +#endif > + > +#endif /* __EARLY_PRINTK_H__ */ > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > index 13e24e2fe1..9c9412152a 100644 > --- a/xen/arch/riscv/setup.c > +++ b/xen/arch/riscv/setup.c > @@ -1,13 +1,17 @@ > #include <xen/compile.h> > #include <xen/init.h> > > +#include <asm/early_printk.h> > + > /* Xen stack for bringing up the first CPU. */ > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > __aligned(STACK_SIZE); > > void __init noreturn start_xen(void) > { > - for ( ;; ) > + early_printk("Hello from C env\n"); > + > + for ( ; ; ) > asm volatile ("wfi"); > > unreachable(); > -- > 2.39.0 > Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
On 16/01/2023 2:39 pm, Oleksii Kurochko wrote: > diff --git a/xen/arch/riscv/Kconfig.debug b/xen/arch/riscv/Kconfig.debug > index e69de29bb2..e139e44873 100644 > --- a/xen/arch/riscv/Kconfig.debug > +++ b/xen/arch/riscv/Kconfig.debug > @@ -0,0 +1,6 @@ > +config EARLY_PRINTK > + bool "Enable early printk" > + default DEBUG > + help > + > + Enables early printk debug messages Kconfig indentation is a little hard to get used to. It's one tab for the main block, and one tab + 2 spaces for the help text. Also, drop the blank line after help. > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > index fd916e1004..1a4f1a6015 100644 > --- a/xen/arch/riscv/Makefile > +++ b/xen/arch/riscv/Makefile > @@ -1,3 +1,4 @@ > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > obj-$(CONFIG_RISCV_64) += riscv64/ > obj-y += sbi.o > obj-y += setup.o > diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c > new file mode 100644 > index 0000000000..6bc29a1942 > --- /dev/null > +++ b/xen/arch/riscv/early_printk.c > @@ -0,0 +1,45 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * RISC-V early printk using SBI > + * > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com> > + */ > +#include <asm/early_printk.h> > +#include <asm/sbi.h> > + > +/* > + * early_*() can be called from head.S with MMU-off. > + * > + * The following requiremets should be honoured for early_*() to > + * work correctly: > + * It should use PC-relative addressing for accessing symbols. > + * To achieve that GCC cmodel=medany should be used. > + */ > +#ifndef __riscv_cmodel_medany > +#error "early_*() can be called from head.S before relocate so it should not use absolute addressing." > +#endif This is incorrect. What *this* file is compiled with has no bearing on how head.S calls us. The RISC-V documentation explaining __riscv_cmodel_medany vs __riscv_cmodel_medlow calls this point out specifically. There's nothing you can put here to check that head.S gets compiled with medany. Right now, there's nothing in this file dependent on either mode, and it's not liable to change in the short term. Furthermore, Xen isn't doing any relocation in the first place. We will want to support XIP in due course, and that will be compiled __riscv_cmodel_medlow, which is a fine and legitimate usecase. The build system sets the model up consistently. All you are doing by putting this in is creating work that someone is going to have to delete for legitimate reasons in the future. > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > index 13e24e2fe1..9c9412152a 100644 > --- a/xen/arch/riscv/setup.c > +++ b/xen/arch/riscv/setup.c > @@ -1,13 +1,17 @@ > #include <xen/compile.h> > #include <xen/init.h> > > +#include <asm/early_printk.h> > + > /* Xen stack for bringing up the first CPU. */ > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > __aligned(STACK_SIZE); > > void __init noreturn start_xen(void) > { > - for ( ;; ) > + early_printk("Hello from C env\n"); > + > + for ( ; ; ) Rebasing error? ~Andrew
On Tue, 17 Jan 2023 at 23:57, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote: > On 16/01/2023 2:39 pm, Oleksii Kurochko wrote: > > diff --git a/xen/arch/riscv/Kconfig.debug b/xen/arch/riscv/Kconfig.debug > > index e69de29bb2..e139e44873 100644 > > --- a/xen/arch/riscv/Kconfig.debug > > +++ b/xen/arch/riscv/Kconfig.debug > > @@ -0,0 +1,6 @@ > > +config EARLY_PRINTK > > + bool "Enable early printk" > > + default DEBUG > > + help > > + > > + Enables early printk debug messages > > Kconfig indentation is a little hard to get used to. > > It's one tab for the main block, and one tab + 2 spaces for the help text. > > Also, drop the blank line after help. > > > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > > index fd916e1004..1a4f1a6015 100644 > > --- a/xen/arch/riscv/Makefile > > +++ b/xen/arch/riscv/Makefile > > @@ -1,3 +1,4 @@ > > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > obj-$(CONFIG_RISCV_64) += riscv64/ > > obj-y += sbi.o > > obj-y += setup.o > > diff --git a/xen/arch/riscv/early_printk.c > b/xen/arch/riscv/early_printk.c > > new file mode 100644 > > index 0000000000..6bc29a1942 > > --- /dev/null > > +++ b/xen/arch/riscv/early_printk.c > > @@ -0,0 +1,45 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * RISC-V early printk using SBI > > + * > > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com> > > + */ > > +#include <asm/early_printk.h> > > +#include <asm/sbi.h> > > + > > +/* > > + * early_*() can be called from head.S with MMU-off. > > + * > > + * The following requiremets should be honoured for early_*() to > > + * work correctly: > > + * It should use PC-relative addressing for accessing symbols. > > + * To achieve that GCC cmodel=medany should be used. > > + */ > > +#ifndef __riscv_cmodel_medany > > +#error "early_*() can be called from head.S before relocate so it > should not use absolute addressing." > > +#endif > > This is incorrect. Aside the part about the relocation, I do not see what is wrong with it (see below) > > > What *this* file is compiled with has no bearing on how head.S calls > us. The RISC-V documentation explaining __riscv_cmodel_medany vs > __riscv_cmodel_medlow calls this point out specifically. There's > nothing you can put here to check that head.S gets compiled with medany. I believe you misunderstood the goal here. It is not to check how head.S will call it but to ensure the function is safe to be called when the MMU is off (e.g VA != VA). > > Right now, there's nothing in this file dependent on either mode, and > it's not liable to change in the short term. The whole point is to make sure this don’t change without us knowing. Furthermore, Xen isn't > doing any relocation in the first place. > > We will want to support XIP in due course, and that will be compiled > __riscv_cmodel_medlow, which is a fine and legitimate usecase. > > > The build system sets the model up consistently. All you are doing by > putting this in is creating work that someone is going to have to delete > for legitimate reasons in the future. Are you saying that a code compiled with medlow can also work with MMU off and no relocation needed? If not, then the check is correct. It would avoid hours of debugging… Cheers, > > > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > > index 13e24e2fe1..9c9412152a 100644 > > --- a/xen/arch/riscv/setup.c > > +++ b/xen/arch/riscv/setup.c > > @@ -1,13 +1,17 @@ > > #include <xen/compile.h> > > #include <xen/init.h> > > > > +#include <asm/early_printk.h> > > + > > /* Xen stack for bringing up the first CPU. */ > > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > > __aligned(STACK_SIZE); > > > > void __init noreturn start_xen(void) > > { > > - for ( ;; ) > > + early_printk("Hello from C env\n"); > > + > > + for ( ; ; ) > > Rebasing error? > > ~Andrew >
On Tue, 2023-01-17 at 23:57 +0000, Andrew Cooper wrote: > On 16/01/2023 2:39 pm, Oleksii Kurochko wrote: > > diff --git a/xen/arch/riscv/Kconfig.debug > > b/xen/arch/riscv/Kconfig.debug > > index e69de29bb2..e139e44873 100644 > > --- a/xen/arch/riscv/Kconfig.debug > > +++ b/xen/arch/riscv/Kconfig.debug > > @@ -0,0 +1,6 @@ > > +config EARLY_PRINTK > > + bool "Enable early printk" > > + default DEBUG > > + help > > + > > + Enables early printk debug messages > > Kconfig indentation is a little hard to get used to. > > It's one tab for the main block, and one tab + 2 spaces for the help > text. > > Also, drop the blank line after help. > > > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > > index fd916e1004..1a4f1a6015 100644 > > --- a/xen/arch/riscv/Makefile > > +++ b/xen/arch/riscv/Makefile > > @@ -1,3 +1,4 @@ > > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > obj-$(CONFIG_RISCV_64) += riscv64/ > > obj-y += sbi.o > > obj-y += setup.o > > diff --git a/xen/arch/riscv/early_printk.c > > b/xen/arch/riscv/early_printk.c > > new file mode 100644 > > index 0000000000..6bc29a1942 > > --- /dev/null > > +++ b/xen/arch/riscv/early_printk.c > > @@ -0,0 +1,45 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * RISC-V early printk using SBI > > + * > > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com> > > + */ > > +#include <asm/early_printk.h> > > +#include <asm/sbi.h> > > + > > +/* > > + * early_*() can be called from head.S with MMU-off. > > + * > > + * The following requiremets should be honoured for early_*() to > > + * work correctly: > > + * It should use PC-relative addressing for accessing symbols. > > + * To achieve that GCC cmodel=medany should be used. > > + */ > > +#ifndef __riscv_cmodel_medany > > +#error "early_*() can be called from head.S before relocate so it > > should not use absolute addressing." > > +#endif > > This is incorrect. > > What *this* file is compiled with has no bearing on how head.S calls > us. The RISC-V documentation explaining __riscv_cmodel_medany vs > __riscv_cmodel_medlow calls this point out specifically. There's > nothing you can put here to check that head.S gets compiled with > medany. > > Right now, there's nothing in this file dependent on either mode, and > it's not liable to change in the short term. Furthermore, Xen isn't > doing any relocation in the first place. > > We will want to support XIP in due course, and that will be compiled > __riscv_cmodel_medlow, which is a fine and legitimate usecase. > > > The build system sets the model up consistently. All you are doing > by > putting this in is creating work that someone is going to have to > delete > for legitimate reasons in the future. > > > diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c > > index 13e24e2fe1..9c9412152a 100644 > > --- a/xen/arch/riscv/setup.c > > +++ b/xen/arch/riscv/setup.c > > @@ -1,13 +1,17 @@ > > #include <xen/compile.h> > > #include <xen/init.h> > > > > +#include <asm/early_printk.h> > > + > > /* Xen stack for bringing up the first CPU. */ > > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > > __aligned(STACK_SIZE); > > > > void __init noreturn start_xen(void) > > { > > - for ( ;; ) > > + early_printk("Hello from C env\n"); > > + > > + for ( ; ; ) > > Rebasing error? > If you are not speaking about adding of the space between "; ;" than it is rebasing error. I will double-check it during work on new version of the patch series. > ~Andrew ~Oleksii
diff --git a/xen/arch/riscv/Kconfig.debug b/xen/arch/riscv/Kconfig.debug index e69de29bb2..e139e44873 100644 --- a/xen/arch/riscv/Kconfig.debug +++ b/xen/arch/riscv/Kconfig.debug @@ -0,0 +1,6 @@ +config EARLY_PRINTK + bool "Enable early printk" + default DEBUG + help + + Enables early printk debug messages diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index fd916e1004..1a4f1a6015 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -1,3 +1,4 @@ +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o obj-$(CONFIG_RISCV_64) += riscv64/ obj-y += sbi.o obj-y += setup.o diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c new file mode 100644 index 0000000000..6bc29a1942 --- /dev/null +++ b/xen/arch/riscv/early_printk.c @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * RISC-V early printk using SBI + * + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com> + */ +#include <asm/early_printk.h> +#include <asm/sbi.h> + +/* + * early_*() can be called from head.S with MMU-off. + * + * The following requiremets should be honoured for early_*() to + * work correctly: + * It should use PC-relative addressing for accessing symbols. + * To achieve that GCC cmodel=medany should be used. + */ +#ifndef __riscv_cmodel_medany +#error "early_*() can be called from head.S before relocate so it should not use absolute addressing." +#endif + +/* + * TODO: + * sbi_console_putchar is already planned for deprecation + * so it should be reworked to use UART directly. +*/ +void early_puts(const char *s, size_t nr) +{ + while ( nr-- > 0 ) + { + if ( *s == '\n' ) + sbi_console_putchar('\r'); + sbi_console_putchar(*s); + s++; + } +} + +void early_printk(const char *str) +{ + while ( *str ) + { + early_puts(str, 1); + str++; + } +} diff --git a/xen/arch/riscv/include/asm/early_printk.h b/xen/arch/riscv/include/asm/early_printk.h new file mode 100644 index 0000000000..05106e160d --- /dev/null +++ b/xen/arch/riscv/include/asm/early_printk.h @@ -0,0 +1,12 @@ +#ifndef __EARLY_PRINTK_H__ +#define __EARLY_PRINTK_H__ + +#include <xen/early_printk.h> + +#ifdef CONFIG_EARLY_PRINTK +void early_printk(const char *str); +#else +static inline void early_printk(const char *s) {}; +#endif + +#endif /* __EARLY_PRINTK_H__ */ diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c index 13e24e2fe1..9c9412152a 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -1,13 +1,17 @@ #include <xen/compile.h> #include <xen/init.h> +#include <asm/early_printk.h> + /* Xen stack for bringing up the first CPU. */ unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE); void __init noreturn start_xen(void) { - for ( ;; ) + early_printk("Hello from C env\n"); + + for ( ; ; ) asm volatile ("wfi"); unreachable();
Because printk() relies on a serial driver (like the ns16550 driver) and drivers require working virtual memory (ioremap()) there is not print functionality early in Xen boot. The patch introduces the basic stuff of early_printk functionality which will be enough to print 'hello from C environment". Originally early_printk.{c,h} was introduced by Bobby Eshleman (https://github.com/glg-rv/xen/commit/a3c9916bbdff7ad6030055bbee7e53d1aab71384) but some functionality was changed: early_printk() function was changed in comparison with the original as common isn't being built now so there is no vscnprintf. This commit adds early printk implementation built on the putc SBI call. As sbi_console_putchar() is already being planned for deprecation it is used temporarily now and will be removed or reworked after real uart will be ready. Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V4: - Remove "depends on RISCV*" from Kconfig.debug as it is located in arch specific folder so by default RISCV configs should be ebabled. - Add "ifdef __riscv_cmodel_medany" to be sure that PC-relative addressing is used as early_*() functions can be called from head.S with MMU-off and before relocation (if it would be needed at all for RISC-V) - fix code style --- Changes in V3: - reorder headers in alphabetical order - merge changes related to start_xen() function from "[PATCH v2 7/8] xen/riscv: print hello message from C env" to this patch - remove unneeded parentheses in definition of STACK_SIZE --- Changes in V2: - introduce STACK_SIZE define. - use consistent padding between instruction mnemonic and operand(s) --- --- xen/arch/riscv/Kconfig.debug | 6 +++ xen/arch/riscv/Makefile | 1 + xen/arch/riscv/early_printk.c | 45 +++++++++++++++++++++++ xen/arch/riscv/include/asm/early_printk.h | 12 ++++++ xen/arch/riscv/setup.c | 6 ++- 5 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 xen/arch/riscv/early_printk.c create mode 100644 xen/arch/riscv/include/asm/early_printk.h