On 2025/5/18 22:34, Jan Beulich wrote:
> On 09.05.2025 11:05, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/msi.c
>> +++ b/xen/drivers/vpci/msi.c
>> @@ -270,7 +270,7 @@ static int cf_check init_msi(struct pci_dev *pdev)
>>
>> return 0;
>> }
>> -REGISTER_VPCI_INIT(init_msi, VPCI_PRIORITY_LOW);
>> +REGISTER_VPCI_LEGACY_CAP(PCI_CAP_ID_MSI, init_msi, NULL);
>
> To keep identifier length bounded, how about REGISTER_VPCI_CAP() here
> and ...
>
>> --- a/xen/drivers/vpci/rebar.c
>> +++ b/xen/drivers/vpci/rebar.c
>> @@ -118,7 +118,7 @@ static int cf_check init_rebar(struct pci_dev *pdev)
>>
>> return 0;
>> }
>> -REGISTER_VPCI_INIT(init_rebar, VPCI_PRIORITY_LOW);
>> +REGISTER_VPCI_EXTENDED_CAP(PCI_EXT_CAP_ID_REBAR, init_rebar, NULL);
>
> ... and REGISTER_VPCI_EXTCAP() here?
If so, I need to change the name of REGISTER_VPCI_CAP to be _REGISTER_VPCI_CAP ?
#define REGISTER_VPCI_CAP(cap, finit, fclean, ext) \
static vpci_capability_t finit##_t = { \
.id = (cap), \
.init = (finit), \
.cleanup = (fclean), \
.is_ext = (ext), \
}; \
static vpci_capability_t *const finit##_entry \
__used_section(".data.vpci") = &finit##_t
>
>> @@ -83,6 +83,35 @@ static int assign_virtual_sbdf(struct pci_dev *pdev)
>>
>> #endif /* CONFIG_HAS_VPCI_GUEST_SUPPORT */
>>
>> +static int vpci_init_capabilities(struct pci_dev *pdev)
>> +{
>> + for ( unsigned int i = 0; i < NUM_VPCI_INIT; i++ )
>> + {
>> + const vpci_capability_t *capability = __start_vpci_array[i];
>> + const unsigned int cap = capability->id;
>> + const bool is_ext = capability->is_ext;
>> + unsigned int pos;
>> + int rc;
>> +
>> + if ( !is_hardware_domain(pdev->domain) && is_ext )
>> + continue;
>
> Fold this into ...
>
>> + if ( !is_ext )
>> + pos = pci_find_cap_offset(pdev->sbdf, cap);
>> + else
>> + pos = pci_find_ext_capability(pdev->sbdf, cap);
>
> ... this by adding a middle "else if()"?
It seems better, will do.
Thanks.
>
> Jan
--
Best regards,
Jiqian Chen.
