Re: [PATCH v8 5/5] xen/x86: switch x86 to use generic implemetation of bug.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-03-16 at 10:52 +0100, Jan Beulich wrote:
> On 15.03.2023 18:21, Oleksii Kurochko wrote:
> > The following changes were made:
> > * Make GENERIC_BUG_FRAME mandatory for X86
> > * Update asm/bug.h using generic implementation in <xen/bug.h>
> > * Update do_invalid_op using generic do_bug_frame()
> > * Define BUG_DEBUGGER_TRAP_FATAL to
> > debugger_trap_fatal(X86_EXC_GP,regs)
> > * type of eip variable was changed to 'const void *'
> > * add '#include <xen/debugger.h>'
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > Changes in V8:
> >  * move <xen/debuger.h> from <asm/bug.h> to <common/bug.c> to fix
> > compilation issue.
> >    The following compilation issue occurs:
> >      In file included from ./arch/x86/include/asm/smp.h:10,
> >                  from ./include/xen/smp.h:4,
> >                  from ./arch/x86/include/asm/processor.h:10,
> >                  from ./arch/x86/include/asm/system.h:6,
> >                  from ./arch/x86/include/asm/atomic.h:5,
> >                  from ./include/xen/gdbstub.h:24,
> >                  from ./arch/x86/include/asm/debugger.h:10,
> >                  from ./include/xen/debugger.h:24,
> >                  from ./arch/x86/include/asm/bug.h:7,
> >                  from ./include/xen/bug.h:15,
> >                  from ./include/xen/lib.h:27,
> >                  from ./include/xen/perfc.h:6,
> >                  from arch/x86/x86_64/asm-offsets.c:9:
> >      ./include/xen/cpumask.h: In function 'cpumask_check':
> >      ./include/xen/cpumask.h:84:9: error: implicit declaration of
> > function 'ASSERT' [-Werror=implicit-function-declaration]
> >                     84 |         ASSERT(cpu < nr_cpu_ids);
> 
> I'm going to post a patch to address this specific failure. But
> something
> similar may then surface elsewhere.
> 
> >    It happens in case when CONFIG_CRASH_DEBUG is enabled
> 
> I have to admit that I don't see the connection to CRASH_DEBUG: It's
> the
> asm/atomic.h inclusion that's problematic afaics, yet that
> (needlessly)
> happens outside the respective #ifdef in xen/gdbstub.h.
> 
> If another instance of this header interaction issue would surface
> despite
> my to-be-posted patch, I'd be okay with going this route for the
> moment.
> But I think the real issue here is xen/lib.h including xen/bug.h.
> Instead
> of that, some stuff that's presently in xen/lib.h should instead move
> to
> xen/bug.h, and the inclusion there be dropped. Any parties actually
> using
> stuff from xen/bug.h (xen/lib.h then won't anymore) should then
> include
> that header themselves.
I tried to remove dependency between xen/lib.h and xen/bug.h but it is
still the same issue but for different compilation unit:

mmanual-endbr -fno-jump-tables '-D__OBJECT_LABEL__=asm-offsets.s' -
mpreferred-stack-boundary=3 -S -g0 -o asm-offsets.s.new -MQ asm-
offsets.s arch/x86/x86_64/asm-offsets.c
In file included from ./include/public/domctl.h:21,
                 from ./include/public/sysctl.h:18,
                 from ./arch/x86/include/asm/cpuid.h:14,
                 from ./arch/x86/include/asm/cpufeature.h:10,
                 from ./arch/x86/include/asm/system.h:7,
                 from ./arch/x86/include/asm/atomic.h:5,
                 from ./include/xen/gdbstub.h:24,
                 from ./arch/x86/include/asm/debugger.h:10,
                 from ./include/xen/debugger.h:24,
                 from ./arch/x86/include/asm/bug.h:6,
                 from ./include/xen/bug.h:15,
                 from ./arch/x86/include/asm/alternative.h:7,
                 from ./arch/x86/include/asm/bitops.h:8,
                 from ./include/xen/bitops.h:106,
                 from ./arch/x86/include/asm/smp.h:8,
                 from ./include/xen/smp.h:4,
                 from ./include/xen/perfc.h:7,
                 from arch/x86/x86_64/asm-offsets.c:9:
~ Oleksii