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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Mar 24, 2016 at 01:57:59PM +0800, Quan Xu wrote:
> If Device-TLB flush timed out, we would hide the target ATS
> device and crash the domain owning this ATS device. If impacted
> domain is hardware domain, just throw out a warning.
> 
> The hidden device should be disallowed to be further assigned
> to any domain.
> 
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> ---
>  xen/drivers/passthrough/pci.c         |  6 ++--
>  xen/drivers/passthrough/vtd/extern.h  |  3 +-
>  xen/drivers/passthrough/vtd/qinval.c  | 58 +++++++++++++++++++++++++++++++++--
>  xen/drivers/passthrough/vtd/x86/ats.c | 11 ++++---
>  xen/include/xen/pci.h                 |  1 +
>  5 files changed, 68 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 9f1716a..9a214c6 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -420,7 +420,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
>      xfree(pdev);
>  }
>  
> -static void _pci_hide_device(struct pci_dev *pdev)
> +void pci_hide_existing_device(struct pci_dev *pdev)
>  {
>      if ( pdev->domain )
>          return;
> @@ -437,7 +437,7 @@ int __init pci_hide_device(int bus, int devfn)
>      pdev = alloc_pdev(get_pseg(0), bus, devfn);
>      if ( pdev )
>      {
> -        _pci_hide_device(pdev);
> +        pci_hide_existing_device(pdev);
>          rc = 0;
>      }
>      pcidevs_unlock();
> @@ -467,7 +467,7 @@ int __init pci_ro_device(int seg, int bus, int devfn)
>      }
>  
>      __set_bit(PCI_BDF2(bus, devfn), pseg->ro_map);
> -    _pci_hide_device(pdev);
> +    pci_hide_existing_device(pdev);
>  
>      return 0;
>  }
> diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h
> index 6d3187d..94e2c11 100644
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -62,7 +62,8 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did,
>  int qinval_device_iotlb(struct iommu *iommu,
>                          u32 max_invs_pend, u16 sid, u16 size, u64 addr);
>  int qinval_device_iotlb_sync(struct iommu *iommu, u32 max_invs_pend,
> -                             u16 sid, u16 size, u64 addr);
> +                             u16 did, u16 seg, u8 bus, u8 devfn,
> +                             u16 size, u64 addr);
>  
>  unsigned int get_cache_line_size(void);
>  void cacheline_flush(char *);
> diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
> index ad9e265..10c5684 100644
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -216,6 +216,58 @@ static int queue_invalidate_iotlb_sync(struct iommu *iommu,
>      return invalidate_sync(iommu);
>  }
>  
> +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)
> +    {
> +        if ( ( pdev->seg == seg ) &&
> +             ( pdev->bus == bus ) &&
> +             ( pdev->devfn == devfn ) )
> +        {
> +            ASSERT ( pdev->domain );

Oddly enough (and I don't see this in the StyleGuide), the ASSERTS
do not require the spaces. So it can be:

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);

The description said something about 'just throw out a warning' (if
the domain owning it is a hardware domain). That seems to be missing?


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