qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v2 2/3] hw/iommu: enable iommu with -device


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH v2 2/3] hw/iommu: enable iommu with -device
Date: Sun, 5 Jun 2016 11:46:13 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0

On 06/03/2016 07:07 PM, Michael S. Tsirkin wrote:
On Thu, Jun 02, 2016 at 11:15:54PM +0300, Marcel Apfelbaum wrote:
Use the standard '-device iommu' instead of '-machine,iommu=on'
to create the IOMMU device.

Signed-off-by: Marcel Apfelbaum <address@hidden>


Hi Michael,
Thank you for the review.

Why can't we keep support for the old flag?


We can, but IMO we don't need it for several reasons:

The current vIOMMU before the fantastic work of Aviv and Peter
is not really usable, is there only as a "lab" feature with no
clear interesting scenario.

If we keep it, we should also support the "x-iommu-type" for
AMD IOMMU, so we add "legacy" code we don't want.

It is easy to add additional options with -device,
but how will we add them to -machine,iommu=on? an extra machine option?

Finally, if we do have current users, asking them for a minimum command line
change is not such a big deal.

Thanks,
Marcel

---
  hw/core/machine.c     | 20 --------------------
  hw/i386/intel_iommu.c | 17 +++++++++++++++++
  hw/pci-host/q35.c     | 28 ----------------------------
  qemu-options.hx       |  3 ---
  4 files changed, 17 insertions(+), 51 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index ccdd5fa..8f94301 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -300,20 +300,6 @@ static void machine_set_firmware(Object *obj, const char 
*value, Error **errp)
      ms->firmware = g_strdup(value);
  }

-static bool machine_get_iommu(Object *obj, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    return ms->iommu;
-}
-
-static void machine_set_iommu(Object *obj, bool value, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    ms->iommu = value;
-}
-
  static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp)
  {
      MachineState *ms = MACHINE(obj);
@@ -493,12 +479,6 @@ static void machine_initfn(Object *obj)
      object_property_set_description(obj, "firmware",
                                      "Firmware image",
                                      NULL);
-    object_property_add_bool(obj, "iommu",
-                             machine_get_iommu,
-                             machine_set_iommu, NULL);
-    object_property_set_description(obj, "iommu",
-                                    "Set on/off to enable/disable Intel IOMMU 
(VT-d)",
-                                    NULL);
      object_property_add_bool(obj, "suppress-vmdesc",
                               machine_get_suppress_vmdesc,
                               machine_set_suppress_vmdesc, NULL);
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 347718f..9af5d6b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -24,6 +24,8 @@
  #include "exec/address-spaces.h"
  #include "intel_iommu_internal.h"
  #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/i386/pc.h"

  /*#define DEBUG_INTEL_IOMMU*/
  #ifdef DEBUG_INTEL_IOMMU
@@ -2014,8 +2016,20 @@ static void vtd_reset(DeviceState *dev)
      vtd_init(s);
  }

+static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
+{
+    IntelIOMMUState *s = opaque;
+    VTDAddressSpace *vtd_as;
+
+    assert(0 <= devfn && devfn <= VTD_PCI_DEVFN_MAX);
+
+    vtd_as = vtd_find_add_as(s, bus, devfn);
+    return &vtd_as->as;
+}
+
  static void vtd_realize(DeviceState *dev, Error **errp)
  {
+    PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus;
      IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);

      VTD_DPRINTF(GENERAL, "");
@@ -2029,6 +2043,9 @@ static void vtd_realize(DeviceState *dev, Error **errp)
      s->vtd_as_by_busptr = g_hash_table_new_full(vtd_uint64_hash, 
vtd_uint64_equal,
                                                g_free, g_free);
      vtd_init(s);
+    sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
+    bus->iommu_fn = vtd_host_dma_iommu;
+    bus->iommu_opaque = dev;
  }

  static void vtd_class_init(ObjectClass *klass, void *data)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 70f897e..ea684c7 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -424,30 +424,6 @@ static void mch_reset(DeviceState *qdev)
      mch_update(mch);
  }

-static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
-{
-    IntelIOMMUState *s = opaque;
-    VTDAddressSpace *vtd_as;
-
-    assert(0 <= devfn && devfn <= VTD_PCI_DEVFN_MAX);
-
-    vtd_as = vtd_find_add_as(s, bus, devfn);
-    return &vtd_as->as;
-}
-
-static void mch_init_dmar(MCHPCIState *mch)
-{
-    PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch)));
-
-    mch->iommu = INTEL_IOMMU_DEVICE(qdev_create(NULL, 
TYPE_INTEL_IOMMU_DEVICE));
-    object_property_add_child(OBJECT(mch), "intel-iommu",
-                              OBJECT(mch->iommu), NULL);
-    qdev_init_nofail(DEVICE(mch->iommu));
-    sysbus_mmio_map(SYS_BUS_DEVICE(mch->iommu), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
-
-    pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch->iommu);
-}
-
  static void mch_realize(PCIDevice *d, Error **errp)
  {
      int i;
@@ -506,10 +482,6 @@ static void mch_realize(PCIDevice *d, Error **errp)
                   mch->pci_address_space, &mch->pam_regions[i+1],
                   PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
      }
-    /* Intel IOMMU (VT-d) */
-    if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) {
-        mch_init_dmar(mch);
-    }
  }

  uint64_t mch_mcfg_base(void)
diff --git a/qemu-options.hx b/qemu-options.hx
index 6106520..2953baf 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -38,7 +38,6 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
      "                kvm_shadow_mem=size of KVM shadow MMU\n"
      "                dump-guest-core=on|off include guest memory in a core dump 
(default=on)\n"
      "                mem-merge=on|off controls memory merge support (default: 
on)\n"
-    "                iommu=on|off controls emulated Intel IOMMU (VT-d) support 
(default=off)\n"
      "                igd-passthru=on|off controls IGD GFX passthrough support 
(default=off)\n"
      "                aes-key-wrap=on|off controls support for AES key wrapping 
(default=on)\n"
      "                dea-key-wrap=on|off controls support for DEA key wrapping 
(default=on)\n"
@@ -73,8 +72,6 @@ Include guest memory in a core dump. The default is on.
  Enables or disables memory merge support. This feature, when supported by
  the host, de-duplicates identical memory pages among VMs instances
  (enabled by default).
address@hidden iommu=on|off
-Enables or disables emulated Intel IOMMU (VT-d) support. The default is off.
  @item aes-key-wrap=on|off
  Enables or disables AES key wrapping support on s390-ccw hosts. This feature
  controls whether AES wrapping keys will be created to allow
--
2.4.3




reply via email to

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