Re: [Xen-devel] [PATCH] x86/link: Don't merge .init.text and .init.data

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Jan 11, 2018 at 02:17:57PM +0000, Andrew Cooper wrote:
> c/s 1308f0170c merged .init.text and .init.data, because EFI might properly
> write-protect r/o sections.
> 
> However, this change makes xen-syms unusable for disassembly analysis.  In

s/this makes/that made/
> particular, searching for indirect branches as part of the SP2/Spectre
> mitigation series.
> 
> Revert the relevent bits of 1308f0170c and instead modify the EFI relocation
> code to disable CR0.WP, which is how we deal with relocations in r/o mappings
> elsewhere.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

with that change above:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> ---
>  xen/arch/x86/efi/efi-boot.h | 12 ++++++++++++
>  xen/arch/x86/efi/mkreloc.c  |  5 -----
>  xen/arch/x86/xen.lds.S      |  7 +++----
>  3 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index d30f688..0b5f218 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -47,8 +47,17 @@ extern const struct pe_base_relocs {
>  
>  static void __init efi_arch_relocate_image(unsigned long delta)
>  {
> +    unsigned long cr0;
>      const struct pe_base_relocs *base_relocs;
>  
> +    /*
> +     * Conditionally disable CR0.WP in case there are relocations present in
> +     * read-only mappings.
> +     */
> +    cr0 = read_cr0();
> +    if ( cr0 & X86_CR0_WP )
> +        write_cr0(cr0 & ~X86_CR0_WP);
> +
>      for ( base_relocs = __base_relocs_start; base_relocs < 
> __base_relocs_end; )
>      {
>          unsigned int i = 0, n;
> @@ -96,6 +105,9 @@ static void __init efi_arch_relocate_image(unsigned long 
> delta)
>          }
>          base_relocs = (const void *)(base_relocs->entries + i + (i & 1));
>      }
> +
> +    if ( cr0 & X86_CR0_WP )
> +        write_cr0(cr0);
>  }
>  
>  extern const s32 __trampoline_rel_start[], __trampoline_rel_stop[];
> diff --git a/xen/arch/x86/efi/mkreloc.c b/xen/arch/x86/efi/mkreloc.c
> index 1aca796..509fd83 100644
> --- a/xen/arch/x86/efi/mkreloc.c
> +++ b/xen/arch/x86/efi/mkreloc.c
> @@ -267,11 +267,6 @@ static void diff_sections(const unsigned char *ptr1, 
> const unsigned char *ptr2,
>              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;
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index d5e8821..6a7bbb8 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -159,7 +159,7 @@ SECTIONS
>    __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot 
> only). */
>    . = ALIGN(PAGE_SIZE);             /* Init code and data */
>    __init_begin = .;
> -  .init : {
> +  .init.text : {
>         _sinittext = .;
>         *(.init.text)
>         /*
> @@ -169,9 +169,8 @@ SECTIONS
>          */
>         *(.altinstr_replacement)
>         _einittext = .;
> -
> -       . = ALIGN(SMP_CACHE_BYTES);
> -
> +  } :text
> +  .init.data : {
>         *(.init.rodata)
>         *(.init.rodata.rel)
>         *(.init.rodata.str*)
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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