[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] intel_iommu: check misordered init when real
From: |
Pankaj Gupta |
Subject: |
Re: [Qemu-devel] [PATCH v2] intel_iommu: check misordered init when realize |
Date: |
Fri, 24 Feb 2017 01:35:10 -0500 (EST) |
>
> 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 <address@hidden>
> > > ---
> > > 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
>
Re: [Qemu-devel] [PATCH v2] intel_iommu: check misordered init when realize, Marcel Apfelbaum, 2017/02/28