Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
28.03.2021 18:25, Dmitry Osipenko пишет: > 03.03.2021 12:47, Dmitry Osipenko пишет: >> 03.03.2021 02:08, Nicolin Chen пишет: >>> On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: 25.02.2021 09:27, Nicolin Chen пишет: ... >> The partially revert should be okay, but it's not clear to me what makes >> difference for T124 since I don't see that problem on T30, which also >> has active display at a boot time. > Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach > or from of_dma_configure_id/arch_setup_dma_ops? > I applied yours debug-patch, please see dmesg.txt attached to the email. Seems probe-defer of the tegra-dc driver prevents the implicit tegra_smmu_attach_dev, so it happens to work by accident. [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 [0.328641] [] (tegra_smmu_of_xlate) from [] (of_iommu_xlate+0x51/0x70) [0.328740] [] (of_iommu_xlate) from [] (of_iommu_configure+0x127/0x150) [0.328896] [] (of_iommu_configure) from [] (of_dma_configure_id+0x1fb/0x2ec) [0.329060] [] (of_dma_configure_id) from [] (really_probe+0x7b/0x2a0) [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 [0.332234] [] (tegra_smmu_probe_device) from [] (__iommu_probe_device+0x35/0x1c4) [0.332391] [] (__iommu_probe_device) from [] (iommu_probe_device+0x19/0xec) [0.332545] [] (iommu_probe_device) from [] (of_iommu_configure+0xfb/0x150) [0.332701] [] (of_iommu_configure) from [] (of_dma_configure_id+0x1fb/0x2ec) [0.332804] [] (of_dma_configure_id) from [] (really_probe+0x7b/0x2a0) [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, 909: 1: drm [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, 1543: type 3 [0.336101] iommu: --iommu_group_alloc_default_domain: platform, (null), drm [0.336187] -tegra_smmu_domain_alloc, 284: type 3 >>> [0.336968] [] (tegra_smmu_domain_alloc) from [] >>> (iommu_group_alloc_default_domain+0x4b/0xfa) [0.337127] [] (iommu_group_alloc_default_domain) from [] (iommu_probe_device+0x69/0xec) [0.337285] [] (iommu_probe_device) from [] (of_iommu_configure+0xfb/0x150) [0.337441] [] (of_iommu_configure) from [] (of_dma_configure_id+0x1fb/0x2ec) [0.337599] [] (of_dma_configure_id) from [] (really_probe+0x7b/0x2a0) [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 >>> Hmm..not sure where this EPROBE_DEFER comes from. >> DC driver on Nexus 7 depends on LVDS bridge and display panel, which >> cause the probe defer. >> >>> But you are right, >>> as of_dma_configure_id() returns because of that so it didn't run to >>> arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain >>> and attaches DC to it on Tegra124. >>> >>> By the way, anyone can accept this change? It doesn't feel right to >>> leave a regression in the newer release... > > Guys, I have a good and bad news. > > The good news is that I figured out why I didn't see this problem on > Nexus 7 and the reason is that I had CONFIG_ARM_DMA_USE_IOMMU=n. > > The other good news is that I have a simple workaround which fixes the > implicit IOMMU problem by deferring the ASID enabling for display clients. > > The bad news is that CONFIG_ARM_DMA_USE_IOMMU=y breaks GPU (DRM, host1x) > drivers because they aren't properly prepared to this case and > CONFIG_ARM_DMA_USE_IOMMU is enabled in multi-platform kernel config. I > will try to fix up the drivers, but not sure how much time this may take. > Oh, actually the old workaround with arm_iommu_detach_device() still works, so we just need to bring it back. I'll prepare the patches. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
03.03.2021 12:47, Dmitry Osipenko пишет: > 03.03.2021 02:08, Nicolin Chen пишет: >> On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: >>> 25.02.2021 09:27, Nicolin Chen пишет: >>> ... > The partially revert should be okay, but it's not clear to me what makes > difference for T124 since I don't see that problem on T30, which also > has active display at a boot time. Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach or from of_dma_configure_id/arch_setup_dma_ops? >>> I applied yours debug-patch, please see dmesg.txt attached to the email. >>> Seems probe-defer of the tegra-dc driver prevents the implicit >>> tegra_smmu_attach_dev, so it happens to work by accident. >>> [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 >>> [0.328641] [] (tegra_smmu_of_xlate) from [] >>> (of_iommu_xlate+0x51/0x70) >>> [0.328740] [] (of_iommu_xlate) from [] >>> (of_iommu_configure+0x127/0x150) >>> [0.328896] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.329060] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 >>> [0.332234] [] (tegra_smmu_probe_device) from [] >>> (__iommu_probe_device+0x35/0x1c4) >>> [0.332391] [] (__iommu_probe_device) from [] >>> (iommu_probe_device+0x19/0xec) >>> [0.332545] [] (iommu_probe_device) from [] >>> (of_iommu_configure+0xfb/0x150) >>> [0.332701] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.332804] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 >>> [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 >>> [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, 909: >>> 1: drm >>> [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 >>> [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 >>> [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 >>> [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, >>> 1543: type 3 >>> [0.336101] iommu: --iommu_group_alloc_default_domain: platform, >>> (null), drm >>> [0.336187] -tegra_smmu_domain_alloc, 284: type 3 >> [0.336968] [] (tegra_smmu_domain_alloc) from [] >> (iommu_group_alloc_default_domain+0x4b/0xfa) >>> [0.337127] [] (iommu_group_alloc_default_domain) from >>> [] (iommu_probe_device+0x69/0xec) >>> [0.337285] [] (iommu_probe_device) from [] >>> (of_iommu_configure+0xfb/0x150) >>> [0.337441] [] (of_iommu_configure) from [] >>> (of_dma_configure_id+0x1fb/0x2ec) >>> [0.337599] [] (of_dma_configure_id) from [] >>> (really_probe+0x7b/0x2a0) >>> [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 >>> [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 >> Hmm..not sure where this EPROBE_DEFER comes from. > DC driver on Nexus 7 depends on LVDS bridge and display panel, which > cause the probe defer. > >> But you are right, >> as of_dma_configure_id() returns because of that so it didn't run to >> arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain >> and attaches DC to it on Tegra124. >> >> By the way, anyone can accept this change? It doesn't feel right to >> leave a regression in the newer release... Guys, I have a good and bad news. The good news is that I figured out why I didn't see this problem on Nexus 7 and the reason is that I had CONFIG_ARM_DMA_USE_IOMMU=n. The other good news is that I have a simple workaround which fixes the implicit IOMMU problem by deferring the ASID enabling for display clients. The bad news is that CONFIG_ARM_DMA_USE_IOMMU=y breaks GPU (DRM, host1x) drivers because they aren't properly prepared to this case and CONFIG_ARM_DMA_USE_IOMMU is enabled in multi-platform kernel config. I will try to fix up the drivers, but not sure how much time this may take. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Thu, Mar 11, 2021 at 03:06:25PM +0300, Dmitry Osipenko wrote: > 11.03.2021 01:17, Nicolin Chen пишет: > > On Wed, Mar 10, 2021 at 11:22:57PM +0300, Dmitry Osipenko wrote: > >> 10.03.2021 22:13, Dmitry Osipenko пишет: > >>> I found that this patch introduced a serious regression on Tegra30 using > >>> today's linux-next. Tegra30 has two 3d h/w blocks connected in SLI and > >>> only one of the blocks is now attached to IOMMU domain, meaning that GPU > >>> is unusable now. All 3d, 2d and display devices share the same "DRM" > >>> group on Tegra30. > >>> > >>> Nicolin, please let me know if have any suggestions. I may take a closer > >>> look a day later, for now I'll just revert this patch locally. Thanks in > >>> advance. > >>> > >> > >> Actually, this was easy to fix: > >> > >> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > >> index 97eb62f667d2..639d5ceab60b 100644 > >> --- a/drivers/iommu/tegra-smmu.c > >> +++ b/drivers/iommu/tegra-smmu.c > >> @@ -853,8 +853,6 @@ static struct iommu_device > >> *tegra_smmu_probe_device(struct device *dev) > >> > >>if (err < 0) > >>return ERR_PTR(err); > >> - > >> - break; > > > > Hmm..I don't understand why this "break" causes problems on Tegra30. > > The older versions that used _find()+configure() had it also, e.g.: > > https://elixir.bootlin.com/linux/v5.9.16/source/drivers/iommu/tegra-smmu.c#L760 > > > > Dmitry, do you have any idea? > > > > The older variant of tegra_smmu_attach_dev() didn't use fwspec [1], that > makes the difference. In other words, the older variant of > tegra_smmu_probe_device() was already buggy, but the bug was masked by > the tegra_smmu_attach_dev() that didn't use the fwspec. > > [1] > https://elixir.bootlin.com/linux/v5.10.22/source/drivers/iommu/tegra-smmu.c#L476 Hmm...would you mind submitting a fix from your side? I think it'd be more appropriate to do so, as you can definitely write a better commit message than I can for this bug. Thanks! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
11.03.2021 01:17, Nicolin Chen пишет: > On Wed, Mar 10, 2021 at 11:22:57PM +0300, Dmitry Osipenko wrote: >> 10.03.2021 22:13, Dmitry Osipenko пишет: >>> I found that this patch introduced a serious regression on Tegra30 using >>> today's linux-next. Tegra30 has two 3d h/w blocks connected in SLI and >>> only one of the blocks is now attached to IOMMU domain, meaning that GPU >>> is unusable now. All 3d, 2d and display devices share the same "DRM" >>> group on Tegra30. >>> >>> Nicolin, please let me know if have any suggestions. I may take a closer >>> look a day later, for now I'll just revert this patch locally. Thanks in >>> advance. >>> >> >> Actually, this was easy to fix: >> >> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c >> index 97eb62f667d2..639d5ceab60b 100644 >> --- a/drivers/iommu/tegra-smmu.c >> +++ b/drivers/iommu/tegra-smmu.c >> @@ -853,8 +853,6 @@ static struct iommu_device >> *tegra_smmu_probe_device(struct device *dev) >> >> if (err < 0) >> return ERR_PTR(err); >> - >> -break; > > Hmm..I don't understand why this "break" causes problems on Tegra30. > The older versions that used _find()+configure() had it also, e.g.: > https://elixir.bootlin.com/linux/v5.9.16/source/drivers/iommu/tegra-smmu.c#L760 > > Dmitry, do you have any idea? > The older variant of tegra_smmu_attach_dev() didn't use fwspec [1], that makes the difference. In other words, the older variant of tegra_smmu_probe_device() was already buggy, but the bug was masked by the tegra_smmu_attach_dev() that didn't use the fwspec. [1] https://elixir.bootlin.com/linux/v5.10.22/source/drivers/iommu/tegra-smmu.c#L476 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Wed, Mar 10, 2021 at 11:22:57PM +0300, Dmitry Osipenko wrote: > 10.03.2021 22:13, Dmitry Osipenko пишет: > > I found that this patch introduced a serious regression on Tegra30 using > > today's linux-next. Tegra30 has two 3d h/w blocks connected in SLI and > > only one of the blocks is now attached to IOMMU domain, meaning that GPU > > is unusable now. All 3d, 2d and display devices share the same "DRM" > > group on Tegra30. > > > > Nicolin, please let me know if have any suggestions. I may take a closer > > look a day later, for now I'll just revert this patch locally. Thanks in > > advance. > > > > Actually, this was easy to fix: > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > index 97eb62f667d2..639d5ceab60b 100644 > --- a/drivers/iommu/tegra-smmu.c > +++ b/drivers/iommu/tegra-smmu.c > @@ -853,8 +853,6 @@ static struct iommu_device > *tegra_smmu_probe_device(struct device *dev) > > if (err < 0) > return ERR_PTR(err); > - > - break; Hmm..I don't understand why this "break" causes problems on Tegra30. The older versions that used _find()+configure() had it also, e.g.: https://elixir.bootlin.com/linux/v5.9.16/source/drivers/iommu/tegra-smmu.c#L760 Dmitry, do you have any idea? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
10.03.2021 22:13, Dmitry Osipenko пишет:
> 19.02.2021 01:07, Nicolin Chen пишет:
>> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
>> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
>> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
>> tegra_smmu_configure() that are typically done in the IOMMU core also.
>>
>> This approach works for both existing devices that have DT nodes and other
>> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
>> upon testing. However, Page Fault errors are reported 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])
>>
>> After debugging, I found that the mentioned commit changed some function
>> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
>> client before display driver gets initialized. I couldn't reproduce exact
>> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>>
>> Actually this Page Fault is a known issue, as on most of Tegra platforms,
>> display gets enabled by the bootloader for the splash screen feature, so
>> it keeps filling the framebuffer memory. A proper fix to this issue is to
>> 1:1 linear map the framebuffer memory to IOVA space so the SMMU will have
>> the same address as the physical address in its page table. Yet, Thierry
>> has been working on the solution above for a year, and it hasn't merged.
>>
>> Therefore, let's partially revert the mentioned commit to fix the errors.
>>
>> The reason why we do a partial revert here is that we can still set priv
>> in ->of_xlate() callback for PCI devices. Meanwhile, devices existing in
>> DT, like display, will go through tegra_smmu_configure() at the stage of
>> bus_set_iommu() when SMMU gets probed(), as what it did before we merged
>> the mentioned commit.
>>
>> Once we have the linear map solution for framebuffer memory, this change
>> can be cleaned away.
>>
>> [Big thank to Guillaume who reported and helped debugging/verification]
>>
>> Fixes: 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
>> Reported-by: Guillaume Tucker
>> Signed-off-by: Nicolin Chen
>> ---
>>
>> Guillaume, would you please give a "Tested-by" to this change? Thanks!
>>
>> 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, >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,
>> + ) == 0) {
>> +smmu = tegra_smmu_find(args.np);
>> +if (smmu) {
>> +err = tegra_smmu_configure(smmu, dev, );
>> +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);
>>
>> @@
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
19.02.2021 01:07, Nicolin Chen пишет:
> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> tegra_smmu_configure() that are typically done in the IOMMU core also.
>
> This approach works for both existing devices that have DT nodes and other
> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> upon testing. However, Page Fault errors are reported 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])
>
> After debugging, I found that the mentioned commit changed some function
> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> client before display driver gets initialized. I couldn't reproduce exact
> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>
> Actually this Page Fault is a known issue, as on most of Tegra platforms,
> display gets enabled by the bootloader for the splash screen feature, so
> it keeps filling the framebuffer memory. A proper fix to this issue is to
> 1:1 linear map the framebuffer memory to IOVA space so the SMMU will have
> the same address as the physical address in its page table. Yet, Thierry
> has been working on the solution above for a year, and it hasn't merged.
>
> Therefore, let's partially revert the mentioned commit to fix the errors.
>
> The reason why we do a partial revert here is that we can still set priv
> in ->of_xlate() callback for PCI devices. Meanwhile, devices existing in
> DT, like display, will go through tegra_smmu_configure() at the stage of
> bus_set_iommu() when SMMU gets probed(), as what it did before we merged
> the mentioned commit.
>
> Once we have the linear map solution for framebuffer memory, this change
> can be cleaned away.
>
> [Big thank to Guillaume who reported and helped debugging/verification]
>
> Fixes: 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> Reported-by: Guillaume Tucker
> Signed-off-by: Nicolin Chen
> ---
>
> Guillaume, would you please give a "Tested-by" to this change? Thanks!
>
> 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, >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,
> + ) == 0) {
> + smmu = tegra_smmu_find(args.np);
> + if (smmu) {
> + err = tegra_smmu_configure(smmu, dev, );
> + 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)
>
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Thu, Feb 18, 2021 at 02:07:02PM -0800, Nicolin Chen wrote: > drivers/iommu/tegra-smmu.c | 72 +- > 1 file changed, 71 insertions(+), 1 deletion(-) Applied for v5.12, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
03.03.2021 02:08, Nicolin Chen пишет: > On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: >> 25.02.2021 09:27, Nicolin Chen пишет: >> ... The partially revert should be okay, but it's not clear to me what makes difference for T124 since I don't see that problem on T30, which also has active display at a boot time. >>> >>> Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach >>> or from of_dma_configure_id/arch_setup_dma_ops? >>> >> >> I applied yours debug-patch, please see dmesg.txt attached to the email. >> Seems probe-defer of the tegra-dc driver prevents the implicit >> tegra_smmu_attach_dev, so it happens to work by accident. > >> [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 >> [0.328641] [] (tegra_smmu_of_xlate) from [] >> (of_iommu_xlate+0x51/0x70) >> [0.328740] [] (of_iommu_xlate) from [] >> (of_iommu_configure+0x127/0x150) >> [0.328896] [] (of_iommu_configure) from [] >> (of_dma_configure_id+0x1fb/0x2ec) >> [0.329060] [] (of_dma_configure_id) from [] >> (really_probe+0x7b/0x2a0) >> [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 >> [0.332234] [] (tegra_smmu_probe_device) from [] >> (__iommu_probe_device+0x35/0x1c4) >> [0.332391] [] (__iommu_probe_device) from [] >> (iommu_probe_device+0x19/0xec) >> [0.332545] [] (iommu_probe_device) from [] >> (of_iommu_configure+0xfb/0x150) >> [0.332701] [] (of_iommu_configure) from [] >> (of_dma_configure_id+0x1fb/0x2ec) >> [0.332804] [] (of_dma_configure_id) from [] >> (really_probe+0x7b/0x2a0) >> [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 >> [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 >> [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, 909: >> 1: drm >> [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 >> [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 >> [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 >> [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, >> 1543: type 3 >> [0.336101] iommu: --iommu_group_alloc_default_domain: platform, >> (null), drm >> [0.336187] -tegra_smmu_domain_alloc, 284: type 3 > [0.336968] [] (tegra_smmu_domain_alloc) from [] > (iommu_group_alloc_default_domain+0x4b/0xfa) >> [0.337127] [] (iommu_group_alloc_default_domain) from >> [] (iommu_probe_device+0x69/0xec) >> [0.337285] [] (iommu_probe_device) from [] >> (of_iommu_configure+0xfb/0x150) >> [0.337441] [] (of_iommu_configure) from [] >> (of_dma_configure_id+0x1fb/0x2ec) >> [0.337599] [] (of_dma_configure_id) from [] >> (really_probe+0x7b/0x2a0) >> [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 >> [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 > > Hmm..not sure where this EPROBE_DEFER comes from. DC driver on Nexus 7 depends on LVDS bridge and display panel, which cause the probe defer. > But you are right, > as of_dma_configure_id() returns because of that so it didn't run to > arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain > and attaches DC to it on Tegra124. > > By the way, anyone can accept this change? It doesn't feel right to > leave a regression in the newer release... > I think Thierry should give ack. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Thu, Feb 18, 2021 at 02:07:02PM -0800, Nicolin Chen wrote:
> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> tegra_smmu_configure() that are typically done in the IOMMU core also.
>
> This approach works for both existing devices that have DT nodes and other
> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> upon testing. However, Page Fault errors are reported 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])
>
> After debugging, I found that the mentioned commit changed some function
> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> client before display driver gets initialized. I couldn't reproduce exact
> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>
> Actually this Page Fault is a known issue, as on most of Tegra platforms,
> display gets enabled by the bootloader for the splash screen feature, so
> it keeps filling the framebuffer memory. A proper fix to this issue is to
> 1:1 linear map the framebuffer memory to IOVA space so the SMMU will have
> the same address as the physical address in its page table. Yet, Thierry
> has been working on the solution above for a year, and it hasn't merged.
>
> Therefore, let's partially revert the mentioned commit to fix the errors.
>
> The reason why we do a partial revert here is that we can still set priv
> in ->of_xlate() callback for PCI devices. Meanwhile, devices existing in
> DT, like display, will go through tegra_smmu_configure() at the stage of
> bus_set_iommu() when SMMU gets probed(), as what it did before we merged
> the mentioned commit.
>
> Once we have the linear map solution for framebuffer memory, this change
> can be cleaned away.
>
> [Big thank to Guillaume who reported and helped debugging/verification]
>
> Fixes: 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> Reported-by: Guillaume Tucker
> Signed-off-by: Nicolin Chen
> ---
>
> Guillaume, would you please give a "Tested-by" to this change? Thanks!
>
> drivers/iommu/tegra-smmu.c | 72 +-
> 1 file changed, 71 insertions(+), 1 deletion(-)
Acked-by: Thierry Reding
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Sat, Feb 27, 2021 at 12:59:17PM +0300, Dmitry Osipenko wrote: > 25.02.2021 09:27, Nicolin Chen пишет: > ... > >> The partially revert should be okay, but it's not clear to me what makes > >> difference for T124 since I don't see that problem on T30, which also > >> has active display at a boot time. > > > > Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach > > or from of_dma_configure_id/arch_setup_dma_ops? > > > > I applied yours debug-patch, please see dmesg.txt attached to the email. > Seems probe-defer of the tegra-dc driver prevents the implicit > tegra_smmu_attach_dev, so it happens to work by accident. > [0.327826] tegra-dc 5420.dc: ---tegra_smmu_of_xlate: id 1 > [0.328641] [] (tegra_smmu_of_xlate) from [] > (of_iommu_xlate+0x51/0x70) > [0.328740] [] (of_iommu_xlate) from [] > (of_iommu_configure+0x127/0x150) > [0.328896] [] (of_iommu_configure) from [] > (of_dma_configure_id+0x1fb/0x2ec) > [0.329060] [] (of_dma_configure_id) from [] > (really_probe+0x7b/0x2a0) > [0.331438] tegra-dc 5420.dc: tegra_smmu_probe_device, 822 > [0.332234] [] (tegra_smmu_probe_device) from [] > (__iommu_probe_device+0x35/0x1c4) > [0.332391] [] (__iommu_probe_device) from [] > (iommu_probe_device+0x19/0xec) > [0.332545] [] (iommu_probe_device) from [] > (of_iommu_configure+0xfb/0x150) > [0.332701] [] (of_iommu_configure) from [] > (of_dma_configure_id+0x1fb/0x2ec) > [0.332804] [] (of_dma_configure_id) from [] > (really_probe+0x7b/0x2a0) > [0.335202] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1572 > [0.335292] tegra-dc 5420.dc: -tegra_smmu_device_group, 862 > [0.335474] tegra-dc 5420.dc: -tegra_smmu_device_group, 909: > 1: drm > [0.335566] tegra-dc 5420.dc: -iommu_group_get_for_dev, 1574 > [0.335718] tegra-dc 5420.dc: -iommu_group_add_device, 858 > [0.335862] tegra-dc 5420.dc: Adding to iommu group 1 > [0.335955] tegra-dc 5420.dc: -iommu_alloc_default_domain, > 1543: type 3 > [0.336101] iommu: --iommu_group_alloc_default_domain: platform, > (null), drm > [0.336187] -tegra_smmu_domain_alloc, 284: type 3 [0.336968] [] (tegra_smmu_domain_alloc) from [] (iommu_group_alloc_default_domain+0x4b/0xfa) > [0.337127] [] (iommu_group_alloc_default_domain) from > [] (iommu_probe_device+0x69/0xec) > [0.337285] [] (iommu_probe_device) from [] > (of_iommu_configure+0xfb/0x150) > [0.337441] [] (of_iommu_configure) from [] > (of_dma_configure_id+0x1fb/0x2ec) > [0.337599] [] (of_dma_configure_id) from [] > (really_probe+0x7b/0x2a0) > [0.339913] tegra-dc 5420.dc: -iommu_probe_device, 272 > [0.348144] tegra-dc 5420.dc: failed to probe RGB output: -517 Hmm..not sure where this EPROBE_DEFER comes from. But you are right, as of_dma_configure_id() returns because of that so it didn't run to arch_setup_dma_ops() call, which allocates an UNMANAGED iommu domain and attaches DC to it on Tegra124. By the way, anyone can accept this change? It doesn't feel right to leave a regression in the newer release... ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
25.02.2021 09:27, Nicolin Chen пишет: ... >> The partially revert should be okay, but it's not clear to me what makes >> difference for T124 since I don't see that problem on T30, which also >> has active display at a boot time. > > Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach > or from of_dma_configure_id/arch_setup_dma_ops? > I applied yours debug-patch, please see dmesg.txt attached to the email. Seems probe-defer of the tegra-dc driver prevents the implicit tegra_smmu_attach_dev, so it happens to work by accident. [0.00] Booting Linux on physical CPU 0x0 [0.00] Linux version 5.11.0-next-20210226-3-g89069e0f4a28 (dima@dimapc) (armv7a-hardfloat-linux-gnueabi-gcc (Gentoo 9.3.0-r2 p4) 9.3.0, GNU ld (Gentoo 2.26.1 p1.0) 2.26.1) #7012 SMP PREEMPT Sat Feb 27 11:49:27 MSK 2021 [0.00] CPU: ARMv7 Processor [412fc099] revision 9 (ARMv7), cr=50c5387d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] OF: fdt: Machine model: ASUS Google Nexus 7 (Project Bach / ME370TG) E1565 [0.00] Memory policy: Data cache writealloc [0.00] Reserved memory: created CMA memory pool at 0xa000, size 256 MiB [0.00] OF: reserved mem: initialized node linux,cma@8000, compatible id shared-dma-pool [0.00] Zone ranges: [0.00] Normal [mem 0x-0x9f7f] [0.00] HighMem [mem 0x9f80-0xbfdf] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x8000-0xbfdf] [0.00] Initmem setup node 0 [mem 0x8000-0xbfdf] [0.00] On node 0 totalpages: 261632 [0.00] Normal zone: 1134 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 129024 pages, LIFO batch:31 [0.00] HighMem zone: 132608 pages, LIFO batch:31 [0.00] percpu: Embedded 21 pages/cpu s53324 r8192 d24500 u86016 [0.00] pcpu-alloc: s53324 r8192 d24500 u86016 alloc=21*4096 [0.00] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 260498 [0.00] Kernel command line: tegraid=30.1.3.0.0 mem=1022M@2048M android.commchip=0 vmalloc=512M androidboot.serialno=015d3f18c9081210 video=tegrafb no_console_suspend=1 console=none debug_uartport=hsport usbcore.old_scheme_first=1 lp0_vec=8192@0xbddf9000 tegra_fbmem=8195200@0xabe01000 core_edp_mv=0 audio_codec=rt5640 board_info=f41:a00:1:44:2 root=/dev/sda1 rw rootwait tegraboot=sdmmc gpt gpt_sector=61079551 androidboot.bootloader=4.23 androidboot.baseband=1231_0.18.0_0409 init=/usr/lib/systemd/systemd root=/dev/nfs nfsroot=192.168.3.100:/nfs/root,v3,tcp rw ip=192.168.3.101::192.168.3.100:255.255.255.0::usb0 no_console_suspend gpt gpt_sector=61079551 console=tty1 no_console_suspend [0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes, linear) [0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] Memory: 754680K/1046528K available (10240K kernel code, 1579K rwdata, 5404K rodata, 1024K init, 418K bss, 29704K reserved, 262144K cma-reserved, 268224K highmem) [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1 [0.00] ftrace: allocating 46557 entries in 91 pages [0.00] [ cut here ] [0.00] WARNING: CPU: 0 PID: 0 at arch/arm/kernel/insn.c:15 __arm_gen_branch+0x7f/0x84 [0.00] Modules linked in: [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 5.11.0-next-20210226-3-g89069e0f4a28 #7012 [0.00] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [0.00] [] (unwind_backtrace) from [] (show_stack+0x11/0x14) [0.00] [] (show_stack) from [] (dump_stack+0x8d/0x9a) [0.00] [] (dump_stack) from [] (__warn+0xb7/0xc8) [0.00] [] (__warn) from [] (warn_slowpath_fmt+0x45/0x78) [0.00] [] (warn_slowpath_fmt) from [] (__arm_gen_branch+0x7f/0x84) [0.00] [] (__arm_gen_branch) from [] (ftrace_make_nop+0xf/0x20) [0.00] [] (ftrace_make_nop) from [] (ftrace_process_locs+0x211/0x320) [0.00] [] (ftrace_process_locs) from [] (ftrace_init+0x6f/0xbe) [0.00] [] (ftrace_init) from [] (start_kernel+0x42d/0x662) [0.00] [] (start_kernel) from [<>] (0x0) [0.00] random: get_random_bytes called from print_oops_end_marker+0x25/0x64 with crng_init=0 [0.00] ---[ end trace ]--- [0.00] [ ftrace bug ] [0.00] ftrace failed to modify [0.00] [] crash_notes_memory_init+0x4/0x34 [0.00] actual: 23:f0:9c:ec [0.00] Initializing ftrace call sites [0.00] ftrace record flags: 0
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On Tue, Feb 23, 2021 at 08:10:41AM +0300, Dmitry Osipenko wrote:
> 23.02.2021 05:13, Nicolin Chen пишет:
> > Hi Dmitry,
> >
> > On Sat, Feb 20, 2021 at 08:16:22AM +0300, Dmitry Osipenko wrote:
> >> 19.02.2021 01:07, Nicolin Chen пишет:
> >>> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> >>> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> >>> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> >>> tegra_smmu_configure() that are typically done in the IOMMU core also.
> >>>
> >>> This approach works for both existing devices that have DT nodes and other
> >>> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> >>> upon testing. However, Page Fault errors are reported 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])
> >>>
> >>> After debugging, I found that the mentioned commit changed some function
> >>> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> >>> client before display driver gets initialized. I couldn't reproduce exact
> >>> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
> >>
> >> Hello Nicolin,
> >>
> >> Could you please explain in a more details what exactly makes the
> >> difference for the callback sequence?
> >
> > Here is a log with 5.11.0-rc6:
> > https://lava.collabora.co.uk/scheduler/job/3187849
> > [dump_stack was added in some tegra-smmu functions]
> >
> > And here is a corresponding log with reverting the original commit:
> > https://lava.collabora.co.uk/scheduler/job/3187851
> >
> > Here is a log with 5.11.0-rc7-next-20210210:
> > https://lava.collabora.co.uk/scheduler/job/3210245
> >
> > And here is a corresponding log with reverting the original commit:
> > https://lava.collabora.co.uk/scheduler/job/3210596
> >
> > Both failing logs show that mc errors started right after client DC
> > got enabled by ->attach_dev() callback that in the passing logs was
> > not called until Host1x driver init. And note that two failing logs
> > show that ->attach_dev() could be called from two different sources,
> > of_dma_configure_id() or arch_setup_dma_ops().
> >
> > The reason why ->attach_dev() gets called is probably related to the
> > following reasons (sorry, can't be 100% sure as I don't have Tegra124
> > or other 32bit Tegra board to test):
> > 1) With the commit reverted, all clients are probed in "arch" stage,
> >which is even prior to iommu core initialization -- including it
> >setting default domain type. This probably messed up the type of
> >allocating domains against the default domain type. Also internal
> >group is somehow affected. So some condition check in iommu core
> >failed and then it bypassed ->attach_dev callback in really_probe
> >stage, until Host1x driver does attach_dev again.
> >
> > 2) 32bit ARM has arch_setup_dma_ops() does an additional set of iommu
> >domain allocation + attach_dev(), after of_dma_configure_id() did
> >once. This isn't reproducible for me on Tegra210.
> >
> > As debugging online isn't very efficient, and given that Thierry has
> > been working on the linear mapping of framebuffer carveout, I choose
> > to partially revert as a quick fix.
>
> The partially revert should be okay, but it's not clear to me what makes
> difference for T124 since I don't see that problem on T30, which also
> has active display at a boot time.
Hmm..do you see ->attach_dev() is called from host1x_client_iommu_attach
or from of_dma_configure_id/arch_setup_dma_ops?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
23.02.2021 05:13, Nicolin Chen пишет:
> Hi Dmitry,
>
> On Sat, Feb 20, 2021 at 08:16:22AM +0300, Dmitry Osipenko wrote:
>> 19.02.2021 01:07, Nicolin Chen пишет:
>>> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
>>> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
>>> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
>>> tegra_smmu_configure() that are typically done in the IOMMU core also.
>>>
>>> This approach works for both existing devices that have DT nodes and other
>>> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
>>> upon testing. However, Page Fault errors are reported 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])
>>>
>>> After debugging, I found that the mentioned commit changed some function
>>> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
>>> client before display driver gets initialized. I couldn't reproduce exact
>>> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>>
>> Hello Nicolin,
>>
>> Could you please explain in a more details what exactly makes the
>> difference for the callback sequence?
>
> Here is a log with 5.11.0-rc6:
> https://lava.collabora.co.uk/scheduler/job/3187849
> [dump_stack was added in some tegra-smmu functions]
>
> And here is a corresponding log with reverting the original commit:
> https://lava.collabora.co.uk/scheduler/job/3187851
>
> Here is a log with 5.11.0-rc7-next-20210210:
> https://lava.collabora.co.uk/scheduler/job/3210245
>
> And here is a corresponding log with reverting the original commit:
> https://lava.collabora.co.uk/scheduler/job/3210596
>
> Both failing logs show that mc errors started right after client DC
> got enabled by ->attach_dev() callback that in the passing logs was
> not called until Host1x driver init. And note that two failing logs
> show that ->attach_dev() could be called from two different sources,
> of_dma_configure_id() or arch_setup_dma_ops().
>
> The reason why ->attach_dev() gets called is probably related to the
> following reasons (sorry, can't be 100% sure as I don't have Tegra124
> or other 32bit Tegra board to test):
> 1) With the commit reverted, all clients are probed in "arch" stage,
>which is even prior to iommu core initialization -- including it
>setting default domain type. This probably messed up the type of
>allocating domains against the default domain type. Also internal
>group is somehow affected. So some condition check in iommu core
>failed and then it bypassed ->attach_dev callback in really_probe
>stage, until Host1x driver does attach_dev again.
>
> 2) 32bit ARM has arch_setup_dma_ops() does an additional set of iommu
>domain allocation + attach_dev(), after of_dma_configure_id() did
>once. This isn't reproducible for me on Tegra210.
>
> As debugging online isn't very efficient, and given that Thierry has
> been working on the linear mapping of framebuffer carveout, I choose
> to partially revert as a quick fix.
The partially revert should be okay, but it's not clear to me what makes
difference for T124 since I don't see that problem on T30, which also
has active display at a boot time.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
Hi Dmitry,
On Sat, Feb 20, 2021 at 08:16:22AM +0300, Dmitry Osipenko wrote:
> 19.02.2021 01:07, Nicolin Chen пишет:
> > Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> > removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> > of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> > tegra_smmu_configure() that are typically done in the IOMMU core also.
> >
> > This approach works for both existing devices that have DT nodes and other
> > devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> > upon testing. However, Page Fault errors are reported 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])
> >
> > After debugging, I found that the mentioned commit changed some function
> > callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> > client before display driver gets initialized. I couldn't reproduce exact
> > same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>
> Hello Nicolin,
>
> Could you please explain in a more details what exactly makes the
> difference for the callback sequence?
Here is a log with 5.11.0-rc6:
https://lava.collabora.co.uk/scheduler/job/3187849
[dump_stack was added in some tegra-smmu functions]
And here is a corresponding log with reverting the original commit:
https://lava.collabora.co.uk/scheduler/job/3187851
Here is a log with 5.11.0-rc7-next-20210210:
https://lava.collabora.co.uk/scheduler/job/3210245
And here is a corresponding log with reverting the original commit:
https://lava.collabora.co.uk/scheduler/job/3210596
Both failing logs show that mc errors started right after client DC
got enabled by ->attach_dev() callback that in the passing logs was
not called until Host1x driver init. And note that two failing logs
show that ->attach_dev() could be called from two different sources,
of_dma_configure_id() or arch_setup_dma_ops().
The reason why ->attach_dev() gets called is probably related to the
following reasons (sorry, can't be 100% sure as I don't have Tegra124
or other 32bit Tegra board to test):
1) With the commit reverted, all clients are probed in "arch" stage,
which is even prior to iommu core initialization -- including it
setting default domain type. This probably messed up the type of
allocating domains against the default domain type. Also internal
group is somehow affected. So some condition check in iommu core
failed and then it bypassed ->attach_dev callback in really_probe
stage, until Host1x driver does attach_dev again.
2) 32bit ARM has arch_setup_dma_ops() does an additional set of iommu
domain allocation + attach_dev(), after of_dma_configure_id() did
once. This isn't reproducible for me on Tegra210.
As debugging online isn't very efficient, and given that Thierry has
been working on the linear mapping of framebuffer carveout, I choose
to partially revert as a quick fix.
Thanks
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
On 18/02/2021 22:07, Nicolin Chen wrote:
> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> tegra_smmu_configure() that are typically done in the IOMMU core also.
>
> This approach works for both existing devices that have DT nodes and other
> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> upon testing. However, Page Fault errors are reported 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])
>
> After debugging, I found that the mentioned commit changed some function
> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> client before display driver gets initialized. I couldn't reproduce exact
> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
>
> Actually this Page Fault is a known issue, as on most of Tegra platforms,
> display gets enabled by the bootloader for the splash screen feature, so
> it keeps filling the framebuffer memory. A proper fix to this issue is to
> 1:1 linear map the framebuffer memory to IOVA space so the SMMU will have
> the same address as the physical address in its page table. Yet, Thierry
> has been working on the solution above for a year, and it hasn't merged.
>
> Therefore, let's partially revert the mentioned commit to fix the errors.
>
> The reason why we do a partial revert here is that we can still set priv
> in ->of_xlate() callback for PCI devices. Meanwhile, devices existing in
> DT, like display, will go through tegra_smmu_configure() at the stage of
> bus_set_iommu() when SMMU gets probed(), as what it did before we merged
> the mentioned commit.
>
> Once we have the linear map solution for framebuffer memory, this change
> can be cleaned away.
>
> [Big thank to Guillaume who reported and helped debugging/verification]
>
> Fixes: 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> Reported-by: Guillaume Tucker
You're welcome. I would actually prefer to see it as reported by
kernelci.org since I wouldn't have found it without the automated
testing and bisection. If you're OK to change this trailer:
Reported-by: "kernelci.org bot"
> Signed-off-by: Nicolin Chen
> ---
>
> Guillaume, would you please give a "Tested-by" to this change? Thanks!
Sure. https://lava.collabora.co.uk/scheduler/job/3249387
Tested-by: Guillaume Tucker
Thanks,
Guillaume
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
19.02.2021 01:07, Nicolin Chen пишет:
> Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
> removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
> of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
> tegra_smmu_configure() that are typically done in the IOMMU core also.
>
> This approach works for both existing devices that have DT nodes and other
> devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
> upon testing. However, Page Fault errors are reported 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])
>
> After debugging, I found that the mentioned commit changed some function
> callback sequence of tegra-smmu's, resulting in enabling SMMU for display
> client before display driver gets initialized. I couldn't reproduce exact
> same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
Hello Nicolin,
Could you please explain in a more details what exactly makes the
difference for the callback sequence?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/tegra-smmu: Fix mc errors on tegra124-nyan
Commit 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
removed certain hack in the tegra_smmu_probe() by relying on IOMMU core to
of_xlate SMMU's SID per device, so as to get rid of tegra_smmu_find() and
tegra_smmu_configure() that are typically done in the IOMMU core also.
This approach works for both existing devices that have DT nodes and other
devices (like PCI device) that don't exist in DT, on Tegra210 and Tegra3
upon testing. However, Page Fault errors are reported 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])
After debugging, I found that the mentioned commit changed some function
callback sequence of tegra-smmu's, resulting in enabling SMMU for display
client before display driver gets initialized. I couldn't reproduce exact
same issue on Tegra210 as Tegra124 (arm-32) differs at arch-level code.
Actually this Page Fault is a known issue, as on most of Tegra platforms,
display gets enabled by the bootloader for the splash screen feature, so
it keeps filling the framebuffer memory. A proper fix to this issue is to
1:1 linear map the framebuffer memory to IOVA space so the SMMU will have
the same address as the physical address in its page table. Yet, Thierry
has been working on the solution above for a year, and it hasn't merged.
Therefore, let's partially revert the mentioned commit to fix the errors.
The reason why we do a partial revert here is that we can still set priv
in ->of_xlate() callback for PCI devices. Meanwhile, devices existing in
DT, like display, will go through tegra_smmu_configure() at the stage of
bus_set_iommu() when SMMU gets probed(), as what it did before we merged
the mentioned commit.
Once we have the linear map solution for framebuffer memory, this change
can be cleaned away.
[Big thank to Guillaume who reported and helped debugging/verification]
Fixes: 25938c73cd79 ("iommu/tegra-smmu: Rework tegra_smmu_probe_device()")
Reported-by: Guillaume Tucker
Signed-off-by: Nicolin Chen
---
Guillaume, would you please give a "Tested-by" to this change? Thanks!
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, >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,
+ ) == 0) {
+ smmu = tegra_smmu_find(args.np);
+ if (smmu) {
+ err = tegra_smmu_configure(smmu, dev, );
+ 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
+
