Re: [PATCH v3 2/4] iommu/io-pgtable-arm: Add read_and_clear_dirty() support

From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"will@kernel.org" <will@kernel.org>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"ryan.roberts@arm.com" <ryan.roberts@arm.com>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"mshavit@google.com" <mshavit@google.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
	"jiangkunkun@huawei.com" <jiangkunkun@huawei.com>,
	"zhukeqian1@huawei.com" <zhukeqian1@huawei.com>,
	"linuxarm@huawei.com" <linuxarm@huawei.com>
Subject: Re: [PATCH v3 2/4] iommu/io-pgtable-arm: Add read_and_clear_dirty() support
Date: Wed, 22 May 2024 09:37:44 -0300	[thread overview]
Message-ID: <20240522123744.GU20229@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB5276ABF99FDF6CF0F863501C8CEB2@BN9PR11MB5276.namprd11.prod.outlook.com>

On Wed, May 22, 2024 at 07:12:27AM +0000, Tian, Kevin wrote:
> > +static int arm_lpae_read_and_clear_dirty(struct io_pgtable_ops *ops,
> > +					 unsigned long iova, size_t size,
> > +					 unsigned long flags,
> > +					 struct iommu_dirty_bitmap *dirty)
> > +{
> > +	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
> > +	struct io_pgtable_cfg *cfg = &data->iop.cfg;
> > +	struct io_pgtable_walk_data walk_data = {
> > +		.dirty = dirty,
> > +		.flags = flags,
> > +		.addr = iova,
> > +		.end = iova + size,
> > +	};
> > +	arm_lpae_iopte *ptep = data->pgd;
> > +	int lvl = data->start_level;
> > +
> > +	if (WARN_ON(!size))
> > +		return -EINVAL;
> > +	if (WARN_ON((iova + size - 1) & ~(BIT(cfg->ias) - 1)))
> > +		return -EINVAL;
> > +	if (data->iop.fmt != ARM_64_LPAE_S1)
> > +		return -EINVAL;
> > +
> > +	return __arm_lpae_iopte_walk_dirty(data, &walk_data, ptep, lvl);
> 
> Intel/AMD drivers also checks:
> 
>         if (!dmar_domain->dirty_tracking && dirty->bitmap)
>                 return -EINVAL;

Those both seem redundant checks on a fast path?

Jason