qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qemu v10 03/14] spapr_pci: Convert finish_realiz


From: Alexey Kardashevskiy
Subject: Re: [Qemu-devel] [PATCH qemu v10 03/14] spapr_pci: Convert finish_realize() to dma_capabilities_update()+dma_init_window()
Date: Tue, 7 Jul 2015 10:28:42 +1000
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1

On 07/07/2015 02:41 AM, Laurent Vivier wrote:


On 06/07/2015 04:10, Alexey Kardashevskiy wrote:
This reworks finish_realize() which used to finalize DMA setup with
an assumption that it will not change later.

New callbacks supports various window parameters such as page and
windows sizes. The new callback return error code rather than Error**.

This is a mechanical change so no change in behaviour is expected.
This is a part of getting rid of spapr-pci-vfio-host-bridge type.

Signed-off-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: David Gibson <address@hidden>
---
Changes:
v8:
* moved spapr_phb_dma_capabilities_update() higher to avoid forward
declaration in following patches and keep DMA code together (i.e. next
to spapr_pci_dma_iommu())
---
  hw/ppc/spapr_pci.c          | 59 ++++++++++++++++++++++++++-------------------
  hw/ppc/spapr_pci_vfio.c     | 53 ++++++++++++++++------------------------
  include/hw/pci-host/spapr.h |  8 +++++-
  3 files changed, 62 insertions(+), 58 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a8f79d8..c1ca13d 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -808,6 +808,28 @@ static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb, 
PCIDevice *pdev)
      return buf;
  }

+static int spapr_phb_dma_capabilities_update(sPAPRPHBState *sphb)
+{
+    sphb->dma32_window_start = 0;
+    sphb->dma32_window_size = SPAPR_PCI_DMA32_SIZE;
+
+    return 0;
+}
+
+static int spapr_phb_dma_init_window(sPAPRPHBState *sphb,
+                                     uint32_t liobn, uint32_t page_shift,
+                                     uint64_t window_size)
+{
+    uint64_t bus_offset = sphb->dma32_window_start;
+    sPAPRTCETable *tcet;
+
+    tcet = spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_shift,
+                               window_size >> page_shift,
+                               false);
+
+    return tcet ? 0 : -1;
+}
+
  /* Macros to operate with address in OF binding to PCI */
  #define b_x(x, p, l)    (((x) & ((1<<(l))-1)) << (p))
  #define b_n(x)          b_x((x), 31, 1) /* 0 if relocatable */
@@ -1220,6 +1242,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
      int i;
      PCIBus *bus;
      uint64_t msi_window_size = 4096;
+    sPAPRTCETable *tcet;

      if (sphb->index != (uint32_t)-1) {
          hwaddr windows_base;
@@ -1369,33 +1392,18 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
          }
      }

-    if (!info->finish_realize) {
-        error_setg(errp, "finish_realize not defined");
-        return;
-    }
-
-    info->finish_realize(sphb, errp);
-
-    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
-}
-
-static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
-{
-    sPAPRTCETable *tcet;
-    uint32_t nb_table;
-
-    nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
-    tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
-                               0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
+    info->dma_capabilities_update(sphb);
+    info->dma_init_window(sphb, sphb->dma_liobn, SPAPR_TCE_PAGE_SHIFT,
+                          sphb->dma32_window_size);
+    tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
      if (!tcet) {
-        error_setg(errp, "Unable to create TCE table for %s",
-                   sphb->dtbusname);
-        return ;
+        error_setg(errp, "failed to create TCE table");
+        return;
      }
-
-    /* Register default 32bit DMA window */
-    memory_region_add_subregion(&sphb->iommu_root, 0,
+    memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
                                  spapr_tce_get_iommu(tcet));
+
+    sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free);
  }

  static int spapr_phb_children_reset(Object *child, void *opaque)
@@ -1543,9 +1551,10 @@ static void spapr_phb_class_init(ObjectClass *klass, 
void *data)
      dc->vmsd = &vmstate_spapr_pci;
      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
      dc->cannot_instantiate_with_device_add_yet = false;
-    spc->finish_realize = spapr_phb_finish_realize;
      hp->plug = spapr_phb_hot_plug_child;
      hp->unplug = spapr_phb_hot_unplug_child;
+    spc->dma_capabilities_update = spapr_phb_dma_capabilities_update;
+    spc->dma_init_window = spapr_phb_dma_init_window;
  }

  static const TypeInfo spapr_phb_info = {
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index cca45ed..6e3e17b 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -28,48 +28,36 @@ static Property spapr_phb_vfio_properties[] = {
      DEFINE_PROP_END_OF_LIST(),
  };

-static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
+static int spapr_phb_vfio_dma_capabilities_update(sPAPRPHBState *sphb)
  {
      sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
      struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
      int ret;
-    sPAPRTCETable *tcet;
-    uint32_t liobn = svphb->phb.dma_liobn;

-    if (svphb->iommugroupid == -1) {
-        error_setg(errp, "Wrong IOMMU group ID %d", svphb->iommugroupid);
-        return;
-    }
-
-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
-                               VFIO_CHECK_EXTENSION,
-                               (void *) VFIO_SPAPR_TCE_IOMMU);


This ioctl() disappears completely, was it useless ?


It is called in hw/vfio/common.c and if it fails there, we never get here so there is no point in calling it here too.





--
Alexey



reply via email to

[Prev in Thread] Current Thread [Next in Thread]