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 <bobbyeshle...@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
