qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qemu v8 03/14] spapr_pci_vfio: Enable multiple g

From: Alexey Kardashevskiy
Subject: Re: [Qemu-devel] [PATCH qemu v8 03/14] spapr_pci_vfio: Enable multiple groups per container
Date: Tue, 30 Jun 2015 13:32:45 +1000
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 
On 06/26/2015 05:59 AM, Alex Williamson wrote:

On Thu, 2015-06-18 at 21:37 +1000, Alexey Kardashevskiy wrote:

This enables multiple IOMMU groups in one VFIO container which means
that multiple devices from different groups can share the same IOMMU
table (or tables if DDW).

This removes a group id from vfio_container_ioctl(). The kernel support
is required for this; if the host kernel does not have the support,
it will allow only one group per container. The PHB's "iommuid" property
is ignored. The ioctl is called for every container attached to
the address space. At the moment there is just one container anyway.

If there is no container attached to the address space,
vfio_container_do_ioctl() returns -1.

This removes casts to sPAPRPHBVFIOState as none of sPAPRPHBVFIOState
members is accessed here.

Signed-off-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: David Gibson <address@hidden>
---
  hw/ppc/spapr_pci_vfio.c | 21 ++++++---------------
  hw/vfio/common.c        | 20 ++++++--------------
  include/hw/vfio/vfio.h  |  2 +-
  3 files changed, 13 insertions(+), 30 deletions(-)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 99a1be5..e89cbff 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -35,12 +35,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState 
*sphb, Error **errp)
      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,
+    ret = vfio_container_ioctl(&svphb->phb.iommu_as,
                                 VFIO_CHECK_EXTENSION,
                                 (void *) VFIO_SPAPR_TCE_IOMMU);
      if (ret != 1) {
@@ -49,7 +44,7 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState 
*sphb, Error **errp)
          return;
      }

-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+    ret = vfio_container_ioctl(&sphb->iommu_as,
                                 VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
      if (ret) {
          error_setg_errno(errp, -ret,
@@ -79,7 +74,6 @@ static void spapr_phb_vfio_reset(DeviceState *qdev)
  static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState *sphb,
                                           unsigned int addr, int option)
  {
-    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
      struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
      int ret;

@@ -116,7 +110,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState 
*sphb,
          return RTAS_OUT_PARAM_ERROR;
      }

-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+    ret = vfio_container_ioctl(&sphb->iommu_as,
                                 VFIO_EEH_PE_OP, &op);
      if (ret < 0) {
          return RTAS_OUT_HW_ERROR;
@@ -127,12 +121,11 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState 
*sphb,

  static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState *sphb, int *state)
  {
-    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
      struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
      int ret;

      op.op = VFIO_EEH_PE_GET_STATE;
-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+    ret = vfio_container_ioctl(&sphb->iommu_as,
                                 VFIO_EEH_PE_OP, &op);
      if (ret < 0) {
          return RTAS_OUT_PARAM_ERROR;
@@ -144,7 +137,6 @@ static int spapr_phb_vfio_eeh_get_state(sPAPRPHBState 
*sphb, int *state)

  static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, int option)
  {
-    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
      struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
      int ret;

@@ -162,7 +154,7 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, 
int option)
          return RTAS_OUT_PARAM_ERROR;
      }

-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+    ret = vfio_container_ioctl(&sphb->iommu_as,
                                 VFIO_EEH_PE_OP, &op);
      if (ret < 0) {
          return RTAS_OUT_HW_ERROR;
@@ -173,12 +165,11 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb, 
int option)

  static int spapr_phb_vfio_eeh_configure(sPAPRPHBState *sphb)
  {
-    sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
      struct vfio_eeh_pe_op op = { .argsz = sizeof(op) };
      int ret;

      op.op = VFIO_EEH_PE_CONFIGURE;
-    ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
+    ret = vfio_container_ioctl(&sphb->iommu_as,
                                 VFIO_EEH_PE_OP, &op);
      if (ret < 0) {
          return RTAS_OUT_PARAM_ERROR;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 3e4c685..369e564 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -793,34 +793,26 @@ void vfio_put_base_device(VFIODevice *vbasedev)
      close(vbasedev->fd);
  }

-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
+static int vfio_container_do_ioctl(AddressSpace *as,
                                     int req, void *param)
  {
-    VFIOGroup *group;
      VFIOContainer *container;
      int ret = -1;
+    VFIOAddressSpace *space = vfio_get_address_space(as);

-    group = vfio_get_group(groupid, as);
-    if (!group) {
-        error_report("vfio: group %d not registered", groupid);
-        return ret;
-    }
-
-    container = group->container;
-    if (group->container) {
+    QLIST_FOREACH(container, &space->containers, next) {
          ret = ioctl(container->fd, req, param);
          if (ret < 0) {
              error_report("vfio: failed to ioctl %d to container: ret=%d, %s",
                           _IOC_NR(req) - VFIO_BASE, ret, strerror(errno));
+            return -errno;
          }
      }



How does this work with VFIO_EEH_PE_OP?  Suddenly we're potentially
calling it on multiple containers?


Gavin says yes.



Should all of this container ioctl
code also be pulled out to spapr?


Well, I could but I won't as I am dropping 2/14 at all.



There's clearly an approach here where the patches could be split into
separate ppc and vfio series rather than this horrible practice of
intermixing them.  Thanks,
Oh. Ok. I'll move 3/14 further. I just wanted to put 2/14 and 3/14 earlier as it allowed me to avoid changing same lines twice in the same patchset. 



Alex


-    vfio_put_group(group);
-
      return ret;
  }

-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+int vfio_container_ioctl(AddressSpace *as,
                           int req, void *param)
  {
      /* We allow only certain ioctls to the container */
@@ -835,5 +827,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
          return -1;
      }

-    return vfio_container_do_ioctl(as, groupid, req, param);
+    return vfio_container_do_ioctl(as, req, param);
  }
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
index 0b26cd8..76b5744 100644
--- a/include/hw/vfio/vfio.h
+++ b/include/hw/vfio/vfio.h
@@ -3,7 +3,7 @@

  #include "qemu/typedefs.h"

-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+extern int vfio_container_ioctl(AddressSpace *as,
                                  int req, void *param);

  #endif







--
Alexey
reply via email to
[Prev in Thread] Current Thread [Next in Thread]