> > On Fri, Feb 24, 2017 at 12:07:33AM -0500, Pankaj Gupta wrote: > > Hello Peter, > > Hi, Pankaj! > > > > > 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. > > Yes this is my intention to emphasize that we should possibly put > intel-iommu even before all the PCI devices. As mentioned above (and > also in the commit message), although vfio-pci is the only one that is > affected by this, we should probably put intel-iommu even before all > the rest of PCI devices. E.g., in the future we can have new devices > that need this dependency as well (that vt-d being inited before that > device), which is still legal imho.
yes, something like this can help "intel-iommu should be specified as first device"? Or we can just check for "intel-iommu" as first device at the start in place of checking for "vfio-pci". This will also help us to check for all other devices. Thanks. > > Meanwhile, I intended to avoid naming "vfio-pci" here since it'll let > user think of "something bad with vfio-pci" but actually vfio-pci did > nothing wrong. o.k > > Thanks, > > -- peterx >