Re: [PATCH v1 03/20] iommu/arm-smmu-v3: Implement arm_smmu_domain_test_dev

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Oct 20, 2025 at 01:32:31PM -0300, Jason Gunthorpe wrote:
> On Sun, Oct 12, 2025 at 05:05:00PM -0700, Nicolin Chen wrote:
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index a33fbd12a0dd9..3448e55bbcdbb 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2765,9 +2765,6 @@ static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
> >  
> >  	iommu_group_mutex_assert(master->dev);
> >  
> > -	if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
> > -		return -EOPNOTSUPP;
> 
> Stuff like this is also optimizing the codegen, it shouldn't be
> removed.

Okay. I assume we should just copy it to test_dev() then.

> > +int arm_smmu_domain_test_dev(struct iommu_domain *domain, struct device *dev,
> > +			     ioasid_t pasid, struct iommu_domain *old_domain)
> > +{
> > +	struct arm_smmu_domain *device_domain = to_smmu_domain_devices(domain);
> > +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +
> > +	if (!dev_iommu_fwspec_get(dev))
> > +		return -ENOENT;
> > +
> > +	switch (domain->type) {
> > +	case IOMMU_DOMAIN_NESTED: {
> > +		struct arm_smmu_nested_domain *nested_domain =
> > +			to_smmu_nested_domain(domain);
> > +
> > +		if (WARN_ON(pasid != IOMMU_NO_PASID))
> > +			return -EOPNOTSUPP;
> > +		if (nested_domain->vsmmu->smmu != master->smmu)
> > +			return -EINVAL;
> > +		if (arm_smmu_ssids_in_use(&master->cd_table))
> > +			return -EBUSY;
> 
> This gives me alot of pause.. Here we are detecting if a S1 PASID is
> installed for a S2 attach, but to your purpose this can be made
> inconsistent by userspace during a FLR..

Ah right, the used_ssids could mismatch with the group->domain!

> I don't see any reasonable way to mitigate this??

Right. It can't simply go through a regular attach_dev call since
driver wouldn't expect any inconsistency in the core.

Driver would have to be aware of the reset state, and make a copy
of the old domain's CD/STE to use for a test_dev() during a reset.

> Which makes me wonder if we should just try to solve the simple
> obvious things like direct, permanent incompatability and still have
> some kind of recovery code to leave things in blocking if they fail to
> attach

I don't quite get this. Mind elaborating?

Thanks
Nicolin