> From: Jan Beulich
> Sent: Wednesday, September 15, 2021 5:13 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 "found" to "bool".
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1657,7 +1657,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 +1737,33 @@ 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.
what is changed above?
> */
> - for_each_pdev ( domain, pdev )
> + for ( found = false; ; domain = dom_xen )
honesty speaking I had to read several times to understand the break
condition of this loop. It'd be easier to understand if putting for_each_
pdev(){} into a function and then:
found = func(domain);
if (!found && is_hardware_domain(domain))
found = func(dom_xen);
> {
> - if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn )
> - continue;
> -
> - drhd = acpi_find_matched_drhd_unit(pdev);
> - if ( drhd && drhd->iommu == iommu )
> + for_each_pdev ( domain, pdev )
> {
> - found = 1;
> - break;
> + if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn ==
> devfn )
> + continue;
> +
> + drhd = acpi_find_matched_drhd_unit(pdev);
> + if ( drhd && drhd->iommu == iommu )
> + {
> + found = true;
> + break;
> + }
> }
> +
> + /*
> + * 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) )
> + break;
> }
>
> - if ( found == 0 )
> + if ( !found )
> {
> clear_bit(iommu->index, &dom_iommu(domain)-
> >arch.vtd.iommu_bitmap);
> cleanup_domid_map(domain, iommu);
>
