Re: [PATCH v1 0/8] Basic early_printk and smoke test implementation

[Andrew Cooper <Andrew.Cooper3@citrix.com>]

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

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

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

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

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

~Andrew