Re: [Qemu-devel] [PATCH] intel_iommu: make sure its init before PCI dev

[Jintack Lim]

On Wed, Feb 22, 2017 at 12:49 AM, Peter Xu <address@hidden> wrote:

> Intel vIOMMU devices are created with "-device" parameter, while here
> actually we need to make sure this device will be created before some
> other PCI devices (like vfio-pci devices) so that we know iommu_fn will
> be setup correctly before realizations of those PCI devices.
>
> Here we do explicit check to make sure intel-iommu device will be inited
> before all the rest of the PCI devices. This is done by checking against
> the devices dangled under current root PCIe bus and we should see
> nothing there besides integrated ICH9 ones.
>
> If the user violated this rule, we abort the program.
>

Hi Peter,

After applying this patch, qemu gave the following error and was
terminated, but I believe I passed parameters in a right order?

[kvm-node ~]$sudo qemu-system-x86_64 \
> -device intel-iommu,intremap=on,eim=off,caching-mode=on \
> -M q35,accel=kvm,kernel-irqchip=split \
> -m 12G \
> -drive file=/mydata/guest0.img,format=raw --nographic -cpu host \
> -smp 4,sockets=4,cores=1,threads=1 \
> -device vfio-pci,host=08:00.0
qemu-system-x86_64: -device
intel-iommu,intremap=on,eim=off,caching-mode=on: Please init intel-iommu
before other PCI devices

 Thanks,
Jintack


> Maybe one day we will be able to manage the ordering of device
> initialization, and then we can grant VT-d devices a higher init
> priority. But before that, let's have this explicit check to make sure
> of it.
>
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  hw/i386/intel_iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 22d8226..db74124 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -31,6 +31,7 @@
>  #include "hw/i386/apic-msidef.h"
>  #include "hw/boards.h"
>  #include "hw/i386/x86-iommu.h"
> +#include "hw/i386/ich9.h"
>  #include "hw/pci-host/q35.h"
>  #include "sysemu/kvm.h"
>  #include "hw/i386/apic_internal.h"
> @@ -2560,6 +2561,41 @@ static bool vtd_decide_config(IntelIOMMUState *s,
> Error **errp)
>      return true;
>  }
>
> +static bool vtd_has_inited_pci_devices(PCIBus *bus, Error **errp)
> +{
> +    int i;
> +    uint8_t func;
> +
> +    /* We check against root bus */
> +    assert(bus && pci_bus_is_root(bus));
> +
> +    /*
> +     * We need to make sure vIOMMU device is created before other PCI
> +     * devices other than the integrated ICH9 ones, so that they can
> +     * get correct iommu_fn setup even during its realize(). Some
> +     * devices (e.g., vfio-pci) will need a correct iommu_fn to work.
> +     */
> +    for (i = 1; i < PCI_FUNC_MAX * PCI_SLOT_MAX; i++) {
> +        /* Skip the checking against ICH9 integrated devices */
> +        if (PCI_SLOT(i) == ICH9_LPC_DEV) {
> +            func = PCI_FUNC(i);
> +            if (func == ICH9_LPC_FUNC ||
> +                func == ICH9_SATA1_FUNC ||
> +                func == ICH9_SMB_FUNC) {
> +                continue;
> +            }
> +        }
> +
> +        if (bus->devices[i]) {
> +            error_setg(errp, "Please init intel-iommu before "
> +                       "other PCI devices");
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
>  static void vtd_realize(DeviceState *dev, Error **errp)
>  {
>      PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> @@ -2567,6 +2603,10 @@ static void vtd_realize(DeviceState *dev, Error
> **errp)
>      IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
>      X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>
> +    if (vtd_has_inited_pci_devices(bus, errp)) {
> +        return;
> +    }
> +
>      VTD_DPRINTF(GENERAL, "");
>      x86_iommu->type = TYPE_INTEL;
>
> --
> 2.7.4
>
>
>