Re: [Xen-devel] [PATCH v3 2/2] VT-d: Fix vt-d flush timeout issue.

["Xu, Quan" <quan.xu@xxxxxxxxx>]

> On 15.12.2015 at 8:32pm, <JBeulich@xxxxxxxx> wrote:
> >>> On 15.12.15 at 12:42,  wrote:
> >>>> On 15.12.15 at 11:24, <quan.xu@xxxxxxxxx> wrote:
> > >>  On 15.12.2015 at 5:17pm, <JBeulich@xxxxxxxx> wrote:
> > >> >>> On 15.12.15 at 09:15, <quan.xu@xxxxxxxxx> wrote:
> > >> >> On 14.12.2015 at 5:28pm, <JBeulich@xxxxxxxx> wrote:
> > >> >> >>> On 12.12.15 at 14:21, <quan.xu@xxxxxxxxx> wrote:
> > >> >> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > >> >> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > >> >> > @@ -1890,6 +1890,9 @@ static int intel_iommu_add_device(u8
> > >> >> > devfn, struct
> > >> >> pci_dev *pdev)
> > >> >> >      if ( !pdev->domain )
> > >> >> >          return -EINVAL;
> > >> >> >
> > >> >> > +    if ( is_pdev_unassignable(pdev) )
> > >> >> > +        return -EACCES;
> > >> >>
> > >> >> Is this case possible at all (i.e. a newly added device being
> > > unassignable)?
> > >> >>
> > >> >
> > >> > IMO, it is possible, and I have checked and printed the invoke
> > >> > flow when
> > > Xen
> > >> init.
> > >> > iommu_setup() / iommu_hwdom_init() are called before
> > >> > pci_add_device()
> > >> when Xen init.
> > >> > If it flushes error in iommu_setup() / iommu_hwdom_init(), it
> > >> > will mark device as unassignable.
> > >>
> > >> Please be more precise: I can't see how e.g. iommu_setup() would
> > >> manage to time out on invalidation on a particular device. I.e.
> > >> please provide an exact scenario where the flag could get set
> > >> before the
> > > device
> > >> gets reported through iommu_add_device().
> > >>
> > >> Furthermore I think that boot time invalidations shouldn't lead to
> > >> any
> > > devices
> > >> getting marked un-assignable. Instead such would probably better
> > >> result in
> > > the
> > >> IOMMU to be turned of altogether.
> > >
> > >
> > > ...
> > > construct_dom0()
> > > |--iommu_hwdom_init()
> > >   |--hd->platform_ops->hwdom_init(d) [intel_iommu_hwdom_init() for
> x86]
> > >     |--iommu_flush_all()
> > >     ...
> > >
> > >
> > > If it is failed in iommu_flush_all(), it will mask all of dom0's
> > > devices as unassignable.
> > > And all of that called before pci_add_device() ..   right?
> >
> > Agreed. But as said, I don't think that's the right action in this
> > specific case.
> 
> Having thought about it some more over lunch, I think leaving the conditional 
> in
> place is going to be fine, and much less of a change than trying to unroll
> IOMMU setup at that point.
> 
Thanks Jan!
Do you mean that I can ignore it in intel_iommu_add_device()?

Quan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel