On 2025/4/15 17:49, Jan Beulich wrote:
> On 09.04.2025 08:45, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -815,6 +815,39 @@ static int vpci_init_capability_list(struct pci_dev
>> *pdev)
>> return rc;
>> }
>>
>> +static int vpci_init_ext_capability_list(struct pci_dev *pdev)
>> +{
>> + int rc;
>> + u32 header;
>> + unsigned int pos = 0x100U, ttl = 480;
>> +
>> + if ( !is_hardware_domain(pdev->domain) )
>> + {
>> + /* Extended capabilities read as zero, write ignore */
>> + rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
>> + pos, 4, (void *)0);
>> + if ( rc )
>> + return rc;
>> + }
>> +
>> + while ( pos && ttl-- )
>> + {
>> + header = pci_conf_read32(pdev->sbdf, pos);
>> +
>> + rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
>> + pos, 4, (void *)(uintptr_t)header);
>> + if ( rc )
>> + return rc;
>> +
>> + if ( (header == 0) || (header == -1) )
>> + return 0;
>
> I realize pci_find_next_ext_capability() also has such a check, but even
> there it's not really clear to me why compare not only against 0, but also
> again -1 (aka ~0).
Thank you for raising this question.
When I coded this part, I also had this confuse since
pci_find_next_ext_capability() has this check,
so I chose to keep the same check.
Do you think I need to remove this -1 check?
>
> Jan
--
Best regards,
Jiqian Chen.
