RE: [PATCH v2 1/3] VT-d: consider hidden devices when unmapping

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

> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Friday, September 17, 2021 7:00 PM
> 
> Whether to clear an IOMMU's bit in the domain's bitmap should depend on
> all devices the domain can control. For the hardware domain this
> includes hidden devices, which are associated with DomXEN.
> 
> While touching related logic
> - convert the "current device" exclusion check to a simple pointer
>   comparison,
> - convert "found" to "bool",
> - adjust style and correct a typo in an existing comment.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

> ---
> v2: Introduce helper function.
> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1650,6 +1650,27 @@ int domain_context_unmap_one(
>      return rc;
>  }
> 
> +static bool any_pdev_behind_iommu(const struct domain *d,
> +                                  const struct pci_dev *exclude,
> +                                  const struct vtd_iommu *iommu)
> +{
> +    const struct pci_dev *pdev;
> +
> +    for_each_pdev ( d, pdev )
> +    {
> +        const struct acpi_drhd_unit *drhd;
> +
> +        if ( pdev == exclude )
> +            continue;
> +
> +        drhd = acpi_find_matched_drhd_unit(pdev);
> +        if ( drhd && drhd->iommu == iommu )
> +            return true;
> +    }
> +
> +    return false;
> +}
> +
>  static int domain_context_unmap(struct domain *domain, u8 devfn,
>                                  struct pci_dev *pdev)
>  {
> @@ -1657,7 +1678,7 @@ static int domain_context_unmap(struct d
>      struct vtd_iommu *iommu = drhd ? drhd->iommu : NULL;
>      int ret;
>      u8 seg = pdev->seg, bus = pdev->bus, tmp_bus, tmp_devfn, secbus;
> -    int found = 0;
> +    bool found;
> 
>      switch ( pdev->type )
>      {
> @@ -1737,23 +1758,18 @@ static int domain_context_unmap(struct d
>          return ret;
> 
>      /*
> -     * if no other devices under the same iommu owned by this domain,
> -     * clear iommu in iommu_bitmap and clear domain_id in domid_bitmp
> +     * If no other devices under the same iommu owned by this domain,
> +     * clear iommu in iommu_bitmap and clear domain_id in domid_bitmap.
>       */
> -    for_each_pdev ( domain, pdev )
> -    {
> -        if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
> -            continue;
> -
> -        drhd = acpi_find_matched_drhd_unit(pdev);
> -        if ( drhd && drhd->iommu == iommu )
> -        {
> -            found = 1;
> -            break;
> -        }
> -    }
> +    found = any_pdev_behind_iommu(domain, pdev, iommu);
> +    /*
> +     * Hidden devices are associated with DomXEN but usable by the
> hardware
> +     * domain. Hence they need considering here as well.
> +     */
> +    if ( !found && is_hardware_domain(domain) )
> +        found = any_pdev_behind_iommu(dom_xen, pdev, iommu);
> 
> -    if ( found == 0 )
> +    if ( !found )
>      {
>          clear_bit(iommu->index, &dom_iommu(domain)-
> >arch.vtd.iommu_bitmap);
>          cleanup_domid_map(domain, iommu);