Re: [PATCH v2 12/14] xen/riscv: introduce an implementation of macros from <asm/bug.h>

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

Hi Julien,
On Fri, 2023-01-27 at 16:02 +0000, Julien Grall wrote:
> Hi Oleksii,
> 
> On 27/01/2023 13:59, Oleksii Kurochko wrote:
> > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > assert_failed.
> > 
> > The implementation uses "ebreak" instruction in combination with
> > diffrent bug frame tables (for each type) which contains useful
> > information.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> > Changes:
> >    - Remove __ in define namings
> >    - Update run_in_exception_handler() with
> >      register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> >    - Remove bug_instr_t type and change it's usage to uint32_t
> > ---
> >   xen/arch/riscv/include/asm/bug.h | 118
> > ++++++++++++++++++++++++++++
> >   xen/arch/riscv/traps.c           | 128
> > +++++++++++++++++++++++++++++++
> >   xen/arch/riscv/xen.lds.S         |  10 +++
> >   3 files changed, 256 insertions(+)
> >   create mode 100644 xen/arch/riscv/include/asm/bug.h
> > 
> > diff --git a/xen/arch/riscv/include/asm/bug.h
> > b/xen/arch/riscv/include/asm/bug.h
> > new file mode 100644
> > index 0000000000..4b15d8eba6
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/bug.h
> > @@ -0,0 +1,118 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2012 Regents of the University of California
> > + * Copyright (C) 2021-2023 Vates
> 
> I have to question the two copyrights here given that the majority of
> the code seems to be taken from the arm implementation (see 
> arch/arm/include/asm/bug.h).
> 
> With that said, we should consolidate the code rather than
> duplicating 
> it on every architecture.
> 
Copyrights should be removed. They were taken from the previous
implementation of bug.h for RISC-V so I just forgot to remove them.

It looks like we should have common bug.h for ARM and RISCV but I am
not sure that I know how to do that better.
Probably the code wants to be moved to xen/include/bug.h and using
ifdef ARM && RISCV ...
But still I am not sure that this is the best one option as at least we
have different implementation for x86_64.
> Cheers,
> 
~ Oleksii