[PATCH v5 08/17] iommu/arm-smmu-v3: Do not change the STE twice during arm_smmu_attach_dev()

[Jason Gunthorpe]

On Tue, Feb 13, 2024 at 03:40:34PM +0000, Mostafa Saleh wrote:
> > @@ -2928,9 +2922,18 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
> >  static void arm_smmu_release_device(struct device *dev)
> >  {
> >  	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +	struct arm_smmu_ste target;
> >  
> >  	if (WARN_ON(arm_smmu_master_sva_enabled(master)))
> >  		iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
> > +
> > +	/* Put the STE back to what arm_smmu_init_strtab() sets */
> > +	if (disable_bypass && !dev->iommu->require_direct)
> > +		arm_smmu_make_abort_ste(&target);
> > +	else
> > +		arm_smmu_make_bypass_ste(&target);
> > +	arm_smmu_install_ste_for_dev(master, &target);
> > +
> >  	arm_smmu_detach_dev(master);
> >  	arm_smmu_disable_pasid(master);
> >  	arm_smmu_remove_master(master);

> I am still reviewing patch-1 and the hitless machinery (also I think -or hope-
> this can be simplified), with the assumption that
> arm_smmu_install_ste_for_dev()/arm_smmu_write_ste() will do the right thing,
> that good looks good to me.

I'm interested if you can come up with something. Let me know if you
want to bounce some ideas.

> However, as it changes the current behavior of the driver where
> disable_bypass used to override require_direct, I am not sure if
> this would break any existing setups.

Yes, the commit message explains this. It is a little bug.

require_direct takes precedence when building the initial STE, release
should restore the STE back to how it was before probe.

I don't imagine a case where a system was fine with the STE during
boot but doesn't like that same STE during devuce hot unplug???

Jason