03.10.2020 09:59, Nicolin Chen пишет: > The bus_set_iommu() in tegra_smmu_probe() enumerates all clients > to call in tegra_smmu_probe_device() where each client searches > its DT node for smmu pointer and swgroup ID, so as to configure > an fwspec. But this requires a valid smmu pointer even before mc > and smmu drivers are probed. So in tegra_smmu_probe() we added a > line of code to fill mc->smmu, marking "a bit of a hack". > > This works for most of clients in the DTB, however, doesn't work > for a client that doesn't exist in DTB, a PCI device for example. > > Actually, if we return ERR_PTR(-ENODEV) in ->probe_device() when > it's called from bus_set_iommu(), iommu core will let everything > carry on. Then when a client gets probed, of_iommu_configure() in > iommu core will search DTB for swgroup ID and call ->of_xlate() > to prepare an fwspec, similar to tegra_smmu_probe_device() and > tegra_smmu_configure(). Then it'll call tegra_smmu_probe_device() > again, and this time we shall return smmu->iommu pointer properly. > > So we can get rid of tegra_smmu_find() and tegra_smmu_configure() > along with DT polling code by letting the iommu core handle every > thing, except a problem that we search iommus property in DTB not > only for swgroup ID but also for mc node to get mc->smmu pointer > to call dev_iommu_priv_set() and return the smmu->iommu pointer. > So we'll need to find another way to get smmu pointer. > > Referencing the implementation of sun50i-iommu driver, of_xlate() > has client's dev pointer, mc node and swgroup ID. This means that > we can call dev_iommu_priv_set() in of_xlate() instead, so we can > simply get smmu pointer in ->probe_device(). > > This patch reworks tegra_smmu_probe_device() by: > 1) Removing mc->smmu hack in tegra_smmu_probe() so as to return > ERR_PTR(-ENODEV) in tegra_smmu_probe_device() during stage of > tegra_smmu_probe/tegra_mc_probe(). > 2) Moving dev_iommu_priv_set() to of_xlate() so we can get smmu > pointer in tegra_smmu_probe_device() to replace DTB polling. > 3) Removing tegra_smmu_configure() accordingly since iommu core > takes care of it. > > This also fixes a problem that previously we added all clients to > iommu groups before iommu core initializes its default domain: > ubuntu@jetson:~$ dmesg | grep iommu > platform smmu_benchmark: Adding to iommu group 0 > platform 1003000.pcie: Adding to iommu group 1 > platform 50000000.host1x: Adding to iommu group 2 > platform 57000000.gpu: Adding to iommu group 3 > platform 7000c400.i2c: Adding to iommu group 4 > platform 7000c500.i2c: Adding to iommu group 4 > platform 7000c700.i2c: Adding to iommu group 4 > platform 7000d000.i2c: Adding to iommu group 4 > iommu: Default domain type: Translated > > Though it works fine with IOMMU_DOMAIN_UNMANAGED, but will have > warnings if switching to IOMMU_DOMAIN_DMA: > iommu: Failed to allocate default IOMMU domain of type 0 for > group (null) - Falling back to IOMMU_DOMAIN_DMA > iommu: Failed to allocate default IOMMU domain of type 0 for > group (null) - Falling back to IOMMU_DOMAIN_DMA > > Now, bypassing the first probe_device() call from bus_set_iommu() > fixes the sequence: > ubuntu@jetson:~$ dmesg | grep iommu > iommu: Default domain type: Translated > tegra-i2c 7000c400.i2c: Adding to iommu group 0 > tegra-i2c 7000c500.i2c: Adding to iommu group 0 > tegra-i2c 7000d000.i2c: Adding to iommu group 0 > tegra-pcie 1003000.pcie: Adding to iommu group 1 > ... > > Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED. > > Signed-off-by: Nicolin Chen <nicoleotsuka@xxxxxxxxx> > --- Everything looks good to me, apart from the very minor pending question about the NULL-checking. Thanks! Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx> Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx>
