[Qemu-devel] [PATCH V5 0/5] enable iommu with -device

[Qemu-devel] [PATCH V5 0/5] enable iommu with -device

[Marcel Apfelbaum]

Create the iommu device with '-device intel-iommu' instead of '-machine,iommu=on'.

The device is part of the machine properties because we wanted
to ensure is created before any other PCI device.

The alternative is to skip the bus_master_enable_region at
the time the device is created. We can create this region
at machine_done phase. (patch 2)

Then we need to enable sysbus devices(*) for PC machines (patch 3),
since intel-iommu is a sysbus device.

Patch 4 moves the IOMMU init proces into iommu's realize function
and allows the device creation in both ways.

Finally patch 5 removes the iommu machine property.

v4->v5:
  - patch 1/5 - new patch solving make check mac99 issue. (Michael)
  - patch 4/5 - fix commit message (Paolo)

v3 -> v4:
  - Rebased on mst/pci tree (Michael).

v2 -> v3:
  - Add machine_done notifier in pci_bus realize and remove it unrealize (Paolo).
  - Add comments for 'cannot_instantiate_with_device_add_yet' (Markus).
  - Split adding the -device iommu support and removing the iommu machine property (Michael).
  - Use pci_setup_iommu as before (Peter)
  - Mark intel-iommu as not hot-pluggable (Peter)
  - Rebased on master

v1 -> v2:
  - Enable bus_master also on init if the guest OS already booted to enable hotplug (Paolo).
  - Add a machine_done notifier to PCIBus instead of adding functionality
    for q35 machine_done callback. The main reason is we don't want to replicate
    the code for all platforms that support PCI and is also cleaner this way.
  - Added 'cannot_instantiate_with_device_add_yet' to sysbus devices that lead
    to crashes if added with -device.
  - Rebased on master


(*) Creates a new problem since we have now a bunch of
    new devices that can be created with -device on Q35:
	name "q35-pcihost", bus System
	name "sysbus-ohci", bus System, desc "OHCI USB Controller"
	name "allwinner-ahci", bus System
	name "cfi.pflash01", bus System
	name "esp", bus System
	name "SUNW,fdtwo", bus System
	name "sysbus-ahci", bus System
	name "sysbus-fdc", bus System
	name "vfio-amd-xgbe", bus System, desc "VFIO AMD XGBE"
	name "vfio-calxeda-xgmac", bus System, desc "VFIO Calxeda XGMAC"
	name "virtio-mmio", bus System
	name "fw_cfg", bus System
	name "fw_cfg_io", bus System
	name "fw_cfg_mem", bus System
	name "generic-sdhci", bus System
	name "hpet", bus System
	name "i440FX-pcihost", bus System
	name "intel-iommu", bus System
	name "ioapic", bus System
	name "isabus-bridge", bus System
	name "kvm-ioapic", bus System
	name "kvmclock", bus System
	name "kvmvapic", bus System
	name "pxb-host", bus System

Took care of the ones creating immediate issues (like crashes) by marking them
as 'cannot_instantiate_with_device_add_yet'. I didn't mark them all because:
  - libvirt will mask them anyway
  - some of them have already a "protection" in place
  - it is possible that some of them can be actually used with -device on other platform. 
  - those are not 'interesting' scenarios.
If somebody spots devices in the list that cannot be added with -device on any platform
please let me know and I'll mark them.


Thanks,
Marcel

Marcel Apfelbaum (5):
  hw/ppc: realize the PCI root bus as part of mac99 init
  hw/pci: delay bus_master_enable_region initialization
  q35: allow dynamic sysbus
  hw/iommu: enable iommu with -device
  machine: remove iommu property

 hw/core/machine.c                   | 20 ------------------
 hw/i386/intel_iommu.c               | 16 +++++++++++++++
 hw/i386/pc_q35.c                    |  2 +-
 hw/pci-bridge/pci_expander_bridge.c |  2 ++
 hw/pci-host/piix.c                  |  2 ++
 hw/pci-host/q35.c                   | 31 +++-------------------------
 hw/pci/pci.c                        | 41 +++++++++++++++++++++++++++++--------
 hw/ppc/mac_newworld.c               |  1 +
 include/hw/pci-host/q35.h           |  1 -
 include/hw/pci/pci_bus.h            |  2 ++
 include/sysemu/sysemu.h             |  1 +
 qemu-options.hx                     |  3 ---
 vl.c                                |  5 +++++
 13 files changed, 65 insertions(+), 62 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH V5 1/5] hw/ppc: realize the PCI root bus as part of mac99 init

[Marcel Apfelbaum]

Mac99's PCI root bus is not part of a host bridge,
realize it manually.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/ppc/mac_newworld.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 32e88b3..7d25106 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -380,6 +380,7 @@ static void ppc_core99_init(MachineState *machine)
         pci_bus = pci_pmac_init(pic, get_system_memory(), get_system_io());
         machine_arch = ARCH_MAC99;
     }
+    object_property_set_bool(OBJECT(pci_bus), true, "realized", &error_abort);
 
     machine->usb |= defaults_enabled() && !machine->usb_disabled;
 
-- 
2.4.3

[Qemu-devel] [PATCH V5 2/5] hw/pci: delay bus_master_enable_region initialization

[Marcel Apfelbaum]

Skip bus_master_enable region creation on PCI device init
in order to be sure the IOMMU device (if present) would
be created in advance. Add this memory region at machine_done time.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci/pci.c             | 41 ++++++++++++++++++++++++++++++++---------
 include/hw/pci/pci_bus.h |  2 ++
 include/sysemu/sysemu.h  |  1 +
 vl.c                     |  5 +++++
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4b585f4..ee385eb 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -78,10 +78,37 @@ static const VMStateDescription vmstate_pcibus = {
     }
 };
 
+static void pci_init_bus_master(PCIDevice *pci_dev)
+{
+    AddressSpace *dma_as = pci_device_iommu_address_space(pci_dev);
+
+    memory_region_init_alias(&pci_dev->bus_master_enable_region,
+                             OBJECT(pci_dev), "bus master",
+                             dma_as->root, 0, memory_region_size(dma_as->root));
+    memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
+    address_space_init(&pci_dev->bus_master_as,
+                       &pci_dev->bus_master_enable_region, pci_dev->name);
+}
+
+static void pcibus_machine_done(Notifier *notifier, void *data)
+{
+    PCIBus *bus = container_of(notifier, PCIBus, machine_done);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
+        if (bus->devices[i]) {
+            pci_init_bus_master(bus->devices[i]);
+        }
+    }
+}
+
 static void pci_bus_realize(BusState *qbus, Error **errp)
 {
     PCIBus *bus = PCI_BUS(qbus);
 
+    bus->machine_done.notify = pcibus_machine_done;
+    qemu_add_machine_init_done_notifier(&bus->machine_done);
+
     vmstate_register(NULL, -1, &vmstate_pcibus, bus);
 }
 
@@ -89,6 +116,8 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
 {
     PCIBus *bus = PCI_BUS(qbus);
 
+    qemu_remove_machine_init_done_notifier(&bus->machine_done);
+
     vmstate_unregister(NULL, &vmstate_pcibus, bus);
 }
 
@@ -920,7 +949,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     PCIConfigReadFunc *config_read = pc->config_read;
     PCIConfigWriteFunc *config_write = pc->config_write;
     Error *local_err = NULL;
-    AddressSpace *dma_as;
     DeviceState *dev = DEVICE(pci_dev);
 
     pci_dev->bus = bus;
@@ -961,15 +989,10 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
 
     pci_dev->devfn = devfn;
     pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);
-    dma_as = pci_device_iommu_address_space(pci_dev);
-
-    memory_region_init_alias(&pci_dev->bus_master_enable_region,
-                             OBJECT(pci_dev), "bus master",
-                             dma_as->root, 0, memory_region_size(dma_as->root));
-    memory_region_set_enabled(&pci_dev->bus_master_enable_region, false);
-    address_space_init(&pci_dev->bus_master_as, &pci_dev->bus_master_enable_region,
-                       name);
 
+    if (qdev_hotplug) {
+        pci_init_bus_master(pci_dev);
+    }
     pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
     pci_dev->irq_state = 0;
     pci_config_alloc(pci_dev);
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 403fec6..5484a9b 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -39,6 +39,8 @@ struct PCIBus {
        Keep a count of the number of devices with raised IRQs.  */
     int nirq;
     int *irq_count;
+
+    Notifier machine_done;
 };
 
 typedef struct PCIBridgeWindows PCIBridgeWindows;
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 7313673..ee7c760 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -75,6 +75,7 @@ void qemu_add_exit_notifier(Notifier *notify);
 void qemu_remove_exit_notifier(Notifier *notify);
 
 void qemu_add_machine_init_done_notifier(Notifier *notify);
+void qemu_remove_machine_init_done_notifier(Notifier *notify);
 
 void hmp_savevm(Monitor *mon, const QDict *qdict);
 int load_vmstate(const char *name);
diff --git a/vl.c b/vl.c
index c85833a..90189b9 100644
--- a/vl.c
+++ b/vl.c
@@ -2692,6 +2692,11 @@ void qemu_add_machine_init_done_notifier(Notifier *notify)
     }
 }
 
+void qemu_remove_machine_init_done_notifier(Notifier *notify)
+{
+    notifier_remove(notify);
+}
+
 static void qemu_run_machine_init_done_notifiers(void)
 {
     notifier_list_notify(&machine_init_done_notifiers, NULL);
-- 
2.4.3

[Qemu-devel] [PATCH V5 3/5] q35: allow dynamic sysbus

[Marcel Apfelbaum]

Allow adding sysbus devices with -device on Q35.

At first Q35 will support only intel-iommu to be added this way,
however the command line will support all sysbus devices.

Mark with 'cannot_instantiate_with_device_add_yet' the ones
causing immediate problems (e.g. crashes).

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/pc_q35.c                    | 1 +
 hw/pci-bridge/pci_expander_bridge.c | 2 ++
 hw/pci-host/piix.c                  | 2 ++
 hw/pci-host/q35.c                   | 2 ++
 4 files changed, 7 insertions(+)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 97a8835..2ab62ee 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -281,6 +281,7 @@ static void pc_q35_machine_options(MachineClass *m)
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
     m->no_floppy = 1;
+    m->has_dynamic_sysbus = true;
 }
 
 static void pc_q35_2_7_machine_options(MachineClass *m)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index ba320bd..ab86121 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -149,6 +149,8 @@ static void pxb_host_class_init(ObjectClass *class, void *data)
     PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
 
     dc->fw_name = "pci";
+    /* Reason: Internal part of the pxb/pxb-pcie device, not usable by itself */
+    dc->cannot_instantiate_with_device_add_yet = true;
     sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address;
     hc->root_bus_path = pxb_host_root_bus_path;
 }
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index df2b0e2..7167e58 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -865,6 +865,8 @@ static void i440fx_pcihost_class_init(ObjectClass *klass, void *data)
     dc->realize = i440fx_pcihost_realize;
     dc->fw_name = "pci";
     dc->props = i440fx_props;
+    /* Reason: needs to be wired up by pc_init1 */
+    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo i440fx_pcihost_info = {
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 70f897e..141ba5b 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -138,6 +138,8 @@ static void q35_host_class_init(ObjectClass *klass, void *data)
     hc->root_bus_path = q35_host_root_bus_path;
     dc->realize = q35_host_realize;
     dc->props = mch_props;
+    /* Reason: needs to be wired up by pc_q35_init */
+    dc->cannot_instantiate_with_device_add_yet = true;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
     dc->fw_name = "pci";
 }
-- 
2.4.3

[Qemu-devel] [PATCH V5 4/5] hw/iommu: enable iommu with -device

[Marcel Apfelbaum]

Use the standard '-device intel-iommu' to create the IOMMU device.
The legacy '-machine,iommu=on' can still be used.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/i386/intel_iommu.c | 16 ++++++++++++++++
 hw/i386/pc_q35.c      |  1 -
 hw/pci-host/q35.c     | 17 +----------------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 347718f..ea25585 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -24,6 +24,7 @@
 #include "exec/address-spaces.h"
 #include "intel_iommu_internal.h"
 #include "hw/pci/pci.h"
+#include "hw/i386/pc.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -2014,8 +2015,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 +2042,8 @@ 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);
+    pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
 }
 
 static void vtd_class_init(ObjectClass *klass, void *data)
@@ -2039,6 +2054,7 @@ static void vtd_class_init(ObjectClass *klass, void *data)
     dc->realize = vtd_realize;
     dc->vmsd = &vtd_vmstate;
     dc->props = vtd_properties;
+    dc->hotpluggable = false;
 }
 
 static const TypeInfo vtd_info = {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2ab62ee..6b33591 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -169,7 +169,6 @@ static void pc_q35_init(MachineState *machine)
     qdev_init_nofail(DEVICE(q35_host));
     phb = PCI_HOST_BRIDGE(q35_host);
     host_bus = phb->bus;
-    pcms->bus = phb->bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,
                                           ICH9_LPC_FUNC), true,
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 141ba5b..4bd5fb5 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -52,6 +52,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
                            s->mch.pci_address_space, s->mch.address_space_io,
                            0, TYPE_PCIE_BUS);
+    PC_MACHINE(qdev_get_machine())->bus = pci->bus;
     qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
     qdev_init_nofail(DEVICE(&s->mch));
 }
@@ -426,28 +427,12 @@ 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)
-- 
2.4.3

[Qemu-devel] [PATCH V5 5/5] machine: remove iommu property

[Marcel Apfelbaum]

Since iommu devices can be created with '-device' there is
no need to keep iommu as machine and mch property.

Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/core/machine.c         | 20 --------------------
 hw/pci-host/q35.c         | 12 ------------
 include/hw/pci-host/q35.h |  1 -
 qemu-options.hx           |  3 ---
 4 files changed, 36 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/pci-host/q35.c b/hw/pci-host/q35.c
index 4bd5fb5..181bc3b 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -427,14 +427,6 @@ static void mch_reset(DeviceState *qdev)
     mch_update(mch);
 }
 
-static void mch_init_dmar(MCHPCIState *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));
-}
-
 static void mch_realize(PCIDevice *d, Error **errp)
 {
     int i;
@@ -493,10 +485,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/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index c5c073d..3dee058 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -60,7 +60,6 @@ typedef struct MCHPCIState {
     ram_addr_t above_4g_mem_size;
     uint64_t pci_hole64_size;
     uint32_t short_root_bus;
-    IntelIOMMUState *iommu;
 } MCHPCIState;
 
 typedef struct Q35PCIHost {
diff --git a/qemu-options.hx b/qemu-options.hx
index 44c658f..1d3c02e 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 in bytes\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).
-@item 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

Re: [Qemu-devel] [PATCH V5 1/5] hw/ppc: realize the PCI root bus as part of mac99 init

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

On Mon, Jun 27, 2016 at 06:38:31PM +0300, Marcel Apfelbaum wrote:
> Mac99's PCI root bus is not part of a host bridge,
> realize it manually.

Um.. how did this ever work?

> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/ppc/mac_newworld.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 32e88b3..7d25106 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -380,6 +380,7 @@ static void ppc_core99_init(MachineState *machine)
>          pci_bus = pci_pmac_init(pic, get_system_memory(), get_system_io());
>          machine_arch = ARCH_MAC99;
>      }
> +    object_property_set_bool(OBJECT(pci_bus), true, "realized", &error_abort);
>  
>      machine->usb |= defaults_enabled() && !machine->usb_disabled;
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH V5 5/5] machine: remove iommu property

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5088 bytes --]

On Mon, Jun 27, 2016 at 06:38:35PM +0300, Marcel Apfelbaum wrote:
> Since iommu devices can be created with '-device' there is
> no need to keep iommu as machine and mch property.

Doesn't this break backwards compatibility?

> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/core/machine.c         | 20 --------------------
>  hw/pci-host/q35.c         | 12 ------------
>  include/hw/pci-host/q35.h |  1 -
>  qemu-options.hx           |  3 ---
>  4 files changed, 36 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/pci-host/q35.c b/hw/pci-host/q35.c
> index 4bd5fb5..181bc3b 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -427,14 +427,6 @@ static void mch_reset(DeviceState *qdev)
>      mch_update(mch);
>  }
>  
> -static void mch_init_dmar(MCHPCIState *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));
> -}
> -
>  static void mch_realize(PCIDevice *d, Error **errp)
>  {
>      int i;
> @@ -493,10 +485,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/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
> index c5c073d..3dee058 100644
> --- a/include/hw/pci-host/q35.h
> +++ b/include/hw/pci-host/q35.h
> @@ -60,7 +60,6 @@ typedef struct MCHPCIState {
>      ram_addr_t above_4g_mem_size;
>      uint64_t pci_hole64_size;
>      uint32_t short_root_bus;
> -    IntelIOMMUState *iommu;
>  } MCHPCIState;
>  
>  typedef struct Q35PCIHost {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 44c658f..1d3c02e 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 in bytes\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).
> -@item 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

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH V5 1/5] hw/ppc: realize the PCI root bus as part of mac99 init

[Marcel Apfelbaum]

On 06/28/2016 05:56 AM, David Gibson wrote:
> On Mon, Jun 27, 2016 at 06:38:31PM +0300, Marcel Apfelbaum wrote:
>> Mac99's PCI root bus is not part of a host bridge,
>> realize it manually.
>
> Um.. how did this ever work?

Well, the only thing the PCI bus realize does is
to register the VM migration state, so only migration was affected.

However, patch 2/5 adds to the realize function bus_master initialization code
for all devices attached to the bridge.

Thanks,
Marcel

>
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>>   hw/ppc/mac_newworld.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>> index 32e88b3..7d25106 100644
>> --- a/hw/ppc/mac_newworld.c
>> +++ b/hw/ppc/mac_newworld.c
>> @@ -380,6 +380,7 @@ static void ppc_core99_init(MachineState *machine)
>>           pci_bus = pci_pmac_init(pic, get_system_memory(), get_system_io());
>>           machine_arch = ARCH_MAC99;
>>       }
>> +    object_property_set_bool(OBJECT(pci_bus), true, "realized", &error_abort);
>>
>>       machine->usb |= defaults_enabled() && !machine->usb_disabled;
>>
>

Re: [Qemu-devel] [PATCH V5 5/5] machine: remove iommu property

[Marcel Apfelbaum]

On 06/28/2016 05:57 AM, David Gibson wrote:
> On Mon, Jun 27, 2016 at 06:38:35PM +0300, Marcel Apfelbaum wrote:
>> Since iommu devices can be created with '-device' there is
>> no need to keep iommu as machine and mch property.
>
> Doesn't this break backwards compatibility?
>


Hi David,

Intel IOMMU was a kind of POC until recent development.
The new IOMMU features will require more command line options,
so even if we keep the machine property around, it can't be really used.

More on this was discussed in a prev thread.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg377385.html

Thanks,
Marcel

>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>>   hw/core/machine.c         | 20 --------------------
>>   hw/pci-host/q35.c         | 12 ------------
>>   include/hw/pci-host/q35.h |  1 -
>>   qemu-options.hx           |  3 ---
>>   4 files changed, 36 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/pci-host/q35.c b/hw/pci-host/q35.c
>> index 4bd5fb5..181bc3b 100644
>> --- a/hw/pci-host/q35.c
>> +++ b/hw/pci-host/q35.c
>> @@ -427,14 +427,6 @@ static void mch_reset(DeviceState *qdev)
>>       mch_update(mch);
>>   }
>>
>> -static void mch_init_dmar(MCHPCIState *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));
>> -}
>> -
>>   static void mch_realize(PCIDevice *d, Error **errp)
>>   {
>>       int i;
>> @@ -493,10 +485,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/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
>> index c5c073d..3dee058 100644
>> --- a/include/hw/pci-host/q35.h
>> +++ b/include/hw/pci-host/q35.h
>> @@ -60,7 +60,6 @@ typedef struct MCHPCIState {
>>       ram_addr_t above_4g_mem_size;
>>       uint64_t pci_hole64_size;
>>       uint32_t short_root_bus;
>> -    IntelIOMMUState *iommu;
>>   } MCHPCIState;
>>
>>   typedef struct Q35PCIHost {
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 44c658f..1d3c02e 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 in bytes\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).
>> -@item 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
>

Re: [Qemu-devel] [PATCH V5 1/5] hw/ppc: realize the PCI root bus as part of mac99 init

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

On Tue, Jun 28, 2016 at 11:00:18AM +0300, Marcel Apfelbaum wrote:
> On 06/28/2016 05:56 AM, David Gibson wrote:
> > On Mon, Jun 27, 2016 at 06:38:31PM +0300, Marcel Apfelbaum wrote:
> > > Mac99's PCI root bus is not part of a host bridge,
> > > realize it manually.
> > 
> > Um.. how did this ever work?
> 
> Well, the only thing the PCI bus realize does is
> to register the VM migration state, so only migration was affected.
> 
> However, patch 2/5 adds to the realize function bus_master initialization code
> for all devices attached to the bridge.

Ah, ok.  In that case, ppc portions are

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> 
> Thanks,
> Marcel
> 
> > 
> > > 
> > > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> > > ---
> > >   hw/ppc/mac_newworld.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> > > index 32e88b3..7d25106 100644
> > > --- a/hw/ppc/mac_newworld.c
> > > +++ b/hw/ppc/mac_newworld.c
> > > @@ -380,6 +380,7 @@ static void ppc_core99_init(MachineState *machine)
> > >           pci_bus = pci_pmac_init(pic, get_system_memory(), get_system_io());
> > >           machine_arch = ARCH_MAC99;
> > >       }
> > > +    object_property_set_bool(OBJECT(pci_bus), true, "realized", &error_abort);
> > > 
> > >       machine->usb |= defaults_enabled() && !machine->usb_disabled;
> > > 
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH V5 5/5] machine: remove iommu property

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On Tue, Jun 28, 2016 at 11:07:52AM +0300, Marcel Apfelbaum wrote:
> On 06/28/2016 05:57 AM, David Gibson wrote:
> > On Mon, Jun 27, 2016 at 06:38:35PM +0300, Marcel Apfelbaum wrote:
> > > Since iommu devices can be created with '-device' there is
> > > no need to keep iommu as machine and mch property.
> > 
> > Doesn't this break backwards compatibility?
> > 
> 
> 
> Hi David,
> 
> Intel IOMMU was a kind of POC until recent development.
> The new IOMMU features will require more command line options,
> so even if we keep the machine property around, it can't be really used.
> 
> More on this was discussed in a prev thread.
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg377385.html

Ok, fair enough.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]