On Wed, Feb 22, 2017 at 6:42 AM, Jintack Lim <jint...@cs.columbia.edu>
wrote:
>
>
> On Wed, Feb 22, 2017 at 12:49 AM, Peter Xu <pet...@redhat.com> 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?
>
FYI, I've applied this patch to your vtd-vfio-enablement-v7 branch.
>
> [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 <pet...@redhat.com>
>> ---
>> 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
>>
>>
>>
>
