* [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes @ 2020-11-25 10:10 Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get Nicolin Chen ` (6 more replies) 0 siblings, 7 replies; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh Changelog v1->v2: * Added Thierry's acks to PATCH-3~5 This is a merged set of resend for previously two series of patches that were reviewed/acked a month ago yet have not got applied. Series-1: https://lkml.org/lkml/2020/9/29/73 "[PATCH v4 0/2] iommu/tegra-smmu: Two followup changes" Series-2: https://lkml.org/lkml/2020/10/9/808 "[PATCH v7 0/3] iommu/tegra-smmu: Add PCI support" Nicolin Chen (5): iommu/tegra-smmu: Unwrap tegra_smmu_group_get iommu/tegra-smmu: Expand mutex protection range iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev iommu/tegra-smmu: Rework tegra_smmu_probe_device() iommu/tegra-smmu: Add PCI support drivers/iommu/tegra-smmu.c | 240 ++++++++++++++----------------------- 1 file changed, 88 insertions(+), 152 deletions(-) -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RESEND v2 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen @ 2020-11-25 10:10 ` Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 2/5] iommu/tegra-smmu: Expand mutex protection range Nicolin Chen ` (5 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh The tegra_smmu_group_get was added to group devices in different SWGROUPs and it'd return a NULL group pointer upon a mismatch at tegra_smmu_find_group(), so for most of clients/devices, it very likely would mismatch and need a fallback generic_device_group(). But now tegra_smmu_group_get handles devices in same SWGROUP too, which means that it would allocate a group for every new SWGROUP or would directly return an existing one upon matching a SWGROUP, i.e. any device will go through this function. So possibility of having a NULL group pointer in device_group() is upon failure of either devm_kzalloc() or iommu_group_alloc(). In either case, calling generic_device_group() no longer makes a sense. Especially for devm_kzalloc() failing case, it'd cause a problem if it fails at devm_kzalloc() yet succeeds at a fallback generic_device_group(), because it does not create a group->list for other devices to match. This patch simply unwraps the function to clean it up. Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Acked-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 0becdbfea306..ec4c9dafff95 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -903,10 +903,12 @@ static void tegra_smmu_group_release(void *iommu_data) mutex_unlock(&smmu->lock); } -static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu, - unsigned int swgroup) +static struct iommu_group *tegra_smmu_device_group(struct device *dev) { + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + struct tegra_smmu *smmu = dev_iommu_priv_get(dev); const struct tegra_smmu_group_soc *soc; + unsigned int swgroup = fwspec->ids[0]; struct tegra_smmu_group *group; struct iommu_group *grp; @@ -950,19 +952,6 @@ static struct iommu_group *tegra_smmu_group_get(struct tegra_smmu *smmu, return group->group; } -static struct iommu_group *tegra_smmu_device_group(struct device *dev) -{ - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); - struct tegra_smmu *smmu = dev_iommu_priv_get(dev); - struct iommu_group *group; - - group = tegra_smmu_group_get(smmu, fwspec->ids[0]); - if (!group) - group = generic_device_group(dev); - - return group; -} - static int tegra_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) { -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND v2 2/5] iommu/tegra-smmu: Expand mutex protection range 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get Nicolin Chen @ 2020-11-25 10:10 ` Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev Nicolin Chen ` (4 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh This is used to protect potential race condition at use_count. since probes of client drivers, calling attach_dev(), may run concurrently. Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Acked-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index ec4c9dafff95..6a3ecc334481 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -256,26 +256,19 @@ static int tegra_smmu_alloc_asid(struct tegra_smmu *smmu, unsigned int *idp) { unsigned long id; - mutex_lock(&smmu->lock); - id = find_first_zero_bit(smmu->asids, smmu->soc->num_asids); - if (id >= smmu->soc->num_asids) { - mutex_unlock(&smmu->lock); + if (id >= smmu->soc->num_asids) return -ENOSPC; - } set_bit(id, smmu->asids); *idp = id; - mutex_unlock(&smmu->lock); return 0; } static void tegra_smmu_free_asid(struct tegra_smmu *smmu, unsigned int id) { - mutex_lock(&smmu->lock); clear_bit(id, smmu->asids); - mutex_unlock(&smmu->lock); } static bool tegra_smmu_capable(enum iommu_cap cap) @@ -420,17 +413,21 @@ static int tegra_smmu_as_prepare(struct tegra_smmu *smmu, struct tegra_smmu_as *as) { u32 value; - int err; + int err = 0; + + mutex_lock(&smmu->lock); if (as->use_count > 0) { as->use_count++; - return 0; + goto unlock; } as->pd_dma = dma_map_page(smmu->dev, as->pd, 0, SMMU_SIZE_PD, DMA_TO_DEVICE); - if (dma_mapping_error(smmu->dev, as->pd_dma)) - return -ENOMEM; + if (dma_mapping_error(smmu->dev, as->pd_dma)) { + err = -ENOMEM; + goto unlock; + } /* We can't handle 64-bit DMA addresses */ if (!smmu_dma_addr_valid(smmu, as->pd_dma)) { @@ -453,24 +450,35 @@ static int tegra_smmu_as_prepare(struct tegra_smmu *smmu, as->smmu = smmu; as->use_count++; + mutex_unlock(&smmu->lock); + return 0; err_unmap: dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE); +unlock: + mutex_unlock(&smmu->lock); + return err; } static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu, struct tegra_smmu_as *as) { - if (--as->use_count > 0) + mutex_lock(&smmu->lock); + + if (--as->use_count > 0) { + mutex_unlock(&smmu->lock); return; + } tegra_smmu_free_asid(smmu, as->id); dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE); as->smmu = NULL; + + mutex_unlock(&smmu->lock); } static int tegra_smmu_attach_dev(struct iommu_domain *domain, -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND v2 3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 2/5] iommu/tegra-smmu: Expand mutex protection range Nicolin Chen @ 2020-11-25 10:10 ` Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen ` (3 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh In tegra_smmu_(de)attach_dev() functions, we poll DTB for each client's iommus property to get swgroup ID in order to prepare "as" and enable smmu. Actually tegra_smmu_configure() prepared an fwspec for each client, and added to the fwspec all swgroup IDs of client DT node in DTB. So this patch uses fwspec in tegra_smmu_(de)attach_dev() so as to replace the redundant DT polling code. Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Acked-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 56 ++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 6a3ecc334481..297d49f3f80e 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -484,60 +484,50 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu, static int tegra_smmu_attach_dev(struct iommu_domain *domain, struct device *dev) { + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct tegra_smmu *smmu = dev_iommu_priv_get(dev); struct tegra_smmu_as *as = to_smmu_as(domain); - struct device_node *np = dev->of_node; - struct of_phandle_args args; - unsigned int index = 0; - int err = 0; - - while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, - &args)) { - unsigned int swgroup = args.args[0]; - - if (args.np != smmu->dev->of_node) { - of_node_put(args.np); - continue; - } + unsigned int index; + int err; - of_node_put(args.np); + if (!fwspec) + return -ENOENT; + for (index = 0; index < fwspec->num_ids; index++) { err = tegra_smmu_as_prepare(smmu, as); - if (err < 0) - return err; + if (err) + goto disable; - tegra_smmu_enable(smmu, swgroup, as->id); - index++; + tegra_smmu_enable(smmu, fwspec->ids[index], as->id); } if (index == 0) return -ENODEV; return 0; + +disable: + while (index--) { + tegra_smmu_disable(smmu, fwspec->ids[index], as->id); + tegra_smmu_as_unprepare(smmu, as); + } + + return err; } static void tegra_smmu_detach_dev(struct iommu_domain *domain, struct device *dev) { + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct tegra_smmu_as *as = to_smmu_as(domain); - struct device_node *np = dev->of_node; struct tegra_smmu *smmu = as->smmu; - struct of_phandle_args args; - unsigned int index = 0; - - while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, - &args)) { - unsigned int swgroup = args.args[0]; + unsigned int index; - if (args.np != smmu->dev->of_node) { - of_node_put(args.np); - continue; - } - - of_node_put(args.np); + if (!fwspec) + return; - tegra_smmu_disable(smmu, swgroup, as->id); + for (index = 0; index < fwspec->num_ids; index++) { + tegra_smmu_disable(smmu, fwspec->ids[index], as->id); tegra_smmu_as_unprepare(smmu, as); - index++; } } -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen ` (2 preceding siblings ...) 2020-11-25 10:10 ` [PATCH RESEND v2 3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev Nicolin Chen @ 2020-11-25 10:10 ` Nicolin Chen 2021-02-04 11:10 ` Guillaume Tucker 2020-11-25 10:10 ` [PATCH RESEND v2 5/5] iommu/tegra-smmu: Add PCI support Nicolin Chen ` (2 subsequent siblings) 6 siblings, 1 reply; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh 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 could add clients to iommu groups before iommu core initializes its default domain: ubuntu@jetson:~$ dmesg | grep iommu platform 50000000.host1x: Adding to iommu group 1 platform 57000000.gpu: Adding to iommu group 2 iommu: Default domain type: Translated platform 54200000.dc: Adding to iommu group 3 platform 54240000.dc: Adding to iommu group 3 platform 54340000.vic: Adding to iommu group 4 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-host1x 50000000.host1x: Adding to iommu group 0 tegra-dc 54200000.dc: Adding to iommu group 1 tegra-dc 54240000.dc: Adding to iommu group 1 tegra-vic 54340000.vic: Adding to iommu group 2 nouveau 57000000.gpu: Adding to iommu group 3 Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED. Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Acked-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 96 ++++++-------------------------------- 1 file changed, 15 insertions(+), 81 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 297d49f3f80e..f45ed43cf8db 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -797,75 +797,9 @@ static phys_addr_t tegra_smmu_iova_to_phys(struct iommu_domain *domain, return SMMU_PFN_PHYS(pfn) + SMMU_OFFSET_IN_PAGE(iova); } -static struct tegra_smmu *tegra_smmu_find(struct device_node *np) -{ - struct platform_device *pdev; - struct tegra_mc *mc; - - pdev = of_find_device_by_node(np); - if (!pdev) - return NULL; - - mc = platform_get_drvdata(pdev); - if (!mc) - return NULL; - - return mc->smmu; -} - -static int tegra_smmu_configure(struct tegra_smmu *smmu, struct device *dev, - struct of_phandle_args *args) -{ - const struct iommu_ops *ops = smmu->iommu.ops; - int err; - - err = iommu_fwspec_init(dev, &dev->of_node->fwnode, ops); - if (err < 0) { - dev_err(dev, "failed to initialize fwspec: %d\n", err); - return err; - } - - err = ops->of_xlate(dev, args); - if (err < 0) { - dev_err(dev, "failed to parse SW group ID: %d\n", err); - iommu_fwspec_free(dev); - return err; - } - - return 0; -} - static struct iommu_device *tegra_smmu_probe_device(struct device *dev) { - struct device_node *np = dev->of_node; - struct tegra_smmu *smmu = NULL; - struct of_phandle_args args; - unsigned int index = 0; - int err; - - while (of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, - &args) == 0) { - smmu = tegra_smmu_find(args.np); - if (smmu) { - err = tegra_smmu_configure(smmu, dev, &args); - of_node_put(args.np); - - if (err < 0) - return ERR_PTR(err); - - /* - * Only a single IOMMU master interface is currently - * supported by the Linux kernel, so abort after the - * first match. - */ - dev_iommu_priv_set(dev, smmu); - - break; - } - - of_node_put(args.np); - index++; - } + struct tegra_smmu *smmu = dev_iommu_priv_get(dev); if (!smmu) return ERR_PTR(-ENODEV); @@ -873,10 +807,7 @@ static struct iommu_device *tegra_smmu_probe_device(struct device *dev) return &smmu->iommu; } -static void tegra_smmu_release_device(struct device *dev) -{ - dev_iommu_priv_set(dev, NULL); -} +static void tegra_smmu_release_device(struct device *dev) {} static const struct tegra_smmu_group_soc * tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup) @@ -953,8 +884,21 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) static int tegra_smmu_of_xlate(struct device *dev, struct of_phandle_args *args) { + struct platform_device *iommu_pdev = of_find_device_by_node(args->np); + struct tegra_mc *mc = platform_get_drvdata(iommu_pdev); u32 id = args->args[0]; + /* + * Note: we are here releasing the reference of &iommu_pdev->dev, which + * is mc->dev. Although some functions in tegra_smmu_ops may keep using + * its private data beyond this point, it's still safe to do so because + * the SMMU parent device is the same as the MC, so the reference count + * isn't strictly necessary. + */ + put_device(&iommu_pdev->dev); + + dev_iommu_priv_set(dev, mc->smmu); + return iommu_fwspec_add_ids(dev, &id, 1); } @@ -1079,16 +1023,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev, if (!smmu) return ERR_PTR(-ENOMEM); - /* - * This is a bit of a hack. Ideally we'd want to simply return this - * value. However the IOMMU registration process will attempt to add - * all devices to the IOMMU when bus_set_iommu() is called. In order - * not to rely on global variables to track the IOMMU instance, we - * set it here so that it can be looked up from the .probe_device() - * callback via the IOMMU device's .drvdata field. - */ - mc->smmu = smmu; - size = BITS_TO_LONGS(soc->num_asids) * sizeof(long); smmu->asids = devm_kzalloc(dev, size, GFP_KERNEL); -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2020-11-25 10:10 ` [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen @ 2021-02-04 11:10 ` Guillaume Tucker 2021-02-05 5:24 ` Nicolin Chen 0 siblings, 1 reply; 16+ messages in thread From: Guillaume Tucker @ 2021-02-04 11:10 UTC (permalink / raw) To: Nicolin Chen, will Cc: kernel, kernelci-results, linux-kernel, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, jonathanh Hi Nicolin, A regression was detected by kernelci.org in IGT's drm_read tests on mainline, it was first seen on 17th December 2020. You can find some details here: https://kernelci.org/test/case/id/600b82dc1e3208f123d3dffc/ Then an automated bisection was run and it landed on this patch (v5.10-rc3-4-g25938c73cd79 on mainline). Normally, an email is generated automatically but I had to start this one by hand as there were issues getting it to complete. You can see the failing test cases with this patch: https://lava.collabora.co.uk/results/3126405/0_igt-kms-tegra Some errors are seen around this point in the log: https://lava.collabora.co.uk/scheduler/job/3126405#L1005 [ 3.029729] tegra-mc 70019000.memory-controller: display0a: read @0xfe000000: EMEM address decode error (SMMU translation error [--S]) [ 3.042058] tegra-mc 70019000.memory-controller: display0a: read @0xfe000000: Page fault (SMMU translation error [--S]) Here's the same test passing with this patch reverted: https://lava.collabora.co.uk/results/3126570/0_igt-kms-tegra For completeness, you can see all the test jobs run by the automated bisection here: https://lava.collabora.co.uk/scheduler/device_type/tegra124-nyan-big?dt_length=25&dt_search=bisection-gtucker-12#dt_ Please let us know if you need any help debugging this issue or to try a fix on this platform. Best wishes, Guillaume On 25/11/2020 10:10, Nicolin Chen wrote: > 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 could add clients to > iommu groups before iommu core initializes its default domain: > ubuntu@jetson:~$ dmesg | grep iommu > platform 50000000.host1x: Adding to iommu group 1 > platform 57000000.gpu: Adding to iommu group 2 > iommu: Default domain type: Translated > platform 54200000.dc: Adding to iommu group 3 > platform 54240000.dc: Adding to iommu group 3 > platform 54340000.vic: Adding to iommu group 4 > > 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-host1x 50000000.host1x: Adding to iommu group 0 > tegra-dc 54200000.dc: Adding to iommu group 1 > tegra-dc 54240000.dc: Adding to iommu group 1 > tegra-vic 54340000.vic: Adding to iommu group 2 > nouveau 57000000.gpu: Adding to iommu group 3 > > Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED. > > Reviewed-by: Dmitry Osipenko <digetx@gmail.com> > Tested-by: Dmitry Osipenko <digetx@gmail.com> > Acked-by: Thierry Reding <treding@nvidia.com> > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> > --- > drivers/iommu/tegra-smmu.c | 96 ++++++-------------------------------- > 1 file changed, 15 insertions(+), 81 deletions(-) > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > index 297d49f3f80e..f45ed43cf8db 100644 > --- a/drivers/iommu/tegra-smmu.c > +++ b/drivers/iommu/tegra-smmu.c > @@ -797,75 +797,9 @@ static phys_addr_t tegra_smmu_iova_to_phys(struct iommu_domain *domain, > return SMMU_PFN_PHYS(pfn) + SMMU_OFFSET_IN_PAGE(iova); > } > > -static struct tegra_smmu *tegra_smmu_find(struct device_node *np) > -{ > - struct platform_device *pdev; > - struct tegra_mc *mc; > - > - pdev = of_find_device_by_node(np); > - if (!pdev) > - return NULL; > - > - mc = platform_get_drvdata(pdev); > - if (!mc) > - return NULL; > - > - return mc->smmu; > -} > - > -static int tegra_smmu_configure(struct tegra_smmu *smmu, struct device *dev, > - struct of_phandle_args *args) > -{ > - const struct iommu_ops *ops = smmu->iommu.ops; > - int err; > - > - err = iommu_fwspec_init(dev, &dev->of_node->fwnode, ops); > - if (err < 0) { > - dev_err(dev, "failed to initialize fwspec: %d\n", err); > - return err; > - } > - > - err = ops->of_xlate(dev, args); > - if (err < 0) { > - dev_err(dev, "failed to parse SW group ID: %d\n", err); > - iommu_fwspec_free(dev); > - return err; > - } > - > - return 0; > -} > - > static struct iommu_device *tegra_smmu_probe_device(struct device *dev) > { > - struct device_node *np = dev->of_node; > - struct tegra_smmu *smmu = NULL; > - struct of_phandle_args args; > - unsigned int index = 0; > - int err; > - > - while (of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, > - &args) == 0) { > - smmu = tegra_smmu_find(args.np); > - if (smmu) { > - err = tegra_smmu_configure(smmu, dev, &args); > - of_node_put(args.np); > - > - if (err < 0) > - return ERR_PTR(err); > - > - /* > - * Only a single IOMMU master interface is currently > - * supported by the Linux kernel, so abort after the > - * first match. > - */ > - dev_iommu_priv_set(dev, smmu); > - > - break; > - } > - > - of_node_put(args.np); > - index++; > - } > + struct tegra_smmu *smmu = dev_iommu_priv_get(dev); > > if (!smmu) > return ERR_PTR(-ENODEV); > @@ -873,10 +807,7 @@ static struct iommu_device *tegra_smmu_probe_device(struct device *dev) > return &smmu->iommu; > } > > -static void tegra_smmu_release_device(struct device *dev) > -{ > - dev_iommu_priv_set(dev, NULL); > -} > +static void tegra_smmu_release_device(struct device *dev) {} > > static const struct tegra_smmu_group_soc * > tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup) > @@ -953,8 +884,21 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) > static int tegra_smmu_of_xlate(struct device *dev, > struct of_phandle_args *args) > { > + struct platform_device *iommu_pdev = of_find_device_by_node(args->np); > + struct tegra_mc *mc = platform_get_drvdata(iommu_pdev); > u32 id = args->args[0]; > > + /* > + * Note: we are here releasing the reference of &iommu_pdev->dev, which > + * is mc->dev. Although some functions in tegra_smmu_ops may keep using > + * its private data beyond this point, it's still safe to do so because > + * the SMMU parent device is the same as the MC, so the reference count > + * isn't strictly necessary. > + */ > + put_device(&iommu_pdev->dev); > + > + dev_iommu_priv_set(dev, mc->smmu); > + > return iommu_fwspec_add_ids(dev, &id, 1); > } > > @@ -1079,16 +1023,6 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev, > if (!smmu) > return ERR_PTR(-ENOMEM); > > - /* > - * This is a bit of a hack. Ideally we'd want to simply return this > - * value. However the IOMMU registration process will attempt to add > - * all devices to the IOMMU when bus_set_iommu() is called. In order > - * not to rely on global variables to track the IOMMU instance, we > - * set it here so that it can be looked up from the .probe_device() > - * callback via the IOMMU device's .drvdata field. > - */ > - mc->smmu = smmu; > - > size = BITS_TO_LONGS(soc->num_asids) * sizeof(long); > > smmu->asids = devm_kzalloc(dev, size, GFP_KERNEL); > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-04 11:10 ` Guillaume Tucker @ 2021-02-05 5:24 ` Nicolin Chen 2021-02-05 9:45 ` Nicolin Chen 0 siblings, 1 reply; 16+ messages in thread From: Nicolin Chen @ 2021-02-05 5:24 UTC (permalink / raw) To: Guillaume Tucker Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will Hi Guillaume, On Thu, Feb 04, 2021 at 11:10:15AM +0000, Guillaume Tucker wrote: > Hi Nicolin, > > A regression was detected by kernelci.org in IGT's drm_read tests > on mainline, it was first seen on 17th December 2020. You can > find some details here: > > https://kernelci.org/test/case/id/600b82dc1e3208f123d3dffc/ Thanks for reporting the issue. We did test on Tegra210 and Tegra30 yet not on Tegra124. I am wondering what could go wrong... > Please let us know if you need any help debugging this issue or > to try a fix on this platform. Yes, I don't have any Tegra124 platform to run. It'd be very nice if you can run some debugging patch (I can provide you) and a fix after I root cause the issue. Thanks Nicolin _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-05 5:24 ` Nicolin Chen @ 2021-02-05 9:45 ` Nicolin Chen 2021-02-06 13:40 ` Guillaume Tucker 0 siblings, 1 reply; 16+ messages in thread From: Nicolin Chen @ 2021-02-05 9:45 UTC (permalink / raw) To: Guillaume Tucker Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will [-- Attachment #1: Type: text/plain, Size: 587 bytes --] Hi Guillaume, On Thu, Feb 04, 2021 at 09:24:23PM -0800, Nicolin Chen wrote: > > Please let us know if you need any help debugging this issue or > > to try a fix on this platform. > > Yes, I don't have any Tegra124 platform to run. It'd be very nice > if you can run some debugging patch (I can provide you) and a fix > after I root cause the issue. Would it be possible for you to run with the given debugging patch? It'd be nicer if I can get both logs of the vanilla kernel (failing) and the commit-reverted version (passing), each applying this patch. Thanks in advance! Nicolin [-- Attachment #2: 0001-iommu-debug-tegra-smmu.patch --] [-- Type: text/x-diff, Size: 2365 bytes --] From 80f288d7101101fca0412c5c200cea7e203a675d Mon Sep 17 00:00:00 2001 From: Nicolin Chen <nicoleotsuka@gmail.com> Date: Fri, 5 Feb 2021 01:41:07 -0800 Subject: [PATCH] iommu: debug tegra-smmu Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 4a3f095a1c26..796b7df54b8f 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -363,6 +363,7 @@ static void tegra_smmu_enable(struct tegra_smmu *smmu, unsigned int swgroup, value |= SMMU_ASID_VALUE(asid); value |= SMMU_ASID_ENABLE; smmu_writel(smmu, value, group->reg); + pr_alert("--------%s, swgroup %d: writing %x to reg1 %x\n", __func__, swgroup, value, group->reg); } else { pr_warn("%s group from swgroup %u not found\n", __func__, swgroup); @@ -379,6 +380,7 @@ static void tegra_smmu_enable(struct tegra_smmu *smmu, unsigned int swgroup, value = smmu_readl(smmu, client->smmu.reg); value |= BIT(client->smmu.bit); smmu_writel(smmu, value, client->smmu.reg); + pr_alert("--------%s, swgroup %d: writing %x to reg2 %x\n", __func__, swgroup, value, client->smmu.reg); } } @@ -491,13 +493,19 @@ static int tegra_smmu_attach_dev(struct iommu_domain *domain, unsigned int index; int err; + dev_alert(dev, "-------%s: smmu %s\n", __func__, smmu ? "valid" : "NULL"); + dump_stack(); if (!fwspec) return -ENOENT; + dev_alert(dev, "-------%s: fwspec->num_ids %d\n", __func__, fwspec->num_ids); for (index = 0; index < fwspec->num_ids; index++) { err = tegra_smmu_as_prepare(smmu, as); - if (err) + if (err) { + dev_err(dev, "failed to prepare as(%d) for fwspec %d", + as->id, fwspec->ids[index]); goto disable; + } tegra_smmu_enable(smmu, fwspec->ids[index], as->id); } @@ -805,6 +813,8 @@ static struct iommu_device *tegra_smmu_probe_device(struct device *dev) if (!smmu) return ERR_PTR(-ENODEV); + dev_alert(dev, "--------%s, %d\n", __func__, __LINE__); + dump_stack(); return &smmu->iommu; } @@ -904,6 +914,8 @@ static int tegra_smmu_of_xlate(struct device *dev, dev_iommu_priv_set(dev, mc->smmu); + dev_alert(dev, "-------%s: id %d", __func__, id); + dump_stack(); return iommu_fwspec_add_ids(dev, &id, 1); } -- 2.17.1 [-- Attachment #3: Type: text/plain, Size: 156 bytes --] _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-05 9:45 ` Nicolin Chen @ 2021-02-06 13:40 ` Guillaume Tucker 2021-02-10 8:20 ` Nicolin Chen 0 siblings, 1 reply; 16+ messages in thread From: Guillaume Tucker @ 2021-02-06 13:40 UTC (permalink / raw) To: Nicolin Chen Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will On 05/02/2021 09:45, Nicolin Chen wrote: > Hi Guillaume, > > On Thu, Feb 04, 2021 at 09:24:23PM -0800, Nicolin Chen wrote: >>> Please let us know if you need any help debugging this issue or >>> to try a fix on this platform. >> >> Yes, I don't have any Tegra124 platform to run. It'd be very nice >> if you can run some debugging patch (I can provide you) and a fix >> after I root cause the issue. > > Would it be possible for you to run with the given debugging patch? > > It'd be nicer if I can get both logs of the vanilla kernel (failing) > and the commit-reverted version (passing), each applying this patch. Sure, I've run 3 jobs: * v5.11-rc6 as a reference, to see the original issue: https://lava.collabora.co.uk/scheduler/job/3187848 * + your debug patch: https://lava.collabora.co.uk/scheduler/job/3187849 * + the "breaking" commit reverted, passing the tests: https://lava.collabora.co.uk/scheduler/job/3187851 You can see the history of the test branch I'm using here, with the 3 revisions mentioned above: https://gitlab.collabora.com/gtucker/linux/-/commits/linux-5.11-rc6-nyan-big-drm-read/ Hope that helps, Guillaume _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-06 13:40 ` Guillaume Tucker @ 2021-02-10 8:20 ` Nicolin Chen 2021-02-11 15:50 ` Guillaume Tucker 0 siblings, 1 reply; 16+ messages in thread From: Nicolin Chen @ 2021-02-10 8:20 UTC (permalink / raw) To: Guillaume Tucker Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will [-- Attachment #1: Type: text/plain, Size: 910 bytes --] Hi Guillaume, On Sat, Feb 06, 2021 at 01:40:13PM +0000, Guillaume Tucker wrote: > > It'd be nicer if I can get both logs of the vanilla kernel (failing) > > and the commit-reverted version (passing), each applying this patch. > > Sure, I've run 3 jobs: > > * v5.11-rc6 as a reference, to see the original issue: > https://lava.collabora.co.uk/scheduler/job/3187848 > > * + your debug patch: > https://lava.collabora.co.uk/scheduler/job/3187849 > > * + the "breaking" commit reverted, passing the tests: > https://lava.collabora.co.uk/scheduler/job/3187851 Thanks for the help! I am able to figure out what's probably wrong, yet not so sure about the best solution at this point. Would it be possible for you to run one more time with another debugging patch? I'd like to see the same logs as previous: 1. Vanilla kernel + debug patch 2. Vanilla kernel + Reverted + debug patch Thank you Nicolin [-- Attachment #2: 0001-iommu-debug-tegra-smmu-v2.patch --] [-- Type: text/x-diff, Size: 12404 bytes --] From dce84731e12900b0e98dffc0ff981638a0bb3405 Mon Sep 17 00:00:00 2001 From: Nicolin Chen <nicoleotsuka@gmail.com> Date: Fri, 5 Feb 2021 01:41:07 -0800 Subject: [PATCH] iommu: debug tegra-smmu v2 Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- arch/arm/mm/dma-mapping.c | 6 +++-- drivers/gpu/drm/tegra/dc.c | 1 + drivers/gpu/drm/tegra/drm.c | 4 +++ drivers/iommu/iommu.c | 47 +++++++++++++++++++++++++++------ drivers/iommu/tegra-smmu.c | 31 +++++++++++++++++++++- drivers/memory/tegra/tegra124.c | 2 ++ 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index c4b8df2ad328..6a6715817707 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2136,13 +2136,15 @@ static int __arm_iommu_attach_device(struct device *dev, int err; err = iommu_attach_device(mapping->domain, dev); - if (err) + if (err) { + dev_alert(dev, "------%s failed with err: %d", __func__, err); return err; + } kref_get(&mapping->kref); to_dma_iommu_mapping(dev) = mapping; - pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev)); + pr_alert("Attached IOMMU controller to %s device.\n", dev_name(dev)); return 0; } diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 0ae3a025efe9..96af9186e81a 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -2033,6 +2033,7 @@ static int tegra_dc_init(struct host1x_client *client) struct drm_plane *cursor = NULL; int err; + dev_alert(dc->dev, "-----------%s, %d\n", __func__, __LINE__); /* * XXX do not register DCs with no window groups because we cannot * assign a primary plane to them, which in turn will cause KMS to diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index e9ce7d6992d2..ea6bf26f4df3 100644 --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -906,6 +906,7 @@ int host1x_client_iommu_attach(struct host1x_client *client) struct iommu_group *group = NULL; int err; + dev_alert(client->dev, "-----------%s, %d\n", __func__, __LINE__); /* * If the host1x client is already attached to an IOMMU domain that is * not the shared IOMMU domain, don't try to attach it to a different @@ -914,7 +915,9 @@ int host1x_client_iommu_attach(struct host1x_client *client) if (domain && domain != tegra->domain) return 0; + dev_alert(client->dev, "-----------%s, %d\n", __func__, __LINE__); if (tegra->domain) { + dev_alert(client->dev, "-----------%s, %d\n", __func__, __LINE__); group = iommu_group_get(client->dev); if (!group) return -ENODEV; @@ -932,6 +935,7 @@ int host1x_client_iommu_attach(struct host1x_client *client) client->group = group; + dev_alert(client->dev, "-----------%s, %d\n", __func__, __LINE__); return 0; } diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d0b0a15dba84..890c7c7ecf94 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -220,6 +220,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list group = iommu_group_get_for_dev(dev); if (IS_ERR(group)) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); ret = PTR_ERR(group); goto out_release; } @@ -268,7 +269,9 @@ int iommu_probe_device(struct device *dev) */ iommu_alloc_default_domain(group, dev); + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); if (group->default_domain) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); ret = __iommu_attach_device(group->default_domain, dev); if (ret) { iommu_group_put(group); @@ -852,8 +855,11 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev) mutex_lock(&group->mutex); list_add_tail(&device->list, &group->devices); - if (group->domain && !iommu_is_attach_deferred(group->domain, dev)) + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); + if (group->domain && !iommu_is_attach_deferred(group->domain, dev)) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); ret = __iommu_attach_device(group->domain, dev); + } mutex_unlock(&group->mutex); if (ret) goto err_put_group; @@ -1497,6 +1503,7 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus, { struct iommu_domain *dom; + pr_alert("------%s: %s, %s, %s\n", __func__, bus->name, bus->dev_name, group->name); dom = __iommu_domain_alloc(bus, type); if (!dom && type != IOMMU_DOMAIN_DMA) { dom = __iommu_domain_alloc(bus, IOMMU_DOMAIN_DMA); @@ -1508,6 +1515,7 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus, if (!dom) return -ENOMEM; + pr_alert("------%s: %s: changing default_domain: type %u\n", __func__, group->name, type); group->default_domain = dom; if (!group->domain) group->domain = dom; @@ -1532,6 +1540,7 @@ static int iommu_alloc_default_domain(struct iommu_group *group, type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type; + dev_alert(dev, "---------%s, %d: type %u\n", __func__, __LINE__, type); return iommu_group_alloc_default_domain(dev->bus, group, type); } @@ -1552,22 +1561,30 @@ static struct iommu_group *iommu_group_get_for_dev(struct device *dev) int ret; group = iommu_group_get(dev); - if (group) + if (group) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); return group; + } if (!ops) return ERR_PTR(-EINVAL); + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); group = ops->device_group(dev); + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); if (WARN_ON_ONCE(group == NULL)) return ERR_PTR(-EINVAL); - if (IS_ERR(group)) + if (IS_ERR(group)) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); return group; + } ret = iommu_group_add_device(group, dev); - if (ret) + if (ret) { + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); goto out_put_group; + } return group; @@ -1928,6 +1945,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, struct iommu_domain *iommu_domain_alloc(struct bus_type *bus) { + pr_alert("------%s: %s, %s\n", __func__, bus->name, bus->dev_name); return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED); } EXPORT_SYMBOL_GPL(iommu_domain_alloc); @@ -1943,12 +1961,17 @@ static int __iommu_attach_device(struct iommu_domain *domain, { int ret; - if (unlikely(domain->ops->attach_dev == NULL)) + if (unlikely(domain->ops->attach_dev == NULL)) { + dev_alert(dev, "-------%s: no attach_dev\n", __func__); return -ENODEV; + } ret = domain->ops->attach_dev(domain, dev); if (!ret) trace_attach_device_to_domain(dev); + else + dev_alert(dev, "-------%s: failed to attach_dev: %d\n", __func__, ret); + return ret; } @@ -1958,8 +1981,10 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) int ret; group = iommu_group_get(dev); - if (!group) + if (!group) { + dev_alert(dev, "-------%s: failed to get group\n", __func__); return -ENODEV; + } /* * Lock the group to make sure the device-count doesn't @@ -1967,8 +1992,10 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) */ mutex_lock(&group->mutex); ret = -EINVAL; - if (iommu_group_device_count(group) != 1) + if (iommu_group_device_count(group) != 1) { + dev_alert(dev, "-------%s: bypassing attach_group\n", __func__); goto out_unlock; + } ret = __iommu_attach_group(domain, group); @@ -2283,13 +2310,17 @@ static int __iommu_attach_group(struct iommu_domain *domain, { int ret; - if (group->default_domain && group->domain != group->default_domain) + if (group->default_domain && group->domain != group->default_domain) { + pr_alert("------------%s, domain mismatched\n", __func__); return -EBUSY; + } ret = __iommu_group_for_each_dev(group, domain, iommu_group_do_attach_device); if (ret == 0) group->domain = domain; + else + pr_alert("------------%s, failed: %d\n", __func__, ret); return ret; } diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 4a3f095a1c26..2363a9978674 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -281,6 +281,8 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type) { struct tegra_smmu_as *as; + pr_alert("---------%s, %d: type %u\n", __func__, __LINE__, type); + dump_stack(); if (type != IOMMU_DOMAIN_UNMANAGED) return NULL; @@ -318,6 +320,7 @@ static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type) as->domain.geometry.aperture_end = 0xffffffff; as->domain.geometry.force_aperture = true; + pr_alert("---------%s, %d: type %u\n", __func__, __LINE__, type); return &as->domain; } @@ -363,6 +366,7 @@ static void tegra_smmu_enable(struct tegra_smmu *smmu, unsigned int swgroup, value |= SMMU_ASID_VALUE(asid); value |= SMMU_ASID_ENABLE; smmu_writel(smmu, value, group->reg); + pr_alert("--------%s, swgroup %d: writing %x to reg1 %x\n", __func__, swgroup, value, group->reg); } else { pr_warn("%s group from swgroup %u not found\n", __func__, swgroup); @@ -379,6 +383,7 @@ static void tegra_smmu_enable(struct tegra_smmu *smmu, unsigned int swgroup, value = smmu_readl(smmu, client->smmu.reg); value |= BIT(client->smmu.bit); smmu_writel(smmu, value, client->smmu.reg); + pr_alert("--------%s, swgroup %d: writing %x to reg2 %x\n", __func__, swgroup, value, client->smmu.reg); } } @@ -491,13 +496,22 @@ static int tegra_smmu_attach_dev(struct iommu_domain *domain, unsigned int index; int err; + dev_alert(dev, "-------%s\n", __func__); + if (strstr(dev_name(dev), "dc")) + dump_stack(); if (!fwspec) return -ENOENT; + if (iommu_get_domain_for_dev(dev) != domain) + dev_alert(dev, "-------%s: domain not matched\n", __func__); + for (index = 0; index < fwspec->num_ids; index++) { err = tegra_smmu_as_prepare(smmu, as); - if (err) + if (err) { + dev_err(dev, "failed to prepare as(%d) for fwspec %d", + as->id, fwspec->ids[index]); goto disable; + } tegra_smmu_enable(smmu, fwspec->ids[index], as->id); } @@ -805,6 +819,9 @@ static struct iommu_device *tegra_smmu_probe_device(struct device *dev) if (!smmu) return ERR_PTR(-ENODEV); + dev_alert(dev, "--------%s, %d\n", __func__, __LINE__); + if (strstr(dev_name(dev), "dc")) + dump_stack(); return &smmu->iommu; } @@ -842,6 +859,7 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) struct tegra_smmu_group *group; struct iommu_group *grp; + dev_alert(dev, "---------%s, %d\n", __func__, __LINE__); /* Find group_soc associating with swgroup */ soc = tegra_smmu_find_group(smmu, swgroup); @@ -852,6 +870,10 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) if ((group->swgroup == swgroup) || (soc && group->soc == soc)) { grp = iommu_group_ref_get(group->group); mutex_unlock(&smmu->lock); + if (soc && group->soc == soc) + dev_alert(dev, "---------%s, %d: %u: %s\n", __func__, __LINE__, swgroup, soc->name); + else + dev_alert(dev, "---------%s, %d, %u\n", __func__, __LINE__, swgroup); return grp; } @@ -883,6 +905,10 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) list_add_tail(&group->list, &smmu->groups); mutex_unlock(&smmu->lock); + if (soc) + dev_alert(dev, "---------%s, %d: %u: %s\n", __func__, __LINE__, swgroup, soc->name); + else + dev_alert(dev, "---------%s, %d: %u\n", __func__, __LINE__, swgroup); return group->group; } @@ -904,6 +930,9 @@ static int tegra_smmu_of_xlate(struct device *dev, dev_iommu_priv_set(dev, mc->smmu); + dev_alert(dev, "-------%s: id %d", __func__, id); + if (strstr(dev_name(dev), "dc")) + dump_stack(); return iommu_fwspec_add_ids(dev, &id, 1); } diff --git a/drivers/memory/tegra/tegra124.c b/drivers/memory/tegra/tegra124.c index 459211f50c08..faa4f4c53ac3 100644 --- a/drivers/memory/tegra/tegra124.c +++ b/drivers/memory/tegra/tegra124.c @@ -1062,6 +1062,8 @@ tegra124_mc_of_icc_xlate_extended(struct of_phandle_args *spec, void *data) case TEGRA_SWGROUP_VI: /* these clients are isochronous by default */ ndata->tag = TEGRA_MC_ICC_TAG_ISO; + dev_alert(mc->dev, "-----%s, setting ISO for swgroup %d\n", __func__, client->swgroup); + dump_stack(); break; default: -- 2.17.1 [-- Attachment #3: Type: text/plain, Size: 156 bytes --] _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-10 8:20 ` Nicolin Chen @ 2021-02-11 15:50 ` Guillaume Tucker 2021-02-18 10:35 ` Nicolin Chen 0 siblings, 1 reply; 16+ messages in thread From: Guillaume Tucker @ 2021-02-11 15:50 UTC (permalink / raw) To: Nicolin Chen Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will On 10/02/2021 08:20, Nicolin Chen wrote: > Hi Guillaume, > > On Sat, Feb 06, 2021 at 01:40:13PM +0000, Guillaume Tucker wrote: >>> It'd be nicer if I can get both logs of the vanilla kernel (failing) >>> and the commit-reverted version (passing), each applying this patch. >> >> Sure, I've run 3 jobs: >> >> * v5.11-rc6 as a reference, to see the original issue: >> https://lava.collabora.co.uk/scheduler/job/3187848 >> >> * + your debug patch: >> https://lava.collabora.co.uk/scheduler/job/3187849 >> >> * + the "breaking" commit reverted, passing the tests: >> https://lava.collabora.co.uk/scheduler/job/3187851 > > Thanks for the help! > > I am able to figure out what's probably wrong, yet not so sure > about the best solution at this point. > > Would it be possible for you to run one more time with another > debugging patch? I'd like to see the same logs as previous: > 1. Vanilla kernel + debug patch > 2. Vanilla kernel + Reverted + debug patch As it turns out, next-20210210 is passing all the tests again so it looks like this got fixed in the meantime: https://lava.collabora.co.uk/scheduler/job/3210192 https://lava.collabora.co.uk/results/3210192/0_igt-kms-tegra And here's a more extensive list of IGT tests on next-20210211, all the regressions have been fixed: https://kernelci.org/test/plan/id/60254c42f51df36be53abe62/ I haven't run a reversed bisection to find the fix, but I guess it wouldn't be too hard to find out what happened by hand anyway. I see the drm/tegra/for-5.12-rc1 tag has been merged into linux-next, maybe that solved the issue? FYI I've also run some jobs with your debug patch and with the breaking patch reverted: https://lava.collabora.co.uk/scheduler/job/3210245 https://lava.collabora.co.uk/scheduler/job/3210596 Meanwhile I'll see what can be done to improve the automated bisection so if there are new IGT regressions they would get reported earlier. I guess it would have saved us all some time if it had been bisected in December. Thanks, Guillaume _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-11 15:50 ` Guillaume Tucker @ 2021-02-18 10:35 ` Nicolin Chen 2021-02-18 20:38 ` Guillaume Tucker 0 siblings, 1 reply; 16+ messages in thread From: Nicolin Chen @ 2021-02-18 10:35 UTC (permalink / raw) To: Guillaume Tucker Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will [-- Attachment #1: Type: text/plain, Size: 2206 bytes --] Hi Guillaume, Thank you for the test results! And sorry for my belated reply. On Thu, Feb 11, 2021 at 03:50:05PM +0000, Guillaume Tucker wrote: > > On Sat, Feb 06, 2021 at 01:40:13PM +0000, Guillaume Tucker wrote: > >>> It'd be nicer if I can get both logs of the vanilla kernel (failing) > >>> and the commit-reverted version (passing), each applying this patch. > >> > >> Sure, I've run 3 jobs: > >> > >> * v5.11-rc6 as a reference, to see the original issue: > >> https://lava.collabora.co.uk/scheduler/job/3187848 > >> > >> * + your debug patch: > >> https://lava.collabora.co.uk/scheduler/job/3187849 > >> > >> * + the "breaking" commit reverted, passing the tests: > >> https://lava.collabora.co.uk/scheduler/job/3187851 > > > > Thanks for the help! > > > > I am able to figure out what's probably wrong, yet not so sure > > about the best solution at this point. > > > > Would it be possible for you to run one more time with another > > debugging patch? I'd like to see the same logs as previous: > > 1. Vanilla kernel + debug patch > > 2. Vanilla kernel + Reverted + debug patch > > As it turns out, next-20210210 is passing all the tests again so > it looks like this got fixed in the meantime: > > https://lava.collabora.co.uk/scheduler/job/3210192 I checked this passing log, however, found that the regression is still there though test passed, as the prints below aren't normal: tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: EMEM address decode error (SMMU translation error [--S]) tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: Page fault (SMMU translation error [--S]) I was trying to think of a simpler solution than a revert. However, given the fact that the callback sequence could change -- guessing likely a recent change in iommu core, I feel it safer to revert my previous change, not necessarily being a complete revert though. I attached my partial reverting change in this email. Would it be possible for you to run one more test for me to confirm it? It'd keep the tests passing while eliminating all error prints above. If the fix works, I'll re-send it to mail list by adding a commit message. Thanks! Nicolin [-- Attachment #2: 0001-iommu-tegra-smmu-Fix-mc-errors-on-tegra124-nyan.patch --] [-- Type: text/x-diff, Size: 3125 bytes --] From a9950b6e76e279f19d2c9d06aef1e222b020a9e2 Mon Sep 17 00:00:00 2001 From: Nicolin Chen <nicoleotsuka@gmail.com> Date: Thu, 18 Feb 2021 01:11:59 -0800 Subject: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: EMEM address decode error (SMMU translation error [--S]) tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: Page fault (SMMU translation error [--S]) Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 72 +++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 4a3f095a1c26..97eb62f667d2 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -798,10 +798,70 @@ static phys_addr_t tegra_smmu_iova_to_phys(struct iommu_domain *domain, return SMMU_PFN_PHYS(pfn) + SMMU_OFFSET_IN_PAGE(iova); } +static struct tegra_smmu *tegra_smmu_find(struct device_node *np) +{ + struct platform_device *pdev; + struct tegra_mc *mc; + + pdev = of_find_device_by_node(np); + if (!pdev) + return NULL; + + mc = platform_get_drvdata(pdev); + if (!mc) + return NULL; + + return mc->smmu; +} + +static int tegra_smmu_configure(struct tegra_smmu *smmu, struct device *dev, + struct of_phandle_args *args) +{ + const struct iommu_ops *ops = smmu->iommu.ops; + int err; + + err = iommu_fwspec_init(dev, &dev->of_node->fwnode, ops); + if (err < 0) { + dev_err(dev, "failed to initialize fwspec: %d\n", err); + return err; + } + + err = ops->of_xlate(dev, args); + if (err < 0) { + dev_err(dev, "failed to parse SW group ID: %d\n", err); + iommu_fwspec_free(dev); + return err; + } + + return 0; +} + static struct iommu_device *tegra_smmu_probe_device(struct device *dev) { - struct tegra_smmu *smmu = dev_iommu_priv_get(dev); + struct device_node *np = dev->of_node; + struct tegra_smmu *smmu = NULL; + struct of_phandle_args args; + unsigned int index = 0; + int err; + + while (of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, + &args) == 0) { + smmu = tegra_smmu_find(args.np); + if (smmu) { + err = tegra_smmu_configure(smmu, dev, &args); + of_node_put(args.np); + if (err < 0) + return ERR_PTR(err); + + break; + } + + of_node_put(args.np); + index++; + } + + smmu = dev_iommu_priv_get(dev); if (!smmu) return ERR_PTR(-ENODEV); @@ -1028,6 +1088,16 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev, if (!smmu) return ERR_PTR(-ENOMEM); + /* + * This is a bit of a hack. Ideally we'd want to simply return this + * value. However the IOMMU registration process will attempt to add + * all devices to the IOMMU when bus_set_iommu() is called. In order + * not to rely on global variables to track the IOMMU instance, we + * set it here so that it can be looked up from the .probe_device() + * callback via the IOMMU device's .drvdata field. + */ + mc->smmu = smmu; + size = BITS_TO_LONGS(soc->num_asids) * sizeof(long); smmu->asids = devm_kzalloc(dev, size, GFP_KERNEL); -- 2.17.1 [-- Attachment #3: Type: text/plain, Size: 156 bytes --] _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() 2021-02-18 10:35 ` Nicolin Chen @ 2021-02-18 20:38 ` Guillaume Tucker 0 siblings, 0 replies; 16+ messages in thread From: Guillaume Tucker @ 2021-02-18 20:38 UTC (permalink / raw) To: Nicolin Chen Cc: kernelci-results, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, Dmitry Osipenko, kernel, will On 18/02/2021 10:35, Nicolin Chen wrote: > Hi Guillaume, > > Thank you for the test results! And sorry for my belated reply. No worries :) > On Thu, Feb 11, 2021 at 03:50:05PM +0000, Guillaume Tucker wrote: >>> On Sat, Feb 06, 2021 at 01:40:13PM +0000, Guillaume Tucker wrote: >>>>> It'd be nicer if I can get both logs of the vanilla kernel (failing) >>>>> and the commit-reverted version (passing), each applying this patch. >>>> >>>> Sure, I've run 3 jobs: >>>> >>>> * v5.11-rc6 as a reference, to see the original issue: >>>> https://lava.collabora.co.uk/scheduler/job/3187848 >>>> >>>> * + your debug patch: >>>> https://lava.collabora.co.uk/scheduler/job/3187849 >>>> >>>> * + the "breaking" commit reverted, passing the tests: >>>> https://lava.collabora.co.uk/scheduler/job/3187851 >>> >>> Thanks for the help! >>> >>> I am able to figure out what's probably wrong, yet not so sure >>> about the best solution at this point. >>> >>> Would it be possible for you to run one more time with another >>> debugging patch? I'd like to see the same logs as previous: >>> 1. Vanilla kernel + debug patch >>> 2. Vanilla kernel + Reverted + debug patch >> >> As it turns out, next-20210210 is passing all the tests again so >> it looks like this got fixed in the meantime: >> >> https://lava.collabora.co.uk/scheduler/job/3210192 > > I checked this passing log, however, found that the regression is > still there though test passed, as the prints below aren't normal: > tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: > EMEM address decode error (SMMU translation error [--S]) > tegra-mc 70019000.memory-controller: display0a: read @0xfe056b40: > Page fault (SMMU translation error [--S]) Ah yes sorry, there are other KernelCI checks for kernel errors but that wasn't enabled in the bisection so I didn't notice them. > I was trying to think of a simpler solution than a revert. However, > given the fact that the callback sequence could change -- guessing > likely a recent change in iommu core, I feel it safer to revert my > previous change, not necessarily being a complete revert though. > > I attached my partial reverting change in this email. Would it be > possible for you to run one more test for me to confirm it? It'd > keep the tests passing while eliminating all error prints above. > > If the fix works, I'll re-send it to mail list by adding a commit > message. Sure, here's next-20210218 as a reference: https://lava.collabora.co.uk/scheduler/job/3241236 and here with your patch applied on top of it: https://lava.collabora.co.uk/scheduler/job/3241246 The git branch I've used where your patch is applied: https://gitlab.collabora.com/gtucker/linux/-/commits/next-20210218-nyan-big-drm-read/ The errors seem to have disappeared but I'll let you double check that things are all back to a working state. BTW: This thread is a good example of how having an "on-demand" KernelCI service to let developers re-run tests with extra patches would allow them to fix issues independently. We'll keep that in mind for the future. Best wishes, Guillaume _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH RESEND v2 5/5] iommu/tegra-smmu: Add PCI support 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen ` (3 preceding siblings ...) 2020-11-25 10:10 ` [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen @ 2020-11-25 10:10 ` Nicolin Chen 2020-11-25 11:03 ` [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Will Deacon 2020-11-25 14:05 ` Will Deacon 6 siblings, 0 replies; 16+ messages in thread From: Nicolin Chen @ 2020-11-25 10:10 UTC (permalink / raw) To: will; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh This patch simply adds support for PCI devices. Reviewed-by: Dmitry Osipenko <digetx@gmail.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Acked-by: Thierry Reding <treding@nvidia.com> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index f45ed43cf8db..4a3f095a1c26 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -10,6 +10,7 @@ #include <linux/kernel.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/pci.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -865,7 +866,11 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) group->smmu = smmu; group->soc = soc; - group->group = iommu_group_alloc(); + if (dev_is_pci(dev)) + group->group = pci_device_group(dev); + else + group->group = generic_device_group(dev); + if (IS_ERR(group->group)) { devm_kfree(smmu->dev, group); mutex_unlock(&smmu->lock); @@ -1075,22 +1080,32 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev, iommu_device_set_fwnode(&smmu->iommu, dev->fwnode); err = iommu_device_register(&smmu->iommu); - if (err) { - iommu_device_sysfs_remove(&smmu->iommu); - return ERR_PTR(err); - } + if (err) + goto remove_sysfs; err = bus_set_iommu(&platform_bus_type, &tegra_smmu_ops); - if (err < 0) { - iommu_device_unregister(&smmu->iommu); - iommu_device_sysfs_remove(&smmu->iommu); - return ERR_PTR(err); - } + if (err < 0) + goto unregister; + +#ifdef CONFIG_PCI + err = bus_set_iommu(&pci_bus_type, &tegra_smmu_ops); + if (err < 0) + goto unset_platform_bus; +#endif if (IS_ENABLED(CONFIG_DEBUG_FS)) tegra_smmu_debugfs_init(smmu); return smmu; + +unset_platform_bus: __maybe_unused; + bus_set_iommu(&platform_bus_type, NULL); +unregister: + iommu_device_unregister(&smmu->iommu); +remove_sysfs: + iommu_device_sysfs_remove(&smmu->iommu); + + return ERR_PTR(err); } void tegra_smmu_remove(struct tegra_smmu *smmu) -- 2.17.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen ` (4 preceding siblings ...) 2020-11-25 10:10 ` [PATCH RESEND v2 5/5] iommu/tegra-smmu: Add PCI support Nicolin Chen @ 2020-11-25 11:03 ` Will Deacon 2020-11-25 14:05 ` Will Deacon 6 siblings, 0 replies; 16+ messages in thread From: Will Deacon @ 2020-11-25 11:03 UTC (permalink / raw) To: Nicolin Chen; +Cc: linux-kernel, iommu, thierry.reding, linux-tegra, jonathanh On Wed, Nov 25, 2020 at 02:10:08AM -0800, Nicolin Chen wrote: > Changelog > v1->v2: > * Added Thierry's acks to PATCH-3~5 > > This is a merged set of resend for previously two series of patches > that were reviewed/acked a month ago yet have not got applied. Thanks, and sorry I missed these (I'm doing my best). I'll queue the lot for 5.11. Will _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen ` (5 preceding siblings ...) 2020-11-25 11:03 ` [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Will Deacon @ 2020-11-25 14:05 ` Will Deacon 6 siblings, 0 replies; 16+ messages in thread From: Will Deacon @ 2020-11-25 14:05 UTC (permalink / raw) To: Nicolin Chen Cc: Will Deacon, catalin.marinas, linux-kernel, jonathanh, iommu, thierry.reding, linux-tegra, kernel-team On Wed, 25 Nov 2020 02:10:08 -0800, Nicolin Chen wrote: > Changelog > v1->v2: > * Added Thierry's acks to PATCH-3~5 > > This is a merged set of resend for previously two series of patches > that were reviewed/acked a month ago yet have not got applied. > > [...] Applied to arm64 (for-next/iommu/tegra-smmu), thanks! [1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get https://git.kernel.org/arm64/c/cf910f61aff3 [2/5] iommu/tegra-smmu: Expand mutex protection range https://git.kernel.org/arm64/c/d5f583bf8654 [3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev https://git.kernel.org/arm64/c/8750d207dc98 [4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() https://git.kernel.org/arm64/c/25938c73cd79 [5/5] iommu/tegra-smmu: Add PCI support https://git.kernel.org/arm64/c/541f29bb0643 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-02-18 20:39 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-25 10:10 [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 2/5] iommu/tegra-smmu: Expand mutex protection range Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev Nicolin Chen 2020-11-25 10:10 ` [PATCH RESEND v2 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen 2021-02-04 11:10 ` Guillaume Tucker 2021-02-05 5:24 ` Nicolin Chen 2021-02-05 9:45 ` Nicolin Chen 2021-02-06 13:40 ` Guillaume Tucker 2021-02-10 8:20 ` Nicolin Chen 2021-02-11 15:50 ` Guillaume Tucker 2021-02-18 10:35 ` Nicolin Chen 2021-02-18 20:38 ` Guillaume Tucker 2020-11-25 10:10 ` [PATCH RESEND v2 5/5] iommu/tegra-smmu: Add PCI support Nicolin Chen 2020-11-25 11:03 ` [PATCH RESEND v2 0/5] iommu/tegra-smmu: Some pending reviewed changes Will Deacon 2020-11-25 14:05 ` Will Deacon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).