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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

> > > > > +
> > > > > +#define BUG_FRAME(type, line, ptr, second_frame, msg) do
> > > > > {                   \
> > > > > +    BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH +
> > > > > BUG_LINE_HI_WIDTH));         \
> > > > > +    BUILD_BUG_ON((type) >=
> > > > > BUGFRAME_NR);                                     \
> > > > > +    asm volatile (
> > > > > _ASM_BUGFRAME_TEXT(second_frame)                          \
> > > > > +                   :: _ASM_BUGFRAME_INFO(type, line, ptr,
> > > > > msg)
> > > > > );            \
> > > > > +} while (0)
> > > 
> > > Isn't this tied to BUG_FRAME_STRUCT being defined (or not)? At
> > > least
> > > the 1st BUILD_BUG_ON() looks problematic if an arch wasn't to use
> > > the generic struct: With how you have things right now
> > > BUG_LINE_{LO,HI}_WIDTH may not be defined, and the check would
> > > also
> > > be at risk of causing false positives. Perhaps it's appropriate
> > > to
> > > have a separate #ifdef (incl the distinct identifier used), but
> > > that
> > > first BUILD_BUG_ON() needs abstracting out.
> Missed that. Thanks.
> I'll introduce that a separate #ifdef before BUG_FRAME:
> 
> #ifndef BUILD_BUG_ON_LINE_WIDTH
> #define BUILD_BUG_ON_LINE_WIDTH \
>         BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH +
> BUG_LINE_HI_WIDTH))
> #endif
I think even better will be to do in the following way:

#ifndef LINE_WIDTH
#define LINE_WIDTH (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)
#endif

#define BUG_FRAME(type, line, ptr, second_frame, msg) do {            
\
    BUILD_BUG_ON((line) >> LINE_WIDTH);                               
\
    BUILD_BUG_ON((type) >= BUGFRAME_NR);                              
\
    asm volatile ( _ASM_BUGFRAME_TEXT(second_frame)                   
\
                   :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) );     
\
} while (false)

~ Oleksii