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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/11/2026 8:51 am, Oleksii Kurochko wrote:
>
>
> On 11/12/26 7:58 PM, Andrew Cooper wrote:
>> 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)
>> +
>>          *(.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)
>
> The ordering is preserved, but the alignment between the two isn't.
> Today the ALIGN() before .data sits between .data.read_mostly and
> regular .data.  After this patch it sits before .data.read_mostly, and
> the only thing separating it from *(.sdata/.data) is
> *(.data.page_aligned), which has no inputs on RISC-V, PPC or Arm with
> CONFIG_MPU. So the tail of .data.read_mostly will share a cache line
> with ordinary .data, which is the false sharing __read_mostly is meant
> to avoid (IIUC).

__read_mostly is indeed meant to reduce false sharing, but this patch
does not introduce any.

HYPFS_PARAM should have been read_mostly to begin with, and page
alignment is greater th cache alignment so there's no sharing there either.

~Andrew