Re: [Qemu-devel] [PATCH v3 0/4] enable iommu with -device

[Marcel Apfelbaum <marcel@redhat.com>]

On 06/13/2016 11:51 PM, Michael S. Tsirkin wrote:
>
> On Mon, Jun 13, 2016 at 05:36:56PM +0300, Marcel Apfelbaum wrote:
>> 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 1)
>>
>> Then we need to enable sysbus devices(*) for PC machines (patch 2),
>> since intel-iommu is a sysbus device.
>>
>> Patch 3 moves the IOMMU init proces into iommu's realize function
>> and allows the device creation in both ways.
>
> Looks good. I'm inclined to apply 1-3 initially, see how many
> acks will 4 collect.
> Could you rebase on my tree pls?
>

Sure, I'll resend it.
Thanks,
Marcel

>> Finally patch 4 removes the iommu machine property.
>>
>> 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 (4):
>>    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                        | 42 ++++++++++++++++++++++++++++---------
>>   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 +++++
>>   12 files changed, 64 insertions(+), 63 deletions(-)
>>
>> --
>> 2.4.3