Re: [PATCH v5 03/10] vpci/header: Emulate extended capability list for dom0

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

On 2025/6/5 19:24, Roger Pau Monné wrote:
> On Mon, May 26, 2025 at 05:45:52PM +0800, Jiqian Chen wrote:
>> Add a new function to emulate extended capability list for dom0,
>> and call it in init_header(). So that it will be easy to hide a
>> extended capability whose initialization fails.
>>
>> As for the extended capability list of domU, just move the logic
>> into above function and keep hiding it for domU.
>>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
>> ---
>> cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
>> ---
>> v4->v5 changes:
>> * Add check: if capability list of hardware has a overlap, print warning and 
>> return 0.
>>
>> v3->v4 changes:
>> * Add check "if ( !header )   return 0;" to avoid adding handler for
>>   device that has no extended capabilities.
>>
>> v2->v3 changes:
>> * In vpci_init_ext_capability_list(), when domain is domU, directly return 
>> after adding a handler(hiding all extended capability for domU).
>> * In vpci_init_ext_capability_list(), change condition to be "while ( pos >= 
>> 0x100U && ttl-- )" instead of "while ( pos && ttl-- )".
>> * Add new function vpci_hw_write32, and pass it to extended capability 
>> handler for dom0.
>>
>> v1->v2 changes:
>> new patch
>>
>> Best regards,
>> Jiqian Chen.
>> ---
>>  xen/drivers/vpci/header.c | 47 ++++++++++++++++++++++++++++++++-------
>>  xen/drivers/vpci/vpci.c   |  6 +++++
>>  xen/include/xen/vpci.h    |  2 ++
>>  3 files changed, 47 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>> index d26cbba08ee1..4b2f761c9c24 100644
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -836,6 +836,42 @@ 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;
> 
> I think you can drop the ttl variable, as said by Jan in a previous
> review, the purpose of that counter is to detect overlaps in the PCIe
> config space, but for the context here the call to vpci_add_register()
> already serves that purpose by returning -EEXIST.
Will do in next version.

> 
> Thanks, Roger.

-- 
Best regards,
Jiqian Chen.