On Tue, Oct 17, 2023 at 12:53:01PM -0300, Jason Gunthorpe wrote: > On Tue, Oct 17, 2023 at 08:52:49AM +0000, Tian, Kevin wrote: > > I prefer to removing enforce_cc in attach fn completely then no parent > > trick in this patch. Just keep it in hwpt_alloc and leave to iommu driver to > > figure out the attaching compatibility: > > You are basically saying to set the cc mode during creation because we > know the idev at that moment and can tell if it should be on/off? > > It seems reasonable, but I can't remember why it is in the attach path > at the moment. This was the commit adding it in the alloc path: https://lore.kernel.org/linux-iommu/8e897628-61fa-b3fb-b609-44eeda11b45e@xxxxxxx/ The older code was doing a hwpt "upgrade" from !cc to cc: - /* - * Try to upgrade the domain we have, it is an iommu driver bug to - * report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail - * enforce_cache_coherency when there are no devices attached to the - * domain. - */ - if (idev->enforce_cache_coherency && !hwpt->enforce_cache_coherency) { - if (hwpt->domain->ops->enforce_cache_coherency) - hwpt->enforce_cache_coherency = - hwpt->domain->ops->enforce_cache_coherency( - hwpt->domain); If we remove the enforce_cc call in the attach path and let the driver decide whether to enforce or reject in attach_dev calls, there seems to be no point in tracking an enforce_cache_coherency flag in the IOMMUFD pathway but only for the VFIO_DMA_CC_IOMMU extension check in the vfio-compat pathway? Thanks Nic