xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* Re: [PATCH v10 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-07 12:49  9% ` [PATCH v10 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-02-09  1:37 13%   ` Alistair Francis
  0 siblings, 0 replies; 57+ results
From: Alistair Francis @ 2023-02-09  1:37 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis, Bobby Eshleman

On Tue, Feb 7, 2023 at 10:49 PM Oleksii Kurochko
<oleksii.kurochko@gmail.com> 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>
> Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes in V10:
>  - Miss to add the check in V9 so add it in V10
> ---
> Changes in V9:
>     - Bring "cmodel=medany" check back to be sure that C function is safe to be
>       called in early boot when MMU is off ( e.g. VA != PA )
> ---
> Changes in V8:
>     - Nothing was changed
> ---
> Changes in V7:
>     - Nothing was changed
> ---
> Changes in V6:
>     - Remove __riscv_cmodel_medany check from early_printk.c
> ---
> Changes in V5:
>     - Code style fixes
>     - Change an error message of #error in case of __riscv_cmodel_medany
>       isn't defined
> ---
> 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              |  5 +++
>  xen/arch/riscv/Makefile                   |  1 +
>  xen/arch/riscv/early_printk.c             | 44 +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++
>  xen/arch/riscv/setup.c                    |  4 +++
>  5 files changed, 66 insertions(+)
>  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..608c9ff832 100644
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,5 @@
> +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..dfe4ad77e2
> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,44 @@
> +/* 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>
> +
> +/*
> + * When the MMU is off during early boot, any C function called has to
> + * use PC-relative rather than absolute address because the physical address
> + * may not match the virtual address.
> + *
> + * To guarantee PC-relative address cmodel=medany should be used
> + */
> +#ifndef __riscv_cmodel_medany
> +#error "early_*() can be called from head.S with MMU-off"
> +#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..d09ffe1454 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -1,12 +1,16 @@
>  #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)
>  {
> +    early_printk("Hello from C env\n");
> +
>      for ( ;; )
>          asm volatile ("wfi");
>
> --
> 2.39.0
>
>


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v9 0/2] Basic early_printk and smoke test implementation
  2023-02-07 12:41 11% [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
  2023-02-07 12:41  9% ` [PATCH v9 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-02-07 12:49  0% ` Oleksii
  1 sibling, 0 replies; 57+ results
From: Oleksii @ 2023-02-07 12:49 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Bob Eshleman, Alistair Francis, Connor Davis,
	Doug Goldstein

Hi all,

Please look at V10 and skip V9 as I missed to add the check with
medany.

Sorry.

~ Oleksii
On Tue, 2023-02-07 at 14:41 +0200, Oleksii Kurochko wrote:
> The patch series introduces the following:
> - the minimal set of headers and changes inside them.
> - SBI (RISC-V Supervisor Binary Interface) things necessary for basic
>   early_printk implementation.
> - things needed to set up the stack.
> - early_printk() function to print only strings.
> - RISC-V smoke test which checks if  "Hello from C env" message is
>   present in serial.tmp
> 
> The patch series is rebased on top of the patch "include/types: move
> stddef.h-kind types to common header" [1]
> 
> [1]
> https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/
> 
> ---
> Changes in V9:
>  - Bring "cmodel=medany" check back to be sure that C function is
> safe to be called
>    in early boot when MMU is off as 
> ---
> Changes in V8:
>  - Set "needs: archlinux-current-gcc-riscv64-debug" in test.yaml
>    for RISCV job as CONFIG_EARLY_PRINTK is available only when
>    CONFIG_DEBUG is available.
> ---
> Changes in V7:
>  - Fix dependecy for qemu-smoke-riscv64-gcc job
> ---
> Changes in V6:
>  - Rename container name in test.yaml for .qemu-riscv64.
> ---
> Changes in V5:
>   - Nothing changed
> ---
> Changes in V4:
>   - Nothing changed
> ---
> Changes in V3:
>   - Nothing changed
>   - All mentioned comments by Stefano in Xen mailing list will be
>     fixed as a separate patch out of this patch series.
> ---
> 
> Oleksii Kurochko (2):
>   xen/riscv: introduce early_printk basic stuff
>   automation: add RISC-V smoke test
> 
>  automation/gitlab-ci/test.yaml            | 20 ++++++++++++++
>  automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++++++
>  xen/arch/riscv/Kconfig.debug              |  5 ++++
>  xen/arch/riscv/Makefile                   |  1 +
>  xen/arch/riscv/early_printk.c             | 33
> +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
>  xen/arch/riscv/setup.c                    |  4 +++
>  7 files changed, 95 insertions(+)
>  create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
>  create mode 100644 xen/arch/riscv/early_printk.c
>  create mode 100644 xen/arch/riscv/include/asm/early_printk.h
> 



^ permalink raw reply	[relevance 0%]

* [PATCH v10 0/2] Basic early_printk and smoke test implementation
@ 2023-02-07 12:49 11% Oleksii Kurochko
  2023-02-07 12:49  9% ` [PATCH v10 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-02-07 12:49 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V10:
 - Add changes mentioned in V9 (forget to do in previous version)
---
Changes in V9:
 - Bring "cmodel=medany" check back to be sure that C function is safe to be
   called in early boot when MMU is off ( e.g. VA != PA ) 
---
Changes in V8:
 - Set "needs: archlinux-current-gcc-riscv64-debug" in test.yaml
   for RISCV job as CONFIG_EARLY_PRINTK is available only when
   CONFIG_DEBUG is available.
---
Changes in V7:
 - Fix dependecy for qemu-smoke-riscv64-gcc job
---
Changes in V6:
 - Rename container name in test.yaml for .qemu-riscv64.
---
Changes in V5:
  - Nothing changed
---
Changes in V4:
  - Nothing changed
---
Changes in V3:
  - Nothing changed
  - All mentioned comments by Stefano in Xen mailing list will be
    fixed as a separate patch out of this patch series.
---

Oleksii Kurochko (2):
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 +++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 +++++++++++
 xen/arch/riscv/Kconfig.debug              |  5 +++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 44 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++
 xen/arch/riscv/setup.c                    |  4 +++
 7 files changed, 106 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h

-- 
2.39.0



^ permalink raw reply	[relevance 11%]

* [PATCH v10 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-07 12:49 11% [PATCH v10 " Oleksii Kurochko
@ 2023-02-07 12:49  9% ` Oleksii Kurochko
  2023-02-09  1:37 13%   ` Alistair Francis
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-02-07 12:49 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V10:
 - Miss to add the check in V9 so add it in V10
---
Changes in V9:
    - Bring "cmodel=medany" check back to be sure that C function is safe to be
      called in early boot when MMU is off ( e.g. VA != PA ) 
---
Changes in V8:
    - Nothing was changed
---
Changes in V7:
    - Nothing was changed
---
Changes in V6:
    - Remove __riscv_cmodel_medany check from early_printk.c
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 +++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 44 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 66 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..dfe4ad77e2
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,44 @@
+/* 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>
+
+/*
+ * When the MMU is off during early boot, any C function called has to
+ * use PC-relative rather than absolute address because the physical address
+ * may not match the virtual address.
+ *
+ * To guarantee PC-relative address cmodel=medany should be used
+ */
+#ifndef __riscv_cmodel_medany
+#error "early_*() can be called from head.S with MMU-off"
+#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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* [PATCH v9 0/2] Basic early_printk and smoke test implementation
@ 2023-02-07 12:41 11% Oleksii Kurochko
  2023-02-07 12:41  9% ` [PATCH v9 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-02-07 12:49  0% ` [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii
  0 siblings, 2 replies; 57+ results
From: Oleksii Kurochko @ 2023-02-07 12:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V9:
 - Bring "cmodel=medany" check back to be sure that C function is safe to be called
   in early boot when MMU is off as 
---
Changes in V8:
 - Set "needs: archlinux-current-gcc-riscv64-debug" in test.yaml
   for RISCV job as CONFIG_EARLY_PRINTK is available only when
   CONFIG_DEBUG is available.
---
Changes in V7:
 - Fix dependecy for qemu-smoke-riscv64-gcc job
---
Changes in V6:
 - Rename container name in test.yaml for .qemu-riscv64.
---
Changes in V5:
  - Nothing changed
---
Changes in V4:
  - Nothing changed
---
Changes in V3:
  - Nothing changed
  - All mentioned comments by Stefano in Xen mailing list will be
    fixed as a separate patch out of this patch series.
---

Oleksii Kurochko (2):
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++++++
 xen/arch/riscv/Kconfig.debug              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 7 files changed, 95 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h

-- 
2.39.0



^ permalink raw reply	[relevance 11%]

* [PATCH v9 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-07 12:41 11% [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-02-07 12:41  9% ` Oleksii Kurochko
  2023-02-07 12:49  0% ` [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii
  1 sibling, 0 replies; 57+ results
From: Oleksii Kurochko @ 2023-02-07 12:41 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V9:
    - Bring "cmodel=medany" check back to be sure that C function is safe to be
      called in early boot when MMU is off ( e.g. VA != PA ) 
---
Changes in V8:
    - Nothing was changed
---
Changes in V7:
    - Nothing was changed
---
Changes in V6:
    - Remove __riscv_cmodel_medany check from early_printk.c
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 55 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..b66a11f1bc
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* 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>
+
+/*
+ * 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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* Re: [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 12:37 13%     ` Oleksii
@ 2023-02-07 11:28 13%       ` Oleksii
  0 siblings, 0 replies; 57+ results
From: Oleksii @ 2023-02-07 11:28 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

On Tue, 2023-01-31 at 14:37 +0200, Oleksii wrote:
> Hi Julien,
> 
> On Tue, 2023-01-31 at 11:42 +0000, Julien Grall wrote:
> > Hi Oleksii,
> > 
> > On 31/01/2023 11:17, 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/a3c9916bbdff7ad6030055bbee7e53d
> > > 1aab71384)
> > > 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>
> > > Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> > > ---
> > > Changes in V8:
> > >      - Nothing was changed
> > > ---
> > > Changes in V7:
> > >      - Nothing was changed
> > > ---
> > > Changes in V6:
> > >      - Remove __riscv_cmodel_medany check from early_printk.c
> > 
> > This discussion is still not resolved. I will echo Jan's point [1]
> > and 
> > expand it. There is limited point to keep sending a new version for
> > small changes if there main open points are not addressed.
> > 
> > Can you please look at settling done on the issues first and then
> > send a 
> > new version?
> Sure, I won't provide new patch series until the issue will be
> resolved.
> 
Since Alistair and Bobby agreed that the check is needed I'll send a
new version of the patch series.

> This patch series has been sent to resolve an issue with CI&CD which
> I
> missed after the renaming of task for RISC-V in build.yaml.
> 
> ~ Oleksii
> > 
> > Cheers,
> > 
> > [1] 1d63dd9a-77df-4fca-e35b-886ba19fb35d@suse.com
> > 
> 
~ Oleksii



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-01  9:06 11%             ` Julien Grall
  2023-02-01  9:10 13%               ` Julien Grall
  2023-02-01 17:33 12%               ` Bobby Eshleman
@ 2023-02-06 17:30 13%               ` Oleksii
  2 siblings, 0 replies; 57+ results
From: Oleksii @ 2023-02-06 17:30 UTC (permalink / raw)
  To: Julien Grall, Andrew Cooper, Alistair Francis
  Cc: xen-devel, Bob Eshleman, Alistair Francis, Andrew Cooper,
	Jan Beulich, Stefano Stabellini, Gianluca Guida, Connor Davis,
	Bobby Eshleman

Hi all,

On Wed, 2023-02-01 at 09:06 +0000, Julien Grall wrote:
> Hi Andrew,
> 
> On 01/02/2023 00:21, Andrew Cooper wrote:
> > On 31/01/2023 11:17 pm, Alistair Francis wrote:
> > > On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org>
> > > wrote:
> > > > On 31/01/2023 11:44, Alistair Francis wrote:
> > > > > On Sat, Jan 28, 2023 at 12:15 AM Oleksii
> > > > > <oleksii.kurochko@gmail.com> wrote:
> > > > > 
> > > >   From my understanding, on RISC-V, the use of PC-relative
> > > > address is
> > > > only guaranteed with medany. So if you were going to change the
> > > > cmodel
> > > > (Andrew suggested you would), then early_*() may end up to be
> > > > broken.
> > > > 
> > > > This check serve as a documentation of the assumption and also
> > > > help the
> > > > developer any change in the model and take the appropriate
> > > > action to
> > > > remediate it.
> > > > 
> > > > > I think this is safe to remove.
> > > > Based on what I wrote above, do you still think this is safe?
> > > With that in mind it's probably worth leaving in then. Maybe the
> > > comment should be updated to make it explicit why we want this
> > > check
> > > (I find the current comment not very helpful).
> > 
> > The presence of this check pre-supposes that Xen will always
> > relocate
> > itself, and this simply not true.  XIP for example typically does
> > not
> > 
> > Furthermore it's not checking the property wanted.  The way C is
> > compiled has no bearing on what relocation head.S uses to call it.
> 
> I think we are still cross-talking each other because this is not my 
> point. I am not sure how to explain differently...
> 
> This check is not about how head.S call early_*() but making sure the
> C 
> function can be executed with the MMU off. In which case, you will 
> likely have VA != PA.
> 
> In theory head.S could apply some relocations before hand, but it may
> be 
> too complicated to do it before calling early_*().
> 
> > 
> > It is a given that we compile the hypervisor with a consistent code
> > model, meaning that the properly actually making the check do what
> > is
> > wanted is also the property that means it is not needed in the
> > first place.
> 
> See above.
> 
> > 
> > This check is literally not worth the bytes it's taking up on disk,
> > and
> > not worth the added compiler time, nor the wasted time of whoever
> > comes
> > to support something like XIP in the future finds it to be in the
> > way.
> > Xen as a whole will really genuinely function as intended in models
> > other than medany, and it demonstrates a misunderstanding of the
> > topic
> > to put in such a check to begin with.
> 
> Then enlight me :). So far it looks more like you are not
> understanding 
> my point: I am talking about C function call with MMU off (e.g. VA !=
> PA) and you are talking about Xen as a whole.
> 
> I guess the only way to really know is to implement a different
> model. 
> At which point there are three possible outcome:
>    1) We had the compiler check, it fired and the developper will
> take 
> action to fix it (if needed).
>    2) We have no compiler check, the developper knew what to do it
> and 
> fixed it.
>    3) We have no compiler check, the developper where not aware of
> the 
> problem and we will waste time.
> 
> Even if you disagree with the check, then I think 1 is still the best
> because it would explain our assumption. Yes it may waste a few
> minutes 
> to the developer switching the model. But that probably nothing
> compare 
> to the time you could waste if you don't notice it.
> 
> Anyway, Alistair has now all the information to take an informative 
> decision.
> 

I'll bring back the check and send the new version of the patch
tomorrow as Bobby&Alistair lean toward it.

Andrew, do you have other thoughts?

> Cheers,
> 
~ Oleksii



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-01 17:33 12%               ` Bobby Eshleman
@ 2023-02-04 11:59 13%                 ` Alistair Francis
  0 siblings, 0 replies; 57+ results
From: Alistair Francis @ 2023-02-04 11:59 UTC (permalink / raw)
  To: Bobby Eshleman
  Cc: Julien Grall, Andrew Cooper, Oleksii, xen-devel,
	Alistair Francis, Andrew Cooper, Jan Beulich, Stefano Stabellini,
	Gianluca Guida, Connor Davis, Bobby Eshleman

On Thu, Feb 2, 2023 at 3:34 AM Bobby Eshleman <bobbyeshleman@gmail.com> wrote:
>
> On Wed, Feb 01, 2023 at 09:06:21AM +0000, Julien Grall wrote:
> > Hi Andrew,
> >
> > On 01/02/2023 00:21, Andrew Cooper wrote:
> > > On 31/01/2023 11:17 pm, Alistair Francis wrote:
> > > > On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
> > > > > On 31/01/2023 11:44, Alistair Francis wrote:
> > > > > > On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
> > > > > >
> > > > >   From my understanding, on RISC-V, the use of PC-relative address is
> > > > > only guaranteed with medany. So if you were going to change the cmodel
> > > > > (Andrew suggested you would), then early_*() may end up to be broken.
> > > > >
> > > > > This check serve as a documentation of the assumption and also help the
> > > > > developer any change in the model and take the appropriate action to
> > > > > remediate it.
> > > > >
> > > > > > I think this is safe to remove.
> > > > > Based on what I wrote above, do you still think this is safe?
> > > > With that in mind it's probably worth leaving in then. Maybe the
> > > > comment should be updated to make it explicit why we want this check
> > > > (I find the current comment not very helpful).
> > >
> > > The presence of this check pre-supposes that Xen will always relocate
> > > itself, and this simply not true.  XIP for example typically does not
> > >
> > > Furthermore it's not checking the property wanted.  The way C is
> > > compiled has no bearing on what relocation head.S uses to call it.
> >
> > I think we are still cross-talking each other because this is not my point.
> > I am not sure how to explain differently...
> >
> > This check is not about how head.S call early_*() but making sure the C
> > function can be executed with the MMU off. In which case, you will likely
> > have VA != PA.
> >
> > In theory head.S could apply some relocations before hand, but it may be too
> > complicated to do it before calling early_*().
> >
> > >
> > > It is a given that we compile the hypervisor with a consistent code
> > > model, meaning that the properly actually making the check do what is
> > > wanted is also the property that means it is not needed in the first place.
> >
> > See above.
> >
> > >
> > > This check is literally not worth the bytes it's taking up on disk, and
> > > not worth the added compiler time, nor the wasted time of whoever comes
> > > to support something like XIP in the future finds it to be in the way.
> > > Xen as a whole will really genuinely function as intended in models
> > > other than medany, and it demonstrates a misunderstanding of the topic
> > > to put in such a check to begin with.
> >
> > Then enlight me :). So far it looks more like you are not understanding my
> > point: I am talking about C function call with MMU off (e.g. VA != PA) and
> > you are talking about Xen as a whole.
> >
> > I guess the only way to really know is to implement a different model. At
> > which point there are three possible outcome:
> >   1) We had the compiler check, it fired and the developper will take action
> > to fix it (if needed).
> >   2) We have no compiler check, the developper knew what to do it and fixed
> > it.
> >   3) We have no compiler check, the developper where not aware of the
> > problem and we will waste time.
> >
>
> I tend to favor the check because outcome #1 is so desirable, and I do
> think that checking for medany is sufficient for the bulk of future
> work. But that said, I do see Andrew's point now.
>
> If Xen were to a) not relocate itself, b) be executed under the 2GB
> range, c) be compiled under medlow, and d) the MMU is off or on but Xen
> is identity mapped, then C functions should still be safe to call in
> early boot. Checking medany does protect developers from accidental bad
> configuration now, but it is a somewhat imperfect proxy.

Yeah, I agree. It probably is worth leaving the check in for now, even
if it's imperfect. We can always remove it in the future if required.

Alistair

>
> One alternative that may be more long term is for the self relocation
> code to be Kconfig-able and to require/force medany. Anyone wanting to
> develop something like XIP could deselect relocation, which would allow
> them to use medlow if they wanted/needed. The early C functions would
> still be callable under both in this case. The help strings could offer
> explanation too.
>
> Thanks,
> Bobby
>
> > Even if you disagree with the check, then I think 1 is still the best
> > because it would explain our assumption. Yes it may waste a few minutes to
> > the developer switching the model. But that probably nothing compare to the
> > time you could waste if you don't notice it.
> >
> > Anyway, Alistair has now all the information to take an informative
> > decision.
> >
> > Cheers,
> >
> > --
> > Julien Grall
> >


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-01  9:06 11%             ` Julien Grall
  2023-02-01  9:10 13%               ` Julien Grall
@ 2023-02-01 17:33 12%               ` Bobby Eshleman
  2023-02-04 11:59 13%                 ` Alistair Francis
  2023-02-06 17:30 13%               ` Oleksii
  2 siblings, 1 reply; 57+ results
From: Bobby Eshleman @ 2023-02-01 17:33 UTC (permalink / raw)
  To: Julien Grall
  Cc: Andrew Cooper, Alistair Francis, Oleksii, xen-devel,
	Alistair Francis, Andrew Cooper, Jan Beulich, Stefano Stabellini,
	Gianluca Guida, Connor Davis, Bobby Eshleman

On Wed, Feb 01, 2023 at 09:06:21AM +0000, Julien Grall wrote:
> Hi Andrew,
> 
> On 01/02/2023 00:21, Andrew Cooper wrote:
> > On 31/01/2023 11:17 pm, Alistair Francis wrote:
> > > On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
> > > > On 31/01/2023 11:44, Alistair Francis wrote:
> > > > > On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
> > > > > 
> > > >   From my understanding, on RISC-V, the use of PC-relative address is
> > > > only guaranteed with medany. So if you were going to change the cmodel
> > > > (Andrew suggested you would), then early_*() may end up to be broken.
> > > > 
> > > > This check serve as a documentation of the assumption and also help the
> > > > developer any change in the model and take the appropriate action to
> > > > remediate it.
> > > > 
> > > > > I think this is safe to remove.
> > > > Based on what I wrote above, do you still think this is safe?
> > > With that in mind it's probably worth leaving in then. Maybe the
> > > comment should be updated to make it explicit why we want this check
> > > (I find the current comment not very helpful).
> > 
> > The presence of this check pre-supposes that Xen will always relocate
> > itself, and this simply not true.  XIP for example typically does not
> > 
> > Furthermore it's not checking the property wanted.  The way C is
> > compiled has no bearing on what relocation head.S uses to call it.
> 
> I think we are still cross-talking each other because this is not my point.
> I am not sure how to explain differently...
> 
> This check is not about how head.S call early_*() but making sure the C
> function can be executed with the MMU off. In which case, you will likely
> have VA != PA.
> 
> In theory head.S could apply some relocations before hand, but it may be too
> complicated to do it before calling early_*().
> 
> > 
> > It is a given that we compile the hypervisor with a consistent code
> > model, meaning that the properly actually making the check do what is
> > wanted is also the property that means it is not needed in the first place.
> 
> See above.
> 
> > 
> > This check is literally not worth the bytes it's taking up on disk, and
> > not worth the added compiler time, nor the wasted time of whoever comes
> > to support something like XIP in the future finds it to be in the way.
> > Xen as a whole will really genuinely function as intended in models
> > other than medany, and it demonstrates a misunderstanding of the topic
> > to put in such a check to begin with.
> 
> Then enlight me :). So far it looks more like you are not understanding my
> point: I am talking about C function call with MMU off (e.g. VA != PA) and
> you are talking about Xen as a whole.
> 
> I guess the only way to really know is to implement a different model. At
> which point there are three possible outcome:
>   1) We had the compiler check, it fired and the developper will take action
> to fix it (if needed).
>   2) We have no compiler check, the developper knew what to do it and fixed
> it.
>   3) We have no compiler check, the developper where not aware of the
> problem and we will waste time.
> 

I tend to favor the check because outcome #1 is so desirable, and I do
think that checking for medany is sufficient for the bulk of future
work. But that said, I do see Andrew's point now.

If Xen were to a) not relocate itself, b) be executed under the 2GB
range, c) be compiled under medlow, and d) the MMU is off or on but Xen
is identity mapped, then C functions should still be safe to call in
early boot. Checking medany does protect developers from accidental bad
configuration now, but it is a somewhat imperfect proxy.

One alternative that may be more long term is for the self relocation
code to be Kconfig-able and to require/force medany. Anyone wanting to
develop something like XIP could deselect relocation, which would allow
them to use medlow if they wanted/needed. The early C functions would
still be callable under both in this case. The help strings could offer
explanation too.

Thanks,
Bobby

> Even if you disagree with the check, then I think 1 is still the best
> because it would explain our assumption. Yes it may waste a few minutes to
> the developer switching the model. But that probably nothing compare to the
> time you could waste if you don't notice it.
> 
> Anyway, Alistair has now all the information to take an informative
> decision.
> 
> Cheers,
> 
> -- 
> Julien Grall
> 


^ permalink raw reply	[relevance 12%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-01  9:06 11%             ` Julien Grall
@ 2023-02-01  9:10 13%               ` Julien Grall
  2023-02-01 17:33 12%               ` Bobby Eshleman
  2023-02-06 17:30 13%               ` Oleksii
  2 siblings, 0 replies; 57+ results
From: Julien Grall @ 2023-02-01  9:10 UTC (permalink / raw)
  To: Andrew Cooper, Alistair Francis
  Cc: Oleksii, xen-devel, Bob Eshleman, Alistair Francis,
	Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Connor Davis, Bobby Eshleman



On 01/02/2023 09:06, Julien Grall wrote:
> Hi Andrew,
> 
> On 01/02/2023 00:21, Andrew Cooper wrote:
>> On 31/01/2023 11:17 pm, Alistair Francis wrote:
>>> On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
>>>> On 31/01/2023 11:44, Alistair Francis wrote:
>>>>> On Sat, Jan 28, 2023 at 12:15 AM Oleksii 
>>>>> <oleksii.kurochko@gmail.com> wrote:
>>>>>
>>>>   From my understanding, on RISC-V, the use of PC-relative address is
>>>> only guaranteed with medany. So if you were going to change the cmodel
>>>> (Andrew suggested you would), then early_*() may end up to be broken.
>>>>
>>>> This check serve as a documentation of the assumption and also help the
>>>> developer any change in the model and take the appropriate action to
>>>> remediate it.
>>>>
>>>>> I think this is safe to remove.
>>>> Based on what I wrote above, do you still think this is safe?
>>> With that in mind it's probably worth leaving in then. Maybe the
>>> comment should be updated to make it explicit why we want this check
>>> (I find the current comment not very helpful).
>>
>> The presence of this check pre-supposes that Xen will always relocate
>> itself, and this simply not true.  XIP for example typically does not
>>
>> Furthermore it's not checking the property wanted.  The way C is
>> compiled has no bearing on what relocation head.S uses to call it.
> 
> I think we are still cross-talking each other because this is not my 
> point. I am not sure how to explain differently...
> 
> This check is not about how head.S call early_*() but making sure the C 
> function can be executed with the MMU off. In which case, you will 
> likely have VA != PA.
> 
> In theory head.S could apply some relocations before hand, but it may be 
> too complicated to do it before calling early_*().
> 
>>
>> It is a given that we compile the hypervisor with a consistent code
>> model, meaning that the properly actually making the check do what is
>> wanted is also the property that means it is not needed in the first 
>> place.
> 
> See above.
> 
>>
>> This check is literally not worth the bytes it's taking up on disk, and
>> not worth the added compiler time, nor the wasted time of whoever comes
>> to support something like XIP in the future finds it to be in the way.
>> Xen as a whole will really genuinely function as intended in models
>> other than medany, and it demonstrates a misunderstanding of the topic
>> to put in such a check to begin with.
> 
> Then enlight me :). So far it looks more like you are not understanding 
> my point: I am talking about C function call with MMU off (e.g. VA != 

Just to clarify, I meant a C function executing with MMU off here.

> PA) and you are talking about Xen as a whole.
> 
> I guess the only way to really know is to implement a different model. 
> At which point there are three possible outcome:
>    1) We had the compiler check, it fired and the developper will take 
> action to fix it (if needed).
>    2) We have no compiler check, the developper knew what to do it and 
> fixed it.
>    3) We have no compiler check, the developper where not aware of the 
> problem and we will waste time.
> 
> Even if you disagree with the check, then I think 1 is still the best 
> because it would explain our assumption. Yes it may waste a few minutes 
> to the developer switching the model. But that probably nothing compare 
> to the time you could waste if you don't notice it.
> 
> Anyway, Alistair has now all the information to take an informative 
> decision.
> 
> Cheers,
> 

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-02-01  0:21 12%           ` Andrew Cooper
@ 2023-02-01  9:06 11%             ` Julien Grall
  2023-02-01  9:10 13%               ` Julien Grall
                                 ` (2 more replies)
  0 siblings, 3 replies; 57+ results
From: Julien Grall @ 2023-02-01  9:06 UTC (permalink / raw)
  To: Andrew Cooper, Alistair Francis
  Cc: Oleksii, xen-devel, Bob Eshleman, Alistair Francis,
	Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Connor Davis, Bobby Eshleman

Hi Andrew,

On 01/02/2023 00:21, Andrew Cooper wrote:
> On 31/01/2023 11:17 pm, Alistair Francis wrote:
>> On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
>>> On 31/01/2023 11:44, Alistair Francis wrote:
>>>> On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
>>>>
>>>   From my understanding, on RISC-V, the use of PC-relative address is
>>> only guaranteed with medany. So if you were going to change the cmodel
>>> (Andrew suggested you would), then early_*() may end up to be broken.
>>>
>>> This check serve as a documentation of the assumption and also help the
>>> developer any change in the model and take the appropriate action to
>>> remediate it.
>>>
>>>> I think this is safe to remove.
>>> Based on what I wrote above, do you still think this is safe?
>> With that in mind it's probably worth leaving in then. Maybe the
>> comment should be updated to make it explicit why we want this check
>> (I find the current comment not very helpful).
> 
> The presence of this check pre-supposes that Xen will always relocate
> itself, and this simply not true.  XIP for example typically does not
> 
> Furthermore it's not checking the property wanted.  The way C is
> compiled has no bearing on what relocation head.S uses to call it.

I think we are still cross-talking each other because this is not my 
point. I am not sure how to explain differently...

This check is not about how head.S call early_*() but making sure the C 
function can be executed with the MMU off. In which case, you will 
likely have VA != PA.

In theory head.S could apply some relocations before hand, but it may be 
too complicated to do it before calling early_*().

> 
> It is a given that we compile the hypervisor with a consistent code
> model, meaning that the properly actually making the check do what is
> wanted is also the property that means it is not needed in the first place.

See above.

> 
> This check is literally not worth the bytes it's taking up on disk, and
> not worth the added compiler time, nor the wasted time of whoever comes
> to support something like XIP in the future finds it to be in the way.
> Xen as a whole will really genuinely function as intended in models
> other than medany, and it demonstrates a misunderstanding of the topic
> to put in such a check to begin with.

Then enlight me :). So far it looks more like you are not understanding 
my point: I am talking about C function call with MMU off (e.g. VA != 
PA) and you are talking about Xen as a whole.

I guess the only way to really know is to implement a different model. 
At which point there are three possible outcome:
   1) We had the compiler check, it fired and the developper will take 
action to fix it (if needed).
   2) We have no compiler check, the developper knew what to do it and 
fixed it.
   3) We have no compiler check, the developper where not aware of the 
problem and we will waste time.

Even if you disagree with the check, then I think 1 is still the best 
because it would explain our assumption. Yes it may waste a few minutes 
to the developer switching the model. But that probably nothing compare 
to the time you could waste if you don't notice it.

Anyway, Alistair has now all the information to take an informative 
decision.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[relevance 11%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 23:17 13%         ` Alistair Francis
@ 2023-02-01  0:21 12%           ` Andrew Cooper
  2023-02-01  9:06 11%             ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Andrew Cooper @ 2023-02-01  0:21 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Oleksii, xen-devel, Bob Eshleman, Alistair Francis,
	Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Connor Davis, Bobby Eshleman, Julien Grall

On 31/01/2023 11:17 pm, Alistair Francis wrote:
> On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
>> On 31/01/2023 11:44, Alistair Francis wrote:
>>> On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
>>>
>>  From my understanding, on RISC-V, the use of PC-relative address is
>> only guaranteed with medany. So if you were going to change the cmodel
>> (Andrew suggested you would), then early_*() may end up to be broken.
>>
>> This check serve as a documentation of the assumption and also help the
>> developer any change in the model and take the appropriate action to
>> remediate it.
>>
>>> I think this is safe to remove.
>> Based on what I wrote above, do you still think this is safe?
> With that in mind it's probably worth leaving in then. Maybe the
> comment should be updated to make it explicit why we want this check
> (I find the current comment not very helpful).

The presence of this check pre-supposes that Xen will always relocate
itself, and this simply not true.  XIP for example typically does not.

Furthermore it's not checking the property wanted.  The way C is
compiled has no bearing on what relocation head.S uses to call it.

It is a given that we compile the hypervisor with a consistent code
model, meaning that the properly actually making the check do what is
wanted is also the property that means it is not needed in the first place.

This check is literally not worth the bytes it's taking up on disk, and
not worth the added compiler time, nor the wasted time of whoever comes
to support something like XIP in the future finds it to be in the way. 
Xen as a whole will really genuinely function as intended in models
other than medany, and it demonstrates a misunderstanding of the topic
to put in such a check to begin with.

~Andrew


^ permalink raw reply	[relevance 12%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 12:03 12%       ` Julien Grall
@ 2023-01-31 23:17 13%         ` Alistair Francis
  2023-02-01  0:21 12%           ` Andrew Cooper
  0 siblings, 1 reply; 57+ results
From: Alistair Francis @ 2023-01-31 23:17 UTC (permalink / raw)
  To: Julien Grall
  Cc: Oleksii, xen-devel, Bob Eshleman, Alistair Francis,
	Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Connor Davis, Bobby Eshleman

On Tue, Jan 31, 2023 at 10:03 PM Julien Grall <julien@xen.org> wrote:
>
>
>
> On 31/01/2023 11:44, Alistair Francis wrote:
> > On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
> >>
> >> Hi Alistair, Bobby and community,
> >>
> >> I would like to ask your help with the following check:
> >> +/*
> >> + * 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 with MMU-off"
> >> +#endif
> >
> > I have never seen a check like this before.
>
> The check is in the Linux code, see [3].
>
> > I don't really understand
> > what it's looking for, if the linker is unable to call early_*() I
> > would expect it to throw an error. I'm not sure what this is adding.
>
> When the MMU is off during early boot, you want any C function called to
> use PC-relative address rather than absolute address. This is because
> the physical address may not match the virtual address.

Ah!

I forgot that Xen would be compiled for virtual addresses, I have
spent too much time running on systems without an MMU recently.

>
>  From my understanding, on RISC-V, the use of PC-relative address is
> only guaranteed with medany. So if you were going to change the cmodel
> (Andrew suggested you would), then early_*() may end up to be broken.
>
> This check serve as a documentation of the assumption and also help the
> developer any change in the model and take the appropriate action to
> remediate it.
>
> >
> > I think this is safe to remove.
> Based on what I wrote above, do you still think this is safe?

With that in mind it's probably worth leaving in then. Maybe the
comment should be updated to make it explicit why we want this check
(I find the current comment not very helpful).

Alistair

>
> Cheers,
>
> >> Please take a look at the following messages and help me to decide if
> >> the check mentioned above should be in early_printk.c or not:
> >> [1]
> >> https://lore.kernel.org/xen-devel/599792fa-b08c-0b1e-10c1-0451519d9e4a@xen.org/
> >> [2]
> >> https://lore.kernel.org/xen-devel/0ec33871-96fa-bd9f-eb1b-eb37d3d7c982@xen.org/
>
> [3]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/mm/init.c
>
>
> --
> Julien Grall


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 11:42 13%   ` Julien Grall
@ 2023-01-31 12:37 13%     ` Oleksii
  2023-02-07 11:28 13%       ` Oleksii
  0 siblings, 1 reply; 57+ results
From: Oleksii @ 2023-01-31 12:37 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi Julien,

On Tue, 2023-01-31 at 11:42 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 31/01/2023 11:17, 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/a3c9916bbdff7ad6030055bbee7e53d
> > 1aab71384)
> > 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>
> > Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> > ---
> > Changes in V8:
> >      - Nothing was changed
> > ---
> > Changes in V7:
> >      - Nothing was changed
> > ---
> > Changes in V6:
> >      - Remove __riscv_cmodel_medany check from early_printk.c
> 
> This discussion is still not resolved. I will echo Jan's point [1]
> and 
> expand it. There is limited point to keep sending a new version for 
> small changes if there main open points are not addressed.
> 
> Can you please look at settling done on the issues first and then
> send a 
> new version?
Sure, I won't provide new patch series until the issue will be
resolved.

This patch series has been sent to resolve an issue with CI&CD which I
missed after the renaming of task for RISC-V in build.yaml.

~ Oleksii
> 
> Cheers,
> 
> [1] 1d63dd9a-77df-4fca-e35b-886ba19fb35d@suse.com
> 



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 11:44 13%     ` Alistair Francis
@ 2023-01-31 12:03 12%       ` Julien Grall
  2023-01-31 23:17 13%         ` Alistair Francis
  0 siblings, 1 reply; 57+ results
From: Julien Grall @ 2023-01-31 12:03 UTC (permalink / raw)
  To: Alistair Francis, Oleksii
  Cc: xen-devel, Bob Eshleman, Alistair Francis, Andrew Cooper,
	Jan Beulich, Stefano Stabellini, Gianluca Guida, Connor Davis,
	Bobby Eshleman



On 31/01/2023 11:44, Alistair Francis wrote:
> On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
>>
>> Hi Alistair, Bobby and community,
>>
>> I would like to ask your help with the following check:
>> +/*
>> + * 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 with MMU-off"
>> +#endif
> 
> I have never seen a check like this before. 

The check is in the Linux code, see [3].

> I don't really understand
> what it's looking for, if the linker is unable to call early_*() I
> would expect it to throw an error. I'm not sure what this is adding.

When the MMU is off during early boot, you want any C function called to 
use PC-relative address rather than absolute address. This is because 
the physical address may not match the virtual address.

 From my understanding, on RISC-V, the use of PC-relative address is 
only guaranteed with medany. So if you were going to change the cmodel 
(Andrew suggested you would), then early_*() may end up to be broken.

This check serve as a documentation of the assumption and also help the 
developer any change in the model and take the appropriate action to 
remediate it.

> 
> I think this is safe to remove.
Based on what I wrote above, do you still think this is safe?

Cheers,

>> Please take a look at the following messages and help me to decide if
>> the check mentioned above should be in early_printk.c or not:
>> [1]
>> https://lore.kernel.org/xen-devel/599792fa-b08c-0b1e-10c1-0451519d9e4a@xen.org/
>> [2]
>> https://lore.kernel.org/xen-devel/0ec33871-96fa-bd9f-eb1b-eb37d3d7c982@xen.org/

[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/riscv/mm/init.c


-- 
Julien Grall


^ permalink raw reply	[relevance 12%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 14:15 13%   ` Oleksii
@ 2023-01-31 11:44 13%     ` Alistair Francis
  2023-01-31 12:03 12%       ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Alistair Francis @ 2023-01-31 11:44 UTC (permalink / raw)
  To: Oleksii
  Cc: xen-devel, Bob Eshleman, Alistair Francis, Julien Grall,
	Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Connor Davis, Bobby Eshleman

On Sat, Jan 28, 2023 at 12:15 AM Oleksii <oleksii.kurochko@gmail.com> wrote:
>
> Hi Alistair, Bobby and community,
>
> I would like to ask your help with the following check:
> +/*
> + * 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 with MMU-off"
> +#endif

I have never seen a check like this before. I don't really understand
what it's looking for, if the linker is unable to call early_*() I
would expect it to throw an error. I'm not sure what this is adding.

I think this is safe to remove.

Alistair

>
> Please take a look at the following messages and help me to decide if
> the check mentioned above should be in early_printk.c or not:
> [1]
> https://lore.kernel.org/xen-devel/599792fa-b08c-0b1e-10c1-0451519d9e4a@xen.org/
> [2]
> https://lore.kernel.org/xen-devel/0ec33871-96fa-bd9f-eb1b-eb37d3d7c982@xen.org/
>
> Thanks in advance.
>
> ~ Oleksii
>
> On Fri, 2023-01-27 at 13:39 +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/a3c9916bbdff7ad6030055bbee7e53d1a
> > ab71384)
> > 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>
> > Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> > ---
> > Changes in V7:
> >     - Nothing was changed
> > ---
> > Changes in V6:
> >     - Remove __riscv_cmodel_medany check from early_printk.c
> > ---
> > Changes in V5:
> >     - Code style fixes
> >     - Change an error message of #error in case of
> > __riscv_cmodel_medany
> >       isn't defined
> > ---
> > 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              |  5 ++++
> >  xen/arch/riscv/Makefile                   |  1 +
> >  xen/arch/riscv/early_printk.c             | 33
> > +++++++++++++++++++++++
> >  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
> >  xen/arch/riscv/setup.c                    |  4 +++
> >  5 files changed, 55 insertions(+)
> >  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..608c9ff832 100644
> > --- a/xen/arch/riscv/Kconfig.debug
> > +++ b/xen/arch/riscv/Kconfig.debug
> > @@ -0,0 +1,5 @@
> > +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..b66a11f1bc
> > --- /dev/null
> > +++ b/xen/arch/riscv/early_printk.c
> > @@ -0,0 +1,33 @@
> > +/* 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>
> > +
> > +/*
> > + * 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..d09ffe1454 100644
> > --- a/xen/arch/riscv/setup.c
> > +++ b/xen/arch/riscv/setup.c
> > @@ -1,12 +1,16 @@
> >  #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)
> >  {
> > +    early_printk("Hello from C env\n");
> > +
> >      for ( ;; )
> >          asm volatile ("wfi");
> >
>
>


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 11:17  9% ` [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-31 11:42 13%   ` Julien Grall
  2023-01-31 12:37 13%     ` Oleksii
  0 siblings, 1 reply; 57+ results
From: Julien Grall @ 2023-01-31 11:42 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi Oleksii,

On 31/01/2023 11:17, 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>
> Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> ---
> Changes in V8:
>      - Nothing was changed
> ---
> Changes in V7:
>      - Nothing was changed
> ---
> Changes in V6:
>      - Remove __riscv_cmodel_medany check from early_printk.c

This discussion is still not resolved. I will echo Jan's point [1] and 
expand it. There is limited point to keep sending a new version for 
small changes if there main open points are not addressed.

Can you please look at settling done on the issues first and then send a 
new version?

Cheers,

[1] 1d63dd9a-77df-4fca-e35b-886ba19fb35d@suse.com

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-31 11:17 11% [PATCH v8 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-31 11:17  9% ` Oleksii Kurochko
  2023-01-31 11:42 13%   ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-31 11:17 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V8:
    - Nothing was changed
---
Changes in V7:
    - Nothing was changed
---
Changes in V6:
    - Remove __riscv_cmodel_medany check from early_printk.c
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 55 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..b66a11f1bc
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* 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>
+
+/*
+ * 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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* [PATCH v8 0/2] Basic early_printk and smoke test implementation
@ 2023-01-31 11:17 11% Oleksii Kurochko
  2023-01-31 11:17  9% ` [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-31 11:17 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V8:
 - Set "needs: archlinux-current-gcc-riscv64-debug" in test.yaml
   for RISCV job as CONFIG_EARLY_PRINTK is available only when
   CONFIG_DEBUG is available.
---
Changes in V7:
 - Fix dependecy for qemu-smoke-riscv64-gcc job
---
Changes in V6:
 - Rename container name in test.yaml for .qemu-riscv64.
---
Changes in V5:
  - Nothing changed
---
Changes in V4:
  - Nothing changed
---
Changes in V3:
  - Nothing changed
  - All mentioned comments by Stefano in Xen mailing list will be
    fixed as a separate patch out of this patch series.
---

Oleksii Kurochko (2):
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++++++
 xen/arch/riscv/Kconfig.debug              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 7 files changed, 95 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h

-- 
2.39.0



^ permalink raw reply	[relevance 11%]

* Re: [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:39  9% ` [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-27 14:15 13%   ` Oleksii
  2023-01-31 11:44 13%     ` Alistair Francis
  0 siblings, 1 reply; 57+ results
From: Oleksii @ 2023-01-27 14:15 UTC (permalink / raw)
  To: xen-devel, Bob Eshleman, Alistair Francis, Julien Grall
  Cc: Andrew Cooper, Jan Beulich, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi Alistair, Bobby and community,

I would like to ask your help with the following check:
+/*
+ * 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 with MMU-off"
+#endif

Please take a look at the following messages and help me to decide if
the check mentioned above should be in early_printk.c or not:
[1]
https://lore.kernel.org/xen-devel/599792fa-b08c-0b1e-10c1-0451519d9e4a@xen.org/
[2]
https://lore.kernel.org/xen-devel/0ec33871-96fa-bd9f-eb1b-eb37d3d7c982@xen.org/

Thanks in advance.

~ Oleksii

On Fri, 2023-01-27 at 13:39 +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/a3c9916bbdff7ad6030055bbee7e53d1a
> ab71384)
> 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>
> Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> ---
> Changes in V7:
>     - Nothing was changed
> ---
> Changes in V6:
>     - Remove __riscv_cmodel_medany check from early_printk.c
> ---
> Changes in V5:
>     - Code style fixes
>     - Change an error message of #error in case of
> __riscv_cmodel_medany
>       isn't defined
> ---
> 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              |  5 ++++
>  xen/arch/riscv/Makefile                   |  1 +
>  xen/arch/riscv/early_printk.c             | 33
> +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
>  xen/arch/riscv/setup.c                    |  4 +++
>  5 files changed, 55 insertions(+)
>  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..608c9ff832 100644
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,5 @@
> +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..b66a11f1bc
> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,33 @@
> +/* 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>
> +
> +/*
> + * 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..d09ffe1454 100644
> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -1,12 +1,16 @@
>  #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)
>  {
> +    early_printk("Hello from C env\n");
> +
>      for ( ;; )
>          asm volatile ("wfi");
>  



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:49 13%     ` Oleksii
@ 2023-01-27 12:22 12%       ` Julien Grall
  0 siblings, 0 replies; 57+ results
From: Julien Grall @ 2023-01-27 12:22 UTC (permalink / raw)
  To: Oleksii, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi Oleksii,

On 27/01/2023 11:49, Oleksii wrote:
> On Fri, 2023-01-27 at 11:26 +0000, Julien Grall wrote:
>> Hi,
>>
>> On 27/01/2023 11:15, 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/a3c9916bbdff7ad6030055bbee7e53d
>>> 1aab71384)
>>> 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>
>>> Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
>>> ---
>>> Changes in V6:
>>>       - Remove __riscv_cmodel_medany check from early_printk.c
>>
>> Why? I know Andrew believed this is wrong, but I replied back with my
>> understanding and saw no discussion afterwards explaining why I am
>> incorrect.
>>
>> I am not a maintainer of the code here, but I don't particularly
>> appreciate comments to be ignored. If there was any discussion on
>> IRC,
>> then please summarize them here.
> Sorry, I have to mentioned that in the description of patch series.

I think this should be part of the section --- in this patch as this 
makes easier to know what changes have been done.

> 
> There is no any specific reason to remove and only one reason I decided
> to remove the check was that the check wasn't present in original
> Alistair/Bobby patches and based on my experiments with that patches
> all worked fine. ( at least with some additional patches from my side I
> was able to run Dom0 with console )

The lines you removed only confirm that the file was built with the 
given model and if it is incorrect then the compilation will fail. There 
are no change in behavior expected past the compilation.

So I am not quite too sure what sort of experiment you did. Did you try 
to change the model used to build Xen?

Now if you (or anyone else) can tell me that there will be no issues if 
the model is changed. Then yes, I will agree that the check is unnecessary.

The alternative is you say that you are happy to accept the risk if the 
model is changed. If I were the maintainer, that would not be something 
I would agree with because "wrong" unwritten assumptions are a pain to 
debug. So I much prefer a "wrong" written assumptions that would tip me 
that the author thought the code would not work otherwise.

This is quite easy to remove the check after the fact if it is not correct.

I am not the maintainer of the code, so if this is what they want then 
so be it. But it needs to be explicitely stated. So far, the reviewed-by 
from Bobby was with the check, so it would imply that he was happy with 
it...

Cheers,

-- 
Julien Grall


^ permalink raw reply	[relevance 12%]

* Re: [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:26 13%   ` Julien Grall
@ 2023-01-27 11:49 13%     ` Oleksii
  2023-01-27 12:22 12%       ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Oleksii @ 2023-01-27 11:49 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

On Fri, 2023-01-27 at 11:26 +0000, Julien Grall wrote:
> Hi,
> 
> On 27/01/2023 11:15, 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/a3c9916bbdff7ad6030055bbee7e53d
> > 1aab71384)
> > 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>
> > Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> > ---
> > Changes in V6:
> >      - Remove __riscv_cmodel_medany check from early_printk.c
> 
> Why? I know Andrew believed this is wrong, but I replied back with my
> understanding and saw no discussion afterwards explaining why I am 
> incorrect.
> 
> I am not a maintainer of the code here, but I don't particularly 
> appreciate comments to be ignored. If there was any discussion on
> IRC, 
> then please summarize them here.
Sorry, I have to mentioned that in the description of patch series.

There is no any specific reason to remove and only one reason I decided
to remove the check was that the check wasn't present in original
Alistair/Bobby patches and based on my experiments with that patches 
all worked fine. ( at least with some additional patches from my side I
was able to run Dom0 with console )

I pushed a new version (v7) of the patch series ( as I missed to change
dependency for CI job ) so probably we have to open a discussion again
as Andrew don't answer for a long time.
> 
> Cheers,
> 
~ Oleksii



^ permalink raw reply	[relevance 13%]

* [PATCH v7 0/2] Basic early_printk and smoke test implementation
@ 2023-01-27 11:39 13% Oleksii Kurochko
  2023-01-27 11:39  9% ` [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-27 11:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V7:
  - Fix dependecy for qemu-smoke-riscv64-gcc job
---
Changes in V6:
  - Remove patches that were merged to upstream:
      * xen/include: Change <asm/types.h> to <xen/types.h>
      * xen/riscv: introduce asm/types.h header file
      * xen/riscv: introduce sbi call to putchar to console
  - Remove __riscv_cmodel_medany check from early_printk.C
  - Rename container name in test.yaml for .qemu-riscv64.
---
Changes in V5:
  - Code style fixes
  - Remove size_t from asm/types after rebase on top of the patch
    "include/types: move stddef.h-kind types to common header". [1]

    All other types were back as they are used in <xen/types.h>
  - Update <xen/early_printk.h> after rebase on top of [1] as size_t was moved from
    <asm/types.h> to <xen/types.h>
  - Remove unneeded <xen/errno.h> from sbi.c
  - Change an error message of #error in case of __riscv_cmodel_medany isn't defined
---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv: introduce
      stack stuff" were removed from the patch series as they were merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---

Oleksii Kurochko (2):
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++++++
 xen/arch/riscv/Kconfig.debug              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 7 files changed, 95 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h

-- 
2.39.0



^ permalink raw reply	[relevance 13%]

* [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:39 13% [PATCH v7 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-27 11:39  9% ` Oleksii Kurochko
  2023-01-27 14:15 13%   ` Oleksii
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-27 11:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V7:
    - Nothing was changed
---
Changes in V6:
    - Remove __riscv_cmodel_medany check from early_printk.c
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 55 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..b66a11f1bc
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* 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>
+
+/*
+ * 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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* Re: [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:15  9% ` [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-27 11:26 13%   ` Julien Grall
  2023-01-27 11:49 13%     ` Oleksii
  0 siblings, 1 reply; 57+ results
From: Julien Grall @ 2023-01-27 11:26 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi,

On 27/01/2023 11:15, 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>
> Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> ---
> Changes in V6:
>      - Remove __riscv_cmodel_medany check from early_printk.c

Why? I know Andrew believed this is wrong, but I replied back with my 
understanding and saw no discussion afterwards explaining why I am 
incorrect.

I am not a maintainer of the code here, but I don't particularly 
appreciate comments to be ignored. If there was any discussion on IRC, 
then please summarize them here.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff
  2023-01-27 11:15 14% [PATCH v6 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-27 11:15  9% ` Oleksii Kurochko
  2023-01-27 11:26 13%   ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-27 11:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V6:
    - Remove __riscv_cmodel_medany check from early_printk.c
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 55 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..b66a11f1bc
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* 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>
+
+/*
+ * 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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* [PATCH v6 0/2]  Basic early_printk and smoke test implementation
@ 2023-01-27 11:15 14% Oleksii Kurochko
  2023-01-27 11:15  9% ` [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-27 11:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V6:
  - Remove patches that were merged to upstream:
      * xen/include: Change <asm/types.h> to <xen/types.h>
      * xen/riscv: introduce asm/types.h header file
      * xen/riscv: introduce sbi call to putchar to console
  - Remove __riscv_cmodel_medany check from early_printk.C
  - Rename container name in test.yaml for .qemu-riscv64.
---
Changes in V5:
  - Code style fixes
  - Remove size_t from asm/types after rebase on top of the patch
    "include/types: move stddef.h-kind types to common header". [1]

    All other types were back as they are used in <xen/types.h>
  - Update <xen/early_printk.h> after rebase on top of [1] as size_t was moved from
    <asm/types.h> to <xen/types.h>
  - Remove unneeded <xen/errno.h> from sbi.c
  - Change an error message of #error in case of __riscv_cmodel_medany isn't defined
---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv: introduce
      stack stuff" were removed from the patch series as they were merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---

Oleksii Kurochko (2):
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++++++
 xen/arch/riscv/Kconfig.debug              |  5 ++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 7 files changed, 95 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h

-- 
2.39.0



^ permalink raw reply	[relevance 14%]

* Re: [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff
  2023-01-20  0:48 13%   ` Andrew Cooper
@ 2023-01-20  9:17 13%     ` Julien Grall
  0 siblings, 0 replies; 57+ results
From: Julien Grall @ 2023-01-20  9:17 UTC (permalink / raw)
  To: Andrew Cooper, Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis, Bobby Eshleman

Hi,

On 20/01/2023 00:48, Andrew Cooper wrote:
> On 19/01/2023 2:07 pm, Oleksii Kurochko wrote:
>> diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
>> new file mode 100644
>> index 0000000000..6f590e712b
>> --- /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 with MMU-off"
>> +#endif
> 
> This comment is false, and the check is bogus.

You are already said that in the previous version and ... I reply back 
explaining why I think this is correct (see [1]).

>  > It needs deleting.

That might be the second step. The first step is we settle down on the 
approach.

Cheers,

[1] 
https://lore.kernel.org/xen-devel/CAF3u54C2ewEfBN+ZT6VPaVu4vsqS_+12gr3YJ_jsg1sGHDhZ1A@mail.gmail.com/

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff
  2023-01-19 14:07  9% ` [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-20  0:48 13%   ` Andrew Cooper
  2023-01-20  9:17 13%     ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Andrew Cooper @ 2023-01-20  0:48 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Julien Grall, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

On 19/01/2023 2:07 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
> new file mode 100644
> index 0000000000..6f590e712b
> --- /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 with MMU-off"
> +#endif

This comment is false, and the check is bogus.

It needs deleting.

~Andrew

^ permalink raw reply	[relevance 13%]

* [PATCH v5 0/5]  Basic early_printk and smoke test implementation
@ 2023-01-19 14:07 13% Oleksii Kurochko
  2023-01-19 14:07  9% ` [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, George Dunlap, Wei Liu,
	Bob Eshleman, Alistair Francis, Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

The patch series is rebased on top of the patch "include/types: move
stddef.h-kind types to common header" [1]

[1] https://lore.kernel.org/xen-devel/5a0a9e2a-c116-21b5-8081-db75fe4178d7@suse.com/

---
Changes in V5:
  - Code style fixes
  - Remove size_t from asm/types after rebase on top of the patch
    "include/types: move stddef.h-kind types to common header". [1]

    All other types were back as they are used in <xen/types.h>
  - Update <xen/early_printk.h> after rebase on top of [1] as size_t was moved from
    <asm/types.h> to <xen/types.h>
  - Remove unneeded <xen/errno.h> from sbi.c
  - Change an error message of #error in case of __riscv_cmodel_medany isn't defined
---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv: introduce
      stack stuff" were removed from the patch series as they were merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---

Bobby Eshleman (1):
  xen/riscv: introduce sbi call to putchar to console

Oleksii Kurochko (4):
  xen/include: Change <asm/types.h> to <xen/types.h>
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 +++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 +++++++
 xen/arch/riscv/Kconfig.debug              |  5 ++
 xen/arch/riscv/Makefile                   |  2 +
 xen/arch/riscv/early_printk.c             | 45 +++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++
 xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++
 xen/arch/riscv/include/asm/types.h        | 70 +++++++++++++++++++++++
 xen/arch/riscv/sbi.c                      | 44 ++++++++++++++
 xen/arch/riscv/setup.c                    |  4 ++
 xen/include/xen/early_printk.h            |  2 +-
 11 files changed, 257 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c

-- 
2.39.0



^ permalink raw reply	[relevance 13%]

* [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff
  2023-01-19 14:07 13% [PATCH v5 0/5] " Oleksii Kurochko
@ 2023-01-19 14:07  9% ` Oleksii Kurochko
  2023-01-20  0:48 13%   ` Andrew Cooper
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-19 14:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>
Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>
---
Changes in V5:
    - Code style fixes
    - Change an error message of #error in case of __riscv_cmodel_medany
      isn't defined
---
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              |  5 +++
 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                    |  4 ++
 5 files changed, 67 insertions(+)
 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..608c9ff832 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,5 @@
+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..6f590e712b
--- /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 with MMU-off"
+#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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.39.0



^ permalink raw reply related	[relevance 9%]

* [PATCH v4 0/4] Basic early_printk and smoke test implementation
  2023-01-16 14:39 13% [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-16 14:44 14% ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
@ 2023-01-19 12:45 14% ` Oleksii Kurochko
  2 siblings, 0 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-19 12:45 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv: introduce
      stack stuff" were removed from the patch series as they were merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---


Bobby Eshleman (1):
  xen/riscv: introduce sbi call to putchar to console

Oleksii Kurochko (3):
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++
 xen/arch/riscv/Kconfig.debug              |  6 +++
 xen/arch/riscv/Makefile                   |  2 +
 xen/arch/riscv/early_printk.c             | 45 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++++
 xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++++++++
 xen/arch/riscv/include/asm/types.h        | 43 ++++++++++++++++++++++
 xen/arch/riscv/sbi.c                      | 45 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                    |  6 ++-
 10 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c

-- 
2.39.0



^ permalink raw reply	[relevance 14%]

* [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-17 23:22 13%   ` Bobby Eshleman
  2023-01-17 23:57 12%   ` Andrew Cooper
@ 2023-01-19 12:45  9%   ` Oleksii Kurochko
  2 siblings, 0 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-19 12:45 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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



^ permalink raw reply related	[relevance 9%]

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-17 23:57 12%   ` Andrew Cooper
  2023-01-18  0:33 13%     ` Julien Grall
@ 2023-01-18 11:07 13%     ` Oleksii
  1 sibling, 0 replies; 57+ results
From: Oleksii @ 2023-01-18 11:07 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel
  Cc: Jan Beulich, Julien Grall, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

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


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-17 23:57 12%   ` Andrew Cooper
@ 2023-01-18  0:33 13%     ` Julien Grall
  2023-01-18 11:07 13%     ` Oleksii
  1 sibling, 0 replies; 57+ results
From: Julien Grall @ 2023-01-18  0:33 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Alistair Francis, Bob Eshleman, Bobby Eshleman, Connor Davis,
	Gianluca Guida, Jan Beulich, Julien Grall, Oleksii Kurochko,
	Stefano Stabellini, xen-devel

[-- Attachment #1: Type: text/plain, Size: 4052 bytes --]

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
>
-- 
Julien Grall

[-- Attachment #2: Type: text/html, Size: 6158 bytes --]

^ permalink raw reply	[relevance 13%]

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-17 23:22 13%   ` Bobby Eshleman
@ 2023-01-17 23:57 12%   ` Andrew Cooper
  2023-01-18  0:33 13%     ` Julien Grall
  2023-01-18 11:07 13%     ` Oleksii
  2023-01-19 12:45  9%   ` Oleksii Kurochko
  2 siblings, 2 replies; 57+ results
From: Andrew Cooper @ 2023-01-17 23:57 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Julien Grall, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

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

^ permalink raw reply	[relevance 12%]

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-17 23:22 13%   ` Bobby Eshleman
  2023-01-17 23:57 12%   ` Andrew Cooper
  2023-01-19 12:45  9%   ` Oleksii Kurochko
  2 siblings, 0 replies; 57+ results
From: Bobby Eshleman @ 2023-01-17 23:22 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Alistair Francis,
	Connor Davis, Bobby Eshleman

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>


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v4 0/4] The patch series introduces the following:
  2023-01-16 14:39 13% [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-16 14:44 14% ` Oleksii
  2023-01-19 12:45 14% ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
  2 siblings, 0 replies; 57+ results
From: Oleksii @ 2023-01-16 14:44 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Bob Eshleman, Alistair Francis, Connor Davis,
	Doug Goldstein

Hi all,

Something went wrong with cover letter. I do not know if i have to
make new patch series but cover letter should be the following:


Subject: [PATCH v4 0/4] Basic early_printk and smoke test
implementation

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv:
introduce
      stack stuff" were removed from the patch series as they were
merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is
located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C
env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk
basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged
to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---


Bobby Eshleman (1):
  xen/riscv: introduce sbi call to putchar to console

Oleksii Kurochko (3):
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++
 xen/arch/riscv/Kconfig.debug              |  6 +++
 xen/arch/riscv/Makefile                   |  2 +
 xen/arch/riscv/early_printk.c             | 45 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++++
 xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++++++++
 xen/arch/riscv/include/asm/types.h        | 43 ++++++++++++++++++++++
 xen/arch/riscv/sbi.c                      | 45 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                    |  6 ++-
 10 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c

-- 
2.39.0

On Mon, 2023-01-16 at 16:39 +0200, Oleksii Kurochko wrote:
> ---
> Changes in V4:
>     - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv:
> introduce
>       stack stuff" were removed from the patch series as they were
> merged separately
>       into staging.
>     - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug
> is located
>       in arch specific folder.
>     - fix code style.
>     - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
> ---
> Changes in V3:
>     - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C
> env"
>       was merged with [PATCH v2 3/6] xen/riscv: introduce stack
> stuff.
>     - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
>       merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk
> basic
>       stuff".
>     - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
>       <xen/early_printk.h>" was removed as it has been already merged
> to
>       mainline staging.
>     - code style fixes.
> ---
> Changes in V2:
>     - update commit patches commit messages according to the mailing
>       list comments
>     - Remove unneeded types in <asm/types.h>
>     - Introduce definition of STACK_SIZE
>     - order the files alphabetically in Makefile
>     - Add license to early_printk.c
>     - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
>     - Move dockerfile changes to separate config and sent them as
>       separate patch to mailing list.
>     - Update test.yaml to wire up smoke test
> ---
> 
> 
> Bobby Eshleman (1):
>   xen/riscv: introduce sbi call to putchar to console
> 
> Oleksii Kurochko (3):
>   xen/riscv: introduce asm/types.h header file
>   xen/riscv: introduce early_printk basic stuff
>   automation: add RISC-V smoke test
> 
>  automation/gitlab-ci/test.yaml            | 20 ++++++++++
>  automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++
>  xen/arch/riscv/Kconfig.debug              |  6 +++
>  xen/arch/riscv/Makefile                   |  2 +
>  xen/arch/riscv/early_printk.c             | 45
> +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/early_printk.h | 12 ++++++
>  xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++++++++
>  xen/arch/riscv/include/asm/types.h        | 43
> ++++++++++++++++++++++
>  xen/arch/riscv/sbi.c                      | 45
> +++++++++++++++++++++++
>  xen/arch/riscv/setup.c                    |  6 ++-
>  10 files changed, 232 insertions(+), 1 deletion(-)
>  create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
>  create mode 100644 xen/arch/riscv/early_printk.c
>  create mode 100644 xen/arch/riscv/include/asm/early_printk.h
>  create mode 100644 xen/arch/riscv/include/asm/sbi.h
>  create mode 100644 xen/arch/riscv/include/asm/types.h
>  create mode 100644 xen/arch/riscv/sbi.c
> 



^ permalink raw reply	[relevance 14%]

* [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39 13% [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
@ 2023-01-16 14:39  9% ` Oleksii Kurochko
  2023-01-17 23:22 13%   ` Bobby Eshleman
                     ` (2 more replies)
  2023-01-16 14:44 14% ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
  2023-01-19 12:45 14% ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
  2 siblings, 3 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-16 14:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

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



^ permalink raw reply related	[relevance 9%]

* [PATCH v4 0/4] The patch series introduces the following:
@ 2023-01-16 14:39 13% Oleksii Kurochko
  2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
                   ` (2 more replies)
  0 siblings, 3 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-16 14:39 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

---
Changes in V4:
    - Patches "xen/riscv: introduce dummy asm/init.h" and "xen/riscv: introduce
      stack stuff" were removed from the patch series as they were merged separately
      into staging.
    - Remove "depends on RISCV*" from Kconfig.debug as Kconfig.debug is located
      in arch specific folder.
    - fix code style.
    - Add "ifdef __riscv_cmodel_medany" to early_printk.c.  
---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---


Bobby Eshleman (1):
  xen/riscv: introduce sbi call to putchar to console

Oleksii Kurochko (3):
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce early_printk basic stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++
 xen/arch/riscv/Kconfig.debug              |  6 +++
 xen/arch/riscv/Makefile                   |  2 +
 xen/arch/riscv/early_printk.c             | 45 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++++
 xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++++++++
 xen/arch/riscv/include/asm/types.h        | 43 ++++++++++++++++++++++
 xen/arch/riscv/sbi.c                      | 45 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                    |  6 ++-
 10 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c

-- 
2.39.0



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v3 5/6] xen/riscv: introduce early_printk basic stuff
  2023-01-10 15:17 10% ` [PATCH v3 5/6] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-10 15:57 13%   ` Jan Beulich
  0 siblings, 0 replies; 57+ results
From: Jan Beulich @ 2023-01-10 15:57 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bobby Eshleman, Bob Eshleman, Alistair Francis, Connor Davis,
	xen-devel

On 10.01.2023 16:17, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,7 @@
> +config EARLY_PRINTK
> +    bool "Enable early printk"
> +    default DEBUG
> +    depends on RISCV_64 || RISCV_32

You're in a RISC-V-specific Kconfig - do you really need this line?

> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,33 @@
> +/* 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>
> +
> +/*
> + * 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')

Nit (style): Missing blanks.

> +            sbi_console_putchar('\r');
> +        sbi_console_putchar(*s);
> +        s++;
> +    }
> +}
> +
> +void early_printk(const char *str)
> +{
> +    while (*str)

Again.

> --- a/xen/arch/riscv/setup.c
> +++ b/xen/arch/riscv/setup.c
> @@ -1,12 +1,16 @@
>  #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)
>  {
> +    early_printk("Hello from C env\n");
> +
>      for ( ;; )
>          asm volatile ("wfi");

While this is only context here, it affects an earlier patch in the
series; this wants to be

    for ( ; ; )
        asm volatile ( "wfi" );

Jan


^ permalink raw reply	[relevance 13%]

* [PATCH v3 5/6] xen/riscv: introduce early_printk basic stuff
  2023-01-10 15:17 14% [PATCH v3 0/6] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-10 15:17 10% ` Oleksii Kurochko
  2023-01-10 15:57 13%   ` Jan Beulich
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-10 15:17 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Bobby Eshleman, Bob Eshleman, Alistair Francis,
	Connor Davis, Oleksii Kurochko

From: Bobby Eshleman <bobby.eshleman@gmail.com>

The patch introduces a basic stuff of early_printk functionality
which will be enough to print 'hello from C environment".
early_printk() function was changed in comparison with original as
common isn't being built now so there is no vscnprintf.

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.

This commit adds early printk implementation built on the putc SBI call.

As sbi_console_putchar() is being already planned for deprecation
it is used temporary 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 V3:
    - reorder "include <header>" in erly_printk.c
    - change an author of the commit
---
Changes in V2:
    - add license to early_printk.c
    - add signed-off-by Bobby
    - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
    - update commit message
    - order the files alphabetically in Makefile
---
 xen/arch/riscv/Kconfig.debug              |  7 +++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 xen/arch/riscv/setup.c                    |  4 +++
 5 files changed, 57 insertions(+)
 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..e21649781d 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,7 @@
+config EARLY_PRINTK
+    bool "Enable early printk"
+    default DEBUG
+    depends on RISCV_64 || RISCV_32
+    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..348c21bdaa
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* 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>
+
+/*
+ * 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..d09ffe1454 100644
--- a/xen/arch/riscv/setup.c
+++ b/xen/arch/riscv/setup.c
@@ -1,12 +1,16 @@
 #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)
 {
+    early_printk("Hello from C env\n");
+
     for ( ;; )
         asm volatile ("wfi");
 
-- 
2.38.1



^ permalink raw reply related	[relevance 10%]

* [PATCH v3 0/6] Basic early_printk and smoke test implementation
@ 2023-01-10 15:17 14% Oleksii Kurochko
  2023-01-10 15:17 10% ` [PATCH v3 5/6] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-10 15:17 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

---
Changes in V3:
    - Most of "[PATCH v2 7/8] xen/riscv: print hello message from C env"
      was merged with [PATCH v2 3/6] xen/riscv: introduce stack stuff.
    - "[PATCH v2 7/8] xen/riscv: print hello message from C env" was
      merged with "[PATCH v2 6/8] xen/riscv: introduce early_printk basic
      stuff".
    - "[PATCH v2 5/8] xen/include: include <asm/types.h> in
      <xen/early_printk.h>" was removed as it has been already merged to
      mainline staging.
    - code style fixes.
---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---

Bobby Eshleman (2):
  xen/riscv: introduce sbi call to putchar to console
  xen/riscv: introduce early_printk basic stuff

Oleksii Kurochko (4):
  xen/riscv: introduce dummy asm/init.h
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce stack stuff
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 ++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 ++++++++++
 xen/arch/riscv/Kconfig.debug              |  7 ++++
 xen/arch/riscv/Makefile                   |  3 ++
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++
 xen/arch/riscv/include/asm/config.h       |  2 +
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++++
 xen/arch/riscv/include/asm/init.h         | 12 ++++++
 xen/arch/riscv/include/asm/sbi.h          | 34 +++++++++++++++++
 xen/arch/riscv/include/asm/types.h        | 22 +++++++++++
 xen/arch/riscv/riscv64/head.S             |  6 ++-
 xen/arch/riscv/sbi.c                      | 45 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                    | 18 +++++++++
 13 files changed, 233 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/init.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c
 create mode 100644 xen/arch/riscv/setup.c

-- 
2.38.1



^ permalink raw reply	[relevance 14%]

* Re: [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-10  7:28 13%     ` Bobby Eshleman
@ 2023-01-10 10:47 13%       ` Alistair Francis
  0 siblings, 0 replies; 57+ results
From: Alistair Francis @ 2023-01-10 10:47 UTC (permalink / raw)
  To: Bobby Eshleman
  Cc: Alistair Francis, Andrew Cooper, Connor Davis, Gianluca Guida,
	Jan Beulich, Julien Grall, Oleksii Kurochko, Stefano Stabellini,
	xen-devel

On Tue, Jan 10, 2023 at 5:29 PM Bobby Eshleman <bobby.eshleman@gmail.com> wrote:
>
>
>
> On Mon, Jan 9, 2023 at 4:28 PM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Tue, Jan 10, 2023 at 1:47 AM Oleksii Kurochko
>> <oleksii.kurochko@gmail.com> wrote:
>> >
>> > The patch introduces a basic stuff of early_printk functionality
>> > which will be enough to print 'hello from C environment".
>> > early_printk() function was changed in comparison with original as
>> > common isn't being built now so there is no vscnprintf.
>> >
>> > 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.
>> >
>> > This commit adds early printk implementation built on the putc SBI call.
>> >
>> > As sbi_console_putchar() is being already planned for deprecation
>> > it is used temporary now and will be removed or reworked after
>> > real uart will be ready.
>>
>> There was a discussion to add a new SBI putchar replacement. It
>> doesn't seem to be completed yet, but there might be an SBI
>> replacement for this in the future as well.
>>
>> Alistair
>
>
> Are you referring to the Debug Console Extension (EID #0x4442434E "DBCN")”?
>
> https://lists.riscv.org/g/tech-prs/topic/96051183#84

That's the one!

Alistair

>
> Best,
> Bobby
>
>>
>> >
>> > Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
>> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> > ---
>> > Changes in V2:
>> >     - add license to early_printk.c
>> >     - add signed-off-by Bobby
>> >     - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
>> >     - update commit message
>> >     - order the files alphabetically in Makefile
>> > ---
>> >  xen/arch/riscv/Kconfig.debug              |  7 +++++
>> >  xen/arch/riscv/Makefile                   |  1 +
>> >  xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
>> >  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
>> >  4 files changed, 53 insertions(+)
>> >  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..6ba0bd1e5a 100644
>> > --- a/xen/arch/riscv/Kconfig.debug
>> > +++ b/xen/arch/riscv/Kconfig.debug
>> > @@ -0,0 +1,7 @@
>> > +config EARLY_PRINTK
>> > +    bool "Enable early printk config"
>> > +    default DEBUG
>> > +    depends on RISCV_64 || RISCV_32
>> > +    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..88da5169ed
>> > --- /dev/null
>> > +++ b/xen/arch/riscv/early_printk.c
>> > @@ -0,0 +1,33 @@
>> > +/* SPDX-License-Identifier: GPL-2.0 */
>> > +/*
>> > + * RISC-V early printk using SBI
>> > + *
>> > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
>> > + */
>> > +#include <asm/sbi.h>
>> > +#include <asm/early_printk.h>
>> > +
>> > +/*
>> > + * TODO:
>> > + *   sbi_console_putchar is already planned for deprication
>> > + *   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__ */
>> > --
>> > 2.38.1
>> >
>> >


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-10  0:28 13%   ` Alistair Francis
@ 2023-01-10  7:28 13%     ` Bobby Eshleman
  2023-01-10 10:47 13%       ` Alistair Francis
  0 siblings, 1 reply; 57+ results
From: Bobby Eshleman @ 2023-01-10  7:28 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, Andrew Cooper, Connor Davis, Gianluca Guida,
	Jan Beulich, Julien Grall, Oleksii Kurochko, Stefano Stabellini,
	xen-devel

[-- Attachment #1: Type: text/plain, Size: 4494 bytes --]

On Mon, Jan 9, 2023 at 4:28 PM Alistair Francis <alistair23@gmail.com>
wrote:

> On Tue, Jan 10, 2023 at 1:47 AM Oleksii Kurochko
> <oleksii.kurochko@gmail.com> wrote:
> >
> > The patch introduces a basic stuff of early_printk functionality
> > which will be enough to print 'hello from C environment".
> > early_printk() function was changed in comparison with original as
> > common isn't being built now so there is no vscnprintf.
> >
> > 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.
> >
> > This commit adds early printk implementation built on the putc SBI call.
> >
> > As sbi_console_putchar() is being already planned for deprecation
> > it is used temporary now and will be removed or reworked after
> > real uart will be ready.
>
> There was a discussion to add a new SBI putchar replacement. It
> doesn't seem to be completed yet, but there might be an SBI
> replacement for this in the future as well.
>
> Alistair
>

Are you referring to the Debug Console Extension (EID #0x4442434E "DBCN")”?

https://lists.riscv.org/g/tech-prs/topic/96051183#84

Best,
Bobby


> >
> > Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V2:
> >     - add license to early_printk.c
> >     - add signed-off-by Bobby
> >     - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
> >     - update commit message
> >     - order the files alphabetically in Makefile
> > ---
> >  xen/arch/riscv/Kconfig.debug              |  7 +++++
> >  xen/arch/riscv/Makefile                   |  1 +
> >  xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
> >  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
> >  4 files changed, 53 insertions(+)
> >  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..6ba0bd1e5a 100644
> > --- a/xen/arch/riscv/Kconfig.debug
> > +++ b/xen/arch/riscv/Kconfig.debug
> > @@ -0,0 +1,7 @@
> > +config EARLY_PRINTK
> > +    bool "Enable early printk config"
> > +    default DEBUG
> > +    depends on RISCV_64 || RISCV_32
> > +    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..88da5169ed
> > --- /dev/null
> > +++ b/xen/arch/riscv/early_printk.c
> > @@ -0,0 +1,33 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * RISC-V early printk using SBI
> > + *
> > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
> > + */
> > +#include <asm/sbi.h>
> > +#include <asm/early_printk.h>
> > +
> > +/*
> > + * TODO:
> > + *   sbi_console_putchar is already planned for deprication
> > + *   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__ */
> > --
> > 2.38.1
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 6858 bytes --]

^ permalink raw reply	[relevance 13%]

* Re: [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-09 15:46 10% ` [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-09 16:07 13%   ` Julien Grall
  2023-01-09 16:22 13%   ` Jan Beulich
@ 2023-01-10  0:28 13%   ` Alistair Francis
  2023-01-10  7:28 13%     ` Bobby Eshleman
  2 siblings, 1 reply; 57+ results
From: Alistair Francis @ 2023-01-10  0:28 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis, Bobby Eshleman

On Tue, Jan 10, 2023 at 1:47 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> The patch introduces a basic stuff of early_printk functionality
> which will be enough to print 'hello from C environment".
> early_printk() function was changed in comparison with original as
> common isn't being built now so there is no vscnprintf.
>
> 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.
>
> This commit adds early printk implementation built on the putc SBI call.
>
> As sbi_console_putchar() is being already planned for deprecation
> it is used temporary now and will be removed or reworked after
> real uart will be ready.

There was a discussion to add a new SBI putchar replacement. It
doesn't seem to be completed yet, but there might be an SBI
replacement for this in the future as well.

Alistair

>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>     - add license to early_printk.c
>     - add signed-off-by Bobby
>     - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
>     - update commit message
>     - order the files alphabetically in Makefile
> ---
>  xen/arch/riscv/Kconfig.debug              |  7 +++++
>  xen/arch/riscv/Makefile                   |  1 +
>  xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
>  xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
>  4 files changed, 53 insertions(+)
>  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..6ba0bd1e5a 100644
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,7 @@
> +config EARLY_PRINTK
> +    bool "Enable early printk config"
> +    default DEBUG
> +    depends on RISCV_64 || RISCV_32
> +    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..88da5169ed
> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * RISC-V early printk using SBI
> + *
> + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
> + */
> +#include <asm/sbi.h>
> +#include <asm/early_printk.h>
> +
> +/*
> + * TODO:
> + *   sbi_console_putchar is already planned for deprication
> + *   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__ */
> --
> 2.38.1
>
>


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-09 15:46 10% ` [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-09 16:07 13%   ` Julien Grall
@ 2023-01-09 16:22 13%   ` Jan Beulich
  2023-01-10  0:28 13%   ` Alistair Francis
  2 siblings, 0 replies; 57+ results
From: Jan Beulich @ 2023-01-09 16:22 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman,
	xen-devel

On 09.01.2023 16:46, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,7 @@
> +config EARLY_PRINTK
> +    bool "Enable early printk config"

Nit: Stray "config" in the prompt text.

Jan


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-09 15:46 10% ` [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-09 16:07 13%   ` Julien Grall
  2023-01-09 16:22 13%   ` Jan Beulich
  2023-01-10  0:28 13%   ` Alistair Francis
  2 siblings, 0 replies; 57+ results
From: Julien Grall @ 2023-01-09 16:07 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Bobby Eshleman

Hi,

On 09/01/2023 15:46, Oleksii Kurochko wrote:
> The patch introduces a basic stuff of early_printk functionality
> which will be enough to print 'hello from C environment".
> early_printk() function was changed in comparison with original as
> common isn't being built now so there is no vscnprintf.
> 
> 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.
> 
> This commit adds early printk implementation built on the putc SBI call.
> 
> As sbi_console_putchar() is being already planned for deprecation
> it is used temporary now and will be removed or reworked after
> real uart will be ready.
> 
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>

 From the previous discussion, I was under the impression you agreed 
that the code was mainly written by Bobby. And indeed you put him as the 
first signed-off.

So you want to also add a From tag right at the top of the patch so when 
we commit it, the author tag will be Bobby.

> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V2:
>      - add license to early_printk.c
>      - add signed-off-by Bobby
>      - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
>      - update commit message
>      - order the files alphabetically in Makefile
> ---
>   xen/arch/riscv/Kconfig.debug              |  7 +++++
>   xen/arch/riscv/Makefile                   |  1 +
>   xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
>   xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
>   4 files changed, 53 insertions(+)
>   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..6ba0bd1e5a 100644
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,7 @@
> +config EARLY_PRINTK
> +    bool "Enable early printk config"
> +    default DEBUG
> +    depends on RISCV_64 || RISCV_32
> +    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..88da5169ed
> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * RISC-V early printk using SBI
> + *
> + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
> + */
> +#include <asm/sbi.h>
> +#include <asm/early_printk.h>
> +
> +/*
> + * TODO:
> + *   sbi_console_putchar is already planned for deprication

s/deprication/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__ */

Cheers,

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-09 15:46 13% [PATCH v2 " Oleksii Kurochko
@ 2023-01-09 15:46 10% ` Oleksii Kurochko
  2023-01-09 16:07 13%   ` Julien Grall
                     ` (2 more replies)
  0 siblings, 3 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-09 15:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, Bobby Eshleman

The patch introduces a basic stuff of early_printk functionality
which will be enough to print 'hello from C environment".
early_printk() function was changed in comparison with original as
common isn't being built now so there is no vscnprintf.

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.

This commit adds early printk implementation built on the putc SBI call.

As sbi_console_putchar() is being already planned for deprecation
it is used temporary 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 V2:
    - add license to early_printk.c
    - add signed-off-by Bobby
    - add RISCV_32 to Kconfig.debug to EARLY_PRINTK config
    - update commit message
    - order the files alphabetically in Makefile
---
 xen/arch/riscv/Kconfig.debug              |  7 +++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++++
 4 files changed, 53 insertions(+)
 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..6ba0bd1e5a 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,7 @@
+config EARLY_PRINTK
+    bool "Enable early printk config"
+    default DEBUG
+    depends on RISCV_64 || RISCV_32
+    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..88da5169ed
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * RISC-V early printk using SBI
+ *
+ * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
+ */
+#include <asm/sbi.h>
+#include <asm/early_printk.h>
+
+/*
+ * TODO: 
+ *   sbi_console_putchar is already planned for deprication
+ *   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__ */
-- 
2.38.1



^ permalink raw reply related	[relevance 10%]

* [PATCH v2 0/8] Basic early_printk and smoke test implementation
@ 2023-01-09 15:46 13% Oleksii Kurochko
  2023-01-09 15:46 10% ` [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  0 siblings, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-09 15:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Jan Beulich, Julien Grall, Andrew Cooper, Stefano Stabellini,
	Gianluca Guida, Oleksii Kurochko, Bob Eshleman, Alistair Francis,
	Connor Davis, George Dunlap, Wei Liu, Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

---
Changes in V2:
    - update commit patches commit messages according to the mailing
      list comments
    - Remove unneeded types in <asm/types.h>
    - Introduce definition of STACK_SIZE
    - order the files alphabetically in Makefile
    - Add license to early_printk.c
    - Add RISCV_32 dependecy to config EARLY_PRINTK in Kconfig.debug
    - Move dockerfile changes to separate config and sent them as
      separate patch to mailing list.
    - Update test.yaml to wire up smoke test
---

Oleksii Kurochko (8):
  xen/riscv: introduce dummy asm/init.h
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce stack stuff
  xen/riscv: introduce sbi call to putchar to console
  xen/include: include <asm/types.h> in <xen/early_printk.h>
  xen/riscv: introduce early_printk basic stuff
  xen/riscv: print hello message from C env
  automation: add RISC-V smoke test

 automation/gitlab-ci/test.yaml            | 20 +++++++++++
 automation/scripts/qemu-smoke-riscv64.sh  | 20 +++++++++++
 xen/arch/riscv/Kconfig.debug              |  7 ++++
 xen/arch/riscv/Makefile                   |  3 ++
 xen/arch/riscv/early_printk.c             | 33 +++++++++++++++++
 xen/arch/riscv/include/asm/config.h       |  2 ++
 xen/arch/riscv/include/asm/early_printk.h | 12 +++++++
 xen/arch/riscv/include/asm/init.h         | 12 +++++++
 xen/arch/riscv/include/asm/sbi.h          | 34 ++++++++++++++++++
 xen/arch/riscv/include/asm/types.h        | 22 ++++++++++++
 xen/arch/riscv/riscv64/head.S             |  6 +++-
 xen/arch/riscv/sbi.c                      | 44 +++++++++++++++++++++++
 xen/arch/riscv/setup.c                    | 18 ++++++++++
 xen/include/xen/early_printk.h            |  2 ++
 14 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/init.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c
 create mode 100644 xen/arch/riscv/setup.c

-- 
2.38.1



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-09  9:10 13%     ` Oleksii
@ 2023-01-09 11:14 13%       ` Julien Grall
  0 siblings, 0 replies; 57+ results
From: Julien Grall @ 2023-01-09 11:14 UTC (permalink / raw)
  To: Oleksii, xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis

Hi,

On 09/01/2023 09:10, Oleksii wrote:
> On Fri, 2023-01-06 at 13:51 +0000, Julien Grall wrote:
>> Hi,
>>
>> On 06/01/2023 13:14, Oleksii Kurochko wrote:
>>> The patch introduces a basic stuff of early_printk functionality
>>> which will be enough to print 'hello from C environment"
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> ---
>>>    xen/arch/riscv/Kconfig.debug              |  7 ++++++
>>>    xen/arch/riscv/Makefile                   |  1 +
>>>    xen/arch/riscv/early_printk.c             | 27
>>> +++++++++++++++++++++++
>>>    xen/arch/riscv/include/asm/early_printk.h | 12 ++++++++++
>>>    4 files changed, 47 insertions(+)
>>>    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..940630fd62 100644
>>> --- a/xen/arch/riscv/Kconfig.debug
>>> +++ b/xen/arch/riscv/Kconfig.debug
>>> @@ -0,0 +1,7 @@
>>> +config EARLY_PRINTK
>>> +    bool "Enable early printk config"
>>> +    default DEBUG
>>> +    depends on RISCV_64
>>
>> OOI, why can't this be used for RISCV_32?
>>
> We can. It's my fault we wanted to start from RISCV_64 support so I
> totally forgot about RISCV_32 =)
>>> +    help
>>> +
>>> +      Enables early printk debug messages
>>> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
>>> index 60db415654..e8630fe68d 100644
>>> --- a/xen/arch/riscv/Makefile
>>> +++ b/xen/arch/riscv/Makefile
>>> @@ -1,6 +1,7 @@
>>>    obj-$(CONFIG_RISCV_64) += riscv64/
>>>    obj-y += setup.o
>>>    obj-y += sbi.o
>>> +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>
>> Please order the files alphabetically.
>>
>>>    
>>>    $(TARGET): $(TARGET)-syms
>>>          $(OBJCOPY) -O binary -S $< $@
>>> diff --git a/xen/arch/riscv/early_printk.c
>>> b/xen/arch/riscv/early_printk.c
>>> new file mode 100644
>>> index 0000000000..f357f3220b
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/early_printk.c
>>> @@ -0,0 +1,27 @@
>>
>> Please add an SPDX license (the default for Xen is GPLv2).
>>
>>> +/*
>>> + * RISC-V early printk using SBI
>>> + *
>>> + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
>>
>> So the copyright here is from Bobby. But I don't see any mention in
>> the
>> commit message. Where is this coming from?
>>
> Could you please share with me an example how it should be look like?
> Signed-off: ... ? Or "this file was taken from patch series ..."?

This depends on the context. Do you have a pointer to the original work?

If you are taking the patch mostly as-is, then the author should be 
Bobby. The first signed-off-by would be Bobby and you will be the second 
one.

Otherwise, you could credit him with "Based on the original work from 
...". A link could be added in the commit message or after ---.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* Re: [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-06 13:51 13%   ` Julien Grall
@ 2023-01-09  9:10 13%     ` Oleksii
  2023-01-09 11:14 13%       ` Julien Grall
  0 siblings, 1 reply; 57+ results
From: Oleksii @ 2023-01-09  9:10 UTC (permalink / raw)
  To: Julien Grall, xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis

Hi,

On Fri, 2023-01-06 at 13:51 +0000, Julien Grall wrote:
> Hi,
> 
> On 06/01/2023 13:14, Oleksii Kurochko wrote:
> > The patch introduces a basic stuff of early_printk functionality
> > which will be enough to print 'hello from C environment"
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> >   xen/arch/riscv/Kconfig.debug              |  7 ++++++
> >   xen/arch/riscv/Makefile                   |  1 +
> >   xen/arch/riscv/early_printk.c             | 27
> > +++++++++++++++++++++++
> >   xen/arch/riscv/include/asm/early_printk.h | 12 ++++++++++
> >   4 files changed, 47 insertions(+)
> >   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..940630fd62 100644
> > --- a/xen/arch/riscv/Kconfig.debug
> > +++ b/xen/arch/riscv/Kconfig.debug
> > @@ -0,0 +1,7 @@
> > +config EARLY_PRINTK
> > +    bool "Enable early printk config"
> > +    default DEBUG
> > +    depends on RISCV_64
> 
> OOI, why can't this be used for RISCV_32?
> 
We can. It's my fault we wanted to start from RISCV_64 support so I
totally forgot about RISCV_32 =)
> > +    help
> > +
> > +      Enables early printk debug messages
> > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> > index 60db415654..e8630fe68d 100644
> > --- a/xen/arch/riscv/Makefile
> > +++ b/xen/arch/riscv/Makefile
> > @@ -1,6 +1,7 @@
> >   obj-$(CONFIG_RISCV_64) += riscv64/
> >   obj-y += setup.o
> >   obj-y += sbi.o
> > +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> 
> Please order the files alphabetically.
> 
> >   
> >   $(TARGET): $(TARGET)-syms
> >         $(OBJCOPY) -O binary -S $< $@
> > diff --git a/xen/arch/riscv/early_printk.c
> > b/xen/arch/riscv/early_printk.c
> > new file mode 100644
> > index 0000000000..f357f3220b
> > --- /dev/null
> > +++ b/xen/arch/riscv/early_printk.c
> > @@ -0,0 +1,27 @@
> 
> Please add an SPDX license (the default for Xen is GPLv2).
> 
> > +/*
> > + * RISC-V early printk using SBI
> > + *
> > + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
> 
> So the copyright here is from Bobby. But I don't see any mention in
> the 
> commit message. Where is this coming from?
> 
Could you please share with me an example how it should be look like?
Signed-off: ... ? Or "this file was taken from patch series ..."?
> > + */
> > +#include <asm/sbi.h>
> > +#include <asm/early_printk.h>
> 
> Please order the files alphabetically.
> 
> > +
> > +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__ */
> 



^ permalink raw reply	[relevance 13%]

* Re: [PATCH v1 0/8] Basic early_printk and smoke test implementation
  2023-01-06 13:14 13% [PATCH v1 0/8] Basic early_printk and smoke test implementation Oleksii Kurochko
  2023-01-06 13:14 11% ` [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-06 13:51  0% ` Andrew Cooper
  1 sibling, 0 replies; 57+ results
From: Andrew Cooper @ 2023-01-06 13:51 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis, George Dunlap, Jan Beulich,
	Julien Grall, Wei Liu, Doug Goldstein

On 06/01/2023 1:14 pm, Oleksii Kurochko wrote:
> The patch series introduces the following:
> - the minimal set of headers and changes inside them.
> - SBI (RISC-V Supervisor Binary Interface) things necessary for basic
>   early_printk implementation.
> - things needed to set up the stack.
> - early_printk() function to print only strings.
> - RISC-V smoke test which checks if  "Hello from C env" message is
>   present in serial.tmp
>
> Oleksii Kurochko (8):
>   xen/riscv: introduce dummy asm/init.h
>   xen/riscv: introduce asm/types.h header file
>   xen/riscv: introduce stack stuff
>   xen/riscv: introduce sbi call to putchar to console
>   xen/include: include <asm/types.h> in <xen/early_printk.h>
>   xen/riscv: introduce early_printk basic stuff
>   xen/riscv: print hello message from C env
>   automation: add RISC-V smoke test

Thanks.  This highlights several areas where I think we want some rework
to the current common/arch split.

First, it really shouldn't be necessary for architectures to create
emtpy stub files.  There are two options - first drop some empty files
in xen/include/arch-fallback/asm and put a suitable -I at the end of
CFLAGS.  The other option, which is nicer IMO, is to use __has_include()
although that would require us finally deciding to bump the minimum GCC
version to 5 for x86 (which we need to do for may other reasons too).

Second, the asm/types is absurd.  That should be one common header,
because there's nothing arch specific about making those types appear.

Third, the early printk infrastructure is partially broken (the common
header can't be cleanly included), and unsatisfactory with how it plumbs
into the default console steal function.  With a bit of cleanup, most of
it can be not duplicated per arch.

~Andrew

^ permalink raw reply	[relevance 0%]

* Re: [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-06 13:14 11% ` [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-06 13:51 13%   ` Julien Grall
  2023-01-09  9:10 13%     ` Oleksii
  0 siblings, 1 reply; 57+ results
From: Julien Grall @ 2023-01-06 13:51 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, Bob Eshleman,
	Alistair Francis, Connor Davis

Hi,

On 06/01/2023 13:14, Oleksii Kurochko wrote:
> The patch introduces a basic stuff of early_printk functionality
> which will be enough to print 'hello from C environment"
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
>   xen/arch/riscv/Kconfig.debug              |  7 ++++++
>   xen/arch/riscv/Makefile                   |  1 +
>   xen/arch/riscv/early_printk.c             | 27 +++++++++++++++++++++++
>   xen/arch/riscv/include/asm/early_printk.h | 12 ++++++++++
>   4 files changed, 47 insertions(+)
>   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..940630fd62 100644
> --- a/xen/arch/riscv/Kconfig.debug
> +++ b/xen/arch/riscv/Kconfig.debug
> @@ -0,0 +1,7 @@
> +config EARLY_PRINTK
> +    bool "Enable early printk config"
> +    default DEBUG
> +    depends on RISCV_64

OOI, why can't this be used for RISCV_32?

> +    help
> +
> +      Enables early printk debug messages
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index 60db415654..e8630fe68d 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -1,6 +1,7 @@
>   obj-$(CONFIG_RISCV_64) += riscv64/
>   obj-y += setup.o
>   obj-y += sbi.o
> +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o

Please order the files alphabetically.

>   
>   $(TARGET): $(TARGET)-syms
>   	$(OBJCOPY) -O binary -S $< $@
> diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
> new file mode 100644
> index 0000000000..f357f3220b
> --- /dev/null
> +++ b/xen/arch/riscv/early_printk.c
> @@ -0,0 +1,27 @@

Please add an SPDX license (the default for Xen is GPLv2).

> +/*
> + * RISC-V early printk using SBI
> + *
> + * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>

So the copyright here is from Bobby. But I don't see any mention in the 
commit message. Where is this coming from?

> + */
> +#include <asm/sbi.h>
> +#include <asm/early_printk.h>

Please order the files alphabetically.

> +
> +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__ */

-- 
Julien Grall


^ permalink raw reply	[relevance 13%]

* [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff
  2023-01-06 13:14 13% [PATCH v1 0/8] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-06 13:14 11% ` Oleksii Kurochko
  2023-01-06 13:51 13%   ` Julien Grall
  2023-01-06 13:51  0% ` [PATCH v1 0/8] Basic early_printk and smoke test implementation Andrew Cooper
  1 sibling, 1 reply; 57+ results
From: Oleksii Kurochko @ 2023-01-06 13:14 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Oleksii Kurochko, Bob Eshleman, Alistair Francis, Connor Davis

The patch introduces a basic stuff of early_printk functionality
which will be enough to print 'hello from C environment"

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/Kconfig.debug              |  7 ++++++
 xen/arch/riscv/Makefile                   |  1 +
 xen/arch/riscv/early_printk.c             | 27 +++++++++++++++++++++++
 xen/arch/riscv/include/asm/early_printk.h | 12 ++++++++++
 4 files changed, 47 insertions(+)
 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..940630fd62 100644
--- a/xen/arch/riscv/Kconfig.debug
+++ b/xen/arch/riscv/Kconfig.debug
@@ -0,0 +1,7 @@
+config EARLY_PRINTK
+    bool "Enable early printk config"
+    default DEBUG
+    depends on RISCV_64
+    help
+
+      Enables early printk debug messages
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 60db415654..e8630fe68d 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_RISCV_64) += riscv64/
 obj-y += setup.o
 obj-y += sbi.o
+obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 
 $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
new file mode 100644
index 0000000000..f357f3220b
--- /dev/null
+++ b/xen/arch/riscv/early_printk.c
@@ -0,0 +1,27 @@
+/*
+ * RISC-V early printk using SBI
+ *
+ * Copyright (C) 2021 Bobby Eshleman <bobbyeshleman@gmail.com>
+ */
+#include <asm/sbi.h>
+#include <asm/early_printk.h>
+
+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__ */
-- 
2.38.1



^ permalink raw reply related	[relevance 11%]

* [PATCH v1 0/8] Basic early_printk and smoke test implementation
@ 2023-01-06 13:14 13% Oleksii Kurochko
  2023-01-06 13:14 11% ` [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-06 13:51  0% ` [PATCH v1 0/8] Basic early_printk and smoke test implementation Andrew Cooper
  0 siblings, 2 replies; 57+ results
From: Oleksii Kurochko @ 2023-01-06 13:14 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Oleksii Kurochko, Bob Eshleman, Alistair Francis, Connor Davis,
	George Dunlap, Jan Beulich, Julien Grall, Wei Liu,
	Doug Goldstein

The patch series introduces the following:
- the minimal set of headers and changes inside them.
- SBI (RISC-V Supervisor Binary Interface) things necessary for basic
  early_printk implementation.
- things needed to set up the stack.
- early_printk() function to print only strings.
- RISC-V smoke test which checks if  "Hello from C env" message is
  present in serial.tmp

Oleksii Kurochko (8):
  xen/riscv: introduce dummy asm/init.h
  xen/riscv: introduce asm/types.h header file
  xen/riscv: introduce stack stuff
  xen/riscv: introduce sbi call to putchar to console
  xen/include: include <asm/types.h> in <xen/early_printk.h>
  xen/riscv: introduce early_printk basic stuff
  xen/riscv: print hello message from C env
  automation: add RISC-V smoke test

 automation/build/archlinux/riscv64.dockerfile |  3 +-
 automation/scripts/qemu-smoke-riscv64.sh      | 20 +++++
 xen/arch/riscv/Kconfig.debug                  |  7 ++
 xen/arch/riscv/Makefile                       |  3 +
 xen/arch/riscv/early_printk.c                 | 27 +++++++
 xen/arch/riscv/include/asm/early_printk.h     | 12 +++
 xen/arch/riscv/include/asm/init.h             | 12 +++
 xen/arch/riscv/include/asm/sbi.h              | 34 +++++++++
 xen/arch/riscv/include/asm/types.h            | 73 +++++++++++++++++++
 xen/arch/riscv/riscv64/head.S                 |  6 +-
 xen/arch/riscv/sbi.c                          | 44 +++++++++++
 xen/arch/riscv/setup.c                        | 18 +++++
 xen/include/xen/early_printk.h                |  2 +
 13 files changed, 259 insertions(+), 2 deletions(-)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
 create mode 100644 xen/arch/riscv/early_printk.c
 create mode 100644 xen/arch/riscv/include/asm/early_printk.h
 create mode 100644 xen/arch/riscv/include/asm/init.h
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/include/asm/types.h
 create mode 100644 xen/arch/riscv/sbi.c
 create mode 100644 xen/arch/riscv/setup.c

-- 
2.38.1



^ permalink raw reply	[relevance 13%]

Results 1-57 of 57 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-01-06 13:14 13% [PATCH v1 0/8] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-06 13:14 11% ` [PATCH v1 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-06 13:51 13%   ` Julien Grall
2023-01-09  9:10 13%     ` Oleksii
2023-01-09 11:14 13%       ` Julien Grall
2023-01-06 13:51  0% ` [PATCH v1 0/8] Basic early_printk and smoke test implementation Andrew Cooper
2023-01-09 15:46 13% [PATCH v2 " Oleksii Kurochko
2023-01-09 15:46 10% ` [PATCH v2 6/8] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-09 16:07 13%   ` Julien Grall
2023-01-09 16:22 13%   ` Jan Beulich
2023-01-10  0:28 13%   ` Alistair Francis
2023-01-10  7:28 13%     ` Bobby Eshleman
2023-01-10 10:47 13%       ` Alistair Francis
2023-01-10 15:17 14% [PATCH v3 0/6] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-10 15:17 10% ` [PATCH v3 5/6] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-10 15:57 13%   ` Jan Beulich
2023-01-16 14:39 13% [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
2023-01-16 14:39  9% ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-17 23:22 13%   ` Bobby Eshleman
2023-01-17 23:57 12%   ` Andrew Cooper
2023-01-18  0:33 13%     ` Julien Grall
2023-01-18 11:07 13%     ` Oleksii
2023-01-19 12:45  9%   ` Oleksii Kurochko
2023-01-16 14:44 14% ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
2023-01-19 12:45 14% ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-19 14:07 13% [PATCH v5 0/5] " Oleksii Kurochko
2023-01-19 14:07  9% ` [PATCH v5 4/5] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-20  0:48 13%   ` Andrew Cooper
2023-01-20  9:17 13%     ` Julien Grall
2023-01-27 11:15 14% [PATCH v6 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-27 11:15  9% ` [PATCH v6 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-27 11:26 13%   ` Julien Grall
2023-01-27 11:49 13%     ` Oleksii
2023-01-27 12:22 12%       ` Julien Grall
2023-01-27 11:39 13% [PATCH v7 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-27 11:39  9% ` [PATCH v7 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-27 14:15 13%   ` Oleksii
2023-01-31 11:44 13%     ` Alistair Francis
2023-01-31 12:03 12%       ` Julien Grall
2023-01-31 23:17 13%         ` Alistair Francis
2023-02-01  0:21 12%           ` Andrew Cooper
2023-02-01  9:06 11%             ` Julien Grall
2023-02-01  9:10 13%               ` Julien Grall
2023-02-01 17:33 12%               ` Bobby Eshleman
2023-02-04 11:59 13%                 ` Alistair Francis
2023-02-06 17:30 13%               ` Oleksii
2023-01-31 11:17 11% [PATCH v8 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-31 11:17  9% ` [PATCH v8 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-31 11:42 13%   ` Julien Grall
2023-01-31 12:37 13%     ` Oleksii
2023-02-07 11:28 13%       ` Oleksii
2023-02-07 12:41 11% [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-02-07 12:41  9% ` [PATCH v9 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-02-07 12:49  0% ` [PATCH v9 0/2] Basic early_printk and smoke test implementation Oleksii
2023-02-07 12:49 11% [PATCH v10 " Oleksii Kurochko
2023-02-07 12:49  9% ` [PATCH v10 1/2] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-02-09  1:37 13%   ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).