Re: [PATCH v4 1/5] xen/riscv: introduce decode_cause() stuff

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-02-27 at 13:48 +0100, Jan Beulich wrote:
> On 24.02.2023 12:35, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -4,10 +4,95 @@
> >   *
> >   * RISC-V Trap handlers
> >   */
> > +
> > +#include <xen/errno.h>
> 
> I couldn't spot anything in the file which would require this
> inclusion.
Sure. It can be removed.
> 
> > +#include <xen/lib.h>
> > +
> > +#include <asm/csr.h>
> > +#include <asm/early_printk.h>
> >  #include <asm/processor.h>
> >  #include <asm/traps.h>
> >  
> > -void do_trap(struct cpu_user_regs *cpu_regs)
> > +static const char *decode_trap_cause(unsigned long cause)
> > +{
> > +    static const char *const trap_causes[] = {
> > +        [CAUSE_MISALIGNED_FETCH] = "Instruction Address
> > Misaligned",
> > +        [CAUSE_FETCH_ACCESS] = "Instruction Access Fault",
> > +        [CAUSE_ILLEGAL_INSTRUCTION] = "Illegal Instruction",
> > +        [CAUSE_BREAKPOINT] = "Breakpoint",
> > +        [CAUSE_MISALIGNED_LOAD] = "Load Address Misaligned",
> > +        [CAUSE_LOAD_ACCESS] = "Load Access Fault",
> > +        [CAUSE_MISALIGNED_STORE] = "Store/AMO Address Misaligned",
> > +        [CAUSE_STORE_ACCESS] = "Store/AMO Access Fault",
> > +        [CAUSE_USER_ECALL] = "Environment Call from U-Mode",
> > +        [CAUSE_SUPERVISOR_ECALL] = "Environment Call from S-Mode",
> > +        [CAUSE_MACHINE_ECALL] = "Environment Call from M-Mode",
> > +        [CAUSE_FETCH_PAGE_FAULT] = "Instruction Page Fault",
> > +        [CAUSE_LOAD_PAGE_FAULT] = "Load Page Fault",
> > +        [CAUSE_STORE_PAGE_FAULT] = "Store/AMO Page Fault",
> > +        [CAUSE_FETCH_GUEST_PAGE_FAULT] = "Instruction Guest Page
> > Fault",
> > +        [CAUSE_LOAD_GUEST_PAGE_FAULT] = "Load Guest Page Fault",
> > +        [CAUSE_VIRTUAL_INST_FAULT] = "Virtualized Instruction
> > Fault",
> > +        [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page
> > Fault",
> > +    };
> > +
> > +    if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
> > +        return trap_causes[cause];
> > +    return "UNKNOWN";
> > +}
> > +
> > +const char *decode_reserved_interrupt_cause(unsigned long
> > irq_cause)
> 
> For any non-static function that you add you will need a declaration
> in a header, which the defining C file then includes. I understand
> that during initial bringup functions without (external) callers may
> want to (temporarily) exist, but briefly clarifying what the future
> expectation regarding external uses might help. Any function that's
> not expected to gain external callers should be static.
Thanks. For explanation. I'll make them static.

~ Oleksii