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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.04.16 at 16:47, <quan.xu@xxxxxxxxx> wrote:
> If Device-TLB flush timed out, we would hide the target ATS

Please re-consider the use of the word "would" in all your patch
descriptions.

> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -217,6 +217,58 @@ static int invalidate_sync(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;

This should be accompanied by a comment explaining why
returning here without indicating any error is okay.

> +    pcidevs_lock();
> +
> +    for_each_pdev(d, pdev)
> +    {
> +        if ( ( pdev->seg == seg ) &&
> +             ( pdev->bus == bus ) &&
> +             ( pdev->devfn == devfn ) )

Stray blanks.

> +        {
> +            ASSERT(pdev->domain);
> +            list_del(&pdev->domain_list);
> +            pdev->domain = NULL;
> +            pci_hide_existing_device(pdev);
> +            break;
> +        }
> +    }
> +
> +    pcidevs_unlock();
> +
> +    if ( !is_hardware_domain(d) )
> +        domain_crash(d);

else printk();

> @@ -251,11 +303,13 @@ int qinval_device_iotlb(struct iommu *iommu,
>  }
>  
>  int qinval_device_iotlb_sync(struct iommu *iommu,
> -    u32 max_invs_pend, u16 sid, u16 size, u64 addr)
> +    u32 max_invs_pend, u16 did, u16 seg, u8 bus, u8 devfn, u16 size, u64 
> addr)
>  {
> +    u16 sid = PCI_BDF2(bus, devfn);

Pointless local variable (but being a matter of taste, the VT-d
maintainers will have the final say).

> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -116,6 +116,7 @@ const unsigned long *pci_get_ro_map(u16 seg);
>  int pci_add_device(u16 seg, u8 bus, u8 devfn,
>                     const struct pci_dev_info *, nodeid_t node);
>  int pci_remove_device(u16 seg, u8 bus, u8 devfn);
> +void pci_hide_existing_device(struct pci_dev *pdev);
>  int pci_ro_device(int seg, int bus, int devfn);
>  int pci_hide_device(int bus, int devfn);

Please move the addition here.

Jan


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