[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
On 21.12.20 17:50, Julien Grall wrote:
Hi Jan,
On 15/12/2020 13:59, Jan Beulich wrote:
On 15.12.2020 14:39, Julien Grall wrote:
On 15/12/2020 09:02, Jan Beulich wrote:
On 15.12.2020 07:33, Juergen Gross wrote:
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -15,65 +15,62 @@
struct bug_frame {
signed int loc_disp; /* Relative address to the bug
address */
- signed int file_disp; /* Relative address to the filename */
+ signed int ptr_disp; /* Relative address to the filename or
function */
signed int msg_disp; /* Relative address to the predicate
(for ASSERT) */
uint16_t line; /* Line number */
uint32_t pad0:16; /* Padding for 8-bytes align */
};
#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
-#define bug_file(b) ((const void *)(b) + (b)->file_disp);
+#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp);
#define bug_line(b) ((b)->line)
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
-#define BUGFRAME_warn 0
-#define BUGFRAME_bug 1
-#define BUGFRAME_assert 2
+#define BUGFRAME_run_fn 0
+#define BUGFRAME_warn 1
+#define BUGFRAME_bug 2
+#define BUGFRAME_assert 3
-#define BUGFRAME_NR 3
+#define BUGFRAME_NR 4
/* Many versions of GCC doesn't support the asm %c parameter
which would
* be preferable to this unpleasantness. We use mergeable string
* sections to avoid multiple copies of the string appearing in the
* Xen image.
*/
-#define BUG_FRAME(type, line, file, has_msg, msg) do
{ \
+#define BUG_FRAME(type, line, ptr, msg) do
{ \
BUILD_BUG_ON((line) >>
16); \
BUILD_BUG_ON((type) >=
BUGFRAME_NR); \
asm
("1:"BUG_INSTR"\n" \
- ".pushsection .rodata.str, \"aMS\", %progbits,
1\n" \
- "2:\t.asciz " __stringify(file)
"\n" \
-
"3:\n" \
- ".if " #has_msg
"\n" \
- "\t.asciz " #msg
"\n" \
-
".endif\n" \
-
".popsection\n" \
- ".pushsection .bug_frames." __stringify(type) ", \"a\",
%progbits\n"\
-
"4:\n" \
+ ".pushsection .bug_frames." __stringify(type) ", \"a\",
%%progbits\n"\
+
"2:\n" \
".p2align
2\n" \
- ".long (1b -
4b)\n" \
- ".long (2b -
4b)\n" \
- ".long (3b -
4b)\n" \
+ ".long (1b -
2b)\n" \
+ ".long (%0 -
2b)\n" \
+ ".long (%1 -
2b)\n" \
".hword " __stringify(line) ",
0\n" \
-
".popsection"); \
+ ".popsection" :: "i" (ptr), "i"
(msg)); \
} while (0)
The comment ahead of the construct now looks to be at best stale, if
not entirely pointless. The reference to %c looks quite strange here
to me anyway - I can only guess it appeared here because on x86 one
has to use %c to output constants as operands for .long and alike,
and this was then tried to use on Arm as well without there really
being a need.
Well, %c is one the reason why we can't have a common BUG_FRAME
implementation. So I would like to retain this information before
someone wants to consolidate the code and missed this issue.
Fair enough, albeit I guess this then could do with re-wording.
I agree.
Regarding the mergeable string section, I agree that the comment in now
stale. However, could someone confirm that "i" will still retain the
same behavior as using mergeable string sections?
That's depend on compiler settings / behavior.
Ok. I wanted to see the difference between before and after but it looks
like I can't compile Xen after applying the patch:
In file included from
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:23:0,
from
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/bitmap.h:6,
from bitmap.c:10:
bitmap.c: In function ‘bitmap_allocate_region’:
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: asm operand 0 probably doesn’t match constraints [-Werror]
asm ("1:"BUG_INSTR"\n" \
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: asm operand 1 probably doesn’t match constraints [-Werror]
asm ("1:"BUG_INSTR"\n" \
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: impossible constraint in ‘asm’
asm ("1:"BUG_INSTR"\n" \
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
cc1: all warnings being treated as errors
I am using GCC 7.5.0 built by Linaro (cross-compiler). Native
compilation with GCC 10.2.1 leads to the same error.
@Juergen, which compiler did you use?
gcc 7.4.0 aarch64 cross-compiler (SUSE)
Juergen
Attachment:
OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
|