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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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