* [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init
@ 2014-01-21 16:37 Marcel Apfelbaum
2014-01-23 13:01 ` Marcel Apfelbaum
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Marcel Apfelbaum @ 2014-01-21 16:37 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, mst
Scenario:
- There is a non multifunction pci device A on 00:0X.0.
- Hot-plug another multifunction pci device B at 00:0X.1.
- The operation will fail of course.
- Try to hot-plug the B device 2-3 more times, qemu will crash.
Reason: The error flow leaves the B's address space into global address spaces
list, but the device object is freed. Fixed that.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
hw/pci/pci.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index aa2a395..5f454dd 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -793,6 +793,15 @@ static void pci_config_free(PCIDevice *pci_dev)
g_free(pci_dev->used);
}
+static void do_pci_unregister_device(PCIDevice *pci_dev)
+{
+ pci_dev->bus->devices[pci_dev->devfn] = NULL;
+ pci_config_free(pci_dev);
+
+ address_space_destroy(&pci_dev->bus_master_as);
+ memory_region_destroy(&pci_dev->bus_master_enable_region);
+}
+
/* -1 for devfn means auto assign */
static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
const char *name, int devfn)
@@ -858,7 +867,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
pci_init_mask_bridge(pci_dev);
}
if (pci_init_multifunction(bus, pci_dev)) {
- pci_config_free(pci_dev);
+ do_pci_unregister_device(pci_dev);
return NULL;
}
@@ -873,15 +882,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
return pci_dev;
}
-static void do_pci_unregister_device(PCIDevice *pci_dev)
-{
- pci_dev->bus->devices[pci_dev->devfn] = NULL;
- pci_config_free(pci_dev);
-
- address_space_destroy(&pci_dev->bus_master_as);
- memory_region_destroy(&pci_dev->bus_master_enable_region);
-}
-
static void pci_unregister_io_regions(PCIDevice *pci_dev)
{
PCIIORegion *r;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init
2014-01-21 16:37 [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init Marcel Apfelbaum
@ 2014-01-23 13:01 ` Marcel Apfelbaum
2014-01-23 13:36 ` Markus Armbruster
2014-01-23 14:09 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Marcel Apfelbaum @ 2014-01-23 13:01 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, mst
On Tue, 2014-01-21 at 18:37 +0200, Marcel Apfelbaum wrote:
> Scenario:
> - There is a non multifunction pci device A on 00:0X.0.
> - Hot-plug another multifunction pci device B at 00:0X.1.
> - The operation will fail of course.
> - Try to hot-plug the B device 2-3 more times, qemu will crash.
>
> Reason: The error flow leaves the B's address space into global address spaces
> list, but the device object is freed. Fixed that.
Ping
Thanks,
Marcel
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> hw/pci/pci.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index aa2a395..5f454dd 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -793,6 +793,15 @@ static void pci_config_free(PCIDevice *pci_dev)
> g_free(pci_dev->used);
> }
>
> +static void do_pci_unregister_device(PCIDevice *pci_dev)
> +{
> + pci_dev->bus->devices[pci_dev->devfn] = NULL;
> + pci_config_free(pci_dev);
> +
> + address_space_destroy(&pci_dev->bus_master_as);
> + memory_region_destroy(&pci_dev->bus_master_enable_region);
> +}
> +
> /* -1 for devfn means auto assign */
> static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> const char *name, int devfn)
> @@ -858,7 +867,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> pci_init_mask_bridge(pci_dev);
> }
> if (pci_init_multifunction(bus, pci_dev)) {
> - pci_config_free(pci_dev);
> + do_pci_unregister_device(pci_dev);
> return NULL;
> }
>
> @@ -873,15 +882,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> return pci_dev;
> }
>
> -static void do_pci_unregister_device(PCIDevice *pci_dev)
> -{
> - pci_dev->bus->devices[pci_dev->devfn] = NULL;
> - pci_config_free(pci_dev);
> -
> - address_space_destroy(&pci_dev->bus_master_as);
> - memory_region_destroy(&pci_dev->bus_master_enable_region);
> -}
> -
> static void pci_unregister_io_regions(PCIDevice *pci_dev)
> {
> PCIIORegion *r;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init
2014-01-21 16:37 [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init Marcel Apfelbaum
2014-01-23 13:01 ` Marcel Apfelbaum
@ 2014-01-23 13:36 ` Markus Armbruster
2014-01-23 14:09 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2014-01-23 13:36 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, mst
Marcel Apfelbaum <marcel.a@redhat.com> writes:
> Scenario:
> - There is a non multifunction pci device A on 00:0X.0.
> - Hot-plug another multifunction pci device B at 00:0X.1.
> - The operation will fail of course.
> - Try to hot-plug the B device 2-3 more times, qemu will crash.
>
> Reason: The error flow leaves the B's address space into global address spaces
> list, but the device object is freed. Fixed that.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Looks good to me, but I'm not a PCI expert.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init
2014-01-21 16:37 [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init Marcel Apfelbaum
2014-01-23 13:01 ` Marcel Apfelbaum
2014-01-23 13:36 ` Markus Armbruster
@ 2014-01-23 14:09 ` Michael S. Tsirkin
2 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2014-01-23 14:09 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, armbru
On Tue, Jan 21, 2014 at 06:37:51PM +0200, Marcel Apfelbaum wrote:
> Scenario:
> - There is a non multifunction pci device A on 00:0X.0.
> - Hot-plug another multifunction pci device B at 00:0X.1.
> - The operation will fail of course.
> - Try to hot-plug the B device 2-3 more times, qemu will crash.
>
> Reason: The error flow leaves the B's address space into global address spaces
> list, but the device object is freed. Fixed that.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Thanks, applied.
> ---
> hw/pci/pci.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index aa2a395..5f454dd 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -793,6 +793,15 @@ static void pci_config_free(PCIDevice *pci_dev)
> g_free(pci_dev->used);
> }
>
> +static void do_pci_unregister_device(PCIDevice *pci_dev)
> +{
> + pci_dev->bus->devices[pci_dev->devfn] = NULL;
> + pci_config_free(pci_dev);
> +
> + address_space_destroy(&pci_dev->bus_master_as);
> + memory_region_destroy(&pci_dev->bus_master_enable_region);
> +}
> +
> /* -1 for devfn means auto assign */
> static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> const char *name, int devfn)
> @@ -858,7 +867,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> pci_init_mask_bridge(pci_dev);
> }
> if (pci_init_multifunction(bus, pci_dev)) {
> - pci_config_free(pci_dev);
> + do_pci_unregister_device(pci_dev);
> return NULL;
> }
>
> @@ -873,15 +882,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> return pci_dev;
> }
>
> -static void do_pci_unregister_device(PCIDevice *pci_dev)
> -{
> - pci_dev->bus->devices[pci_dev->devfn] = NULL;
> - pci_config_free(pci_dev);
> -
> - address_space_destroy(&pci_dev->bus_master_as);
> - memory_region_destroy(&pci_dev->bus_master_enable_region);
> -}
> -
> static void pci_unregister_io_regions(PCIDevice *pci_dev)
> {
> PCIIORegion *r;
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-23 14:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-21 16:37 [Qemu-devel] [PATCH] hw/pci: fix error flow in pci multifunction init Marcel Apfelbaum
2014-01-23 13:01 ` Marcel Apfelbaum
2014-01-23 13:36 ` Markus Armbruster
2014-01-23 14:09 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).