qemu-ppc
[Top][All Lists]
Advanced

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

Re: [Qemu-ppc] [Qemu-devel] [PATCH qemu v10 04/14] spapr_iommu: Move tab


From: Alexey Kardashevskiy
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH qemu v10 04/14] spapr_iommu: Move table allocation to helpers
Date: Tue, 7 Jul 2015 01:43: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 01:14 AM, Thomas Huth wrote:
On Mon,  6 Jul 2015 12:11:00 +1000
Alexey Kardashevskiy <address@hidden> wrote:

At the moment presence of vfio-pci devices on a bus affect the way
the guest view table is allocated. If there is no vfio-pci on a PHB
and the host kernel supports KVM acceleration of H_PUT_TCE, a table
is allocated in KVM. However, if there is vfio-pci and we do yet not
KVM acceleration for these, the table has to be allocated by
the userspace. At the moment the table is allocated once at boot time
but next patches will reallocate it.

This moves kvmppc_create_spapr_tce/g_malloc0 and their counterparts
to helpers.

Signed-off-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: David Gibson <address@hidden>
---
  hw/ppc/spapr_iommu.c | 58 +++++++++++++++++++++++++++++++++++-----------------
  trace-events         |  2 +-
  2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index f61504e..0cf5010 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -74,6 +74,37 @@ static IOMMUAccessFlags 
spapr_tce_iommu_access_flags(uint64_t tce)
      }
  }

+static uint64_t *spapr_tce_alloc_table(uint32_t liobn,
+                                       uint32_t nb_table,
+                                       uint32_t page_shift,
+                                       int *fd,
+                                       bool vfio_accel)
+{
+    uint64_t *table = NULL;
+    uint64_t window_size = (uint64_t)nb_table << page_shift;
+
+    if (kvm_enabled() && !(window_size >> 32)) {
+        table = kvmppc_create_spapr_tce(liobn, window_size, fd, vfio_accel);
+    }
+
+    if (!table) {
+        *fd = -1;
+        table = g_malloc0(nb_table * sizeof(uint64_t));
+    }
+
+    trace_spapr_iommu_alloc_table(liobn, table, *fd);
+
+    return table;
+}
+
+static void spapr_tce_free_table(uint64_t *table, int fd, uint32_t nb_table)
+{
+    if (!kvm_enabled() ||
+        (kvmppc_remove_spapr_tce(table, fd, nb_table) != 0)) {
+        g_free(table);
+    }
+}
+
  /* Called from RCU critical section */
  static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr 
addr,
                                                 bool is_write)
@@ -140,21 +171,13 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
  static int spapr_tce_table_realize(DeviceState *dev)
  {
      sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
-    uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;

-    if (kvm_enabled() && !(window_size >> 32)) {
-        tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
-                                              window_size,
-                                              &tcet->fd,
-                                              tcet->vfio_accel);
-    }
-
-    if (!tcet->table) {
-        size_t table_size = tcet->nb_table * sizeof(uint64_t);
-        tcet->table = g_malloc0(table_size);
-    }
-
-    trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
+    tcet->fd = -1;

As far as I can see, spapr_tce_alloc_table() always initialized the fd
to -1 in case the allocation failed, so you can drop the above line, I
think.

Later in the patchset I remove spapr_tce_alloc_table() so it is safer to initialize it here, I guess.


+    tcet->table = spapr_tce_alloc_table(tcet->liobn,
+                                        tcet->nb_table,
+                                        tcet->page_shift,
+                                        &tcet->fd,
+                                        tcet->vfio_accel);

      memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
                               "iommu-spapr",

Apart from the nit above, the patch looks fine to me.

  Thomas



--
Alexey



reply via email to

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