Re: [PATCH v2 7/7] RISC-V: place .sdata / .srodata / .riscv.attributes

On 8/27/26 5:56 PM, Jan Beulich wrote:
> On 27.08.2026 17:40, Oleksii Kurochko wrote:
>> On 8/26/26 2:04 PM, Jan Beulich wrote:
>>> Of the short-data sections, only .sbss is presently mentioned in the
>>> linker script. Place them next to, but ahead of their "normal" data
>>> sections.
>>>
>>> .riscv.attributes can go towards the tail of the image, next to (ahead of)
>>> debug info.
>>>
>>> Signed-off-by: Jan Beulich <[email protected]>
>>> ---
>>> Seeing where .sbss lives, does positioning really not matter at all? I
>>> would have expected that short-data sections want to live close together,
>>> and specifically close to .text / .init.text (seeing that such data is
>>> accessed using AUIPC). I'm puzzled that the psABI doesn't even mention
>>> them, hence leaving it open how exactly they are to be used.
>>
>> It doesn't, and the reason is that the relevant proximity isn't to .text
>> but to __global_pointer$.
> 
> Anything like this still should be set forth by the psABI, so I don't
> quite understand your reply.
> 
>> The small-data sections exist to let a linker
>> script cluster small objects around that anchor so that ld's relaxation
>> pass can fold an auipc+load pair into a single gp-relative access (-+2
>> KiB window).
>>
>> That pass is keyed purely on the symbol being defined
>> riscv_global_pointer_value() returns 0 otherwise and the relaxation is
>> skipped. We define no __global_pointer$ and head.S never loads gp (it
>> appears only as a cpu_user_regs slot in entry.S), so every access stays
>> the medany auipc form regardless of section.
>>
>> I confirmed this by linking the same object twice (look at the script
>> below, with and without the symbol: without it, zero gp-relative
>> accesses; with it, the pairs collapse.
>>
>> Worth noting the relaxation is section-agnostic: in the test mentioned
>> below a 400-byte array in plain .bss got gp-relative too, purely because
>> it landed in range. So the sections are a clustering hint, not a
>> mechanism ld keys off.
> 
> Okay, fine, but what does this mean for placing the small data sections?
> I.e. what does this mean for the patch here (which really it shouldn't
> have been me to write in the first place)?

I just wnated to show that a position of .sbss doesn't really matter 
based on the example and so true for other .s* and not only .s* 
sections. What means I am okay with your suggested places in the current 
patch.


> 
>>> What remains to eliminate orphan section warnings is the placement of
>>> .note.GNU-stack (which perhaps wants dealing with on all of Arm, PPC, and
>>> RISC-V together, ideally unifying with x86) and (odd at the first glance,
>>> but dealt with on x86 as well, i.e. may again want unifying) that of a
>>> number of .rela.* sections.
>>>
>>> --- a/xen/arch/riscv/xen.lds.S
>>> +++ b/xen/arch/riscv/xen.lds.S
>>> @@ -44,6 +44,8 @@ SECTIONS
>>>    
>>>            BUGFRAMES
>>>    
>>> +        *(.srodata)
>>> +        *(.srodata.*)
>>>            *(.rodata)
>>>            *(.rodata.*)
>>>            VPCI_ARRAY
>>> @@ -92,6 +94,7 @@ SECTIONS
>>>            SCHEDULER_ARRAY
>>>            HYPFS_PARAM
>>>    
>>> +        *(.sdata .sdata.*)
>>>            *(.data .data.*)
>>>            CONSTRUCTORS
>>>        } :text
>>> @@ -162,6 +165,8 @@ SECTIONS
>>>        /* Section for the device tree blob (if any). */
>>>        .dtb : { *(.dtb) } :text
>>>    
>>> +    .riscv.attributes : { *(.riscv.attributes) } :text
>>> +
>>
>> Nit: .riscv.attributes is SHT_RISCV_ATTRIBUTES, i.e. non-alloc.
>> :text on it is misleading, and without an explicit address it gets
>> sh_addr from .(location counter) after .dtb. Could we use matching the
>> idiom used for every other non-alloc section in xen.lds.h:
>>     .riscv.attributes 0 : { *(.riscv.attributes) }
>> No functional difference either way (objcopy -O binary drops it, and I
>> verified a non-alloc output section doesn't advance dot, so nothing
>> downstream shifts), so purely consistency.
> 
> Well, I compare attributes rather with notes, which we make part of a
> segment (on x86 at least). I can drop the :text if it's that what's
> needed to get this in, but I'm not fully convinced. But my knowledge
> on the purpose and use of attributes also is still somewhat limited.

As I mentioned from functional point of view I don't think that it will 
be an issue so generally you could keep :text here.

That why I wrote "Nit:".

Reviewed-by: Oleksii Kurochko <[email protected]>

> 
>> Is dropping orphan-handling-y := from arch/riscv/Makefile the intended
>> end of this series?
> 
> It is the intended goal, but not by the end of this series.
> 
>> As if I understand correctly with such defintion we
>> will miss warning so everything of that will be missed:
>>
>> cd xen
>> riscv64-linux-gnu-ld -T arch/riscv/xen.lds prelink.o
>> --orphan-handling=warn -o /tmp/t.elf 2>&1 \
>>     | grep 'orphan section'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section `.note.GNU-stack'
>> from `prelink.o' being placed in section `.note.GNU-stack'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section `.rela.text' from
>> `prelink.o' being placed in section `.rela.dyn'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section `.rela.init.text'
>> from `prelink.o' being placed in section `.rela.dyn'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section
>> `.rela.data.read_mostly' from `prelink.o' being placed in section
>> `.rela.dyn'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section `.rela.init.data'
>> from `prelink.o' being placed in section `.rela.dyn'
>> /usr/bin/riscv64-linux-gnu-ld: warning: orphan section
>> `.rela.text.header' from `prelink.o' being placed in section `.rela.dyn'
> 
> Yes, if the override was dropped, these warnings would appear on every
> build. I thought that may not be wanted, hence the override I put in
> (really everywhere except for x86, where things were already tidied).

Thanks. Got you. It makes sense.

~ Oleksii