Re: [Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability

["Liu, Jing2" <jing2.liu@linux.intel.com>]

Hi Marcel,

On 8/18/2018 12:10 AM, Marcel Apfelbaum wrote:
> Hi Jing,
> 
> On 08/16/2018 12:28 PM, Jing Liu wrote:
>> Clean up the PCI config space of resource reserve capability.
>>
>> Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
>> ---
>>   hw/pci/pci_bridge.c         | 9 +++++++++
>>   include/hw/pci/pci_bridge.h | 1 +
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
>> index 15b055e..dbcee90 100644
>> --- a/hw/pci/pci_bridge.c
>> +++ b/hw/pci/pci_bridge.c
>> @@ -465,6 +465,15 @@ int pci_bridge_qemu_reserve_cap_init(PCIDevice 
>> *dev, int cap_offset,
>>       return 0;
>>   }
>> +void pci_bridge_qemu_reserve_cap_uninit(PCIDevice *dev)
>> +{
>> +    uint8_t pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
>> +
>> +    pci_del_capability(dev, PCI_CAP_ID_VNDR, sizeof(PCIBridgeQemuCap));
> 
> I think that you only need to call pci_del_capability,
> 
>> +    memset(dev->config + pos + PCI_CAP_FLAGS, 0,
>> +           sizeof(PCIBridgeQemuCap) - PCI_CAP_FLAGS);
>> +}
> 
> ... no need for the above line. The reason is pci_del_capability
> will "unlink" the capability, and even if the data remains in
> the configuration space array, it will not be used.
> 
I think I got it: pci_del_capability "unlink" by set the tag
pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST;
so that pdev->config will not be used, right?

> Do you agree? If yes, just call pci_del_capability and you don't need
> this patch.
> 
Yup, I agree with you. And let me remove this patch in next version.

Thanks,
Jing

> 
> Thanks,
> Marcel
> 

[...]