On 23.04.2025 05:49, Chen, Jiqian wrote:
> On 2025/4/23 00:03, Jan Beulich wrote:
>> On 21.04.2025 08:18, Jiqian Chen wrote:
>>> Refactor REGISTER_VPCI_INIT to contain more capability specific
>>> information, this is benifit for follow-on changes to hide capability
>>> which initialization fails.
>>>
>>> What's more, change the definition of init_header() since it is
>>> not a capability and it is needed for all devices' PCI config space.
>>>
>>> Note:
>>> Call vpci_make_msix_hole() in the end of init_msix() since the
>>> change of sequence of init_header() and init_msix().
>>> The fini hook will be implemented in follow-on changes.
>>>
>>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>>
>> From the description I can't derive the need for ...
>>
>>> --- a/xen/include/xen/xen.lds.h
>>> +++ b/xen/include/xen/xen.lds.h
>>> @@ -187,7 +187,7 @@
>>> #define VPCI_ARRAY \
>>> . = ALIGN(POINTER_ALIGN); \
>>> __start_vpci_array = .; \
>>> - *(SORT(.data.vpci.*)) \
>>> + *(.data.vpci.*) \
>>> __end_vpci_array = .;
>>> #else
>>> #define VPCI_ARRAY
>>
>> ... this change.
> As I understand this, this is used for initializing all capabilities
> according to priority before.
> That is msix > header > other capabilities.
> My patch removes the priority and initializing all capabilities doesn't
> depend on priority anymore.
> So I think this is not needed anymore.
Perhaps, but the word "priority" doesn't even appear in the description. So
yes, ...
> Do you mean I should add some explanation in the commit message?
... there's something to add there. But there's also the question of why the
change doesn't go further: With the SORT() dropped, what's the trailing .*
in the section name for? That's apparently connected to the puzzling
+ static vpci_capability_t *const x##_entry \
+ __used_section(".data.vpci.") = &(x##_t)
What's the trailing dot for here?
(As a nit - I also don't see why x##_t would need parenthesizing when
x##_entry doesn't. Is there another Misra gem which makes this necessary?)
Jan
