On 2025/5/19 14:56, Jan Beulich wrote:
> On 19.05.2025 08:43, Chen, Jiqian wrote:
>> On 2025/5/18 22:20, Jan Beulich wrote:
>>> On 09.05.2025 11:05, Jiqian Chen wrote:
>>>> @@ -827,6 +827,34 @@ static int vpci_init_capability_list(struct pci_dev
>>>> *pdev)
>>>> PCI_STATUS_RSVDZ_MASK);
>>>> }
>>>>
>>>> +static int vpci_init_ext_capability_list(struct pci_dev *pdev)
>>>> +{
>>>> + unsigned int pos = PCI_CFG_SPACE_SIZE, ttl = 480;
>>>
>>> The ttl value exists (in the function you took it from) to make sure
>>> the loop below eventually ends. That is, to be able to kind of
>>> gracefully deal with loops in the linked list. Such loops, however,
>>> would ...
>>>
>>>> + if ( !is_hardware_domain(pdev->domain) )
>>>> + /* Extended capabilities read as zero, write ignore for guest */
>>>> + return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
>>>> + pos, 4, (void *)0);
>>>> +
>>>> + while ( pos >= PCI_CFG_SPACE_SIZE && ttl-- )
>>>> + {
>>>> + uint32_t header = pci_conf_read32(pdev->sbdf, pos);
>>>> + int rc;
>>>> +
>>>> + if ( !header )
>>>> + return 0;
>>>> +
>>>> + rc = vpci_add_register(pdev->vpci, vpci_read_val, vpci_hw_write32,
>>>> + pos, 4, (void *)(uintptr_t)header);
>>>
>>> ... mean we may invoke this twice for the same capability. Such
>>> a secondary invocation would fail with -EEXIST, causing device init
>>> to fail altogether. Which is kind of against our aim of exposing
>>> (in a controlled manner) as much of the PCI hardware as possible.
>> May I know what situation that can make this twice for one capability when
>> initialization?
>> Does hardware capability list have a cycle?
>
> Any of this is to work around flawed hardware, I suppose.
>
>>> Imo we ought to be using a bitmap to detect the situation earlier
>>> and hence to be able to avoid redundant register addition. Thoughts?
>> Can we just let it go forward and continue to add register for next
>> capability when rc == -EXIST, instead of returning error ?
>
> Possible, but feels wrong.
How about when EXIST, setting the next bits of previous extended capability to
be zero and return 0? Then we break the cycle.
>
> Jan
--
Best regards,
Jiqian Chen.
