Re: [PATCH v3 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-02-28 at 18:01 +0000, Julien Grall wrote:
> On 28/02/2023 17:21, Oleksii wrote:
> > Hi Julien,
> 
> Hi Oleksii,
> > > > +
> > > > +            for ( i = 0, b = region->frame[id].bugs;
> > > > +                  i < region->frame[id].n_bugs; b++, i++ )
> > > > +            {
> > > > +                if ( bug_loc(b) == pc )
> > > > +                {
> > > > +                    bug = b;
> > > > +                    goto found;
> > > > +                }
> > > > +            }
> > > > +        }
> > > > +    }
> > > > +
> > > > + found:
> > > > +    if ( !bug )
> > > > +        return -EINVAL;
> > > > +
> > > > +    if ( id == BUGFRAME_run_fn )
> > > > +    {
> > > > +#ifdef BUG_FN_REG
> > > > +        void (*fn)(const struct cpu_user_regs *) = (void
> > > > *)regs-
> > > > > BUG_FN_REG;
> > > 
> > > AFAIU, this is necessary so Arm can use the generic
> > > do_bug_frame().
> > > 
> > > I was under the impression that RISC-V and Arm had the similar
> > > issue
> > > with %c. It seems like you managed to resolve it on RISC-V, so
> > > can we
> > > fully switch Arm to the generic implementation of bug?
> > I tried to switch ARM to generic implementation.
> > 
> > Here is the patch: [1]
> 
> I have replied on the other thread.
> > > > +#ifndef BUG_ASM_CONST
> > > > +#define BUG_ASM_CONST ""
> > > > +#endif
> > > 
> > > This line is a bit misterious to me. Would you be able to outline
> > > why
> > > an
> > > architecture would override this?
> > It is needed in case if compiler for an architecture doesn't have
> > proper support of '%c' ( it is so for ARM & RISC-V )
> 
> Hmmm.... Why can't x86 use the same version? IOW what's the benefits
> to 
> differ on x86?
We can't use '%c' for all architectures because not all compiler
supports '%c' fully for all architectures.
There is no any benefits. In case of x86 it is needed to delete
punctuation before immediate. I mean that immediate is passed as $1 (
or # i always missed with ARM ) and to drop $ it is used %c.
> 
> Anyway, documentation is always good to have because it helps the 
> reader/reviewer to understand how such decision was made.
I'll add the comment then before define.
> 
> Cheers,
> 
~ Oleksii