Re: [PATCH] xen/link: Don't separate .data.read_mostly from .data

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi Andrew,

Andrew Cooper <andrew.cooper3@xxxxxxxxxx> writes:

> This appears to be an error in x86 which got copied into all other
> architectures.  .data.read_mostly is an internal detail, and none of the
> similar sections get exposed externally.
>
> Simply place .data.read_mostly in .data.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
> CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
> CC: Timothy Pearson <tpearson@xxxxxxxxxxxxxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>
> This is to simplfy the script checking that xen.efi's section
> attributes/alignments are correct for UEFI SecureBoot.
> ---
>  xen/arch/arm/xen.lds.S   | 8 +++-----
>  xen/arch/ppc/xen.lds.S   | 8 +++-----
>  xen/arch/riscv/xen.lds.S | 7 ++-----
>  xen/arch/x86/xen.lds.S   | 4 +---
>  4 files changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index d1bb6c62d208..8d0954c90afd 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -86,16 +86,14 @@ SECTIONS
>        __ro_after_init_end = .;
>    } : text
>  
> -  .data.read_mostly : {
> -       *(.data.read_mostly)
> -  } :text
> -
>    . = ALIGN(SMP_CACHE_BYTES);
>    .data : {                    /* Data */
> -       *(.data.page_aligned)
> +       *(.data.read_mostly)
>  
>         HYPFS_PARAM
>  
> +       *(.data.page_aligned)

A small question: why are you moving data.page_aligned? I don't think
that this can cause issues, but I want to understand the reason.

> +
>         *(.data .data.*)
>         CONSTRUCTORS
>    } :text
> diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
> index cddb70855307..b2159279169c 100644
> --- a/xen/arch/ppc/xen.lds.S
> +++ b/xen/arch/ppc/xen.lds.S
> @@ -77,16 +77,14 @@ SECTIONS
>          __ro_after_init_end = .;
>      } :text
>  
> -    DECL_SECTION(.data.read_mostly) {
> -        *(.data.read_mostly)
> -    } :text
> -
>      . = ALIGN(PAGE_SIZE);
>      DECL_SECTION(.data) {                    /* Data */
> -        *(.data.page_aligned)
> +        *(.data.read_mostly)
>  
>          HYPFS_PARAM
>  
> +        *(.data.page_aligned)
> +
>          *(.data .data.*)
>          CONSTRUCTORS
>      } :text
> diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
> index 250ee3f36f4f..d1c5ce59c90d 100644
> --- a/xen/arch/riscv/xen.lds.S
> +++ b/xen/arch/riscv/xen.lds.S
> @@ -84,16 +84,13 @@ SECTIONS
>          __ro_after_init_end = .;
>      } : text
>  
> -    .data.read_mostly : {
> -        *(.data.read_mostly)
> -    } :text
> -
>      . = ALIGN(PAGE_SIZE);
>      .data : {                    /* Data */
> -        *(.data.page_aligned)
> +        *(.data.read_mostly)
>  
>          HYPFS_PARAM
>  
> +        *(.data.page_aligned)
>          *(.sdata .sdata.*)
>          *(.data .data.*)
>          CONSTRUCTORS
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 925848980219..99802aed4e13 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -295,13 +295,11 @@ SECTIONS
>  
>    __2M_rwdata_start = .;       /* Start of 2M superpages, mapped RW. */
>    . = ALIGN(SMP_CACHE_BYTES);
> -  DECL_SECTION(.data.read_mostly) {
> +  DECL_SECTION(.data) {
>         *(.data.read_mostly)
>  
>         HYPFS_PARAM
> -  } PHDR(text)
>  
> -  DECL_SECTION(.data) {
>         *(.data.page_aligned)
>         *(.data .data.*)
>    } PHDR(text)
>
> base-commit: 4263fe42b736596cfa5bfec661f2f55f02759aa7
> prerequisite-patch-id: 680071148129321ffaf7bd7435d031904014f556

-- 
WBR, Volodymyr