[Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Andrew Cooper]

Introduce a new ENDDATA() helper which sets type and size together.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/boot/x86_64.S   | 18 +++++++++---------
 xen/include/asm-x86/config.h |  5 +++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 5ab24d73fc..8a4cc7e747 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -65,7 +65,7 @@ l1_identmap:
         .endif
         pfn = pfn + 1
         .endr
-        .size l1_identmap, . - l1_identmap
+ENDDATA(l1_identmap)
 
 /*
  * __page_tables_start does not cover l1_identmap because it (l1_identmap)
@@ -86,7 +86,7 @@ GLOBAL(l2_identmap)
         idx = idx + 1
         .endr
         .fill 4 * L2_PAGETABLE_ENTRIES - 8, 8, 0
-        .size l2_identmap, . - l2_identmap
+ENDDATA(l2_identmap)
 
 /*
  * L2 mapping the 1GB Xen text/data/bss region.  At boot it maps 16MB from
@@ -101,7 +101,7 @@ GLOBAL(l2_xenmap)
         idx = idx + 1
         .endr
         .fill L2_PAGETABLE_ENTRIES - 8, 8, 0
-        .size l2_xenmap, . - l2_xenmap
+ENDDATA(l2_xenmap)
 
 /* L2 mapping the fixmap.  Uses 1x 4k page. */
 l2_fixmap:
@@ -114,7 +114,7 @@ l2_fixmap:
         .endif
         idx = idx + 1
         .endr
-        .size l2_fixmap, . - l2_fixmap
+ENDDATA(l2_fixmap)
 
 /* Identity map, covering the 4 l2_identmap tables.  Uses 1x 4k page. */
 l3_identmap:
@@ -124,7 +124,7 @@ l3_identmap:
         idx = idx + 1
         .endr
         .fill L3_PAGETABLE_ENTRIES - 4, 8, 0
-        .size l3_identmap, . - l3_identmap
+ENDDATA(l3_identmap)
 
 /* L3 mapping the fixmap.  Uses 1x 4k page. */
 l3_xenmap:
@@ -139,7 +139,7 @@ l3_xenmap:
         .endif
         idx = idx + 1
         .endr
-        .size l3_xenmap, . - l3_xenmap
+ENDDATA(l3_xenmap)
 
 /* Top-level master (and idle-domain) page directory. */
 GLOBAL(idle_pg_table)
@@ -155,7 +155,7 @@ GLOBAL(idle_pg_table)
         .endif
         idx = idx + 1
         .endr
-        .size idle_pg_table, . - idle_pg_table
+ENDDATA(idle_pg_table)
 
 GLOBAL(__page_tables_end)
 
@@ -165,8 +165,8 @@ GLOBAL(__page_tables_end)
 
 GLOBAL(l2_bootmap)
         .fill 4 * L2_PAGETABLE_ENTRIES, 8, 0
-        .size l2_bootmap, . - l2_bootmap
+ENDDATA(l2_bootmap)
 
 GLOBAL(l3_bootmap)
         .fill L3_PAGETABLE_ENTRIES, 8, 0
-        .size l3_bootmap, . - l3_bootmap
+ENDDATA(l3_bootmap)
diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
index 22dc795eea..35705441ff 100644
--- a/xen/include/asm-x86/config.h
+++ b/xen/include/asm-x86/config.h
@@ -56,6 +56,11 @@
 #define GLOBAL(name)                            \
   .globl name;                                  \
   name:
+
+#define ENDDATA(name)                           \
+    .type name, STT_OBJECT;                     \
+    .size name, . - name
+
 #endif
 
 #define NR_hypercalls 64
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Wei Liu]

On Wed, Aug 14, 2019 at 11:44:04AM +0100, Andrew Cooper wrote:
[...]
> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
> index 22dc795eea..35705441ff 100644
> --- a/xen/include/asm-x86/config.h
> +++ b/xen/include/asm-x86/config.h
> @@ -56,6 +56,11 @@
>  #define GLOBAL(name)                            \
>    .globl name;                                  \
>    name:
> +
> +#define ENDDATA(name)                           \
> +    .type name, STT_OBJECT;                     \

Isn't the correct syntax

    .type name STT_OBJECT;

?

The comma shouldn't be there according to as manual.

The rest looks good.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Andrew Cooper]

On 14/08/2019 13:00, Wei Liu wrote:
> On Wed, Aug 14, 2019 at 11:44:04AM +0100, Andrew Cooper wrote:
> [...]
>> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
>> index 22dc795eea..35705441ff 100644
>> --- a/xen/include/asm-x86/config.h
>> +++ b/xen/include/asm-x86/config.h
>> @@ -56,6 +56,11 @@
>>  #define GLOBAL(name)                            \
>>    .globl name;                                  \
>>    name:
>> +
>> +#define ENDDATA(name)                           \
>> +    .type name, STT_OBJECT;                     \
> Isn't the correct syntax
>
>     .type name STT_OBJECT;
>
> ?
>
> The comma shouldn't be there according to as manual.

Huh. I'd not even noticed that.  We use a comma with .type everywhere in
Xen, including when using STT_* notation rather than the @ variants.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Jan Beulich]

On 14.08.2019 14:00, Wei Liu wrote:
> On Wed, Aug 14, 2019 at 11:44:04AM +0100, Andrew Cooper wrote:
> [...]
>> diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h
>> index 22dc795eea..35705441ff 100644
>> --- a/xen/include/asm-x86/config.h
>> +++ b/xen/include/asm-x86/config.h
>> @@ -56,6 +56,11 @@
>>   #define GLOBAL(name)                            \
>>     .globl name;                                  \
>>     name:
>> +
>> +#define ENDDATA(name)                           \
>> +    .type name, STT_OBJECT;                     \
> 
> Isn't the correct syntax
> 
>      .type name STT_OBJECT;
> 
> ?
> 
> The comma shouldn't be there according to as manual.

Quote 1:

"ELF Version
  -----------

  For ELF targets, the '.type' directive is used like this:

      .type NAME , TYPE DESCRIPTION
"

The comma is definitely here, unconditionally. But yes, quote 2:

'   The syntaxes supported are:

         .type <name> STT_<TYPE_IN_UPPER_CASE>
         .type <name>,#<type>
         .type <name>,@<type>
         .type <name>,%<type>
         .type <name>,"<type>"'

Judging from the sources all forms treat the comma as optional.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Jan Beulich]

On 14.08.2019 12:44, Andrew Cooper wrote:
> Introduce a new ENDDATA() helper which sets type and size together.

Except this isn't very natural: Setting the size late is quite
common, to avoid the need for an "end" label. But the type would
better be set together with the label definition, i.e. by a
GLOBAL() counterpart (e.g. GLOBAL_DATA()). However, if despite
this remark you think your approach is the way to go:

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Andrew Cooper]

On 27/08/2019 15:36, Jan Beulich wrote:
> On 14.08.2019 12:44, Andrew Cooper wrote:
>> Introduce a new ENDDATA() helper which sets type and size together.
>
> Except this isn't very natural: Setting the size late is quite
> common, to avoid the need for an "end" label. But the type would
> better be set together with the label definition, i.e. by a
> GLOBAL() counterpart (e.g. GLOBAL_DATA()). However, if despite
> this remark you think your approach is the way to go:

Well - this way is fewer moving parts.

GLOBAL and ENTRY are to do with visibility and alignment.  While ENTRY
might not typically be used for data, both are commonly used for code.

We can either have a proliferation of {GLOBAL,ENTRY}{,_DATA,_FUNC,etc}
and a single END, or we can have ENDDATA/ENDFUNC and need no changes to
the existing GLOBAL/ENTRY infrastructure.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] x86/boot: Annotate pagetables with STT_OBJECT

[Jan Beulich]

On 28.08.2019 20:24, Andrew Cooper wrote:
> On 27/08/2019 15:36, Jan Beulich wrote:
>> On 14.08.2019 12:44, Andrew Cooper wrote:
>>> Introduce a new ENDDATA() helper which sets type and size together.
>>
>> Except this isn't very natural: Setting the size late is quite
>> common, to avoid the need for an "end" label. But the type would
>> better be set together with the label definition, i.e. by a
>> GLOBAL() counterpart (e.g. GLOBAL_DATA()). However, if despite
>> this remark you think your approach is the way to go:
> 
> Well - this way is fewer moving parts.
> 
> GLOBAL and ENTRY are to do with visibility and alignment.  While ENTRY
> might not typically be used for data, both are commonly used for code.
> 
> We can either have a proliferation of {GLOBAL,ENTRY}{,_DATA,_FUNC,etc}
> and a single END, or we can have ENDDATA/ENDFUNC and need no changes to
> the existing GLOBAL/ENTRY infrastructure.

Yes, it's slightly more of a change the way I'd prefer, but as said,
it would (imo) yield a more natural result. Omission of END() would
then mean only the symbol's size to not be set, but all other
attributes to be correct nevertheless.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel