On 2025/6/18 22:05, Jan Beulich wrote:
> On 12.06.2025 11:29, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/msix.c
>> +++ b/xen/drivers/vpci/msix.c
>> @@ -703,9 +703,13 @@ static int cf_check init_msix(struct pci_dev *pdev)
>> pdev->vpci->msix = msix;
>> list_add(&msix->next, &d->arch.hvm.msix_tables);
>>
>> - return 0;
>> + spin_lock(&pdev->vpci->lock);
>> + rc = vpci_make_msix_hole(pdev);
>> + spin_unlock(&pdev->vpci->lock);
>
> If you add a call to vpci_make_msix_hole() here, doesn't it need (or at
> least want) removing somewhere else? Otherwise maybe a code comment is
> warranted next to the new call site?
The removing operation in modify_bars() and vpci_deassign_device() is not
enough?
>
>> @@ -29,9 +30,22 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
>> */
>> #define VPCI_MAX_VIRT_DEV (PCI_SLOT(~0) + 1)
>>
>> -#define REGISTER_VPCI_INIT(x, p) \
>> - static vpci_register_init_t *const x##_entry \
>> - __used_section(".data.vpci." p) = (x)
>> +#define REGISTER_VPCI_CAPABILITY(cap, finit, fclean, ext) \
>> + static const vpci_capability_t finit##_t = { \
>> + .id = (cap), \
>> + .init = (finit), \
>> + .cleanup = (fclean), \
>> + .is_ext = (ext), \
>> + }; \
>> + static const vpci_capability_t *const finit##_entry \
>> + __used_section(".data.rel.ro.vpci") = &finit##_t
>
> Could you remind me why the extra level of indirection is necessary here?
> That is, why can't .data.rel.ro.vpci be an array of vpci_capability_t?
You mean I should change to be:
#define REGISTER_VPCI_CAPABILITY(cap, finit, fclean, ext) \
static const vpci_capability_t finit##_t \
__used_section(".data.rel.ro.vpci") = { \
.id = (cap), \
.init = (finit), \
.cleanup = (fclean), \
.is_ext = (ext), \
}
Right?
>
> Jan
--
Best regards,
Jiqian Chen.
