x86/EFI: warn about r/o sections requiring relocations

Commit Message

Jan Beulich Aug. 25, 2017, 8:07 a.m. UTC

EFI implementations may write-protect r/o sections, but we need to
apply relocations. Eliminate the one present case of a r/o section
with relocations (.init.text, which is now being combined with
.init.data into just .init).

Also correct a few other format strings (to account for the possibly
missing NUL in section names).

Signed-off-by: Jan Beulich <[e-mail]>

Comments

Andrew Cooper Aug. 25, 2017, 8:50 a.m. UTC | #1

On 25/08/2017 09:07, Jan Beulich wrote:
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -155,7 +155,11 @@ SECTIONS
>    __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
>    . = ALIGN(PAGE_SIZE);             /* Init code and data */
>    __init_begin = .;
> +#ifndef EFI
>    .init.text : {
> +#else
> +  .init : {
> +#endif

Why special case these for EFI?  Everything else in the boot path treats
init as one single blob.

I don't see an issue with merging them in the non-EFI build as well,
which would avoid this #ifdefary

~Andrew

Jan Beulich Aug. 25, 2017, 9:41 a.m. UTC | #2

>>> On 25.08.17 at 10:50, <[e-mail]> wrote:
> On 25/08/2017 09:07, Jan Beulich wrote:
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -155,7 +155,11 @@ SECTIONS
>>    __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
>>    . = ALIGN(PAGE_SIZE);             /* Init code and data */
>>    __init_begin = .;
>> +#ifndef EFI
>>    .init.text : {
>> +#else
>> +  .init : {
>> +#endif
> 
> Why special case these for EFI?  Everything else in the boot path treats
> init as one single blob.
> 
> I don't see an issue with merging them in the non-EFI build as well,
> which would avoid this #ifdefary

I could certainly do that - I merely wanted to affect the non-EFI
case as little as possible.

Jan

Patch

--- a/xen/arch/x86/efi/mkreloc.c
+++ b/xen/arch/x86/efi/mkreloc.c
@@ -66,8 +66,9 @@  struct coff_section {
     uint16_t relocation_count;
     uint16_t line_number_count;
     uint32_t flags;
-#define COFF_SECTION_BSS         0x00000080
-#define COFF_SECTION_DISCARDABLE 0x02000000
+#define COFF_SECTION_BSS         0x00000080U
+#define COFF_SECTION_DISCARDABLE 0x02000000U
+#define COFF_SECTION_WRITEABLE   0x80000000U
 };
 
 static void usage(const char *cmd, int rc)
@@ -224,7 +225,7 @@  static void diff_sections(const unsigned
         if ( i < disp || i + width - disp > sec->file_size )
         {
             fprintf(stderr,
-                    "Bogus difference at %s:%08" PRIxFAST32 "\n",
+                    "Bogus difference at %.8s:%08" PRIxFAST32 "\n",
                     sec->name, i);
             exit(3);
         }
@@ -235,7 +236,7 @@  static void diff_sections(const unsigned
         if ( delta != diff )
         {
             fprintf(stderr,
-                    "Difference at %s:%08" PRIxFAST32 " is %#" PRIxFAST64
+                    "Difference at %.8s:%08" PRIxFAST32 " is %#" PRIxFAST64
                     " (expected %#" PRIxFAST64 ")\n",
                     sec->name, i, delta, diff);
             continue;
@@ -261,11 +262,16 @@  static void diff_sections(const unsigned
         else if ( rva != cur_rva )
         {
             fprintf(stderr,
-                    "Cannot handle decreasing RVA (at %s:%08" PRIxFAST32 ")\n",
+                    "Cannot handle decreasing RVA (at %.8s:%08" PRIxFAST32 ")\n",
                     sec->name, i);
             exit(3);
         }
 
+        if ( !(sec->flags & COFF_SECTION_WRITEABLE) )
+            fprintf(stderr,
+                    "Warning: relocation to r/o section %.8s:%08" PRIxFAST32 "\n",
+                    sec->name, i);
+
         printf("\t.word (%u << 12) | 0x%03" PRIxFAST32 "\n",
                reloc, sec->rva + i - disp - rva);
         reloc_size += 2;
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -155,7 +155,11 @@  SECTIONS
   __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
+#ifndef EFI
   .init.text : {
+#else
+  .init : {
+#endif
        _sinittext = .;
        *(.init.text)
        /*
@@ -165,8 +169,12 @@  SECTIONS
         */
        *(.altinstr_replacement)
        _einittext = .;
+#ifndef EFI
   } :text
   .init.data : {
+#else
+       . = ALIGN(SMP_CACHE_BYTES);
+#endif
        *(.init.rodata)
        *(.init.rodata.rel)
        *(.init.rodata.str*)