Re: [PATCH v5 04/10] vpci: Refactor REGISTER_VPCI_INIT

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

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 = .;

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 "

And, during booting Xen, all value of __start_vpci_array is incorrect.
Do I miss anything?

> 
> Thanks, Roger.

-- 
Best regards,
Jiqian Chen.