diff mbox

pci_bridge: manually destroy memory regions within PCIBridgeWindows

Message ID 1408549805-20956-1-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Aug. 20, 2014, 3:50 p.m. UTC 
The regions are destroyed and recreated on configuration space accesses.
We need to destroy them before the containing PCIBridgeWindows object
is freed.

Reported-by: Gonglei <arei.gonglei@huawei.com>
Reported-by: Knut Omang <knut.omang@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pci/pci_bridge.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Michael S. Tsirkin Aug. 20, 2014, 7:17 p.m. UTC | #1 
On Wed, Aug 20, 2014 at 05:50:05PM +0200, Paolo Bonzini wrote:
> The regions are destroyed and recreated on configuration space accesses.
> We need to destroy them before the containing PCIBridgeWindows object
> is freed.
> 
> Reported-by: Gonglei <arei.gonglei@huawei.com>
> Reported-by: Knut Omang <knut.omang@oracle.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


Regression in 2.1? Cc stable?

> ---
>  hw/pci/pci_bridge.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 1307265..40c97b1 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -219,6 +219,12 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>  
>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>  {
> +    object_unparent(OBJECT(&w->alias_io));
> +    object_unparent(OBJECT(&w->alias_mem));
> +    object_unparent(OBJECT(&w->alias_pref_mem));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
>      g_free(w);
>  }
>  
> -- 
> 1.8.3.1
Michael S. Tsirkin Aug. 20, 2014, 7:48 p.m. UTC | #2 
On Wed, Aug 20, 2014 at 05:50:05PM +0200, Paolo Bonzini wrote:
> The regions are destroyed and recreated on configuration space accesses.
> We need to destroy them before the containing PCIBridgeWindows object
> is freed.
> 
> Reported-by: Gonglei <arei.gonglei@huawei.com>
> Reported-by: Knut Omang <knut.omang@oracle.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied, thanks!

> ---
>  hw/pci/pci_bridge.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 1307265..40c97b1 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -219,6 +219,12 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>  
>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>  {
> +    object_unparent(OBJECT(&w->alias_io));
> +    object_unparent(OBJECT(&w->alias_mem));
> +    object_unparent(OBJECT(&w->alias_pref_mem));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
>      g_free(w);
>  }
>  
> -- 
> 1.8.3.1
Gonglei (Arei) Aug. 21, 2014, 1:49 a.m. UTC | #3 
> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: Wednesday, August 20, 2014 11:50 PM
> To: qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; Gonglei (Arei); knut.omang@oracle.com;
> mst@redhat.com
> Subject: [PATCH] pci_bridge: manually destroy memory regions within
> PCIBridgeWindows
> 
> The regions are destroyed and recreated on configuration space accesses.
> We need to destroy them before the containing PCIBridgeWindows object
> is freed.
> 
> Reported-by: Gonglei <arei.gonglei@huawei.com>
> Reported-by: Knut Omang <knut.omang@oracle.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/pci/pci_bridge.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 1307265..40c97b1 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -219,6 +219,12 @@ static void pci_bridge_region_del(PCIBridge *br,
> PCIBridgeWindows *w)
> 
>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>  {
> +    object_unparent(OBJECT(&w->alias_io));
> +    object_unparent(OBJECT(&w->alias_mem));
> +    object_unparent(OBJECT(&w->alias_pref_mem));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
>      g_free(w);
>  }
> 
> --
> 1.8.3.1

Tested-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei
Paolo Bonzini Aug. 21, 2014, 8:12 a.m. UTC | #4 
Il 20/08/2014 21:17, Michael S. Tsirkin ha scritto:
> On Wed, Aug 20, 2014 at 05:50:05PM +0200, Paolo Bonzini wrote:
>> The regions are destroyed and recreated on configuration space accesses.
>> We need to destroy them before the containing PCIBridgeWindows object
>> is freed.
>>
>> Reported-by: Gonglei <arei.gonglei@huawei.com>
>> Reported-by: Knut Omang <knut.omang@oracle.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> 
> Regression in 2.1? Cc stable?
> 
>> ---
>>  hw/pci/pci_bridge.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
>> index 1307265..40c97b1 100644
>> --- a/hw/pci/pci_bridge.c
>> +++ b/hw/pci/pci_bridge.c
>> @@ -219,6 +219,12 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>>  
>>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>>  {
>> +    object_unparent(OBJECT(&w->alias_io));
>> +    object_unparent(OBJECT(&w->alias_mem));
>> +    object_unparent(OBJECT(&w->alias_pref_mem));
>> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
>> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
>> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
>>      g_free(w);
>>  }
>>  
>> -- 
>> 1.8.3.1

No, regression from yesterday.

Paolo
Michael S. Tsirkin Aug. 21, 2014, 8:21 a.m. UTC | #5 
On Wed, Aug 20, 2014 at 05:50:05PM +0200, Paolo Bonzini wrote:
> The regions are destroyed and recreated on configuration space accesses.
> We need to destroy them before the containing PCIBridgeWindows object
> is freed.
> 
> Reported-by: Gonglei <arei.gonglei@huawei.com>
> Reported-by: Knut Omang <knut.omang@oracle.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied in my tree.

> ---
>  hw/pci/pci_bridge.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 1307265..40c97b1 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -219,6 +219,12 @@ static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
>  
>  static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
>  {
> +    object_unparent(OBJECT(&w->alias_io));
> +    object_unparent(OBJECT(&w->alias_mem));
> +    object_unparent(OBJECT(&w->alias_pref_mem));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
> +    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
>      g_free(w);
>  }
>  
> -- 
> 1.8.3.1
diff mbox

Patch

diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 1307265..40c97b1 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -219,6 +219,12 @@  static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w)
 
 static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
 {
+    object_unparent(OBJECT(&w->alias_io));
+    object_unparent(OBJECT(&w->alias_mem));
+    object_unparent(OBJECT(&w->alias_pref_mem));
+    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
+    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
+    object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
     g_free(w);
 }