[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit |
Date: |
Wed, 5 Jun 2013 01:03:28 +0300 |
On Tue, Jun 04, 2013 at 08:52:06PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
Please add a commit log with a bit more detail and
documenting the motivation for change.
> ---
> hw/misc/vfio.c | 1 +
> hw/net/vmxnet3.c | 3 +++
> hw/pci/msix.c | 26 ++++++++++++++++++++------
> hw/virtio/virtio-pci.c | 1 +
> include/hw/pci/msix.h | 1 +
> 5 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 3c0dc9f..2bcef7a 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -2167,6 +2167,7 @@ static void vfio_teardown_msi(VFIODevice *vdev)
> if (vdev->msix) {
> msix_uninit(&vdev->pdev, &vdev->bars[vdev->msix->table_bar].mem,
> &vdev->bars[vdev->msix->pba_bar].mem);
> + msix_free(&vdev->pdev);
> }
> }
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 5f483e7..9de2586 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1979,6 +1979,7 @@ vmxnet3_init_msix(VMXNET3State *s)
> if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
> VMW_WRPRN("Failed to use MSI-X vectors, error %d", res);
> msix_uninit(d, &s->msix_bar, &s->msix_bar);
> + msix_free(d);
> s->msix_used = false;
> } else {
> s->msix_used = true;
> @@ -1996,6 +1997,8 @@ vmxnet3_cleanup_msix(VMXNET3State *s)
> msix_vector_unuse(d, VMXNET3_MAX_INTRS);
> msix_uninit(d, &s->msix_bar, &s->msix_bar);
> }
> +
> + msix_free(d);
> }
>
> #define VMXNET3_MSI_NUM_VECTORS (1)
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index 6da75ec..be96b06 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -274,6 +274,10 @@ int msix_init(struct PCIDevice *dev, unsigned short
> nentries,
> dev->wmask[cap + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK |
> MSIX_MASKALL_MASK;
>
> + if (dev->msix_table || dev->msix_pba || dev->msix_entry_used) {
> + msix_free(dev);
> + }
> +
> dev->msix_table = g_malloc0(table_size);
> dev->msix_pba = g_malloc0(pba_size);
> dev->msix_entry_used = g_malloc0(nentries * sizeof
> *dev->msix_entry_used);
Wow msix_init calls msix_free, and not on error path?
What's going on here?
> @@ -359,16 +363,26 @@ void msix_uninit(PCIDevice *dev, MemoryRegion
> *table_bar, MemoryRegion *pba_bar)
> msix_free_irq_entries(dev);
> dev->msix_entries_nr = 0;
> memory_region_del_subregion(pba_bar, &dev->msix_pba_mmio);
> - memory_region_destroy(&dev->msix_pba_mmio);
> - g_free(dev->msix_pba);
> - dev->msix_pba = NULL;
> memory_region_del_subregion(table_bar, &dev->msix_table_mmio);
> - memory_region_destroy(&dev->msix_table_mmio);
> - g_free(dev->msix_table);
> + dev->cap_present &= ~QEMU_PCI_CAP_MSIX;
> +}
> +
> +void msix_free(PCIDevice *dev)
> +{
> + if (dev->msix_pba) {
> + memory_region_destroy(&dev->msix_pba_mmio);
> + g_free(dev->msix_pba);
> + }
> + dev->msix_pba = NULL;
> +
> + if (dev->msix_table) {
> + memory_region_destroy(&dev->msix_table_mmio);
> + g_free(dev->msix_table);
> + }
> dev->msix_table = NULL;
> +
> g_free(dev->msix_entry_used);
> dev->msix_entry_used = NULL;
> - dev->cap_present &= ~QEMU_PCI_CAP_MSIX;
> }
>
> void msix_uninit_exclusive_bar(PCIDevice *dev)
As long as we had init and uninit, it was mostly
self-documenting.
Now, there are two cleanup functions, so please add documentation.
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 444b71a..cbfec6b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -998,6 +998,7 @@ static void virtio_pci_exit(PCIDevice *pci_dev)
> virtio_pci_stop_ioeventfd(proxy);
> memory_region_destroy(&proxy->bar);
> msix_uninit_exclusive_bar(pci_dev);
> + msix_free(pci_dev);
> }
>
> static void virtio_pci_reset(DeviceState *qdev)
> diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
> index 954d82b..34e3ba2 100644
> --- a/include/hw/pci/msix.h
> +++ b/include/hw/pci/msix.h
> @@ -18,6 +18,7 @@ void msix_write_config(PCIDevice *dev, uint32_t address,
> uint32_t val, int len);
> void msix_uninit(PCIDevice *dev, MemoryRegion *table_bar,
> MemoryRegion *pba_bar);
> void msix_uninit_exclusive_bar(PCIDevice *dev);
> +void msix_free(PCIDevice *dev);
>
> unsigned int msix_nr_vectors_allocated(const PCIDevice *dev);
>
> --
> 1.8.1.4
>
- [Qemu-devel] [PATCH 03/39] pci: split exit and finalize, (continued)
- [Qemu-devel] [PATCH 03/39] pci: split exit and finalize, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 04/39] ac97: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 05/39] es1370: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 06/39] hda: split exit and instance_finalize, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 07/39] serial: split exit and instance_finalize, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 08/39] tpci200: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 09/39] pci-assign: split exit and instance_finalize, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 10/39] ahci: split exit and instance_finalize, Paolo Bonzini, 2013/06/04
- [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Paolo Bonzini, 2013/06/04
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Paolo Bonzini, 2013/06/04
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Michael S. Tsirkin, 2013/06/05
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Paolo Bonzini, 2013/06/05
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Michael S. Tsirkin, 2013/06/05
- Re: [Qemu-devel] [PATCH 11/39] msix: split msix_free from msix_uninit, Paolo Bonzini, 2013/06/06
[Qemu-devel] [PATCH 12/39] cmd646: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
[Qemu-devel] [PATCH 13/39] ide/piix: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
[Qemu-devel] [PATCH 14/39] ide/via: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04
[Qemu-devel] [PATCH 15/39] ivshmem: split exit and instance_finalize, Paolo Bonzini, 2013/06/04
[Qemu-devel] [PATCH 16/39] pci-testdev: use instance_finalize instead of exit, Paolo Bonzini, 2013/06/04