| Message ID | 9b85a963db538e4735a9f99fc9090ad79508cb2c.1673278109.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Basic early_printk and smoke test implementation | expand |
On 09.01.2023 16:46, Oleksii Kurochko wrote: > --- /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.eshleman@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__ Didn't you mean to change this? > +#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); Please get indentation right here as well as for the definition. Possible forms are 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); or 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); Jan
Hi, On 09/01/2023 15:46, Oleksii Kurochko wrote: > The patch introduce sbi_putchar() SBI call which is necessary > to implement initial early_printk > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > --- > 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 | 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..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..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.eshleman@gmail.com). Hmmm... I missed this one in v1. Is this mostly code from Bobby? If so, please update the commit message accordingly. FAOD, this comment applies for any future code you take from anyone. I will try to remember to mention it but please take pro-active action to check/mention where the code is coming from. Cheers,
On Mon, 2023-01-09 at 17:00 +0100, Jan Beulich wrote: > On 09.01.2023 16:46, Oleksii Kurochko wrote: > > --- /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.eshleman@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__ > > Didn't you mean to change this? Thanks. My fault. Missed that. I will double check that and take into account in new patch series. > > > +#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); > > Please get indentation right here as well as for the definition. Thanks for clarification. > Possible > forms are > > 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); > > or > > 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); > > Jan
Hi, On Mon, 2023-01-09 at 16:03 +0000, Julien Grall wrote: > Hi, > > On 09/01/2023 15:46, Oleksii Kurochko wrote: > > The patch introduce sbi_putchar() SBI call which is necessary > > to implement initial early_printk > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > --- > > 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 | 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..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..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.eshleman@gmail.com). > Hmmm... I missed this one in v1. Is this mostly code from Bobby? If > so, > please update the commit message accordingly. > > FAOD, this comment applies for any future code you take from anyone. > I > will try to remember to mention it but please take pro-active action > to > check/mention where the code is coming from. > Sure, I will try to be more attentive next time. Probably it is out of scope a little bit but could you please share with me a link or clarify in which cases I have to add Copytrigt(c) or should I add a new comment with "Modfied by Oleksii ... ", or it is not necessary at all? Maybe have I to put some other information related to copyrigts? Thanks in advance. > Cheers, > ~Oleksii
On 10/01/2023 08:21, Oleksii wrote: > Hi, Hi Oleksii, > On Mon, 2023-01-09 at 16:03 +0000, Julien Grall wrote: >> Hi, >> >> On 09/01/2023 15:46, Oleksii Kurochko wrote: >>> The patch introduce sbi_putchar() SBI call which is necessary >>> to implement initial early_printk >>> >>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>> --- >>> 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 | 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..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..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.eshleman@gmail.com). >> Hmmm... I missed this one in v1. Is this mostly code from Bobby? If >> so, >> please update the commit message accordingly. >> >> FAOD, this comment applies for any future code you take from anyone. >> I >> will try to remember to mention it but please take pro-active action >> to >> check/mention where the code is coming from. >> > Sure, I will try to be more attentive next time. > > Probably it is out of scope a little bit but could you please share > with me a link or clarify in which cases I have to add Copytrigt(c) or > should I add a new comment with "Modfied by Oleksii ... ", or it is not > necessary at all? Maybe have I to put some other information related to > copyrigts? From my experience, in Xen, we tend to use the signed-off-by rather in-file copyright. So I don't exactly know what would be the rule. Maybe George can help here? Cheers,
(+ George) On 10/01/2023 12:43, Julien Grall wrote: > > > On 10/01/2023 08:21, Oleksii wrote: >> Hi, > > Hi Oleksii, > >> On Mon, 2023-01-09 at 16:03 +0000, Julien Grall wrote: >>> Hi, >>> >>> On 09/01/2023 15:46, Oleksii Kurochko wrote: >>>> The patch introduce sbi_putchar() SBI call which is necessary >>>> to implement initial early_printk >>>> >>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>>> --- >>>> 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 | 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..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..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.eshleman@gmail.com). >>> Hmmm... I missed this one in v1. Is this mostly code from Bobby? If >>> so, >>> please update the commit message accordingly. >>> >>> FAOD, this comment applies for any future code you take from anyone. >>> I >>> will try to remember to mention it but please take pro-active action >>> to >>> check/mention where the code is coming from. >>> >> Sure, I will try to be more attentive next time. >> >> Probably it is out of scope a little bit but could you please share >> with me a link or clarify in which cases I have to add Copytrigt(c) or >> should I add a new comment with "Modfied by Oleksii ... ", or it is not >> necessary at all? Maybe have I to put some other information related to >> copyrigts? > > From my experience, in Xen, we tend to use the signed-off-by rather > in-file copyright. So I don't exactly know what would be the rule. > > Maybe George can help here? > > Cheers, >
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..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.eshleman@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__ + +#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 // __CPU_SBI_H__ 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.patel@wdc.com). + * + * Modified by Bobby Eshleman (bobby.eshleman@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) +{ + 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); +}
The patch introduce sbi_putchar() SBI call which is necessary to implement initial early_printk Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- 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 | 44 ++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 xen/arch/riscv/include/asm/sbi.h create mode 100644 xen/arch/riscv/sbi.c