xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] The patch series introduces the following:
@ 2023-01-16 14:39 Oleksii Kurochko
  2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
                   ` (6 more replies)
  0 siblings, 7 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
@ 2023-01-16 14:39 ` Oleksii Kurochko
  2023-01-16 14:59   ` Jan Beulich
  2023-01-19 12:45   ` Oleksii Kurochko
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 27+ messages in thread
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

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
    - Clean up types in <asm/types.h> and remain only necessary.
      The following types was removed as they are defined in <xen/types.h>:
      {__|}{u|s}{8|16|32|64}
---
Changes in V3:
    - Nothing changed
---
Changes in V2:
    - Remove unneeded now types from <asm/types.h>
---
 xen/arch/riscv/include/asm/types.h | 43 ++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/types.h

diff --git a/xen/arch/riscv/include/asm/types.h b/xen/arch/riscv/include/asm/types.h
new file mode 100644
index 0000000000..9e55bcf776
--- /dev/null
+++ b/xen/arch/riscv/include/asm/types.h
@@ -0,0 +1,43 @@
+#ifndef __RISCV_TYPES_H__
+#define __RISCV_TYPES_H__
+
+#ifndef __ASSEMBLY__
+
+#if defined(CONFIG_RISCV_32)
+typedef unsigned long long u64;
+typedef unsigned int u32;
+typedef u32 vaddr_t;
+#define PRIvaddr PRIx32
+typedef u64 paddr_t;
+#define INVALID_PADDR (~0ULL)
+#define PRIpaddr "016llx"
+typedef u32 register_t;
+#define PRIregister "x"
+#elif defined (CONFIG_RISCV_64)
+typedef unsigned long u64;
+typedef u64 vaddr_t;
+#define PRIvaddr PRIx64
+typedef u64 paddr_t;
+#define INVALID_PADDR (~0UL)
+#define PRIpaddr "016lx"
+typedef u64 register_t;
+#define PRIregister "lx"
+#endif
+
+#if defined(__SIZE_TYPE__)
+typedef __SIZE_TYPE__ size_t;
+#else
+typedef unsigned long size_t;
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __RISCV_TYPES_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
  2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
@ 2023-01-16 14:39 ` Oleksii Kurochko
  2023-01-17 23:19   ` Bobby Eshleman
                     ` (3 more replies)
  2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
                   ` (4 subsequent siblings)
  6 siblings, 4 replies; 27+ messages in thread
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, Bobby Eshleman, Bob Eshleman, Alistair Francis,
	Connor Davis, Oleksii Kurochko

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

Originally SBI implementation for Xen was introduced by
Bobby Eshleman <bobby.eshleman@gmail.com> but it was removed
all the stuff for simplicity  except SBI call for putting
character to console.

The patch introduces sbi_putchar() SBI call which is necessary
to implement initial early_printk.

Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
    - Nothing changed
---
Changes in V3:
    - update copyright's year
    - rename definition of __CPU_SBI_H__ to __ASM_RISCV_SBI_H__
    - fix identations
    - change an author of the commit
---
Changes in V2:
    - add an explanatory comment about sbi_console_putchar() function.
    - order the files alphabetically in Makefile
---
 xen/arch/riscv/Makefile          |  1 +
 xen/arch/riscv/include/asm/sbi.h | 34 ++++++++++++++++++++++++
 xen/arch/riscv/sbi.c             | 45 ++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/sbi.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 5a67a3f493..fd916e1004 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RISCV_64) += riscv64/
+obj-y += sbi.o
 obj-y += setup.o
 
 $(TARGET): $(TARGET)-syms
diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
new file mode 100644
index 0000000000..0e6820a4ed
--- /dev/null
+++ b/xen/arch/riscv/include/asm/sbi.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later) */
+/*
+ * Copyright (c) 2021-2023 Vates SAS.
+ *
+ * Taken from xvisor, modified by Bobby Eshleman (bobby.eshleman@gmail.com).
+ *
+ * Taken/modified from Xvisor project with the following copyright:
+ *
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef __ASM_RISCV_SBI_H__
+#define __ASM_RISCV_SBI_H__
+
+#define SBI_EXT_0_1_CONSOLE_PUTCHAR		0x1
+
+struct sbiret {
+    long error;
+    long value;
+};
+
+struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
+                        unsigned long arg0, unsigned long arg1,
+                        unsigned long arg2, unsigned long arg3,
+                        unsigned long arg4, unsigned long arg5);
+
+/**
+ * Writes given character to the console device.
+ *
+ * @param ch The data to be written to the console.
+ */
+void sbi_console_putchar(int ch);
+
+#endif /* __ASM_RISCV_SBI_H__ */
diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
new file mode 100644
index 0000000000..dc0eb44bc6
--- /dev/null
+++ b/xen/arch/riscv/sbi.c
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Taken and modified from the xvisor project with the copyright Copyright (c)
+ * 2019 Western Digital Corporation or its affiliates and author Anup Patel
+ * (anup.patel@wdc.com).
+ *
+ * Modified by Bobby Eshleman (bobby.eshleman@gmail.com).
+ *
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ * Copyright (c) 2021-2023 Vates SAS.
+ */
+
+#include <xen/errno.h>
+#include <asm/sbi.h>
+
+struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
+                        unsigned long arg0, unsigned long arg1,
+                        unsigned long arg2, unsigned long arg3,
+                        unsigned long arg4, unsigned long arg5)
+{
+    struct sbiret ret;
+
+    register unsigned long a0 asm ("a0") = arg0;
+    register unsigned long a1 asm ("a1") = arg1;
+    register unsigned long a2 asm ("a2") = arg2;
+    register unsigned long a3 asm ("a3") = arg3;
+    register unsigned long a4 asm ("a4") = arg4;
+    register unsigned long a5 asm ("a5") = arg5;
+    register unsigned long a6 asm ("a6") = fid;
+    register unsigned long a7 asm ("a7") = ext;
+
+    asm volatile ("ecall"
+              : "+r" (a0), "+r" (a1)
+              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
+              : "memory");
+    ret.error = a0;
+    ret.value = a1;
+
+    return ret;
+}
+
+void sbi_console_putchar(int ch)
+{
+    sbi_ecall(SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0);
+}
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
  2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
@ 2023-01-16 14:39 ` Oleksii Kurochko
  2023-01-17 23:22   ` Bobby Eshleman
                     ` (2 more replies)
  2023-01-16 14:39 ` [PATCH v4 4/4] automation: add RISC-V smoke test Oleksii Kurochko
                   ` (3 subsequent siblings)
  6 siblings, 3 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* [PATCH v4 4/4] automation: add RISC-V smoke test
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
                   ` (2 preceding siblings ...)
  2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-16 14:39 ` Oleksii Kurochko
  2023-01-18  1:14   ` Alistair Francis
  2023-01-19 12:45   ` Oleksii Kurochko
  2023-01-16 14:44 ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 27+ messages in thread
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, Doug Goldstein

Add check if there is a message 'Hello from C env' presents
in log file to be sure that stack is set and C part of early printk
is working.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
---
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.
---
 automation/gitlab-ci/test.yaml           | 20 ++++++++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index afd80adfe1..64f47a0ab9 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -54,6 +54,19 @@
   tags:
     - x86_64
 
+.qemu-riscv64:
+  extends: .test-jobs-common
+  variables:
+    CONTAINER: archlinux:riscv64
+    LOGFILE: qemu-smoke-riscv64.log
+  artifacts:
+    paths:
+      - smoke.serial
+      - '*.log'
+    when: always
+  tags:
+    - x86_64
+
 .yocto-test:
   extends: .test-jobs-common
   script:
@@ -234,6 +247,13 @@ qemu-smoke-x86-64-clang-pvh:
   needs:
     - debian-unstable-clang-debug
 
+qemu-smoke-riscv64-gcc:
+  extends: .qemu-riscv64
+  script:
+    - ./automation/scripts/qemu-smoke-riscv64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+    - riscv64-cross-gcc
+
 # Yocto test jobs
 yocto-qemuarm64:
   extends: .yocto-test-arm64
diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
new file mode 100755
index 0000000000..e0f06360bc
--- /dev/null
+++ b/automation/scripts/qemu-smoke-riscv64.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+set -ex
+
+# Run the test
+rm -f smoke.serial
+set +e
+
+timeout -k 1 2 \
+qemu-system-riscv64 \
+    -M virt \
+    -smp 1 \
+    -nographic \
+    -m 2g \
+    -kernel binaries/xen \
+    |& tee smoke.serial
+
+set -e
+(grep -q "Hello from C env" smoke.serial) || exit 1
+exit 0
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 0/4] The patch series introduces the following:
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
                   ` (3 preceding siblings ...)
  2023-01-16 14:39 ` [PATCH v4 4/4] automation: add RISC-V smoke test Oleksii Kurochko
@ 2023-01-16 14:44 ` Oleksii
  2023-01-19 12:45 ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
  2023-01-19 13:23 ` Jan Beulich
  6 siblings, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
@ 2023-01-16 14:59   ` Jan Beulich
  2023-01-17  9:29     ` Oleksii
  2023-01-19 12:45   ` Oleksii Kurochko
  1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2023-01-16 14:59 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On 16.01.2023 15:39, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V4:
>     - Clean up types in <asm/types.h> and remain only necessary.
>       The following types was removed as they are defined in <xen/types.h>:
>       {__|}{u|s}{8|16|32|64}

For one you still typedef u32 and u64. And imo correctly so, until we
get around to move the definition basic types into xen/types.h. Plus
I can't see how things build for you: xen/types.h expects __{u,s}<N>
to be defined in order to then derive {u,}int<N>_t from them.

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/types.h
> @@ -0,0 +1,43 @@
> +#ifndef __RISCV_TYPES_H__
> +#define __RISCV_TYPES_H__
> +
> +#ifndef __ASSEMBLY__
> +
> +#if defined(CONFIG_RISCV_32)
> +typedef unsigned long long u64;
> +typedef unsigned int u32;
> +typedef u32 vaddr_t;
> +#define PRIvaddr PRIx32
> +typedef u64 paddr_t;
> +#define INVALID_PADDR (~0ULL)
> +#define PRIpaddr "016llx"
> +typedef u32 register_t;
> +#define PRIregister "x"
> +#elif defined (CONFIG_RISCV_64)
> +typedef unsigned long u64;
> +typedef u64 vaddr_t;
> +#define PRIvaddr PRIx64
> +typedef u64 paddr_t;
> +#define INVALID_PADDR (~0UL)
> +#define PRIpaddr "016lx"
> +typedef u64 register_t;
> +#define PRIregister "lx"
> +#endif

Any chance you could insert blank lines after #if, around #elif, and
before #endif?

> +#if defined(__SIZE_TYPE__)
> +typedef __SIZE_TYPE__ size_t;
> +#else
> +typedef unsigned long size_t;
> +#endif

I'd appreciate if this part was dropped by re-basing on top of my
"include/types: move stddef.h-kind types to common header" [1], to
avoid that (re-based) patch then needing to drop this from here
again. I would have committed this already, if osstest wasn't
completely broken right now.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html
(since you would not be able to find a patch of the quoted title,
as in the submission I mistakenly referenced stdlib.h)


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-16 14:59   ` Jan Beulich
@ 2023-01-17  9:29     ` Oleksii
  2023-01-17 10:04       ` Jan Beulich
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksii @ 2023-01-17  9:29 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On Mon, 2023-01-16 at 15:59 +0100, Jan Beulich wrote:
> On 16.01.2023 15:39, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V4:
> >     - Clean up types in <asm/types.h> and remain only necessary.
> >       The following types was removed as they are defined in
> > <xen/types.h>:
> >       {__|}{u|s}{8|16|32|64}
> 
> For one you still typedef u32 and u64. And imo correctly so, until we
> get around to move the definition basic types into xen/types.h. Plus
> I can't see how things build for you: xen/types.h expects __{u,s}<N>
It builds because nothing is used <xen/types.h> now so that's why I
missed them but you are right __{u,s}<N> should be backed.
It looks like {__,}{u,s}{8,16,32} are the same for all available in Xen
architectures so probably can I move them to <xen/types.h> instead of
remain them in <asm/types.h>?
> to be defined in order to then derive {u,}int<N>_t from them.
> 
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/types.h
> > @@ -0,0 +1,43 @@
> > +#ifndef __RISCV_TYPES_H__
> > +#define __RISCV_TYPES_H__
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#if defined(CONFIG_RISCV_32)
> > +typedef unsigned long long u64;
> > +typedef unsigned int u32;
> > +typedef u32 vaddr_t;
> > +#define PRIvaddr PRIx32
> > +typedef u64 paddr_t;
> > +#define INVALID_PADDR (~0ULL)
> > +#define PRIpaddr "016llx"
> > +typedef u32 register_t;
> > +#define PRIregister "x"
> > +#elif defined (CONFIG_RISCV_64)
> > +typedef unsigned long u64;
> > +typedef u64 vaddr_t;
> > +#define PRIvaddr PRIx64
> > +typedef u64 paddr_t;
> > +#define INVALID_PADDR (~0UL)
> > +#define PRIpaddr "016lx"
> > +typedef u64 register_t;
> > +#define PRIregister "lx"
> > +#endif
> 
> Any chance you could insert blank lines after #if, around #elif, and
> before #endif?
> 
Sure, I will fix that.
> > +#if defined(__SIZE_TYPE__)
> > +typedef __SIZE_TYPE__ size_t;
> > +#else
> > +typedef unsigned long size_t;
> > +#endif
> 
> I'd appreciate if this part was dropped by re-basing on top of my
> "include/types: move stddef.h-kind types to common header" [1], to
> avoid that (re-based) patch then needing to drop this from here
> again. I would have committed this already, if osstest wasn't
> completely broken right now.
> 
I'll take it into account for the next version of the patch series.
> Jan
> 
> [1]
> https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html
> (since you would not be able to find a patch of the quoted title,
> as in the submission I mistakenly referenced stdlib.h)
Thanks for the link.

~Oleksii



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-17  9:29     ` Oleksii
@ 2023-01-17 10:04       ` Jan Beulich
  2023-01-18 11:23         ` Oleksii
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2023-01-17 10:04 UTC (permalink / raw)
  To: Oleksii
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On 17.01.2023 10:29, Oleksii wrote:
> On Mon, 2023-01-16 at 15:59 +0100, Jan Beulich wrote:
>> On 16.01.2023 15:39, Oleksii Kurochko wrote:
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> ---
>>> Changes in V4:
>>>     - Clean up types in <asm/types.h> and remain only necessary.
>>>       The following types was removed as they are defined in
>>> <xen/types.h>:
>>>       {__|}{u|s}{8|16|32|64}
>>
>> For one you still typedef u32 and u64. And imo correctly so, until we
>> get around to move the definition basic types into xen/types.h. Plus
>> I can't see how things build for you: xen/types.h expects __{u,s}<N>
> It builds because nothing is used <xen/types.h> now so that's why I
> missed them but you are right __{u,s}<N> should be backed.
> It looks like {__,}{u,s}{8,16,32} are the same for all available in Xen
> architectures so probably can I move them to <xen/types.h> instead of
> remain them in <asm/types.h>?

This next step isn't quite as obvious, i.e. has room for being
contentious. In particular deriving fixed width types from C basic
types is setting us up for future problems (especially in the
context of RISC-V think of RV128). Therefore, if we touch and
generalize this, I'd like to sanitize things at the same time.

I'd then prefer to typedef {u,}int<N>_t by using either the "mode"
attribute (requiring us to settle on a prereq of there always being
8 bits per char) or the compiler supplied __{U,}INT<N>_TYPE__
(taking gcc 4.7 as a prereq; didn't check clang yet). Both would
allow {u,}int64_t to also be put in the common header. Yet if e.g.
a prereq assumption faced opposition, some other approach would
need to be found. Plus using either of the named approaches has
issues with the printf() format specifiers, for which I'm yet to
figure out a solution (or maybe someone else knows a good way to
deal with that; using compiler provided headers isn't an option
afaict, as gcc provides stdint.h but not inttypes.h, but maybe
glibc's simplistic approach is good enough - they're targeting
far more architectures than we do and get away with that).

Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
@ 2023-01-17 23:19   ` Bobby Eshleman
  2023-01-17 23:32   ` Andrew Cooper
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 27+ messages in thread
From: Bobby Eshleman @ 2023-01-17 23:19 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Bobby Eshleman,
	Alistair Francis, Connor Davis

On Mon, Jan 16, 2023 at 04:39:30PM +0200, Oleksii Kurochko wrote:
> From: Bobby Eshleman <bobby.eshleman@gmail.com>
> 
> Originally SBI implementation for Xen was introduced by
> Bobby Eshleman <bobby.eshleman@gmail.com> but it was removed
> all the stuff for simplicity  except SBI call for putting
> character to console.
> 
> The patch introduces sbi_putchar() SBI call which is necessary
> to implement initial early_printk.
> 
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V4:
>     - Nothing changed
> ---
> Changes in V3:
>     - update copyright's year
>     - rename definition of __CPU_SBI_H__ to __ASM_RISCV_SBI_H__
>     - fix identations
>     - change an author of the commit
> ---
> Changes in V2:
>     - add an explanatory comment about sbi_console_putchar() function.
>     - order the files alphabetically in Makefile
> ---
>  xen/arch/riscv/Makefile          |  1 +
>  xen/arch/riscv/include/asm/sbi.h | 34 ++++++++++++++++++++++++
>  xen/arch/riscv/sbi.c             | 45 ++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/sbi.h
>  create mode 100644 xen/arch/riscv/sbi.c
> 
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index 5a67a3f493..fd916e1004 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RISCV_64) += riscv64/
> +obj-y += sbi.o
>  obj-y += setup.o
>  
>  $(TARGET): $(TARGET)-syms
> diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
> new file mode 100644
> index 0000000000..0e6820a4ed
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later) */
> +/*
> + * Copyright (c) 2021-2023 Vates SAS.
> + *
> + * Taken from xvisor, modified by Bobby Eshleman (bobby.eshleman@gmail.com).
> + *
> + * Taken/modified from Xvisor project with the following copyright:
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + */
> +
> +#ifndef __ASM_RISCV_SBI_H__
> +#define __ASM_RISCV_SBI_H__
> +
> +#define SBI_EXT_0_1_CONSOLE_PUTCHAR		0x1
> +
> +struct sbiret {
> +    long error;
> +    long value;
> +};
> +
> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> +                        unsigned long arg0, unsigned long arg1,
> +                        unsigned long arg2, unsigned long arg3,
> +                        unsigned long arg4, unsigned long arg5);
> +
> +/**
> + * Writes given character to the console device.
> + *
> + * @param ch The data to be written to the console.
> + */
> +void sbi_console_putchar(int ch);
> +
> +#endif /* __ASM_RISCV_SBI_H__ */
> diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
> new file mode 100644
> index 0000000000..dc0eb44bc6
> --- /dev/null
> +++ b/xen/arch/riscv/sbi.c
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Taken and modified from the xvisor project with the copyright Copyright (c)
> + * 2019 Western Digital Corporation or its affiliates and author Anup Patel
> + * (anup.patel@wdc.com).
> + *
> + * Modified by Bobby Eshleman (bobby.eshleman@gmail.com).
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + * Copyright (c) 2021-2023 Vates SAS.
> + */
> +
> +#include <xen/errno.h>
> +#include <asm/sbi.h>
> +
> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> +                        unsigned long arg0, unsigned long arg1,
> +                        unsigned long arg2, unsigned long arg3,
> +                        unsigned long arg4, unsigned long arg5)
> +{
> +    struct sbiret ret;
> +
> +    register unsigned long a0 asm ("a0") = arg0;
> +    register unsigned long a1 asm ("a1") = arg1;
> +    register unsigned long a2 asm ("a2") = arg2;
> +    register unsigned long a3 asm ("a3") = arg3;
> +    register unsigned long a4 asm ("a4") = arg4;
> +    register unsigned long a5 asm ("a5") = arg5;
> +    register unsigned long a6 asm ("a6") = fid;
> +    register unsigned long a7 asm ("a7") = ext;
> +
> +    asm volatile ("ecall"
> +              : "+r" (a0), "+r" (a1)
> +              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
> +              : "memory");
> +    ret.error = a0;
> +    ret.value = a1;
> +
> +    return ret;
> +}
> +
> +void sbi_console_putchar(int ch)
> +{
> +    sbi_ecall(SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0);
> +}
> -- 
> 2.39.0
> 
> 

Reviewed-by: Bobby Eshleman <bobby.eshleman@gmail.com>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
@ 2023-01-17 23:22   ` Bobby Eshleman
  2023-01-17 23:57   ` Andrew Cooper
  2023-01-19 12:45   ` Oleksii Kurochko
  2 siblings, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
  2023-01-17 23:19   ` Bobby Eshleman
@ 2023-01-17 23:32   ` Andrew Cooper
  2023-01-18  7:38     ` Jan Beulich
  2023-01-18  1:10   ` Alistair Francis
  2023-01-19 12:45   ` Oleksii Kurochko
  3 siblings, 1 reply; 27+ messages in thread
From: Andrew Cooper @ 2023-01-17 23:32 UTC (permalink / raw)
  To: Oleksii Kurochko, xen-devel
  Cc: Jan Beulich, Julien Grall, Stefano Stabellini, Gianluca Guida,
	Bobby Eshleman, Bob Eshleman, Alistair Francis, Connor Davis

On 16/01/2023 2:39 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
> new file mode 100644
> index 0000000000..dc0eb44bc6
> --- /dev/null
> +++ b/xen/arch/riscv/sbi.c
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Taken and modified from the xvisor project with the copyright Copyright (c)
> + * 2019 Western Digital Corporation or its affiliates and author Anup Patel
> + * (anup.patel@wdc.com).
> + *
> + * Modified by Bobby Eshleman (bobby.eshleman@gmail.com).
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + * Copyright (c) 2021-2023 Vates SAS.
> + */
> +
> +#include <xen/errno.h>

Unused header.  All this file needs (in this form at least) is asm/sbi.h

> +#include <asm/sbi.h>
> +
> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> +                        unsigned long arg0, unsigned long arg1,
> +                        unsigned long arg2, unsigned long arg3,
> +                        unsigned long arg4, unsigned long arg5)
> +{
> +    struct sbiret ret;
> +
> +    register unsigned long a0 asm ("a0") = arg0;
> +    register unsigned long a1 asm ("a1") = arg1;
> +    register unsigned long a2 asm ("a2") = arg2;
> +    register unsigned long a3 asm ("a3") = arg3;
> +    register unsigned long a4 asm ("a4") = arg4;
> +    register unsigned long a5 asm ("a5") = arg5;
> +    register unsigned long a6 asm ("a6") = fid;
> +    register unsigned long a7 asm ("a7") = ext;
> +
> +    asm volatile ("ecall"
> +              : "+r" (a0), "+r" (a1)
> +              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
> +              : "memory");

Indentation.  Each colon wants 4 more spaces in front of it.

Both can be fixed on commit.

~Andrew

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-17 23:22   ` Bobby Eshleman
@ 2023-01-17 23:57   ` Andrew Cooper
  2023-01-18  0:33     ` Julien Grall
  2023-01-18 11:07     ` Oleksii
  2023-01-19 12:45   ` Oleksii Kurochko
  2 siblings, 2 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-17 23:57   ` Andrew Cooper
@ 2023-01-18  0:33     ` Julien Grall
  2023-01-18 11:07     ` Oleksii
  1 sibling, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
  2023-01-17 23:19   ` Bobby Eshleman
  2023-01-17 23:32   ` Andrew Cooper
@ 2023-01-18  1:10   ` Alistair Francis
  2023-01-19 12:45   ` Oleksii Kurochko
  3 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2023-01-18  1:10 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Bobby Eshleman, Bob Eshleman,
	Alistair Francis, Connor Davis

On Tue, Jan 17, 2023 at 12:39 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> From: Bobby Eshleman <bobby.eshleman@gmail.com>
>
> Originally SBI implementation for Xen was introduced by
> Bobby Eshleman <bobby.eshleman@gmail.com> but it was removed
> all the stuff for simplicity  except SBI call for putting
> character to console.
>
> The patch introduces sbi_putchar() SBI call which is necessary
> to implement initial early_printk.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

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

Alistair

> ---
> Changes in V4:
>     - Nothing changed
> ---
> Changes in V3:
>     - update copyright's year
>     - rename definition of __CPU_SBI_H__ to __ASM_RISCV_SBI_H__
>     - fix identations
>     - change an author of the commit
> ---
> Changes in V2:
>     - add an explanatory comment about sbi_console_putchar() function.
>     - order the files alphabetically in Makefile
> ---
>  xen/arch/riscv/Makefile          |  1 +
>  xen/arch/riscv/include/asm/sbi.h | 34 ++++++++++++++++++++++++
>  xen/arch/riscv/sbi.c             | 45 ++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100644 xen/arch/riscv/include/asm/sbi.h
>  create mode 100644 xen/arch/riscv/sbi.c
>
> diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
> index 5a67a3f493..fd916e1004 100644
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_RISCV_64) += riscv64/
> +obj-y += sbi.o
>  obj-y += setup.o
>
>  $(TARGET): $(TARGET)-syms
> diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
> new file mode 100644
> index 0000000000..0e6820a4ed
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/sbi.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later) */
> +/*
> + * Copyright (c) 2021-2023 Vates SAS.
> + *
> + * Taken from xvisor, modified by Bobby Eshleman (bobby.eshleman@gmail.com).
> + *
> + * Taken/modified from Xvisor project with the following copyright:
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + */
> +
> +#ifndef __ASM_RISCV_SBI_H__
> +#define __ASM_RISCV_SBI_H__
> +
> +#define SBI_EXT_0_1_CONSOLE_PUTCHAR            0x1
> +
> +struct sbiret {
> +    long error;
> +    long value;
> +};
> +
> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> +                        unsigned long arg0, unsigned long arg1,
> +                        unsigned long arg2, unsigned long arg3,
> +                        unsigned long arg4, unsigned long arg5);
> +
> +/**
> + * Writes given character to the console device.
> + *
> + * @param ch The data to be written to the console.
> + */
> +void sbi_console_putchar(int ch);
> +
> +#endif /* __ASM_RISCV_SBI_H__ */
> diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
> new file mode 100644
> index 0000000000..dc0eb44bc6
> --- /dev/null
> +++ b/xen/arch/riscv/sbi.c
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Taken and modified from the xvisor project with the copyright Copyright (c)
> + * 2019 Western Digital Corporation or its affiliates and author Anup Patel
> + * (anup.patel@wdc.com).
> + *
> + * Modified by Bobby Eshleman (bobby.eshleman@gmail.com).
> + *
> + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> + * Copyright (c) 2021-2023 Vates SAS.
> + */
> +
> +#include <xen/errno.h>
> +#include <asm/sbi.h>
> +
> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> +                        unsigned long arg0, unsigned long arg1,
> +                        unsigned long arg2, unsigned long arg3,
> +                        unsigned long arg4, unsigned long arg5)
> +{
> +    struct sbiret ret;
> +
> +    register unsigned long a0 asm ("a0") = arg0;
> +    register unsigned long a1 asm ("a1") = arg1;
> +    register unsigned long a2 asm ("a2") = arg2;
> +    register unsigned long a3 asm ("a3") = arg3;
> +    register unsigned long a4 asm ("a4") = arg4;
> +    register unsigned long a5 asm ("a5") = arg5;
> +    register unsigned long a6 asm ("a6") = fid;
> +    register unsigned long a7 asm ("a7") = ext;
> +
> +    asm volatile ("ecall"
> +              : "+r" (a0), "+r" (a1)
> +              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
> +              : "memory");
> +    ret.error = a0;
> +    ret.value = a1;
> +
> +    return ret;
> +}
> +
> +void sbi_console_putchar(int ch)
> +{
> +    sbi_ecall(SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0);
> +}
> --
> 2.39.0
>
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 4/4] automation: add RISC-V smoke test
  2023-01-16 14:39 ` [PATCH v4 4/4] automation: add RISC-V smoke test Oleksii Kurochko
@ 2023-01-18  1:14   ` Alistair Francis
  2023-01-19 12:45   ` Oleksii Kurochko
  1 sibling, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2023-01-18  1:14 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: xen-devel, Jan Beulich, Julien Grall, Andrew Cooper,
	Stefano Stabellini, Gianluca Guida, Doug Goldstein

On Tue, Jan 17, 2023 at 12:40 AM Oleksii Kurochko
<oleksii.kurochko@gmail.com> wrote:
>
> Add check if there is a message 'Hello from C env' presents
> in log file to be sure that stack is set and C part of early printk
> is working.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>

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

Alistair

> ---
> 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.
> ---
>  automation/gitlab-ci/test.yaml           | 20 ++++++++++++++++++++
>  automation/scripts/qemu-smoke-riscv64.sh | 20 ++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100755 automation/scripts/qemu-smoke-riscv64.sh
>
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index afd80adfe1..64f47a0ab9 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -54,6 +54,19 @@
>    tags:
>      - x86_64
>
> +.qemu-riscv64:
> +  extends: .test-jobs-common
> +  variables:
> +    CONTAINER: archlinux:riscv64
> +    LOGFILE: qemu-smoke-riscv64.log
> +  artifacts:
> +    paths:
> +      - smoke.serial
> +      - '*.log'
> +    when: always
> +  tags:
> +    - x86_64
> +
>  .yocto-test:
>    extends: .test-jobs-common
>    script:
> @@ -234,6 +247,13 @@ qemu-smoke-x86-64-clang-pvh:
>    needs:
>      - debian-unstable-clang-debug
>
> +qemu-smoke-riscv64-gcc:
> +  extends: .qemu-riscv64
> +  script:
> +    - ./automation/scripts/qemu-smoke-riscv64.sh 2>&1 | tee ${LOGFILE}
> +  needs:
> +    - riscv64-cross-gcc
> +
>  # Yocto test jobs
>  yocto-qemuarm64:
>    extends: .yocto-test-arm64
> diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
> new file mode 100755
> index 0000000000..e0f06360bc
> --- /dev/null
> +++ b/automation/scripts/qemu-smoke-riscv64.sh
> @@ -0,0 +1,20 @@
> +#!/bin/bash
> +
> +set -ex
> +
> +# Run the test
> +rm -f smoke.serial
> +set +e
> +
> +timeout -k 1 2 \
> +qemu-system-riscv64 \
> +    -M virt \
> +    -smp 1 \
> +    -nographic \
> +    -m 2g \
> +    -kernel binaries/xen \
> +    |& tee smoke.serial
> +
> +set -e
> +(grep -q "Hello from C env" smoke.serial) || exit 1
> +exit 0
> --
> 2.39.0
>
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-17 23:32   ` Andrew Cooper
@ 2023-01-18  7:38     ` Jan Beulich
  2023-01-18 11:29       ` Oleksii
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2023-01-18  7:38 UTC (permalink / raw)
  To: Andrew Cooper, Oleksii Kurochko
  Cc: Julien Grall, Stefano Stabellini, Gianluca Guida, Bobby Eshleman,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On 18.01.2023 00:32, Andrew Cooper wrote:
> On 16/01/2023 2:39 pm, Oleksii Kurochko wrote:
>> +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
>> +                        unsigned long arg0, unsigned long arg1,
>> +                        unsigned long arg2, unsigned long arg3,
>> +                        unsigned long arg4, unsigned long arg5)
>> +{
>> +    struct sbiret ret;
>> +
>> +    register unsigned long a0 asm ("a0") = arg0;
>> +    register unsigned long a1 asm ("a1") = arg1;
>> +    register unsigned long a2 asm ("a2") = arg2;
>> +    register unsigned long a3 asm ("a3") = arg3;
>> +    register unsigned long a4 asm ("a4") = arg4;
>> +    register unsigned long a5 asm ("a5") = arg5;
>> +    register unsigned long a6 asm ("a6") = fid;
>> +    register unsigned long a7 asm ("a7") = ext;
>> +
>> +    asm volatile ("ecall"
>> +              : "+r" (a0), "+r" (a1)
>> +              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
>> +              : "memory");
> 
> Indentation.  Each colon wants 4 more spaces in front of it.

Plus, if we're already talking of style, blanks are missing immediately inside
the outermost parentheses, requiring yet one more space of indentation on the
subsequent lines.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-17 23:57   ` Andrew Cooper
  2023-01-18  0:33     ` Julien Grall
@ 2023-01-18 11:07     ` Oleksii
  1 sibling, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-17 10:04       ` Jan Beulich
@ 2023-01-18 11:23         ` Oleksii
  2023-01-18 13:03           ` Jan Beulich
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksii @ 2023-01-18 11:23 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On Tue, 2023-01-17 at 11:04 +0100, Jan Beulich wrote:
> On 17.01.2023 10:29, Oleksii wrote:
> > On Mon, 2023-01-16 at 15:59 +0100, Jan Beulich wrote:
> > > On 16.01.2023 15:39, Oleksii Kurochko wrote:
> > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > > > ---
> > > > Changes in V4:
> > > >     - Clean up types in <asm/types.h> and remain only
> > > > necessary.
> > > >       The following types was removed as they are defined in
> > > > <xen/types.h>:
> > > >       {__|}{u|s}{8|16|32|64}
> > > 
> > > For one you still typedef u32 and u64. And imo correctly so,
> > > until we
> > > get around to move the definition basic types into xen/types.h.
> > > Plus
> > > I can't see how things build for you: xen/types.h expects
> > > __{u,s}<N>
> > It builds because nothing is used <xen/types.h> now so that's why I
> > missed them but you are right __{u,s}<N> should be backed.
> > It looks like {__,}{u,s}{8,16,32} are the same for all available in
> > Xen
> > architectures so probably can I move them to <xen/types.h> instead
> > of
> > remain them in <asm/types.h>?
> 
> This next step isn't quite as obvious, i.e. has room for being
> contentious. In particular deriving fixed width types from C basic
> types is setting us up for future problems (especially in the
> context of RISC-V think of RV128). Therefore, if we touch and
> generalize this, I'd like to sanitize things at the same time.
> 
> I'd then prefer to typedef {u,}int<N>_t by using either the "mode"
> attribute (requiring us to settle on a prereq of there always being
> 8 bits per char) or the compiler supplied __{U,}INT<N>_TYPE__
> (taking gcc 4.7 as a prereq; didn't check clang yet). Both would
> allow {u,}int64_t to also be put in the common header. Yet if e.g.
> a prereq assumption faced opposition, some other approach would
> need to be found. Plus using either of the named approaches has
> issues with the printf() format specifiers, for which I'm yet to
> figure out a solution (or maybe someone else knows a good way to
> deal with that; using compiler provided headers isn't an option
> afaict, as gcc provides stdint.h but not inttypes.h, but maybe
> glibc's simplistic approach is good enough - they're targeting
> far more architectures than we do and get away with that).
> 
Thanks for the explanation.

If back to RISCV's <asm/types.h> it looks that the v2 of the patch
(https://lore.kernel.org/xen-devel/ca2674739cfa71cae0bf084a7b471ad4518026d3.1673278109.git.oleksii.kurochko@gmail.com/)
is the best one option now because as I can see some work is going on
around <xen/types.h> and keeping the minimal set of types now will
allow us to not remove unneeded types for RISCV's port from asm/types.h
in the future.

Even more based on your patch [
https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html ]
RISCV's <asm/types.h> can be empty for this patch series.

> Jan
~Oleksii


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-18  7:38     ` Jan Beulich
@ 2023-01-18 11:29       ` Oleksii
  0 siblings, 0 replies; 27+ messages in thread
From: Oleksii @ 2023-01-18 11:29 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper
  Cc: Julien Grall, Stefano Stabellini, Gianluca Guida, Bobby Eshleman,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On Wed, 2023-01-18 at 08:38 +0100, Jan Beulich wrote:
> On 18.01.2023 00:32, Andrew Cooper wrote:
> > On 16/01/2023 2:39 pm, Oleksii Kurochko wrote:
> > > +struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
> > > +                        unsigned long arg0, unsigned long arg1,
> > > +                        unsigned long arg2, unsigned long arg3,
> > > +                        unsigned long arg4, unsigned long arg5)
> > > +{
> > > +    struct sbiret ret;
> > > +
> > > +    register unsigned long a0 asm ("a0") = arg0;
> > > +    register unsigned long a1 asm ("a1") = arg1;
> > > +    register unsigned long a2 asm ("a2") = arg2;
> > > +    register unsigned long a3 asm ("a3") = arg3;
> > > +    register unsigned long a4 asm ("a4") = arg4;
> > > +    register unsigned long a5 asm ("a5") = arg5;
> > > +    register unsigned long a6 asm ("a6") = fid;
> > > +    register unsigned long a7 asm ("a7") = ext;
> > > +
> > > +    asm volatile ("ecall"
> > > +              : "+r" (a0), "+r" (a1)
> > > +              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r"
> > > (a6), "r" (a7)
> > > +              : "memory");
> > 
> > Indentation.  Each colon wants 4 more spaces in front of it.
> 
> Plus, if we're already talking of style, blanks are missing
> immediately inside
> the outermost parentheses, requiring yet one more space of
> indentation on the
> subsequent lines.
> 
Thanks Andrew and Jan for the comments. I'll take them into account.
> Jan
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-18 11:23         ` Oleksii
@ 2023-01-18 13:03           ` Jan Beulich
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2023-01-18 13:03 UTC (permalink / raw)
  To: Oleksii
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, xen-devel

On 18.01.2023 12:23, Oleksii wrote:
> On Tue, 2023-01-17 at 11:04 +0100, Jan Beulich wrote:
>> On 17.01.2023 10:29, Oleksii wrote:
>>> On Mon, 2023-01-16 at 15:59 +0100, Jan Beulich wrote:
>>>> On 16.01.2023 15:39, Oleksii Kurochko wrote:
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> ---
>>>>> Changes in V4:
>>>>>     - Clean up types in <asm/types.h> and remain only
>>>>> necessary.
>>>>>       The following types was removed as they are defined in
>>>>> <xen/types.h>:
>>>>>       {__|}{u|s}{8|16|32|64}
>>>>
>>>> For one you still typedef u32 and u64. And imo correctly so,
>>>> until we
>>>> get around to move the definition basic types into xen/types.h.
>>>> Plus
>>>> I can't see how things build for you: xen/types.h expects
>>>> __{u,s}<N>
>>> It builds because nothing is used <xen/types.h> now so that's why I
>>> missed them but you are right __{u,s}<N> should be backed.
>>> It looks like {__,}{u,s}{8,16,32} are the same for all available in
>>> Xen
>>> architectures so probably can I move them to <xen/types.h> instead
>>> of
>>> remain them in <asm/types.h>?
>>
>> This next step isn't quite as obvious, i.e. has room for being
>> contentious. In particular deriving fixed width types from C basic
>> types is setting us up for future problems (especially in the
>> context of RISC-V think of RV128). Therefore, if we touch and
>> generalize this, I'd like to sanitize things at the same time.
>>
>> I'd then prefer to typedef {u,}int<N>_t by using either the "mode"
>> attribute (requiring us to settle on a prereq of there always being
>> 8 bits per char) or the compiler supplied __{U,}INT<N>_TYPE__
>> (taking gcc 4.7 as a prereq; didn't check clang yet). Both would
>> allow {u,}int64_t to also be put in the common header. Yet if e.g.
>> a prereq assumption faced opposition, some other approach would
>> need to be found. Plus using either of the named approaches has
>> issues with the printf() format specifiers, for which I'm yet to
>> figure out a solution (or maybe someone else knows a good way to
>> deal with that; using compiler provided headers isn't an option
>> afaict, as gcc provides stdint.h but not inttypes.h, but maybe
>> glibc's simplistic approach is good enough - they're targeting
>> far more architectures than we do and get away with that).
>>
> Thanks for the explanation.
> 
> If back to RISCV's <asm/types.h> it looks that the v2 of the patch
> (https://lore.kernel.org/xen-devel/ca2674739cfa71cae0bf084a7b471ad4518026d3.1673278109.git.oleksii.kurochko@gmail.com/)
> is the best one option now because as I can see some work is going on
> around <xen/types.h> and keeping the minimal set of types now will
> allow us to not remove unneeded types for RISCV's port from asm/types.h
> in the future.

Well, as said before, I'd prefer if that header wasn't populated piecemeal,
but ...

> Even more based on your patch [
> https://lists.xen.org/archives/html/xen-devel/2023-01/msg00720.html ]
> RISCV's <asm/types.h> can be empty for this patch series.

... leaving it empty for now would of course be fine. Once the basic
fixed width types are needed, imo they ought to be populated all in one
got. But maybe by then we've managed to have that stuff in xen/types.h.

Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v4 0/4] Basic early_printk and smoke test implementation
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
                   ` (4 preceding siblings ...)
  2023-01-16 14:44 ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
@ 2023-01-19 12:45 ` Oleksii Kurochko
  2023-01-19 13:23 ` Jan Beulich
  6 siblings, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file
  2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
  2023-01-16 14:59   ` Jan Beulich
@ 2023-01-19 12:45   ` Oleksii Kurochko
  1 sibling, 0 replies; 27+ messages in thread
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

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
    - Clean up types in <asm/types.h> and remain only necessary.
      The following types was removed as they are defined in <xen/types.h>:
      {__|}{u|s}{8|16|32|64}
---
Changes in V3:
    - Nothing changed
---
Changes in V2:
    - Remove unneeded now types from <asm/types.h>
---
 xen/arch/riscv/include/asm/types.h | 43 ++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/types.h

diff --git a/xen/arch/riscv/include/asm/types.h b/xen/arch/riscv/include/asm/types.h
new file mode 100644
index 0000000000..9e55bcf776
--- /dev/null
+++ b/xen/arch/riscv/include/asm/types.h
@@ -0,0 +1,43 @@
+#ifndef __RISCV_TYPES_H__
+#define __RISCV_TYPES_H__
+
+#ifndef __ASSEMBLY__
+
+#if defined(CONFIG_RISCV_32)
+typedef unsigned long long u64;
+typedef unsigned int u32;
+typedef u32 vaddr_t;
+#define PRIvaddr PRIx32
+typedef u64 paddr_t;
+#define INVALID_PADDR (~0ULL)
+#define PRIpaddr "016llx"
+typedef u32 register_t;
+#define PRIregister "x"
+#elif defined (CONFIG_RISCV_64)
+typedef unsigned long u64;
+typedef u64 vaddr_t;
+#define PRIvaddr PRIx64
+typedef u64 paddr_t;
+#define INVALID_PADDR (~0UL)
+#define PRIpaddr "016lx"
+typedef u64 register_t;
+#define PRIregister "lx"
+#endif
+
+#if defined(__SIZE_TYPE__)
+typedef __SIZE_TYPE__ size_t;
+#else
+typedef unsigned long size_t;
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __RISCV_TYPES_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console
  2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
                     ` (2 preceding siblings ...)
  2023-01-18  1:10   ` Alistair Francis
@ 2023-01-19 12:45   ` Oleksii Kurochko
  3 siblings, 0 replies; 27+ messages in thread
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, Bobby Eshleman, Bob Eshleman, Alistair Francis,
	Connor Davis, Oleksii Kurochko

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

Originally SBI implementation for Xen was introduced by
Bobby Eshleman <bobby.eshleman@gmail.com> but it was removed
all the stuff for simplicity  except SBI call for putting
character to console.

The patch introduces sbi_putchar() SBI call which is necessary
to implement initial early_printk.

Signed-off-by: Bobby Eshleman <bobby.eshleman@gmail.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
    - Nothing changed
---
Changes in V3:
    - update copyright's year
    - rename definition of __CPU_SBI_H__ to __ASM_RISCV_SBI_H__
    - fix identations
    - change an author of the commit
---
Changes in V2:
    - add an explanatory comment about sbi_console_putchar() function.
    - order the files alphabetically in Makefile
---
 xen/arch/riscv/Makefile          |  1 +
 xen/arch/riscv/include/asm/sbi.h | 34 ++++++++++++++++++++++++
 xen/arch/riscv/sbi.c             | 45 ++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 xen/arch/riscv/include/asm/sbi.h
 create mode 100644 xen/arch/riscv/sbi.c

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 5a67a3f493..fd916e1004 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_RISCV_64) += riscv64/
+obj-y += sbi.o
 obj-y += setup.o
 
 $(TARGET): $(TARGET)-syms
diff --git a/xen/arch/riscv/include/asm/sbi.h b/xen/arch/riscv/include/asm/sbi.h
new file mode 100644
index 0000000000..0e6820a4ed
--- /dev/null
+++ b/xen/arch/riscv/include/asm/sbi.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later) */
+/*
+ * Copyright (c) 2021-2023 Vates SAS.
+ *
+ * Taken from xvisor, modified by Bobby Eshleman (bobby.eshleman@gmail.com).
+ *
+ * Taken/modified from Xvisor project with the following copyright:
+ *
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef __ASM_RISCV_SBI_H__
+#define __ASM_RISCV_SBI_H__
+
+#define SBI_EXT_0_1_CONSOLE_PUTCHAR		0x1
+
+struct sbiret {
+    long error;
+    long value;
+};
+
+struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
+                        unsigned long arg0, unsigned long arg1,
+                        unsigned long arg2, unsigned long arg3,
+                        unsigned long arg4, unsigned long arg5);
+
+/**
+ * Writes given character to the console device.
+ *
+ * @param ch The data to be written to the console.
+ */
+void sbi_console_putchar(int ch);
+
+#endif /* __ASM_RISCV_SBI_H__ */
diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
new file mode 100644
index 0000000000..dc0eb44bc6
--- /dev/null
+++ b/xen/arch/riscv/sbi.c
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Taken and modified from the xvisor project with the copyright Copyright (c)
+ * 2019 Western Digital Corporation or its affiliates and author Anup Patel
+ * (anup.patel@wdc.com).
+ *
+ * Modified by Bobby Eshleman (bobby.eshleman@gmail.com).
+ *
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ * Copyright (c) 2021-2023 Vates SAS.
+ */
+
+#include <xen/errno.h>
+#include <asm/sbi.h>
+
+struct sbiret sbi_ecall(unsigned long ext, unsigned long fid,
+                        unsigned long arg0, unsigned long arg1,
+                        unsigned long arg2, unsigned long arg3,
+                        unsigned long arg4, unsigned long arg5)
+{
+    struct sbiret ret;
+
+    register unsigned long a0 asm ("a0") = arg0;
+    register unsigned long a1 asm ("a1") = arg1;
+    register unsigned long a2 asm ("a2") = arg2;
+    register unsigned long a3 asm ("a3") = arg3;
+    register unsigned long a4 asm ("a4") = arg4;
+    register unsigned long a5 asm ("a5") = arg5;
+    register unsigned long a6 asm ("a6") = fid;
+    register unsigned long a7 asm ("a7") = ext;
+
+    asm volatile ("ecall"
+              : "+r" (a0), "+r" (a1)
+              : "r" (a2), "r" (a3), "r" (a4), "r" (a5), "r" (a6), "r" (a7)
+              : "memory");
+    ret.error = a0;
+    ret.value = a1;
+
+    return ret;
+}
+
+void sbi_console_putchar(int ch)
+{
+    sbi_ecall(SBI_EXT_0_1_CONSOLE_PUTCHAR, 0, ch, 0, 0, 0, 0, 0);
+}
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff
  2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
  2023-01-17 23:22   ` Bobby Eshleman
  2023-01-17 23:57   ` Andrew Cooper
@ 2023-01-19 12:45   ` Oleksii Kurochko
  2 siblings, 0 replies; 27+ messages in thread
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	[flat|nested] 27+ messages in thread

* [PATCH v4 4/4] automation: add RISC-V smoke test
  2023-01-16 14:39 ` [PATCH v4 4/4] automation: add RISC-V smoke test Oleksii Kurochko
  2023-01-18  1:14   ` Alistair Francis
@ 2023-01-19 12:45   ` Oleksii Kurochko
  1 sibling, 0 replies; 27+ messages in thread
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, Doug Goldstein

Add check if there is a message 'Hello from C env' presents
in log file to be sure that stack is set and C part of early printk
is working.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
---
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.
---
 automation/gitlab-ci/test.yaml           | 20 ++++++++++++++++++++
 automation/scripts/qemu-smoke-riscv64.sh | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100755 automation/scripts/qemu-smoke-riscv64.sh

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index afd80adfe1..64f47a0ab9 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -54,6 +54,19 @@
   tags:
     - x86_64
 
+.qemu-riscv64:
+  extends: .test-jobs-common
+  variables:
+    CONTAINER: archlinux:riscv64
+    LOGFILE: qemu-smoke-riscv64.log
+  artifacts:
+    paths:
+      - smoke.serial
+      - '*.log'
+    when: always
+  tags:
+    - x86_64
+
 .yocto-test:
   extends: .test-jobs-common
   script:
@@ -234,6 +247,13 @@ qemu-smoke-x86-64-clang-pvh:
   needs:
     - debian-unstable-clang-debug
 
+qemu-smoke-riscv64-gcc:
+  extends: .qemu-riscv64
+  script:
+    - ./automation/scripts/qemu-smoke-riscv64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+    - riscv64-cross-gcc
+
 # Yocto test jobs
 yocto-qemuarm64:
   extends: .yocto-test-arm64
diff --git a/automation/scripts/qemu-smoke-riscv64.sh b/automation/scripts/qemu-smoke-riscv64.sh
new file mode 100755
index 0000000000..e0f06360bc
--- /dev/null
+++ b/automation/scripts/qemu-smoke-riscv64.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+
+set -ex
+
+# Run the test
+rm -f smoke.serial
+set +e
+
+timeout -k 1 2 \
+qemu-system-riscv64 \
+    -M virt \
+    -smp 1 \
+    -nographic \
+    -m 2g \
+    -kernel binaries/xen \
+    |& tee smoke.serial
+
+set -e
+(grep -q "Hello from C env" smoke.serial) || exit 1
+exit 0
-- 
2.39.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v4 0/4] Basic early_printk and smoke test implementation
  2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
                   ` (5 preceding siblings ...)
  2023-01-19 12:45 ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
@ 2023-01-19 13:23 ` Jan Beulich
  6 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2023-01-19 13:23 UTC (permalink / raw)
  To: Oleksii Kurochko
  Cc: Julien Grall, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
	Bob Eshleman, Alistair Francis, Connor Davis, Doug Goldstein,
	xen-devel

On 19.01.2023 13:45, 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
> 
> ---
> 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.  

Did you really mean to send v4 another time?

Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2023-01-19 13:24 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-16 14:39 [PATCH v4 0/4] The patch series introduces the following: Oleksii Kurochko
2023-01-16 14:39 ` [PATCH v4 1/4] xen/riscv: introduce asm/types.h header file Oleksii Kurochko
2023-01-16 14:59   ` Jan Beulich
2023-01-17  9:29     ` Oleksii
2023-01-17 10:04       ` Jan Beulich
2023-01-18 11:23         ` Oleksii
2023-01-18 13:03           ` Jan Beulich
2023-01-19 12:45   ` Oleksii Kurochko
2023-01-16 14:39 ` [PATCH v4 2/4] xen/riscv: introduce sbi call to putchar to console Oleksii Kurochko
2023-01-17 23:19   ` Bobby Eshleman
2023-01-17 23:32   ` Andrew Cooper
2023-01-18  7:38     ` Jan Beulich
2023-01-18 11:29       ` Oleksii
2023-01-18  1:10   ` Alistair Francis
2023-01-19 12:45   ` Oleksii Kurochko
2023-01-16 14:39 ` [PATCH v4 3/4] xen/riscv: introduce early_printk basic stuff Oleksii Kurochko
2023-01-17 23:22   ` Bobby Eshleman
2023-01-17 23:57   ` Andrew Cooper
2023-01-18  0:33     ` Julien Grall
2023-01-18 11:07     ` Oleksii
2023-01-19 12:45   ` Oleksii Kurochko
2023-01-16 14:39 ` [PATCH v4 4/4] automation: add RISC-V smoke test Oleksii Kurochko
2023-01-18  1:14   ` Alistair Francis
2023-01-19 12:45   ` Oleksii Kurochko
2023-01-16 14:44 ` [PATCH v4 0/4] The patch series introduces the following: Oleksii
2023-01-19 12:45 ` [PATCH v4 0/4] Basic early_printk and smoke test implementation Oleksii Kurochko
2023-01-19 13:23 ` Jan Beulich

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).