diff mbox series

[2/4] xen: Port linkage.h from kernel code

Message ID 20220804150424.17584-3-jane.malalane@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen/x86: import linkage.h and clean up x86/kexec.S and x86/entry.S | expand

Commit Message

Jane Malalane Aug. 4, 2022, 3:04 p.m. UTC 
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
---
 xen/include/xen/linkage.h | 260 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 260 insertions(+)
 create mode 100644 xen/include/xen/linkage.h

Comments

Jan Beulich Aug. 5, 2022, 9:24 a.m. UTC | #1 
On 04.08.2022 17:04, Jane Malalane wrote:
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>

In the title you say "port", but then you don't say what customization
you've done beyond the obvious adjustment of inclusion guard and
adjustment (actually removal) of #include-s. How much customization we
want to do is important here, after all. I notice you did, for example,
add new flavors of SYM_INNER_LABEL, but then you didn't add anything to
use .hidden (which I have queued as a new patch on top of a supposed v2
of "x86: annotate entry points with type and size"). At the same time
you've left in objtool related commentary, when we don't use that tool
(and no-one knows if we ever will).

I'm further not sure I agree with the naming of some of your additions,
as they appear to not really fit with the naming model used elsewhere.
Your additions also look to not always match style used elsewhere in
this file.

You further want to mention what Linux version this was derived from,
to make it easier to determine what incremental changes to port over
subsequently.

Overall I'm not convinced this is a model we want to go with, first
and foremost because the commonly used macro names are too long for
my taste. What's wrong with ENTRY() and ENDPROC() / ENDDATA()?

Jan
Jane Malalane Aug. 16, 2022, 10:16 a.m. UTC | #2 
On 05/08/2022 10:24, Jan Beulich wrote:
> On 04.08.2022 17:04, Jane Malalane wrote:
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
> 
> In the title you say "port", but then you don't say what customization
> you've done beyond the obvious adjustment of inclusion guard and
> adjustment (actually removal) of #include-s. How much customization we
> want to do is important here, after all. I notice you did, for example,
> add new flavors of SYM_INNER_LABEL, but then you didn't add anything to
> use .hidden (which I have queued as a new patch on top of a supposed v2
> of "x86: annotate entry points with type and size"). At the same time
> you've left in objtool related commentary, when we don't use that tool
> (and no-one knows if we ever will).
> 
> I'm further not sure I agree with the naming of some of your additions,
> as they appear to not really fit with the naming model used elsewhere.
> Your additions also look to not always match style used elsewhere in
> this file.
> 
> You further want to mention what Linux version this was derived from,
> to make it easier to determine what incremental changes to port over
> subsequently.
> 
> Overall I'm not convinced this is a model we want to go with, first
> and foremost because the commonly used macro names are too long for
> my taste. What's wrong with ENTRY() and ENDPROC() / ENDDATA()?
Hi Jan,

Since I have no particular opinion on this I went through the discussion 
that took place before those macros were introduced in Linux. One of the 
points made was that PROC was an ambiguous term to use, since C 
functions are not actually procedures, at least not all. The other was 
that using START/END markers make it easier for a developer to remember 
to add the END marker, and eventhough you suggest using ENTRY and not 
just PROC as the start marker, START might still be clearer.

I welcome other input.

Thank you for your review,

Jane.
Jan Beulich Aug. 16, 2022, 1:06 p.m. UTC | #3 
On 16.08.2022 12:16, Jane Malalane wrote:
> On 05/08/2022 10:24, Jan Beulich wrote:
>> On 04.08.2022 17:04, Jane Malalane wrote:
>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>
>> In the title you say "port", but then you don't say what customization
>> you've done beyond the obvious adjustment of inclusion guard and
>> adjustment (actually removal) of #include-s. How much customization we
>> want to do is important here, after all. I notice you did, for example,
>> add new flavors of SYM_INNER_LABEL, but then you didn't add anything to
>> use .hidden (which I have queued as a new patch on top of a supposed v2
>> of "x86: annotate entry points with type and size"). At the same time
>> you've left in objtool related commentary, when we don't use that tool
>> (and no-one knows if we ever will).
>>
>> I'm further not sure I agree with the naming of some of your additions,
>> as they appear to not really fit with the naming model used elsewhere.
>> Your additions also look to not always match style used elsewhere in
>> this file.
>>
>> You further want to mention what Linux version this was derived from,
>> to make it easier to determine what incremental changes to port over
>> subsequently.
>>
>> Overall I'm not convinced this is a model we want to go with, first
>> and foremost because the commonly used macro names are too long for
>> my taste. What's wrong with ENTRY() and ENDPROC() / ENDDATA()?
> Hi Jan,
> 
> Since I have no particular opinion on this I went through the discussion 
> that took place before those macros were introduced in Linux. One of the 
> points made was that PROC was an ambiguous term to use, since C 
> functions are not actually procedures, at least not all.

Just one remark here: We're talking about assembly code here, so what's
a procedure or function isn't well defined anyway. I wouldn't, btw, mind
ENDFUNC() if that's deemed better than ENDPROC().

Jan

> The other was 
> that using START/END markers make it easier for a developer to remember 
> to add the END marker, and eventhough you suggest using ENTRY and not 
> just PROC as the start marker, START might still be clearer.
> 
> I welcome other input.
> 
> Thank you for your review,
> 
> Jane.
Jane Malalane Aug. 17, 2022, 8:56 a.m. UTC | #4 
On 16/08/2022 14:06, Jan Beulich wrote:
> On 16.08.2022 12:16, Jane Malalane wrote:
>> On 05/08/2022 10:24, Jan Beulich wrote:
>>> On 04.08.2022 17:04, Jane Malalane wrote:
>>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>>
>>> In the title you say "port", but then you don't say what customization
>>> you've done beyond the obvious adjustment of inclusion guard and
>>> adjustment (actually removal) of #include-s. How much customization we
>>> want to do is important here, after all. I notice you did, for example,
>>> add new flavors of SYM_INNER_LABEL, but then you didn't add anything to
>>> use .hidden (which I have queued as a new patch on top of a supposed v2
>>> of "x86: annotate entry points with type and size"). At the same time
>>> you've left in objtool related commentary, when we don't use that tool
>>> (and no-one knows if we ever will).
>>>
>>> I'm further not sure I agree with the naming of some of your additions,
>>> as they appear to not really fit with the naming model used elsewhere.
>>> Your additions also look to not always match style used elsewhere in
>>> this file.
>>>
>>> You further want to mention what Linux version this was derived from,
>>> to make it easier to determine what incremental changes to port over
>>> subsequently.
>>>
>>> Overall I'm not convinced this is a model we want to go with, first
>>> and foremost because the commonly used macro names are too long for
>>> my taste. What's wrong with ENTRY() and ENDPROC() / ENDDATA()?
>> Hi Jan,
>>
>> Since I have no particular opinion on this I went through the discussion
>> that took place before those macros were introduced in Linux. One of the
>> points made was that PROC was an ambiguous term to use, since C
>> functions are not actually procedures, at least not all.
> 
> Just one remark here: We're talking about assembly code here, so what's
> a procedure or function isn't well defined anyway. I wouldn't, btw, mind
> ENDFUNC() if that's deemed better than ENDPROC().
Do you then propose that we use ENTRY() and ENDFUNC() and that for inner 
labels we keep them as is (so just "name:"), since using ENTRY() without 
a closing END() for some "functions" and not for others could get a bit 
confusing?

Jane.
Jan Beulich Aug. 17, 2022, 10:24 a.m. UTC | #5 
On 17.08.2022 10:56, Jane Malalane wrote:
> On 16/08/2022 14:06, Jan Beulich wrote:
>> On 16.08.2022 12:16, Jane Malalane wrote:
>>> On 05/08/2022 10:24, Jan Beulich wrote:
>>>> On 04.08.2022 17:04, Jane Malalane wrote:
>>>>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> Signed-off-by: Jane Malalane <jane.malalane@citrix.com>
>>>>
>>>> In the title you say "port", but then you don't say what customization
>>>> you've done beyond the obvious adjustment of inclusion guard and
>>>> adjustment (actually removal) of #include-s. How much customization we
>>>> want to do is important here, after all. I notice you did, for example,
>>>> add new flavors of SYM_INNER_LABEL, but then you didn't add anything to
>>>> use .hidden (which I have queued as a new patch on top of a supposed v2
>>>> of "x86: annotate entry points with type and size"). At the same time
>>>> you've left in objtool related commentary, when we don't use that tool
>>>> (and no-one knows if we ever will).
>>>>
>>>> I'm further not sure I agree with the naming of some of your additions,
>>>> as they appear to not really fit with the naming model used elsewhere.
>>>> Your additions also look to not always match style used elsewhere in
>>>> this file.
>>>>
>>>> You further want to mention what Linux version this was derived from,
>>>> to make it easier to determine what incremental changes to port over
>>>> subsequently.
>>>>
>>>> Overall I'm not convinced this is a model we want to go with, first
>>>> and foremost because the commonly used macro names are too long for
>>>> my taste. What's wrong with ENTRY() and ENDPROC() / ENDDATA()?
>>> Hi Jan,
>>>
>>> Since I have no particular opinion on this I went through the discussion
>>> that took place before those macros were introduced in Linux. One of the
>>> points made was that PROC was an ambiguous term to use, since C
>>> functions are not actually procedures, at least not all.
>>
>> Just one remark here: We're talking about assembly code here, so what's
>> a procedure or function isn't well defined anyway. I wouldn't, btw, mind
>> ENDFUNC() if that's deemed better than ENDPROC().
> Do you then propose that we use ENTRY() and ENDFUNC() and that for inner 
> labels we keep them as is (so just "name:"), since using ENTRY() without 
> a closing END() for some "functions" and not for others could get a bit 
> confusing?

Almost - I don't see anything wrong with using ENTRY() without any END*()
for inner labels, if the implied alignment is wanted. If no alignment nor
type / size are wanted, we have GLOBAL(). And recall that I already did
post a patch adding various ENDPROC() (which could be converted to
ENDFUNC() if that name is indeed liked better, and which could easily
have ENDDATA() or some such added), see
https://lists.xen.org/archives/html/xen-devel/2022-04/msg00876.html .
With Andrew's odd reply I didn't see fit to post a v2 so far, where I'm
now having a further patch adding .hidden directives into GLOBAL() and
(indirectly) ENTRY(). Not the least because my reply (where I did already
indicate that Linux'es machinery looks a little too involved to me)
didn't have any further responses, which would at least have helped
clarify what - if anything - I had "rejected" long ago.

Jan
diff mbox series

Patch

diff --git a/xen/include/xen/linkage.h b/xen/include/xen/linkage.h
new file mode 100644
index 0000000000..adc00c356b
--- /dev/null
+++ b/xen/include/xen/linkage.h
@@ -0,0 +1,260 @@ 
+#ifndef __XEN_LINKAGE_H
+#define __XEN_LINKAGE_H
+
+/*
+ * Imported from linux-5.19:include/linux/linkage.h
+ */
+
+/* Some toolchains use other characters (e.g. '`') to mark new line in macro */
+#ifndef ASM_NL
+#define ASM_NL		 ;
+#endif
+
+#ifdef __ASSEMBLY__
+
+/* SYM_T_FUNC -- type used by assembler to mark functions */
+#ifndef SYM_T_FUNC
+#define SYM_T_FUNC				STT_FUNC
+#endif
+
+/* SYM_T_OBJECT -- type used by assembler to mark data */
+#ifndef SYM_T_OBJECT
+#define SYM_T_OBJECT				STT_OBJECT
+#endif
+
+/* SYM_T_NONE -- type used by assembler to mark entries of unknown type */
+#ifndef SYM_T_NONE
+#define SYM_T_NONE				STT_NOTYPE
+#endif
+
+/* SYM_A_* -- align the symbol? */
+#define SYM_A_ALIGN				ALIGN
+#define SYM_A_NONE				/* nothing */
+
+/* SYM_L_* -- linkage of symbols */
+#define SYM_L_GLOBAL(name)			.globl name
+#define SYM_L_WEAK(name)			.weak name
+#define SYM_L_LOCAL(name)			/* nothing */
+
+/* === generic annotations === */
+
+/* SYM_ENTRY -- use only if you have to for non-paired symbols */
+#ifndef SYM_ENTRY
+#define SYM_ENTRY(name, linkage, align...)		\
+	linkage(name) ASM_NL				\
+	align ASM_NL					\
+	name:
+#endif
+
+/* SYM_START -- use only if you have to */
+#ifndef SYM_START
+#define SYM_START(name, linkage, align...)		\
+	SYM_ENTRY(name, linkage, align)
+#endif
+
+/* SYM_END -- use only if you have to */
+#ifndef SYM_END
+#define SYM_END(name, sym_type)				\
+	.type name sym_type ASM_NL			\
+	.set .L__sym_size_##name, .-name ASM_NL		\
+	.size name, .L__sym_size_##name
+#endif
+
+/* SYM_ALIAS -- use only if you have to */
+#ifndef SYM_ALIAS
+#define SYM_ALIAS(alias, name, linkage)			\
+	linkage(alias) ASM_NL				\
+	.set alias, name ASM_NL
+#endif
+
+/* === code annotations === */
+
+/*
+ * FUNC -- C-like functions (proper stack frame etc.)
+ * CODE -- non-C code (e.g. irq handlers with different, special stack etc.)
+ *
+ * Objtool validates stack for FUNC, but not for CODE.
+ * Objtool generates debug info for both FUNC & CODE, but needs special
+ * annotations for each CODE's start (to describe the actual stack frame).
+ *
+ * Objtool requires that all code must be contained in an ELF symbol. Symbol
+ * names that have a  .L prefix do not emit symbol table entries. .L
+ * prefixed symbols can be used within a code region, but should be avoided for
+ * denoting a range of code via ``SYM_*_START/END`` annotations.
+ *
+ * ALIAS -- does not generate debug info -- the aliased function will
+ */
+
+/* SYM_INNER_LABEL_ALIGN -- only for labels in the middle of code,
+ * w/ alignment
+ */
+#ifndef SYM_INNER_LABEL_ALIGN
+#define SYM_INNER_LABEL_ALIGN(name, linkage)	\
+	.type name SYM_T_NONE ASM_NL			\
+	SYM_ENTRY(name, linkage, SYM_A_ALIGN)
+#endif
+
+/* SYM_INNER_LABEL_LOCAL -- only for local labels in the middle of code */
+#ifndef SYM_INNER_LABEL_LOCAL
+#define SYM_INNER_LABEL_LOCAL(name)			\
+	.type name SYM_T_NONE ASM_NL			\
+	SYM_ENTRY(name, SYM_L_LOCAL, SYM_A_NONE)
+#endif
+
+/* SYM_INNER_LABEL_GLOBAL -- only for global labels in the middle of code */
+#ifndef SYM_INNER_LABEL_GLOBAL
+#define SYM_INNER_LABEL_GLOBAL(name)				    \
+            .type name SYM_T_NONE ASM_NL                    \
+	    SYM_ENTRY(name, SYM_L_GLOBAL, SYM_A_NONE)
+#endif
+
+/* SYM_FUNC_START -- use for global functions */
+#ifndef SYM_FUNC_START
+#define SYM_FUNC_START(name)				\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)
+#endif
+
+/* SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment */
+#ifndef SYM_FUNC_START_NOALIGN
+#define SYM_FUNC_START_NOALIGN(name)			\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE)
+#endif
+
+/* SYM_FUNC_START_LOCAL -- use for local functions */
+#ifndef SYM_FUNC_START_LOCAL
+#define SYM_FUNC_START_LOCAL(name)			\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_ALIGN)
+#endif
+
+/* SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o alignment */
+#ifndef SYM_FUNC_START_LOCAL_NOALIGN
+#define SYM_FUNC_START_LOCAL_NOALIGN(name)		\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_NONE)
+#endif
+
+/* SYM_FUNC_START_WEAK -- use for weak functions */
+#ifndef SYM_FUNC_START_WEAK
+#define SYM_FUNC_START_WEAK(name)			\
+	SYM_START(name, SYM_L_WEAK, SYM_A_ALIGN)
+#endif
+
+/* SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment */
+#ifndef SYM_FUNC_START_WEAK_NOALIGN
+#define SYM_FUNC_START_WEAK_NOALIGN(name)		\
+	SYM_START(name, SYM_L_WEAK, SYM_A_NONE)
+#endif
+
+/*
+ * SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
+ * SYM_FUNC_START_WEAK, ...
+ */
+#ifndef SYM_FUNC_END
+#define SYM_FUNC_END(name)				\
+	SYM_END(name, SYM_T_FUNC)
+#endif
+
+/*
+ * SYM_FUNC_ALIAS -- define a global alias for an existing function
+ */
+#ifndef SYM_FUNC_ALIAS
+#define SYM_FUNC_ALIAS(alias, name)					\
+	SYM_ALIAS(alias, name, SYM_L_GLOBAL)
+#endif
+
+/*
+ * SYM_FUNC_ALIAS_LOCAL -- define a local alias for an existing function
+ */
+#ifndef SYM_FUNC_ALIAS_LOCAL
+#define SYM_FUNC_ALIAS_LOCAL(alias, name)				\
+	SYM_ALIAS(alias, name, SYM_L_LOCAL)
+#endif
+
+/*
+ * SYM_FUNC_ALIAS_WEAK -- define a weak global alias for an existing function
+ */
+#ifndef SYM_FUNC_ALIAS_WEAK
+#define SYM_FUNC_ALIAS_WEAK(alias, name)				\
+	SYM_ALIAS(alias, name, SYM_L_WEAK)
+#endif
+
+/* SYM_CODE_START -- use for non-C (special) functions */
+#ifndef SYM_CODE_START
+#define SYM_CODE_START(name)				\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)
+#endif
+
+/* SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o alignment */
+#ifndef SYM_CODE_START_NOALIGN
+#define SYM_CODE_START_NOALIGN(name)			\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE)
+#endif
+
+/* SYM_CODE_START_LOCAL -- use for local non-C (special) functions */
+#ifndef SYM_CODE_START_LOCAL
+#define SYM_CODE_START_LOCAL(name)			\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_ALIGN)
+#endif
+
+/*
+ * SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special) functions,
+ * w/o alignment
+ */
+#ifndef SYM_CODE_START_LOCAL_NOALIGN
+#define SYM_CODE_START_LOCAL_NOALIGN(name)		\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_NONE)
+#endif
+
+/* SYM_CODE_END -- the end of SYM_CODE_START_LOCAL, SYM_CODE_START, ... */
+#ifndef SYM_CODE_END
+#define SYM_CODE_END(name)				\
+	SYM_END(name, SYM_T_NONE)
+#endif
+
+/* === data annotations === */
+
+/* SYM_DATA_START -- global data symbol */
+#ifndef SYM_DATA_START
+#define SYM_DATA_START(name)				\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE)
+#endif
+
+/* SYM_DATA_START -- local data symbol */
+#ifndef SYM_DATA_START_LOCAL
+#define SYM_DATA_START_LOCAL(name)			\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_NONE)
+#endif
+
+/* SYM_DATA_END -- the end of SYM_DATA_START symbol */
+#ifndef SYM_DATA_END
+#define SYM_DATA_END(name)				\
+	SYM_END(name, SYM_T_OBJECT)
+#endif
+
+/* SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol */
+#ifndef SYM_DATA_END_LABEL
+#define SYM_DATA_END_LABEL(name, linkage, label)	\
+	linkage(label) ASM_NL				\
+	.type label SYM_T_OBJECT ASM_NL			\
+	label:						\
+	SYM_END(name, SYM_T_OBJECT)
+#endif
+
+/* SYM_DATA -- start+end wrapper around simple global data */
+#ifndef SYM_DATA
+#define SYM_DATA(name, data...)				\
+	SYM_DATA_START(name) ASM_NL				\
+	data ASM_NL						\
+	SYM_DATA_END(name)
+#endif
+
+/* SYM_DATA_LOCAL -- start+end wrapper around simple local data */
+#ifndef SYM_DATA_LOCAL
+#define SYM_DATA_LOCAL(name, data...)			\
+	SYM_DATA_START_LOCAL(name) ASM_NL			\
+	data ASM_NL						\
+	SYM_DATA_END(name)
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __XEN_LINKAGE_H */