Re: [Xen-devel] [PATCH v7 2/2] VT-d: Fix vt-d Device-TLB flush timeout issue

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Thursday, March 17, 2016 5:43 PM
> 
> >>> On 17.03.16 at 09:17, <kevin.tian@xxxxxxxxx> wrote:
> >>  From: Xu, Quan
> >> Sent: Thursday, March 17, 2016 3:13 PM
> >> --- a/xen/drivers/passthrough/vtd/qinval.c
> >> +++ b/xen/drivers/passthrough/vtd/qinval.c
> >> @@ -233,6 +233,57 @@ int qinval_device_iotlb(struct iommu *iommu,
> >>      return 0;
> >>  }
> >>
> >> +static void dev_invalidate_iotlb_timeout(struct iommu *iommu, u16 did,
> >> +                                         u16 seg, u8 bus, u8 devfn)
> >> +{
> >> +    struct domain *d = NULL;
> >> +    struct pci_dev *pdev;
> >> +
> >> +    if ( test_bit(did, iommu->domid_bitmap) )
> >> +        d = rcu_lock_domain_by_id(iommu->domid_map[did]);
> >> +
> >> +    if ( d == NULL )
> >> +        return;
> >> +
> >> +    pcidevs_lock();
> >> +    for_each_pdev(d, pdev)
> >
> > we need a 'safe' version here since you're deleting nodes
> > when walking list. for_each_pdev today is based on
> > list_for_each_entry. Or if it's sure that only one pdev
> > can match, we can break out of the loop to do removal.
> 
> But breaking out of the loop is what is already being done ...
> 
> >> +    {
> >> +        if ( ( pdev->seg == seg ) &&
> >> +             ( pdev->bus == bus ) &&
> >> +             ( pdev->devfn == devfn ) )
> >> +        {
> >> +            ASSERT ( pdev->domain );
> >> +            list_del(&pdev->domain_list);
> >> +            pdev->domain = NULL;
> >> +            pci_hide_existing_device(pdev);
> >> +            break;
> 
> ... here.
> 

however you see list_del happens before breaking out, right?

Thanks
Kevin

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