Re: [PATCH v4 02/10] vpci/header: Emulate legacy capability list for dom0

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2025/5/16 00:29, Roger Pau Monné wrote:
> On Fri, May 09, 2025 at 05:05:34PM +0800, Jiqian Chen wrote:
>> @@ -786,15 +792,18 @@ static int vpci_init_capability_list(struct pci_dev 
>> *pdev)
>>  
>>              next = pci_find_next_cap_ttl(pdev->sbdf,
>>                                           pos + PCI_CAP_LIST_NEXT,
>> -                                         supported_caps,
>> -                                         ARRAY_SIZE(supported_caps), &ttl);
>> +                                         supported_caps, n, &ttl);
>>  
>> -            rc = vpci_add_register(pdev->vpci, vpci_hw_read8, NULL,
>> -                                   pos + PCI_CAP_LIST_ID, 1, NULL);
>> -            if ( rc )
>> -                return rc;
>> +            if ( !is_hwdom )
>> +            {
>> +                rc = vpci_add_register(pdev->vpci, vpci_hw_read8, NULL,
>> +                                       pos + PCI_CAP_LIST_ID, 1, NULL);
>> +                if ( rc )
>> +                    return rc;
>> +            }
>>  
>> -            rc = vpci_add_register(pdev->vpci, vpci_read_val, NULL,
>> +            rc = vpci_add_register(pdev->vpci, vpci_read_val,
>> +                                   is_hwdom ? vpci_hw_write8 : NULL,
>>                                     pos + PCI_CAP_LIST_NEXT, 1,
>>                                     (void *)(uintptr_t)next);
>>              if ( rc )
>> @@ -805,13 +814,17 @@ static int vpci_init_capability_list(struct pci_dev 
>> *pdev)
>>      }
>>  
>>      /* Utilize rsvdp_mask to hide PCI_STATUS_CAP_LIST from the guest. */
>> -    return vpci_add_register_mask(pdev->vpci, vpci_hw_read16, 
>> vpci_hw_write16,
>> -                                  PCI_STATUS, 2, NULL,
>> -                                  PCI_STATUS_RO_MASK &
>> -                                    ~(mask_cap_list ? PCI_STATUS_CAP_LIST : 
>> 0),
>> -                                  PCI_STATUS_RW1C_MASK,
>> -                                  mask_cap_list ? PCI_STATUS_CAP_LIST : 0,
>> -                                  PCI_STATUS_RSVDZ_MASK);
>> +    return is_hwdom ? 0 : vpci_add_register_mask(pdev->vpci, vpci_hw_read16,
>> +                                                 vpci_hw_write16, 
>> PCI_STATUS,
>> +                                                 2, NULL,
>> +                                                 PCI_STATUS_RO_MASK &
>> +                                                    ~(mask_cap_list ?
>> +                                                        PCI_STATUS_CAP_LIST 
>> :
>> +                                                        0),
>> +                                                 PCI_STATUS_RW1C_MASK,
>> +                                                 mask_cap_list ?
>> +                                                    PCI_STATUS_CAP_LIST : 0,
>> +                                                 PCI_STATUS_RSVDZ_MASK);
> 
> Wow, that's a bit too much indentation for my taste.  Do you think you
> could do:
> 
> /* Return early for the hw domain, no masking of PCI_STATUS. */
> if ( is_hwdom )
>     return 0;
> ...
> 
> So that you don't have to touch the current return chunk?
It seems better.
Will do in next version.

> 
> Thanks, Roger.

-- 
Best regards,
Jiqian Chen.