| Message ID | 20200926080719.6822-6-nicoleotsuka@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | iommu/tegra-smmu: Adding PCI support and mappings debugfs node | expand |
26.09.2020 11:07, Nicolin Chen пишет: ... > +static int tegra_smmu_mappings_show(struct seq_file *s, void *data) > +{ > + struct tegra_smmu_group_debug *group_debug = s->private; > + const struct tegra_smmu_swgroup *group; > + struct tegra_smmu_as *as; > + struct tegra_smmu *smmu; > + int pd_index, pt_index; > + u64 pte_count = 0; > + u32 pde_count = 0; > + u32 val, ptb_reg; > + u32 *pd; > + > + if (!group_debug || !group_debug->priv || !group_debug->group) > + return 0; > + > + group = group_debug->group; > + as = group_debug->priv; > + smmu = as->smmu; > + > + val = smmu_readl(smmu, group->reg) & SMMU_ASID_ENABLE; > + if (!val) > + return 0; > + > + pd = page_address(as->pd); > + if (!pd) > + return 0; > + > + seq_printf(s, "\nSWGROUP: %s\nASID: %d\nreg: 0x%x\n", group->name, > + as->id, group->reg); > + > + mutex_lock(&smmu->lock); > + smmu_writel(smmu, as->id & 0x7f, SMMU_PTB_ASID); > + ptb_reg = smmu_readl(smmu, SMMU_PTB_DATA); I think the whole traverse needs a locking protection, doesn't it? > + mutex_unlock(&smmu->lock); > + > + seq_printf(s, "PTB_ASID: 0x%x\nas->pd_dma: 0x%llx\n", ptb_reg, as->pd_dma); > + seq_puts(s, "{\n"); > + > + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { > + struct page *pt; > + u32 *addr; > + > + if (!as->count[pd_index] || !pd[pd_index]) > + continue; > + > + pde_count++; > + pte_count += as->count[pd_index]; > + seq_printf(s, "\t[%d] 0x%x (%d)\n", > + pd_index, pd[pd_index], as->count[pd_index]); > + pt = as->pts[pd_index]; > + addr = page_address(pt); This needs as->lock protection. > + seq_puts(s, "\t{\n"); > + seq_printf(s, "\t\t%-5s %-4s %12s %12s\n", "PDE", "ATTR", "PHYS", "IOVA"); > + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index++) { > + u64 iova; > + > + if (!addr[pt_index]) > + continue; > + > + iova = ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT; > + iova |= ((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > + > + seq_printf(s, "\t\t#%-4d 0x%-4x 0x%-12llx 0x%-12llx\n", > + pt_index, addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, > + SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR), iova); > + } > + seq_puts(s, "\t}\n"); > + } > + seq_puts(s, "}\n"); > + seq_printf(s, "Total PDE count: %d\n", pde_count); > + seq_printf(s, "Total PTE count: %lld\n", pte_count); > + > + return 0; > +} > + > +DEFINE_SHOW_ATTRIBUTE(tegra_smmu_mappings); > + > static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) > { > + const struct tegra_smmu_soc *soc = smmu->soc; > + struct tegra_smmu_group_debug *group_debug; > + struct device *dev = smmu->dev; > + struct dentry *d; > + int i; > + > + group_debug = devm_kzalloc(dev, sizeof(*group_debug) * soc->num_swgroups, GFP_KERNEL); devm_kcalloc()
Hi Dmitry, Thank you for the review. On Sat, Sep 26, 2020 at 05:48:54PM +0300, Dmitry Osipenko wrote: > 26.09.2020 11:07, Nicolin Chen пишет: > ... > > +static int tegra_smmu_mappings_show(struct seq_file *s, void *data) > > +{ > > + struct tegra_smmu_group_debug *group_debug = s->private; > > + const struct tegra_smmu_swgroup *group; > > + struct tegra_smmu_as *as; > > + struct tegra_smmu *smmu; > > + int pd_index, pt_index; > > + u64 pte_count = 0; > > + u32 pde_count = 0; > > + u32 val, ptb_reg; > > + u32 *pd; > > + > > + if (!group_debug || !group_debug->priv || !group_debug->group) > > + return 0; > > + > > + group = group_debug->group; > > + as = group_debug->priv; > > + smmu = as->smmu; > > + > > + val = smmu_readl(smmu, group->reg) & SMMU_ASID_ENABLE; > > + if (!val) > > + return 0; > > + > > + pd = page_address(as->pd); > > + if (!pd) > > + return 0; > > + > > + seq_printf(s, "\nSWGROUP: %s\nASID: %d\nreg: 0x%x\n", group->name, > > + as->id, group->reg); > > + > > + mutex_lock(&smmu->lock); > > + smmu_writel(smmu, as->id & 0x7f, SMMU_PTB_ASID); > > + ptb_reg = smmu_readl(smmu, SMMU_PTB_DATA); > > I think the whole traverse needs a locking protection, doesn't it? Hmm..probably would be nicer. Will move the mutex to the top. > > + mutex_unlock(&smmu->lock); > > + > > + seq_printf(s, "PTB_ASID: 0x%x\nas->pd_dma: 0x%llx\n", ptb_reg, as->pd_dma); > > + seq_puts(s, "{\n"); > > + > > + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { > > + struct page *pt; > > + u32 *addr; > > + > > + if (!as->count[pd_index] || !pd[pd_index]) > > + continue; > > + > > + pde_count++; > > + pte_count += as->count[pd_index]; > > + seq_printf(s, "\t[%d] 0x%x (%d)\n", > > + pd_index, pd[pd_index], as->count[pd_index]); > > + pt = as->pts[pd_index]; > > + addr = page_address(pt); > > This needs as->lock protection. Will add that. > > + seq_puts(s, "\t{\n"); > > + seq_printf(s, "\t\t%-5s %-4s %12s %12s\n", "PDE", "ATTR", "PHYS", "IOVA"); > > + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index++) { > > + u64 iova; > > + > > + if (!addr[pt_index]) > > + continue; > > + > > + iova = ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT; > > + iova |= ((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > > + > > + seq_printf(s, "\t\t#%-4d 0x%-4x 0x%-12llx 0x%-12llx\n", > > + pt_index, addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, > > + SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR), iova); > > + } > > + seq_puts(s, "\t}\n"); > > + } > > + seq_puts(s, "}\n"); > > + seq_printf(s, "Total PDE count: %d\n", pde_count); > > + seq_printf(s, "Total PTE count: %lld\n", pte_count); > > + > > + return 0; > > +} > > + > > +DEFINE_SHOW_ATTRIBUTE(tegra_smmu_mappings); > > + > > static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) > > { > > + const struct tegra_smmu_soc *soc = smmu->soc; > > + struct tegra_smmu_group_debug *group_debug; > > + struct device *dev = smmu->dev; > > + struct dentry *d; > > + int i; > > + > > + group_debug = devm_kzalloc(dev, sizeof(*group_debug) * soc->num_swgroups, GFP_KERNEL); > > devm_kcalloc() Will change it. Thanks Nic
26.09.2020 11:07, Nicolin Chen пишет: ... > + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { > + struct page *pt; > + u32 *addr; > + > + if (!as->count[pd_index] || !pd[pd_index]) > + continue; I guess the idea of this patch is to print out the hardware state, isn't it? Hence the as->count shouldn't be checked here. > + pde_count++; > + pte_count += as->count[pd_index]; > + seq_printf(s, "\t[%d] 0x%x (%d)\n", > + pd_index, pd[pd_index], as->count[pd_index]); > + pt = as->pts[pd_index]; > + addr = page_address(pt); > + > + seq_puts(s, "\t{\n"); > + seq_printf(s, "\t\t%-5s %-4s %12s %12s\n", "PDE", "ATTR", "PHYS", "IOVA"); > + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index++) { > + u64 iova; > + > + if (!addr[pt_index]) > + continue; > + > + iova = ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT; > + iova |= ((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; > + > + seq_printf(s, "\t\t#%-4d 0x%-4x 0x%-12llx 0x%-12llx\n", > + pt_index, addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, > + SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR), iova); > Would be nice if you could improve the output formatting by printing out contiguous ranges that have the same ATTRs, otherwise it could be a bit too large and unpractical output in a case if lots of pages are mapped.
27.09.2020 00:24, Dmitry Osipenko пишет: > 26.09.2020 11:07, Nicolin Chen пишет: > ... >> + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { >> + struct page *pt; >> + u32 *addr; >> + >> + if (!as->count[pd_index] || !pd[pd_index]) >> + continue; > > I guess the idea of this patch is to print out the hardware state, isn't > it? Hence the as->count shouldn't be checked here. Perhaps also will be good to check whether the state of !as->count[pd_index] matches state of !pd[pd_index]. WARN_ON_ONCE(!as->count[pd_index] ^ !pd[pd_index])
26.09.2020 23:47, Nicolin Chen пишет: ... >>> + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { >>> + struct page *pt; >>> + u32 *addr; >>> + >>> + if (!as->count[pd_index] || !pd[pd_index]) >>> + continue; >>> + >>> + pde_count++; >>> + pte_count += as->count[pd_index]; >>> + seq_printf(s, "\t[%d] 0x%x (%d)\n", >>> + pd_index, pd[pd_index], as->count[pd_index]); >>> + pt = as->pts[pd_index]; >>> + addr = page_address(pt); >> >> This needs as->lock protection. > > Will add that. > >>> + seq_puts(s, "\t{\n"); >>> + seq_printf(s, "\t\t%-5s %-4s %12s %12s\n", "PDE", "ATTR", "PHYS", "IOVA"); >>> + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index++) { >>> + u64 iova; >>> + >>> + if (!addr[pt_index]) >>> + continue; >>> + >>> + iova = ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT; >>> + iova |= ((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; >>> + >>> + seq_printf(s, "\t\t#%-4d 0x%-4x 0x%-12llx 0x%-12llx\n", >>> + pt_index, addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, >>> + SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR), iova); Please also note that the addr[pt_index] needs to be protected as well. Perhaps you could temporally bump the as->count.
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 9dbc5d7183cc..53160d1ca086 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -20,6 +20,11 @@ #include <soc/tegra/ahb.h> #include <soc/tegra/mc.h> +struct tegra_smmu_group_debug { + const struct tegra_smmu_swgroup *group; + void *priv; +}; + struct tegra_smmu_group { struct list_head list; struct tegra_smmu *smmu; @@ -48,6 +53,8 @@ struct tegra_smmu { struct dentry *debugfs; struct iommu_device iommu; /* IOMMU Core code handle */ + + struct tegra_smmu_group_debug *group_debug; }; struct tegra_smmu_as { @@ -155,6 +162,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) { @@ -337,7 +347,7 @@ static void tegra_smmu_domain_free(struct iommu_domain *domain) } static const struct tegra_smmu_swgroup * -tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup) +tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup, int *index) { const struct tegra_smmu_swgroup *group = NULL; unsigned int i; @@ -345,6 +355,8 @@ tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup) for (i = 0; i < smmu->soc->num_swgroups; i++) { if (smmu->soc->swgroups[i].swgroup == swgroup) { group = &smmu->soc->swgroups[i]; + if (index) + *index = i; break; } } @@ -353,19 +365,22 @@ tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup) } static void tegra_smmu_enable(struct tegra_smmu *smmu, unsigned int swgroup, - unsigned int asid) + struct tegra_smmu_as *as) { const struct tegra_smmu_swgroup *group; + unsigned int asid = as->id; unsigned int i; u32 value; - group = tegra_smmu_find_swgroup(smmu, swgroup); + group = tegra_smmu_find_swgroup(smmu, swgroup, &i); if (group) { value = smmu_readl(smmu, group->reg); value &= ~SMMU_ASID_MASK; value |= SMMU_ASID_VALUE(asid); value |= SMMU_ASID_ENABLE; smmu_writel(smmu, value, group->reg); + if (smmu->group_debug) + smmu->group_debug[i].priv = as; } else { pr_warn("%s group from swgroup %u not found\n", __func__, swgroup); @@ -392,13 +407,15 @@ static void tegra_smmu_disable(struct tegra_smmu *smmu, unsigned int swgroup, unsigned int i; u32 value; - group = tegra_smmu_find_swgroup(smmu, swgroup); + group = tegra_smmu_find_swgroup(smmu, swgroup, &i); if (group) { value = smmu_readl(smmu, group->reg); value &= ~SMMU_ASID_MASK; value |= SMMU_ASID_VALUE(asid); value &= ~SMMU_ASID_ENABLE; smmu_writel(smmu, value, group->reg); + if (smmu->group_debug) + smmu->group_debug[i].priv = NULL; } for (i = 0; i < smmu->soc->num_clients; i++) { @@ -501,7 +518,7 @@ static int tegra_smmu_attach_dev(struct iommu_domain *domain, if (err) goto err_disable; - tegra_smmu_enable(smmu, fwspec->ids[index], as->id); + tegra_smmu_enable(smmu, fwspec->ids[index], as); } if (index == 0) @@ -1078,8 +1095,96 @@ static int tegra_smmu_clients_show(struct seq_file *s, void *data) DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients); +static int tegra_smmu_mappings_show(struct seq_file *s, void *data) +{ + struct tegra_smmu_group_debug *group_debug = s->private; + const struct tegra_smmu_swgroup *group; + struct tegra_smmu_as *as; + struct tegra_smmu *smmu; + int pd_index, pt_index; + u64 pte_count = 0; + u32 pde_count = 0; + u32 val, ptb_reg; + u32 *pd; + + if (!group_debug || !group_debug->priv || !group_debug->group) + return 0; + + group = group_debug->group; + as = group_debug->priv; + smmu = as->smmu; + + val = smmu_readl(smmu, group->reg) & SMMU_ASID_ENABLE; + if (!val) + return 0; + + pd = page_address(as->pd); + if (!pd) + return 0; + + seq_printf(s, "\nSWGROUP: %s\nASID: %d\nreg: 0x%x\n", group->name, + as->id, group->reg); + + mutex_lock(&smmu->lock); + smmu_writel(smmu, as->id & 0x7f, SMMU_PTB_ASID); + ptb_reg = smmu_readl(smmu, SMMU_PTB_DATA); + mutex_unlock(&smmu->lock); + + seq_printf(s, "PTB_ASID: 0x%x\nas->pd_dma: 0x%llx\n", ptb_reg, as->pd_dma); + seq_puts(s, "{\n"); + + for (pd_index = 0; pd_index < SMMU_NUM_PDE; pd_index++) { + struct page *pt; + u32 *addr; + + if (!as->count[pd_index] || !pd[pd_index]) + continue; + + pde_count++; + pte_count += as->count[pd_index]; + seq_printf(s, "\t[%d] 0x%x (%d)\n", + pd_index, pd[pd_index], as->count[pd_index]); + pt = as->pts[pd_index]; + addr = page_address(pt); + + seq_puts(s, "\t{\n"); + seq_printf(s, "\t\t%-5s %-4s %12s %12s\n", "PDE", "ATTR", "PHYS", "IOVA"); + for (pt_index = 0; pt_index < SMMU_NUM_PTE; pt_index++) { + u64 iova; + + if (!addr[pt_index]) + continue; + + iova = ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT; + iova |= ((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT; + + seq_printf(s, "\t\t#%-4d 0x%-4x 0x%-12llx 0x%-12llx\n", + pt_index, addr[pt_index] >> SMMU_PTE_ATTR_SHIFT, + SMMU_PFN_PHYS(addr[pt_index] & ~SMMU_PTE_ATTR), iova); + } + seq_puts(s, "\t}\n"); + } + seq_puts(s, "}\n"); + seq_printf(s, "Total PDE count: %d\n", pde_count); + seq_printf(s, "Total PTE count: %lld\n", pte_count); + + return 0; +} + +DEFINE_SHOW_ATTRIBUTE(tegra_smmu_mappings); + static void tegra_smmu_debugfs_init(struct tegra_smmu *smmu) { + const struct tegra_smmu_soc *soc = smmu->soc; + struct tegra_smmu_group_debug *group_debug; + struct device *dev = smmu->dev; + struct dentry *d; + int i; + + group_debug = devm_kzalloc(dev, sizeof(*group_debug) * soc->num_swgroups, GFP_KERNEL); + if (!group_debug) + return; + smmu->debugfs = debugfs_create_dir("smmu", NULL); if (!smmu->debugfs) return; @@ -1088,6 +1193,21 @@ 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); + d = debugfs_create_dir("mappings", smmu->debugfs); + + for (i = 0; i < soc->num_swgroups; i++) { + const struct tegra_smmu_swgroup *group = &soc->swgroups[i]; + + if (!group->name) + continue; + + group_debug[i].group = group; + + debugfs_create_file(group->name, 0444, d, &group_debug[i], + &tegra_smmu_mappings_fops); + } + + smmu->group_debug = group_debug; } 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". Ataching an example: SWGROUP: hc ASID: 0 reg: 0x250 PTB_ASID: 0xe00bb880 as->pd_dma: 0xbb880000 { [1023] 0xf00bb882 (1) { PDE ATTR PHYS IOVA #1023 0x5 0x159f5d000 0xfffff000 } } Total PDE count: 1 Total PTE count: 1 Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/iommu/tegra-smmu.c | 130 +++++++++++++++++++++++++++++++++++-- 1 file changed, 125 insertions(+), 5 deletions(-)