On 2025/4/23 15:36, Jan Beulich wrote:
> 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, ...
I will add description about "priority" removal in commit message in next
version.
>
>> 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?
Thanks for catching this problem.
I forgot to delete the dot and ".*", will delete them in next version.
>
> (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?)
Oh, I will delete the parentheses in next version.
>
> Jan
--
Best regards,
Jiqian Chen.
