Hi Michal,
On Fri, 2023-01-06 at 16:19 +0100, Michal Orzel wrote:
> Hi Oleksii,
>
> On 06/01/2023 14:14, Oleksii Kurochko wrote:
> >
> >
> > The patch introduce sbi_putchar() SBI call which is necessary
> > to implement initial early_printk
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com>
> > ---
> > xen/arch/riscv/Makefile | 1 +
> > xen/arch/riscv/include/asm/sbi.h | 34 ++++++++++++++++++++++++
> > xen/arch/riscv/sbi.c | 44
> > ++++++++++++++++++++++++++++++++
> > 3 files changed, 79 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..60db415654 100644
> > --- a/xen/arch/riscv/Makefile
> > +++ b/xen/arch/riscv/Makefile
> > @@ -1,5 +1,6 @@
> > obj-$(CONFIG_RISCV_64) += riscv64/
> > obj-y += setup.o
> > +obj-y += sbi.o
> >
> > $(TARGET): $(TARGET)-syms
> > $(OBJCOPY) -O binary -S $< $@
> > diff --git a/xen/arch/riscv/include/asm/sbi.h
> > b/xen/arch/riscv/include/asm/sbi.h
> > new file mode 100644
> > index 0000000000..34b53f8eaf
> > --- /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 Vates SAS.
> > + *
> > + * Taken from xvisor, modified by Bobby Eshleman
> > (bobby.eshle...@gmail.com).
> > + *
> > + * Taken/modified from Xvisor project with the following
> > copyright:
> > + *
> > + * Copyright (c) 2019 Western Digital Corporation or its
> > affiliates.
> > + */
> > +
> > +#ifndef __CPU_SBI_H__
> > +#define __CPU_SBI_H__
> I wonder where does CPU come from. Shouldn't this be called
> __ASM_RISCV_SBI_H__ ?
>
I missed that when get this files from Bobby's patch series.
It makes sense to rename the define.
Thanks.
> > +
> > +#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);
> The arguments needs to be aligned.
>
> > +
> > +/**
> > + * Writes given character to the console device.
> > + *
> > + * @param ch The data to be written to the console.
> > + */
> > +void sbi_console_putchar(int ch);
> > +
> > +#endif // __CPU_SBI_H__
> // should be replaced with /* */
>
> > diff --git a/xen/arch/riscv/sbi.c b/xen/arch/riscv/sbi.c
> > new file mode 100644
> > index 0000000000..67cf5dd982
> > --- /dev/null
> > +++ b/xen/arch/riscv/sbi.c
> > @@ -0,0 +1,44 @@
> > +/* 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.pa...@wdc.com).
> > + *
> > + * Modified by Bobby Eshleman (bobby.eshle...@gmail.com).
> > + *
> > + * Copyright (c) 2019 Western Digital Corporation or its
> > affiliates.
> > + * Copyright (c) 2021 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)
> The arguments needs to be aligned.
>
It looks that I mixed tabs with space or vice versa.
Will double check.
Thanks.
> > +{
> > + struct sbiret ret;
> Could you please add an empty line here.
>
> > + 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.38.1
> >
> >
>
> ~Michal
~Oleksii