|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
On Thu, 2023-02-16 at 12:19 +0000, Andrew Cooper wrote:
> On 16/02/2023 12:09 pm, Oleksii wrote:
> > On Thu, 2023-02-16 at 12:44 +0200, Oleksii wrote:
> > > 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(...).
> > >
> > I think we can avoid 'i' by using 'r' + some kind of mov
> > instruction to
> > write a register value to memory. The code below is pseudo-code:
> > #define _ASM_BUGFRAME_TEXT(second_frame)
> > ...
> > "loc_disp:\n"
> > " .long 0x0"
> > "mov [loc_disp], some_register"
> > ...
> > And the we have to define mov for each architecture.
>
> No, you really cannot. This is the asm equivalent of writing
> something
> like this:
>
> static struct bugframe __section(.bug_frames.1) foo = {
> .loc = insn - &foo + LINE_LO,
> .msg = "bug string" - &foo + LINE_HI,
> };
>
> It is a data structure, not executable code.
Thanks for the clarification.
What about mainly using C for BUG_FRAME and only allocating bug_frame
sections in assembly?
Please look at POC below:
#include <stdio.h>
#include <stdint.h>
#define BUG_DISP_WIDTH 24
#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
struct bug_frame {
signed int loc_disp:BUG_DISP_WIDTH;
unsigned int line_hi:BUG_LINE_HI_WIDTH;
signed int ptr_disp:BUG_DISP_WIDTH;
unsigned int line_lo:BUG_LINE_LO_WIDTH;
signed int msg_disp[];
};
#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
#define bug_line(b) (((((b)->line_hi + ((b)->loc_disp < 0)) & \
((1 << BUG_LINE_HI_WIDTH) - 1)) << \
BUG_LINE_LO_WIDTH) + \
(((b)->line_lo + ((b)->ptr_disp < 0)) &
\
((1 << BUG_LINE_LO_WIDTH) - 1)))
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
#define ALLOCATE_BF_SECTION(has_msg) do { \
asm (".pushsection bug_frames \n"
\
".long 0, 0 \n"
\
".if " #has_msg "\n"
\
"\t.long 0 \n"
\
"\t.long 0 \n"
\
".endif\n"
\
".popsection" );
\
} while (0)
#define MERGE_(a,b) a##b
#define UNIQUE_BUG_INSTR_LABEL(a) MERGE_(unique_name_, a)
#define BUG_FRAME(type, line, file, has_msg, msg) do {
\
unsigned int line_lo = (((line) >> BUG_LINE_LO_WIDTH) <<
BUG_DISP_WIDTH);
\
unsigned int line_hi = ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) <<
BUG_DISP_WIDTH);
\
ALLOCATE_BF_SECTION(1);
\
*(signed int *)(&bug_frames) = (unsigned long)
&&UNIQUE_BUG_INSTR_LABEL(line) - (unsigned long)&bug_frames + line_lo;
\
*((signed int *)(&bug_frames) + 1) = (unsigned long)file -
(unsigned long)&bug_frames + line_hi;
\
bug_var.msg_disp[1] = (signed int)((unsigned long)#msg - (unsigned
long)&bug_frames);
\
UNIQUE_BUG_INSTR_LABEL(line):
\
asm volatile ("nop");
} while (0)
extern char bug_frames[];
static struct bug_frame bug_var __attribute__((section("bug_frames")));
int main() {
BUG_FRAME(1, __LINE__, __FILE__, 1, "TEST");
printf("bug_loc: %p\n", bug_loc(&bug_var));
printf("bug_ptr: 0x%x\n", bug_ptr(&bug_var));
printf("__FILE__: %s\n", (char *)bug_ptr(&bug_var));
printf("bug_line: %d\n", bug_line(&bug_var));
printf("msg: %s\n", bug_msg(&bug_var));
BUG_FRAME(1, __LINE__, __FILE__, 1, "NEW TEST");
printf("bug_loc: %p\n", bug_loc(&bug_var));
printf("bug_ptr: 0x%x\n", bug_ptr(&bug_var));
printf("__FILE__: %s\n", (char *)bug_ptr(&bug_var));
printf("bug_line: %d\n", bug_line(&bug_var));
printf("msg: %s\n", bug_msg(&bug_var));
return 0;
}
Do you think it makes any sense? In this case, all BUG_FRAME can be re-
used among all architectures, and only bug instructions should be
changed.
>
> ~Andrew
~ Oleksii
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |