Re: [PATCH v6 4/6] xen/riscv: introduce trap_init()

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-05-30 at 17:44 +0200, Jan Beulich wrote:
> On 29.05.2023 14:13, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -12,6 +12,31 @@
> >  #include <asm/processor.h>
> >  #include <asm/traps.h>
> >  
> > +#define cast_to_bug_frame(addr) \
> > +    (const struct bug_frame *)(addr)
> 
> I can't find a use for this; should it be dropped or moved to some
> later patch? In any event, if ti's intended to survive, it needs yet
> another pair of parentheses.
You are right. It should be a part of the next patch.
Thanks.

> 
> > +/*
> > + * Initialize the trap handling.
> > + *
> > + * The function is called after MMU is enabled.
> > + */
> > +void trap_init(void)
> 
> Is this going to be used for secondary processors as well? If not,
> it will want to be __init.
I think I'll use it for secondary processors.

> 
> > +{
> > +    /*
> > +     * When the MMU is off, addr varialbe will be a physical
> > address otherwise
> > +     * it would be a virtual address.
> > +     *
> > +     * It will work fine as:
> > +     *  - access to addr is PC-relative.
> > +     *  - -nopie is used. -nopie really suppresses the compiler
> > emitting
> > +     *    code going through .got (which then indeed would mean
> > using absolute
> > +     *    addresses).
> > +     */
> 
> Is all of this comment still relevant not that you're running with
> the MMU already enabled.
Not really. I think comment above trap_init() function will be enough.
I'll remove this comment.

~ Oleksii