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

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

> From: Xu, Quan
> Sent: Thursday, December 10, 2015 5:33 PM
> 
> If IOTLB/Context/IETC flush is timeout, we should think
> all devices under this IOMMU cannot function correctly.
> So for each device under this IOMMU we'll mark it as
> unassignable and kill the domain owning the device.
> 
> If Device-TLB flush is timeout, we'll mark the target
> ATS device as unassignable and kill the domain owning
> this device.
> 
> If impacted domain is hardware domain, just throw out
> a warning. It's an open here whether we want to kill
> hardware domain (or directly panic hypervisor). Comments
> are welcomed.
> 
> Device marked as unassignable will be disallowed to be
> further assigned to any domain.
> 
> Signed-off-by: Quan Xu <quan.xu@intel.com>
> ---
>  xen/drivers/passthrough/vtd/extern.h  |  4 ++
>  xen/drivers/passthrough/vtd/iommu.c   |  6 +++
>  xen/drivers/passthrough/vtd/iommu.h   |  5 ++
>  xen/drivers/passthrough/vtd/qinval.c  | 86
> ++++++++++++++++++++++++++++++++++-
>  xen/drivers/passthrough/vtd/x86/ats.c | 16 +++++++
>  xen/include/xen/pci.h                 |  7 +++
>  6 files changed, 122 insertions(+), 2 deletions(-)
> 
[...]
> diff --git a/xen/drivers/passthrough/vtd/iommu.h
> b/xen/drivers/passthrough/vtd/iommu.h
> index ac71ed1..c3beaa6 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -452,6 +452,11 @@ struct qinval_entry {
> 
>  #define RESERVED_VAL        0
> 
> +#define INVALID_DID    ((u16)~0)
> +#define INVALID_SEG    ((u16)~0)
> +#define INVALID_BUS    ((u8)~0)
> +#define INVALID_DEVFN  ((u8)~0)
> +

Are those invalid values defined by specification? Or if they
are software defined, does related mgmt. code guarantee
that they won't be allocated?

>  #define TYPE_INVAL_CONTEXT      0x1
>  #define TYPE_INVAL_IOTLB        0x2
>  #define TYPE_INVAL_DEVICE_IOTLB 0x3
> diff --git a/xen/drivers/passthrough/vtd/qinval.c b/xen/drivers/passthrough/vtd/qinval.c
> index 990baf2..bf7f5b0 100644
> --- a/xen/drivers/passthrough/vtd/qinval.c
> +++ b/xen/drivers/passthrough/vtd/qinval.c
> @@ -27,12 +27,62 @@
>  #include "dmar.h"
>  #include "vtd.h"
>  #include "extern.h"
> +#include "../ats.h"
> 
>  static int __read_mostly iommu_qi_timeout_ms = 1;
>  integer_param("iommu_qi_timeout_ms", iommu_qi_timeout_ms);
> 
>  #define IOMMU_QI_TIMEOUT (iommu_qi_timeout_ms * MILLISECS(1))
> 
> +void invalidate_timeout(struct iommu *iommu, int type, u16 did,
> +                        u16 seg, u8 bus, u8 devfn)
> +{
> +    struct domain *d;
> +    unsigned long nr_dom, i;
> +    struct pci_dev *pdev;
> +
> +    switch (type) {
> +    case TYPE_INVAL_IOTLB:
> +    case TYPE_INVAL_CONTEXT:
> +    case TYPE_INVAL_IEC:
> +        nr_dom = cap_ndoms(iommu->cap);
> +        i = find_first_bit(iommu->domid_bitmap, nr_dom);
> +        while ( i < nr_dom ) {
> +            d = rcu_lock_domain_by_id(iommu->domid_map[i]);
> +            ASSERT(d);
> +
> +            /* Mark the devices as unassignable. */
> +            for_each_pdev(d, pdev)
> +                mark_pdev_unassignable(pdev);
> +            if ( d != hardware_domain )
> +                domain_kill(d);
> +
> +            rcu_unlock_domain(d);
> +            i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
> +        }
> +        break;
> +
> +    case TYPE_INVAL_DEVICE_IOTLB:
> +        d = rcu_lock_domain_by_id(iommu->domid_map[did]);
> +        ASSERT(d);
> +        for_each_pdev(d, pdev)
> +            if ( (pdev->seg == seg) &&
> +                 (pdev->bus == bus) &&
> +                 (pdev->devfn == devfn) )
> +                mark_pdev_unassignable(pdev);

Once found you can break the for loop immediately since BDF is unique.

Thanks
Kevin