diff mbox series

[v4,2/4] xen/riscv: introduce sbi call to putchar to console

Message ID 06ad9f6c8cbc87284ef4ecd4b85d9c7df33bd2c1.1673877778.git.oleksii.kurochko@gmail.com (mailing list archive)
State New, archived
Headers show
Series The patch series introduces the following: | expand

Commit Message

Oleksii Kurochko Jan. 16, 2023, 2:39 p.m. UTC 
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

Comments

Bobby Eshleman Jan. 17, 2023, 11:19 p.m. UTC | #1 
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>
Andrew Cooper Jan. 17, 2023, 11:32 p.m. UTC | #2 
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
Alistair Francis Jan. 18, 2023, 1:10 a.m. UTC | #3 
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
>
>
Jan Beulich Jan. 18, 2023, 7:38 a.m. UTC | #4 
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
Oleksii Kurochko Jan. 18, 2023, 11:29 a.m. UTC | #5 
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
>
diff mbox series

Patch

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);
+}