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.
