1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/3] hw/pci: PCI resource reserve capability
  4. View patch

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

Jing Liu posted 3 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability
Posted by Jing Liu 7 years, 2 months ago 
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));
+    memset(dev->config + pos + PCI_CAP_FLAGS, 0,
+           sizeof(PCIBridgeQemuCap) - PCI_CAP_FLAGS);
+}
+
 static const TypeInfo pci_bridge_type_info = {
     .name = TYPE_PCI_BRIDGE,
     .parent = TYPE_PCI_DEVICE,
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index 6186a32..b1e25ad 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -147,4 +147,5 @@ typedef struct PCIResReserve {
 int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
                                PCIResReserve res_reserve, Error **errp);
 
+void pci_bridge_qemu_reserve_cap_uninit(PCIDevice *dev);
 #endif /* QEMU_PCI_BRIDGE_H */
-- 
1.8.3.1
Re: [Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability
Posted by Marcel Apfelbaum 7 years, 2 months ago 
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.

Do you agree? If yes, just call pci_del_capability and you don't need
this patch.


Thanks,
Marcel

> +
>   static const TypeInfo pci_bridge_type_info = {
>       .name = TYPE_PCI_BRIDGE,
>       .parent = TYPE_PCI_DEVICE,
> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> index 6186a32..b1e25ad 100644
> --- a/include/hw/pci/pci_bridge.h
> +++ b/include/hw/pci/pci_bridge.h
> @@ -147,4 +147,5 @@ typedef struct PCIResReserve {
>   int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
>                                  PCIResReserve res_reserve, Error **errp);
>   
> +void pci_bridge_qemu_reserve_cap_uninit(PCIDevice *dev);
>   #endif /* QEMU_PCI_BRIDGE_H */
Re: [Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability
Posted by Liu, Jing2 7 years, 2 months ago 
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
> 

[...]
Re: [Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability
Posted by Marcel Apfelbaum 7 years, 2 months ago 
Hi Jing,

On 08/20/2018 05:58 AM, Liu, Jing2 wrote:
> 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?

If is the latest capability in the list, yes.
Otherwise it will simply link 'prev' with 'next' using config array offsets.

Thanks,
Marcel

>
>> 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
>>
>
> [...]
Re: [Qemu-devel] [PATCH v2 2/3] hw/pci: add teardown function for PCI resource reserve capability
Posted by Liu, Jing2 7 years, 2 months ago 
Hi Marcel,

On 8/20/2018 9:38 PM, Marcel Apfelbaum wrote:
> Hi Jing,
> 
> On 08/20/2018 05:58 AM, Liu, Jing2 wrote:
>> 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?
> 
> If is the latest capability in the list, yes.
> Otherwise it will simply link 'prev' with 'next' using config array 
> offsets.
I got it! Thanks very much for the details!

Jing

> 
> Thanks,
> Marcel
> 
>>
>>> 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
>>>
>>
>> [...]
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.