Hello Peter,
This solution looks to check dependency of 'vfio-pci' over 'intel-iommu'
before 'intel-iommu' is not initialized.
Overall it looks good to me, just a small nit below.
>
> Intel vIOMMU devices are created with "-device" parameter, while here
> actually we need to make sure the dmar device be created before other
> PCI devices (like vfio-pci) so that we know iommu_fn will be setup
> correctly before realizations of those PCI devices (it is sensible that
> PCI device fetch these info during its realization). Now this ordering
> yet cannot be achieved elsewhere, and devices will be created in the
> order that user specified. That might be dangerous.
>
> Here we add one more function to detect this kind of misordering issue,
> then report to guest. Currently, the only known device that is affected
> by this VT-d defect is the vfio-pci typed devices. So for now we just
> check against it to make sure we are safe.
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> hw/i386/intel_iommu.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 22d8226..b723ece 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2560,6 +2560,24 @@ static bool vtd_decide_config(IntelIOMMUState *s,
> Error **errp)
> return true;
> }
>
> +/*
> + * TODO: we should have a better way to achieve the ordering rather
> + * than this misorder check explicitly against vfio-pci. After all, no
> + * one should be blamed for this, and vfio-pci did nothing wrong.
> + */
> +static bool vtd_detected_misorder_init(Error **errp)
> +{
> + Object *dev = object_resolve_path_type("", "vfio-pci", NULL);
> +
> + if (dev) {
> + error_setg(errp, "Please specify \"intel-iommu\" before all the rest
"before all the rest" does not give much clue to user. Do you think
better
error message would help? just a thought.
> "
> + "of the devices.");
> + return true;
> + }
> +
> + return false;
> +}
> +
> static void vtd_realize(DeviceState *dev, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> @@ -2567,6 +2585,10 @@ static void vtd_realize(DeviceState *dev, Error
> **errp)
> IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
> X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>
> + if (vtd_detected_misorder_init(errp)) {
> + return;
> + }
> +
> VTD_DPRINTF(GENERAL, "");
> x86_iommu->type = TYPE_INTEL;
>
> --
> 2.7.4
>
>
>
