On 2025/6/9 18:40, Roger Pau Monné wrote:
> On Mon, Jun 09, 2025 at 10:18:42AM +0000, Chen, Jiqian wrote:
>> On 2025/6/9 17:29, Roger Pau Monné wrote:
>>> On Mon, Jun 09, 2025 at 07:50:21AM +0000, Chen, Jiqian wrote:
>>>> On 2025/6/6 17:14, Roger Pau Monné wrote:
>>>>> On Fri, Jun 06, 2025 at 09:05:48AM +0200, Jan Beulich wrote:
>>>>>> On 06.06.2025 08:29, Chen, Jiqian wrote:
>>>>>>> On 2025/6/5 20:50, Roger Pau Monné wrote:
>>>>>>>> On Mon, May 26, 2025 at 05:45:53PM +0800, Jiqian Chen wrote:
>>>>>>>>> + }; \
>>>>>>>>> + static vpci_capability_t *const finit##_entry \
>>>>>>>>> + __used_section(".data.vpci") = &finit##_t
>>>>>>>>
>>>>>>>> IMO this should better use .rodata instead of .data.
>>>>>>> Is below change correct?
>>>>>>>
>>>>>>> + static const vpci_capability_t *const finit##_entry \
>>>>>>> + __used_section(".rodata") = &finit##_t
>>>>>>
>>>>>> No, specifically because ...
>>>>>>
>>>>>>>> Not that it matters much in practice, as we place it in .rodata
>>>>>>>> anyway. Note
>>>>>>>> however you will have to move the placement of the VPCI_ARRAY in the
>>>>>>>> linker script ahead of *(.rodata.*), otherwise that section match will
>>>>>>>> consume the vPCI data.
>>>>>>> I am sorry, how to move it ahead of *(.rodata.*) ?
>>>>>>> Is below change correct?
>>>>>>>
>>>>>>> diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
>>>>>>> index 793d0e11450c..3817642135aa 100644
>>>>>>> --- a/xen/include/xen/xen.lds.h
>>>>>>> +++ b/xen/include/xen/xen.lds.h
>>>>>>> @@ -188,7 +188,7 @@
>>>>>>> #define VPCI_ARRAY \
>>>>>>> . = ALIGN(POINTER_ALIGN); \
>>>>>>> __start_vpci_array = .; \
>>>>>>> - *(SORT(.data.vpci.*)) \
>>>>>>> + *(.rodata) \
>>>>>>
>>>>>> ... this isn't - you'd move _all_ of .rodata into here, which definitely
>>>>>> isn't what you want. What I understand Roger meant was a .rodata-like
>>>>>> section, e.g. .rodata.vpci.* (much like it was .data.vpci.* before).
>>>>>
>>>>> Indeed, my suggestion was merely to use .rodata instead of .data, as
>>>>> that's more accurate IMO. I think it should be *(.rodata.vpci) (and
>>>>> same section change for the __used_section() attribute.
>>>>
>>>> If I understand correctly, the next version will be:
>>>>
>>>> + static const vpci_capability_t *const finit##_entry \
>>>> + __used_section(".rodata.vpci") = &finit##_t
>>>> +
>>>>
>>>> and
>>>>
>>>> #define VPCI_ARRAY \
>>>> . = ALIGN(POINTER_ALIGN); \
>>>> __start_vpci_array = .; \
>>>> - *(SORT(.data.vpci.*)) \
>>>> + *(.rodata.vpci) \
>>>> __end_vpci_array = .;
>>>
>>> Did you also move the instances of VPCI_ARRAY in the linker scripts so
>>> it's before the catch-all *(.rodata.*)?
>>>
>>>>
>>>> But, that encountered an warning when compiling.
>>>> " {standard input}: Assembler messages:
>>>> {standard input}:1160: Warning: setting incorrect section attributes for
>>>> .rodata.vpci
>>>> {standard input}: Assembler messages:
>>>> {standard input}:3034: Warning: setting incorrect section attributes for
>>>> .rodata.vpci
>>>> {standard input}: Assembler messages:
>>>> {standard input}:6686: Warning: setting incorrect section attributes for
>>>> .rodata.vpci "
>>>
>>> What are the attributes for .rodata.vpci in the object files? You can
>>> get those using objdump or readelf, for example:
>>>
>>> $ objdump -h xen/drivers/vpci/msi.o
>>> [...]
>>> 17 .data.vpci.9 00000008 0000000000000000 0000000000000000 00000a50
>>> 2**3
>>> CONTENTS, ALLOC, LOAD, RELOC, DATA
>>>
>>> It should be READONLY, otherwise you will get those messages.
>>>
>>>> And, during booting Xen, all value of __start_vpci_array is incorrect.
>>>> Do I miss anything?
>>>
>>> I think that's likely because you haven't moved the instance of
>>> VPCI_ARRAY in the linker script?
>> Oh, right. Sorry, I forgot to move it.
>> After changing this, it works now.
>>
>> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
>> index bf956b6c5fc0..c88fd62f4f0d 100644
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -134,6 +134,7 @@ SECTIONS
>> BUGFRAMES
>>
>> *(.rodata)
>> + VPCI_ARRAY
>> *(.rodata.*)
>> *(.data.rel.ro)
>> *(.data.rel.ro.*)
>> @@ -148,7 +149,6 @@ SECTIONS
>> *(.note.gnu.build-id)
>> __note_gnu_build_id_end = .;
>> #endif
>> - VPCI_ARRAY
>> } PHDR(text)
>
> FWIW, I would put it ahead of *(.rodata). Remember to also modify the
> linker script for all the other arches, not just x86.Whether before *(.rodata.*) or before *(.rodata), there still is the warning " Warning: setting incorrect section attributes for .rodata.vpci " And the objdump shows "rodata.vpci" has no "readonly" word. But starting Xen dom0 is OK. I attached the new this patch and the result of "objdump -h xen/drivers/vpci/msi.o" > > Thanks, Roger. -- Best regards, Jiqian Chen.
./xen/drivers/vpci/msi.o: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .text 00000000 0000000000000000 0000000000000000 00000040 2**0
CONTENTS, ALLOC, LOAD, READONLY, CODE
1 .data 00000000 0000000000000000 0000000000000000 00000040 2**0
CONTENTS, ALLOC, LOAD, DATA
2 .bss 00000000 0000000000000000 0000000000000000 00000040 2**0
ALLOC
3 .text._can_read_lock 00000052 0000000000000000 0000000000000000 00000040
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
4 .text.address_read 0000000b 0000000000000000 0000000000000000 00000092
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
5 .text.address_hi_read 00000010 0000000000000000 0000000000000000
0000009d 2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
6 .text.data_read 0000000d 0000000000000000 0000000000000000 000000ad 2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
7 .text.mask_read 0000000c 0000000000000000 0000000000000000 000000ba 2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
8 .text.mask_write 00000078 0000000000000000 0000000000000000 000000c6
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
9 .text.address_hi_write 00000027 0000000000000000 0000000000000000
0000013e 2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
10 .text.control_read 0000004e 0000000000000000 0000000000000000 00000165
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
11 .text.control_write 00000144 0000000000000000 0000000000000000 000001b3
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
12 .altinstructions 00000070 0000000000000000 0000000000000000 000002f7
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
13 .discard 00000010 0000000000000000 0000000000000000 00000367 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
14 .altinstr_replacement 00000000 0000000000000000 0000000000000000
00000377 2**0
CONTENTS, ALLOC, LOAD, READONLY, CODE
15 .text._read_trylock 00000090 0000000000000000 0000000000000000 00000377
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
16 .text._read_unlock 00000036 0000000000000000 0000000000000000 00000407
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
17 .text.cleanup_msi 000000dc 0000000000000000 0000000000000000 0000043d
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
18 .rodata.init_msi.str1.1 0000002a 0000000000000000 0000000000000000
00000519 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
19 .text.init_msi 00000270 0000000000000000 0000000000000000 00000543 2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
20 .bug_frames.3 00000020 0000000000000000 0000000000000000 000007b4 2**2
CONTENTS, ALLOC, LOAD, RELOC, READONLY, DATA
21 .text.address_write 00000039 0000000000000000 0000000000000000 000007d4
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
22 .text.data_write 00000028 0000000000000000 0000000000000000 0000080d
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
23 .rodata.vpci_dump_msi.str1.1 0000009a 0000000000000000 0000000000000000
00000835 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
24 .rodata.vpci_dump_msi.str1.8 0000007f 0000000000000000 0000000000000000
000008d0 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
25 .text.vpci_dump_msi 000002af 0000000000000000 0000000000000000 0000094f
2**0
CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
26 .rodata.vpci 00000008 0000000000000000 0000000000000000 00000c00 2**3
CONTENTS, ALLOC, LOAD, RELOC, DATA
27 .data.rel.ro.local.init_msi_t 00000018 0000000000000000 0000000000000000
00000c10 2**4
CONTENTS, ALLOC, LOAD, RELOC, DATA
28 .debug_info 0000ae83 0000000000000000 0000000000000000 00000c28 2**0
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
29 .debug_abbrev 00000892 0000000000000000 0000000000000000 0000baab 2**0
CONTENTS, READONLY, DEBUGGING, OCTETS
30 .debug_loclists 00000ffc 0000000000000000 0000000000000000 0000c33d 2**0
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
31 .debug_aranges 00000120 0000000000000000 0000000000000000 0000d339 2**0
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
32 .debug_rnglists 00000212 0000000000000000 0000000000000000 0000d459 2**0
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
33 .debug_line 00000e30 0000000000000000 0000000000000000 0000d66b 2**0
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
34 .debug_str 0000502b 0000000000000000 0000000000000000 0000e49b 2**0
CONTENTS, READONLY, DEBUGGING, OCTETS
35 .debug_line_str 00000470 0000000000000000 0000000000000000 000134c6 2**0
CONTENTS, READONLY, DEBUGGING, OCTETS
36 .comment 0000002c 0000000000000000 0000000000000000 00013936 2**0
CONTENTS, READONLY
37 .note.GNU-stack 00000000 0000000000000000 0000000000000000 00013962 2**0
CONTENTS, READONLY
38 .note.gnu.property 00000020 0000000000000000 0000000000000000 00013968
2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
39 .debug_frame 000002d0 0000000000000000 0000000000000000 00013988 2**3
CONTENTS, RELOC, READONLY, DEBUGGING, OCTETS
0003-vpci-Refactor-REGISTER_VPCI_INIT.patch
Description: 0003-vpci-Refactor-REGISTER_VPCI_INIT.patch
