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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-02-16 at 08:31 +0100, Jan Beulich wrote:
> On 15.02.2023 18:59, Oleksii wrote:
> > Hello Jan and community,
> > 
> > I experimented and switched RISC-V to x86 implementation. All that
> > I
> > changed in x86 implementation for RISC-V was _ASM_BUGFRAME_TEXT.
> > Other
> > things are the same as for x86.
> > 
> > For RISC-V it is fine to skip '%c' modifier so _ASM_BUGFRAME_TEXT
> > will
> > look like:
> > 
> > #define _ASM_BUGFRAME_TEXT(second_frame) \
> >     ".Lbug%=: ebreak\n"   
> >     ".pushsection .bug_frames.%[bf_type], \"a\", @progbits\n"
> >     ".p2align 2\n"
> >     ".Lfrm%=:\n"
> >     ".long (.Lbug%= - .Lfrm%=) + %[bf_line_hi]\n"
> >     ".long (%[bf_ptr] - .Lfrm%=) + %[bf_line_lo]\n"
> >     ".if " #second_frame "\n"
> >     ".long 0, %[bf_msg] - .Lfrm%=\n"
> >     ".endif\n"
> >     ".popsection\n"
> 
> I expect this could be further abstracted such that only the actual
> instruction is arch-specific.
Generally, the only thing that can't be abstracted ( as you mentioned
is an instruction ).

So we can make additional defines:
  1. #define MODIFIER "" or "c" (depends on architecture) and use it  
the following way:
   ".pushsection .bug_frames.%"MODIFIER"[bf_type], \"a\", @progbits\n"
   ...
  2. #define BUG_INSTR "ebreak" | "ud2" | "..." and use it in the
following way:
   ".Lbug%=: "BUG_ISNTR"\n"
   ...
Except for these defines which should be specified for each
architecture
all other code will be the same for all architectures.

But as I understand with modifier 'c' can be issues that not all
compilers support but for ARM and  x86 before immediate is present
punctuation # or $ which are removed by modifier 'c'. And I am
wondering what other ways to remove punctuation before immediate exist.

Do you think we should consider that as an issue?

> 
> > The only thing I am worried about is:
> > 
> > #define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
> >   [bf_type] "i" (type), ...
> > because as I understand it can be an issue with 'i' modifier in
> > case of
> > PIE. I am not sure that Xen enables PIE somewhere but still...
> > If it is not an issue then we can use x86 implementation as a
> > generic
> > one.
> 
> "i" is not generally an issue with PIE, it only is when the value is
> the
> address of a symbol. Here "type" is a constant in all cases. (Or else
> how would you express an immediate operand of an instruction in an
> asm()?)
Hmm. I don't know other ways to express an immediate operand except
'i'.

It looks like type, line, msg are always 'constants' 
(
they possibly can be passed without 'i' and used directly inside asm():
   ...
   ".pushsection .bug_frames." __stringify(type) ", \"a\",
%progbits\n"\
   ...
) but the issue will be with 'ptr' for which we have to use 'i'.

And I am not sure about all 'constants'. For example, I think that it
can be an issue with 'line' = '((line & ((1 << BUG_LINE_LO_WIDTH) - 1))
<< BUG_DISP_WIDTH)' if we will use that directly inside asm(...).

> 
> Jan

~ Oleksii