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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.12.15 at 14:01, <quan.xu@xxxxxxxxx> wrote:
>>  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()?

Ignore? Just keep the code the way it was (still seen above).

Jan


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