| Message ID | 20211209073822.26728-7-nicolinc@nvidia.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | iommu/tegra-smmu: Add pagetable mappings to debugfs | expand |
09.12.2021 10:38, Nicolin Chen пишет: > @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, > if (group->swgrp != swgrp) > continue; > group->as = NULL; > + > + if (smmu->debugfs_mappings) { Do we really need this check? Looks like all debugfs_create_dir() usages in this driver are incorrect, that function never returns NULL. Please fix this. > + struct dentry *d; The file name is wrong here. if (group->soc) name = group->soc->name; else name = group->swgrp->name; > + d = debugfs_lookup(group->swgrp->name, > + smmu->debugfs_mappings); > + debugfs_remove(d); > + } This now looks problematic to me. You created debugfs file when the first member of the shared group was attached to AS, now you remove this file when any device is detached. The shared debugfs file should be refcounted or something.
09.12.2021 10:38, Nicolin Chen пишет: > +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) > +{ > + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | > + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > +} I'd change the return type to u32 here, for consistency.
On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 10:38, Nicolin Chen пишет: > > +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) > > +{ > > + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | > > + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > > +} > > I'd change the return type to u32 here, for consistency. The whole file defines iova using "unsigned long", which I see as the consistency... If we change it to u32, it'd be probably necessary to change all iova types to u32 too... So I'd rather keep it "unsigned long" here. If you see a big necessity to do that, an additional patch would be nicer IMHO.
On Thu, Dec 09, 2021 at 05:47:18PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 10:38, Nicolin Chen пишет: > > @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, > > if (group->swgrp != swgrp) > > continue; > > group->as = NULL; > > + > > + if (smmu->debugfs_mappings) { > > Do we really need this check? > > Looks like all debugfs_create_dir() usages in this driver are incorrect, > that function never returns NULL. Please fix this. debugfs_create_dir returns ERR_PTR on failure. So here should be to check !IS_ERR. Thanks for pointing it out! > > + struct dentry *d; > > The file name is wrong here. > > if (group->soc) > name = group->soc->name; > else > name = group->swgrp->name; Yea, I'll add this. > > > + d = debugfs_lookup(group->swgrp->name, > > + smmu->debugfs_mappings); > > + debugfs_remove(d); > > + } > > This now looks problematic to me. You created debugfs file when the > first member of the shared group was attached to AS, now you remove this > file when any device is detached. The shared debugfs file should be > refcounted or something.a Will see how to handle it. Thanks Nic
09.12.2021 22:32, Nicolin Chen пишет: > On Thu, Dec 09, 2021 at 05:47:18PM +0300, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 09.12.2021 10:38, Nicolin Chen пишет: >>> @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, >>> if (group->swgrp != swgrp) >>> continue; >>> group->as = NULL; >>> + >>> + if (smmu->debugfs_mappings) { >> Do we really need this check? >> >> Looks like all debugfs_create_dir() usages in this driver are incorrect, >> that function never returns NULL. Please fix this. > debugfs_create_dir returns ERR_PTR on failure. So here should be > to check !IS_ERR. Thanks for pointing it out! > All debugfs functions handle IS_ERR(). GregKH removes all such checks all over the kernel. So the check shouldn't be needed at all, please remove it if it's unneeded or prove that it's needed.
09.12.2021 22:24, Nicolin Chen пишет: > On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 09.12.2021 10:38, Nicolin Chen пишет: >>> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) >>> +{ >>> + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | >>> + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; >>> +} >> >> I'd change the return type to u32 here, for consistency. > > The whole file defines iova using "unsigned long", which I see > as the consistency... If we change it to u32, it'd be probably > necessary to change all iova types to u32 too... So I'd rather > keep it "unsigned long" here. If you see a big necessity to do > that, an additional patch would be nicer IMHO. > In general IOVA is "unsigned long", of course. But in case of Tegra SMMU, we know that is always u32. Alright, I see now that there are other places in the driver code that use "unsigned long", so need to change it in this patch.
On Thu, Dec 09, 2021 at 10:40:42PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 22:32, Nicolin Chen пишет: > > On Thu, Dec 09, 2021 at 05:47:18PM +0300, Dmitry Osipenko wrote: > >> External email: Use caution opening links or attachments > >> > >> > >> 09.12.2021 10:38, Nicolin Chen пишет: > >>> @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, > >>> if (group->swgrp != swgrp) > >>> continue; > >>> group->as = NULL; > >>> + > >>> + if (smmu->debugfs_mappings) { > >> Do we really need this check? > >> > >> Looks like all debugfs_create_dir() usages in this driver are incorrect, > >> that function never returns NULL. Please fix this. > > debugfs_create_dir returns ERR_PTR on failure. So here should be > > to check !IS_ERR. Thanks for pointing it out! > > > > All debugfs functions handle IS_ERR(). GregKH removes all such checks > all over the kernel. So the check shouldn't be needed at all, please > remove it if it's unneeded or prove that it's needed. debugfs_create_file can handle a NULL parent, but not ERR_PTR one, and then it puts the new node under the root. So either passing an ERR_PTR parent or creating orphan nodes here doesn't sound good...
On Thu, Dec 09, 2021 at 10:44:25PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 22:24, Nicolin Chen пишет: > > On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: > >> External email: Use caution opening links or attachments > >> > >> > >> 09.12.2021 10:38, Nicolin Chen пишет: > >>> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) > >>> +{ > >>> + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | > >>> + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > >>> +} > >> > >> I'd change the return type to u32 here, for consistency. > > > > The whole file defines iova using "unsigned long", which I see > > as the consistency... If we change it to u32, it'd be probably > > necessary to change all iova types to u32 too... So I'd rather > > keep it "unsigned long" here. If you see a big necessity to do > > that, an additional patch would be nicer IMHO. > > > > In general IOVA is "unsigned long", of course. But in case of Tegra > SMMU, we know that is always u32. > > Alright, I see now that there are other places in the driver code that > use "unsigned long", so need to change it in this patch. I think we should do that in a separate patch that changes the iova type in the entire tegra-smmu file. I can add one in this series, if this makes you happy...
09.12.2021 22:51, Nicolin Chen пишет: > On Thu, Dec 09, 2021 at 10:40:42PM +0300, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 09.12.2021 22:32, Nicolin Chen пишет: >>> On Thu, Dec 09, 2021 at 05:47:18PM +0300, Dmitry Osipenko wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> >>>> 09.12.2021 10:38, Nicolin Chen пишет: >>>>> @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, >>>>> if (group->swgrp != swgrp) >>>>> continue; >>>>> group->as = NULL; >>>>> + >>>>> + if (smmu->debugfs_mappings) { >>>> Do we really need this check? >>>> >>>> Looks like all debugfs_create_dir() usages in this driver are incorrect, >>>> that function never returns NULL. Please fix this. >>> debugfs_create_dir returns ERR_PTR on failure. So here should be >>> to check !IS_ERR. Thanks for pointing it out! >>> >> >> All debugfs functions handle IS_ERR(). GregKH removes all such checks >> all over the kernel. So the check shouldn't be needed at all, please >> remove it if it's unneeded or prove that it's needed. > > debugfs_create_file can handle a NULL parent, but not ERR_PTR one, > and then it puts the new node under the root. So either passing an > ERR_PTR parent or creating orphan nodes here doesn't sound good... > What makes you say so? Please show the exact source code that will cause the problem. The smmu->debugfs_mappings can't ever be NULL and debugfs_create_file handles the ERR_PTR [1][2]. [1] https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L397 [2] https://elixir.bootlin.com/linux/latest/source/fs/debugfs/inode.c#L330
09.12.2021 22:54, Nicolin Chen пишет: > On Thu, Dec 09, 2021 at 10:44:25PM +0300, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 09.12.2021 22:24, Nicolin Chen пишет: >>> On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> >>>> 09.12.2021 10:38, Nicolin Chen пишет: >>>>> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) >>>>> +{ >>>>> + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | >>>>> + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; >>>>> +} >>>> >>>> I'd change the return type to u32 here, for consistency. >>> >>> The whole file defines iova using "unsigned long", which I see >>> as the consistency... If we change it to u32, it'd be probably >>> necessary to change all iova types to u32 too... So I'd rather >>> keep it "unsigned long" here. If you see a big necessity to do >>> that, an additional patch would be nicer IMHO. >>> >> >> In general IOVA is "unsigned long", of course. But in case of Tegra >> SMMU, we know that is always u32. >> >> Alright, I see now that there are other places in the driver code that >> use "unsigned long", so need to change it in this patch. > > I think we should do that in a separate patch that changes the iova > type in the entire tegra-smmu file. I can add one in this series, if > this makes you happy... > Please keep it "unsigned long", no need for extra patches.
On Thu, Dec 09, 2021 at 10:58:32PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 22:54, Nicolin Chen пишет: > > On Thu, Dec 09, 2021 at 10:44:25PM +0300, Dmitry Osipenko wrote: > >> External email: Use caution opening links or attachments > >> > >> > >> 09.12.2021 22:24, Nicolin Chen пишет: > >>> On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: > >>>> External email: Use caution opening links or attachments > >>>> > >>>> > >>>> 09.12.2021 10:38, Nicolin Chen пишет: > >>>>> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) > >>>>> +{ > >>>>> + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | > >>>>> + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > >>>>> +} > >>>> > >>>> I'd change the return type to u32 here, for consistency. > >>> > >>> The whole file defines iova using "unsigned long", which I see > >>> as the consistency... If we change it to u32, it'd be probably > >>> necessary to change all iova types to u32 too... So I'd rather > >>> keep it "unsigned long" here. If you see a big necessity to do > >>> that, an additional patch would be nicer IMHO. > >>> > >> > >> In general IOVA is "unsigned long", of course. But in case of Tegra > >> SMMU, we know that is always u32. > >> > >> Alright, I see now that there are other places in the driver code that > >> use "unsigned long", so need to change it in this patch. > > > > I think we should do that in a separate patch that changes the iova > > type in the entire tegra-smmu file. I can add one in this series, if > > this makes you happy... > > > > Please keep it "unsigned long", no need for extra patches. Oh, I guess that "so need to change it in this patch" should be "so (no) need to change it in this patch" then?
09.12.2021 23:01, Nicolin Chen пишет: > On Thu, Dec 09, 2021 at 10:58:32PM +0300, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 09.12.2021 22:54, Nicolin Chen пишет: >>> On Thu, Dec 09, 2021 at 10:44:25PM +0300, Dmitry Osipenko wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> >>>> 09.12.2021 22:24, Nicolin Chen пишет: >>>>> On Thu, Dec 09, 2021 at 05:49:09PM +0300, Dmitry Osipenko wrote: >>>>>> External email: Use caution opening links or attachments >>>>>> >>>>>> >>>>>> 09.12.2021 10:38, Nicolin Chen пишет: >>>>>>> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) >>>>>>> +{ >>>>>>> + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | >>>>>>> + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; >>>>>>> +} >>>>>> >>>>>> I'd change the return type to u32 here, for consistency. >>>>> >>>>> The whole file defines iova using "unsigned long", which I see >>>>> as the consistency... If we change it to u32, it'd be probably >>>>> necessary to change all iova types to u32 too... So I'd rather >>>>> keep it "unsigned long" here. If you see a big necessity to do >>>>> that, an additional patch would be nicer IMHO. >>>>> >>>> >>>> In general IOVA is "unsigned long", of course. But in case of Tegra >>>> SMMU, we know that is always u32. >>>> >>>> Alright, I see now that there are other places in the driver code that >>>> use "unsigned long", so need to change it in this patch. >>> >>> I think we should do that in a separate patch that changes the iova >>> type in the entire tegra-smmu file. I can add one in this series, if >>> this makes you happy... >>> >> >> Please keep it "unsigned long", no need for extra patches. > > Oh, I guess that "so need to change it in this patch" should be > "so (no) need to change it in this patch" then? > Indeed, I missed that typo, sorry.
On Thu, Dec 09, 2021 at 10:58:15PM +0300, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 09.12.2021 22:51, Nicolin Chen пишет: > > On Thu, Dec 09, 2021 at 10:40:42PM +0300, Dmitry Osipenko wrote: > >> External email: Use caution opening links or attachments > >> > >> > >> 09.12.2021 22:32, Nicolin Chen пишет: > >>> On Thu, Dec 09, 2021 at 05:47:18PM +0300, Dmitry Osipenko wrote: > >>>> External email: Use caution opening links or attachments > >>>> > >>>> > >>>> 09.12.2021 10:38, Nicolin Chen пишет: > >>>>> @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, > >>>>> if (group->swgrp != swgrp) > >>>>> continue; > >>>>> group->as = NULL; > >>>>> + > >>>>> + if (smmu->debugfs_mappings) { > >>>> Do we really need this check? > >>>> > >>>> Looks like all debugfs_create_dir() usages in this driver are incorrect, > >>>> that function never returns NULL. Please fix this. > >>> debugfs_create_dir returns ERR_PTR on failure. So here should be > >>> to check !IS_ERR. Thanks for pointing it out! > >>> > >> > >> All debugfs functions handle IS_ERR(). GregKH removes all such checks > >> all over the kernel. So the check shouldn't be needed at all, please > >> remove it if it's unneeded or prove that it's needed. > > > > debugfs_create_file can handle a NULL parent, but not ERR_PTR one, > > and then it puts the new node under the root. So either passing an > > ERR_PTR parent or creating orphan nodes here doesn't sound good... > > > > What makes you say so? Please show the exact source code that will cause > the problem. > > The smmu->debugfs_mappings can't ever be NULL and debugfs_create_file > handles the ERR_PTR [1][2]. Ah...my tool jumps to start_creating in fs/tracefs/inode.c instead. Thanks for the reply. I will remove the if line then.
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 454504aa6602..cbd1a52f2a9f 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -47,6 +47,7 @@ struct tegra_smmu { struct list_head list; struct dentry *debugfs; + struct dentry *debugfs_mappings; struct iommu_device iommu; /* IOMMU Core code handle */ }; @@ -154,6 +155,9 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu, unsigned long offset) #define SMMU_PDE_ATTR (SMMU_PDE_READABLE | SMMU_PDE_WRITABLE | \ SMMU_PDE_NONSECURE) +#define SMMU_PTE_ATTR (SMMU_PTE_READABLE | SMMU_PTE_WRITABLE | \ + SMMU_PTE_NONSECURE) +#define SMMU_PTE_ATTR_SHIFT 29 static unsigned int iova_pd_index(unsigned long iova) { @@ -165,6 +169,12 @@ static unsigned int iova_pt_index(unsigned long iova) return (iova >> SMMU_PTE_SHIFT) & (SMMU_NUM_PTE - 1); } +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int pt_index) +{ + return (pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT | + (pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; +} + static bool smmu_dma_addr_valid(struct tegra_smmu *smmu, dma_addr_t addr) { addr >>= 12; @@ -498,6 +508,156 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu, mutex_unlock(&smmu->lock); } +static int tegra_smmu_debugfs_mappings_show(struct seq_file *s, void *data) +{ + struct tegra_smmu_group *group = s->private; + const struct tegra_smmu_group_soc *soc; + const struct tegra_smmu_swgroup *swgrp; + struct tegra_smmu_as *as; + struct tegra_smmu *smmu; + unsigned int pd_index; + unsigned int pt_index; + unsigned long flags; + u64 pte_count = 0; + u32 pde_count = 0; + u32 *pd, val; + + if (!group || !group->as || !group->swgrp) + return 0; + + swgrp = group->swgrp; + smmu = group->smmu; + soc = group->soc; + as = group->as; + + mutex_lock(&smmu->lock); + + val = smmu_readl(smmu, swgrp->reg); + if (!(val & SMMU_ASID_ENABLE)) + goto unlock; + + pd = page_address(as->pd); + if (!pd) + goto unlock; + + seq_puts(s, "[SWGROUP: "); + /* List all the swgroup names in the same group_soc */ + if (soc) { + bool first_swgroup = true; + unsigned int i; + + for (i = 0; i < soc->num_swgroups; i++) { + swgrp = tegra_smmu_find_swgrp(smmu, soc->swgroups[i]); + if (WARN_ON(!swgrp)) + goto unlock; + + val = smmu_readl(smmu, swgrp->reg); + if (!(val & SMMU_ASID_ENABLE)) + continue; + + if (WARN_ON((val & SMMU_ASID_MASK) != as->id)) + continue; + + if (first_swgroup) + first_swgroup = false; + else + seq_puts(s, ", "); + + seq_printf(s, "%s", swgrp->name); + } + } else { + WARN_ON((val & SMMU_ASID_MASK) != as->id); + seq_printf(s, "%s", swgrp->name); + } + seq_puts(s, "] "); + + seq_printf(s, "[as: (id: %d), ", as->id); + seq_printf(s, "(attr: %c|%c|%c), ", + as->attr & SMMU_PD_READABLE ? 'R' : '-', + as->attr & SMMU_PD_WRITABLE ? 'W' : '-', + as->attr & SMMU_PD_NONSECURE ? '-' : 'S'); + seq_printf(s, "(pd_dma: %pad)]\n", &as->pd_dma); + seq_puts(s, "{\n"); + + spin_lock_irqsave(&as->lock, flags); + + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { + struct page *pt_page; + unsigned int i; + u32 *addr; + + /* An empty PDE should not have a pte use count */ + WARN_ON_ONCE(!pd[pd_index] ^ !as->count[pd_index]); + + /* Skip this empty PDE */ + if (!pd[pd_index]) + continue; + + pde_count++; + pte_count += as->count[pd_index]; + seq_printf(s, "\t[index: %u] 0x%x (count: %d)\n", + pd_index, pd[pd_index], as->count[pd_index]); + pt_page = as->pts[pd_index]; + addr = page_address(pt_page); + + seq_puts(s, "\t{\n"); + seq_printf(s, "\t\t%-14s | %-4s | %-10s%s | %-10s | %-11s\n", + "PTE RANGE", "ATTR", + "PHYS", sizeof(phys_addr_t) > 4 ? " " : "", + "IOVA", "SIZE"); + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index += i) { + size_t size = SMMU_SIZE_PT; + dma_addr_t iova; + phys_addr_t pa; + + i = 1; + + if (!addr[pt_index]) + continue; + + iova = pd_pt_index_iova(pd_index, pt_index); + pa = SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR); + + /* Check contiguous mappings and increase size */ + while (pt_index + i < SMMU_NUM_PTE) { + dma_addr_t next_iova; + phys_addr_t next_pa; + + if (!addr[pt_index + i]) + break; + + next_iova = pd_pt_index_iova(pd_index, pt_index + i); + next_pa = SMMU_PFN_PHYS(addr[pt_index + i] & ~SMMU_PTE_ATTR); + + /* Break at the end of a linear mapping */ + if ((next_iova - iova != SMMU_SIZE_PT * i) || + (next_pa - pa != SMMU_SIZE_PT * i)) + break; + + i++; + } + + seq_printf(s, "\t\t[#%-4u, #%-4u] | 0x%-2x | %pa | 0x%-8x | 0x%-9zx\n", + pt_index, pt_index + i - 1, + addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, + &pa, (u32)iova, size * i); + } + seq_puts(s, "\t}\n"); + } + + spin_unlock_irqrestore(&as->lock, flags); + + seq_puts(s, "}\n"); + seq_printf(s, "Total PDEs: %u, total PTEs: %llu\n ", pde_count, pte_count); + +unlock: + mutex_unlock(&smmu->lock); + + return 0; +} + +DEFINE_SHOW_ATTRIBUTE(tegra_smmu_debugfs_mappings); + static void tegra_smmu_attach_as(struct tegra_smmu *smmu, struct tegra_smmu_as *as, unsigned int swgroup) @@ -522,6 +682,20 @@ static void tegra_smmu_attach_as(struct tegra_smmu *smmu, dev_warn(smmu->dev, "overwriting group->as for swgroup: %s\n", swgrp->name); group->as = as; + + if (smmu->debugfs_mappings) { + const char *name; + + if (group->soc) + name = group->soc->name; + else + name = group->swgrp->name; + + debugfs_create_file(name, 0444, + smmu->debugfs_mappings, group, + &tegra_smmu_debugfs_mappings_fops); + } + break; } @@ -545,6 +719,15 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu, if (group->swgrp != swgrp) continue; group->as = NULL; + + if (smmu->debugfs_mappings) { + struct dentry *d; + + d = debugfs_lookup(group->swgrp->name, + smmu->debugfs_mappings); + debugfs_remove(d); + } + break; } @@ -1137,6 +1320,8 @@ static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) &tegra_smmu_swgroups_fops); debugfs_create_file("clients", S_IRUGO, smmu->debugfs, smmu, &tegra_smmu_clients_fops); + + smmu->debugfs_mappings = debugfs_create_dir("mappings", smmu->debugfs); } static void tegra_smmu_debugfs_exit(struct tegra_smmu *smmu)
This patch dumps all active mapping entries from pagetable to a debugfs directory named "mappings". Part of this patch for listing all swgroup names in a group_soc is provided by Dmitry Osipenko <digetx@gmail.com> Attaching an example: [SWGROUP: xusb_host] [as: (id: 5), (attr: R|W|-), (pd_dma: 0x0000000080005000)] { [index: 1023] 0xf0080007 (count: 52) { PTE RANGE | ATTR | PHYS | IOVA | SIZE [#913 , #913 ] | 0x7 | 0x0000000101fbe000 | 0xfff91000 | 0x1000 [#914 , #914 ] | 0x7 | 0x0000000101fbd000 | 0xfff92000 | 0x1000 [#915 , #915 ] | 0x7 | 0x0000000101fbc000 | 0xfff93000 | 0x1000 [#916 , #916 ] | 0x7 | 0x0000000101fbb000 | 0xfff94000 | 0x1000 [#921 , #921 ] | 0x7 | 0x00000000fcc02000 | 0xfff99000 | 0x1000 [#922 , #922 ] | 0x7 | 0x0000000101fb7000 | 0xfff9a000 | 0x1000 [#923 , #923 ] | 0x7 | 0x0000000101fb5000 | 0xfff9b000 | 0x1000 [#948 , #948 ] | 0x7 | 0x0000000101fb2000 | 0xfffb4000 | 0x1000 [#949 , #949 ] | 0x7 | 0x0000000101fb1000 | 0xfffb5000 | 0x1000 [#950 , #950 ] | 0x7 | 0x0000000101faf000 | 0xfffb6000 | 0x1000 [#951 , #951 ] | 0x7 | 0x0000000101fae000 | 0xfffb7000 | 0x1000 [#952 , #952 ] | 0x7 | 0x000000010263d000 | 0xfffb8000 | 0x1000 [#953 , #953 ] | 0x7 | 0x000000010263c000 | 0xfffb9000 | 0x1000 [#954 , #954 ] | 0x7 | 0x000000010263b000 | 0xfffba000 | 0x1000 [#955 , #955 ] | 0x7 | 0x000000010263a000 | 0xfffbb000 | 0x1000 [#956 , #956 ] | 0x7 | 0x0000000102639000 | 0xfffbc000 | 0x1000 [#957 , #957 ] | 0x7 | 0x0000000102638000 | 0xfffbd000 | 0x1000 [#958 , #958 ] | 0x7 | 0x0000000102637000 | 0xfffbe000 | 0x1000 [#959 , #959 ] | 0x7 | 0x0000000102636000 | 0xfffbf000 | 0x1000 [#960 , #992 ] | 0x7 | 0x0000000102613000 | 0xfffc0000 | 0x21000 } } Total PDEs: 1, total PTEs: 52 Note that the example above was output after I locally enabled IOMMU_DOMAIN_DMA, which is not merged to mainline yet due to a known framebuffer issue. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/tegra-smmu.c | 185 +++++++++++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+)