Re: [PATCH v2 3/4] iommu/arm-smmu-v3: Add support for dirty tracking in domain alloc

From: Jason Gunthorpe <jgg@nvidia.com>
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
	iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	joro@8bytes.org, kevin.tian@intel.com, nicolinc@nvidia.com,
	mshavit@google.com, robin.murphy@arm.com, will@kernel.org,
	joao.m.martins@oracle.com, jiangkunkun@huawei.com,
	zhukeqian1@huawei.com, linuxarm@huawei.com
Subject: Re: [PATCH v2 3/4] iommu/arm-smmu-v3: Add support for dirty tracking in domain alloc
Date: Tue, 23 Apr 2024 13:39:54 -0300	[thread overview]
Message-ID: <20240423163954.GA941030@nvidia.com> (raw)
In-Reply-To: <fa1968eb-9ad0-41a4-8259-6c500fc88521@arm.com>

On Tue, Apr 23, 2024 at 05:27:09PM +0100, Ryan Roberts wrote:

> >  	switch (smmu_domain->stage) {
> >  	case ARM_SMMU_DOMAIN_S1:
> >  		ias = (smmu->features & ARM_SMMU_FEAT_VAX) ? 52 : 48;
> > -		ias = min_t(unsigned long, ias, VA_BITS);
> > -		oas = smmu->ias;
> > +		pgtbl_cfg.ias = min_t(unsigned long, ias, VA_BITS);
> 
> I know this isn't changed by this patch, but do we really mean VA_BITS here?
> Don't we want vabits_actual? I'm guessing we are intending to limit ias to the
> size the kernel is using.

I think the intention here is to allow CONFIG_ARM64_VA_BITS to choose
the number of page table levels in the iommu. Perhaps a SMMU specific
config option would be clearer.

There is no relationship to the configuration of the S1 paging domain
and what the MM is doing, there should be no crossing between those
two worlds.

vabits_actual is used in the SVA code to match the SMMU SVA S1 to the
mm_struct.

> > @@ -3206,6 +3212,10 @@ arm_smmu_domain_alloc_user(struct device *dev, u32 flags,
> >  	if (user_data)
> >  		return ERR_PTR(-EINVAL);
> >  
> > +	if (enforce_dirty &&
> > +	    !device_iommu_capable(dev, IOMMU_CAP_DIRTY_TRACKING))
> > +		return ERR_PTR(-EOPNOTSUPP);
> 
> I'm guessing the intention is that only a stage 1 will ever be marked with
> IOMMU_CAP_DIRTY_TRACKING (there are a few places that assume/check we are
> dealing with S1)? But is there a reason why stage 2 can't be supported as well?

Stage 2 really should ultimately be supported..

I'd say this is a first step as iommufd will naturally prefer the S1
configuration when not using nesting. After the nesting support is
merged the vSMMU will require the S2 to do the dirty tracking.

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel