> On Dec 22, 2023, at 15:12, Tian, Kevin <kevin.tian@xxxxxxxxx> wrote: > > >> >> From: Liu, Yi L <yi.l.liu@xxxxxxxxx> >> Sent: Friday, December 22, 2023 3:02 PM >> >> >>>> On Dec 22, 2023, at 14:47, Tian, Kevin <kevin.tian@xxxxxxxxx> wrote: >>> >>> >>>> >>>> From: Yang, Weijiang <weijiang.yang@xxxxxxxxx> >>>> Sent: Friday, December 22, 2023 11:56 AM >>>>> + >>>>> + xa_for_each(&domain->iommu_array, i, info) { >>>>> + nested_flush_pasid_iotlb(info->iommu, domain, addr, >>>> npages, 0); >>>>> + >>>>> + if (domain->has_iotlb_device) >>>>> + continue; >>>> >>>> Shouldn't this be if (!domain->has_iotlb_device)? >>> >>> yes that is wrong. >>> >>> actually it's weird to put domain check in a loop of domain->iommu_array. >>> >>> that check along with devtlb flush should be done out of that loop. >> >> Maybe adding a bool, set it out of the loop, check the bool in the loop. > > the point is that dev iotlb doesn't rely on info->iommu: > > nested_flush_dev_iotlb(domain, addr, mask, &fault); > > then why do it in the loop of info->iommu? yes. It should have another device loop instead.