| Message ID | 1373021097-32420-15-git-send-email-hdoyu@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 07/05/2013 04:44 AM, Hiroshi Doyu wrote: > Register platform_devices to IOMMU dynamically via > ops->{add,remove}_device(). > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > +/* > + * ASID[0] for the system default > + * ASID[1] for PPCS, which has SDMMC I have no idea what PPCS is. The patch description for 21/22 implies much more than SDMMC is "in PPCS"... > + * ASID[2][3].. open for drivers, first come, first served. > + */ > +enum { > + SYSTEM_DEFAULT, > + SYSTEM_PROTECTED, > +}; Why hard-code this mapping? Can't devices be assigned to ASIDs based on a DT property? I assume there's nothing in the SMMU HW that requires specific ASIDs to be used? I don't see anything in this series which implements the "open for drivers" part of the description for ASID 2/3/... Perhaps not so relevant here since this patch only uses SYSTEM_DEFAULT as the ASID number, but don't you need to validate that smmu->num_as is large enough to support all values in the enum definition above? Since this patch doesn't actually use anything other than SYSTEM_DEFAULT, I wonder if this patch shouldn't be squashed into patch 21/22, where multiple ASIDs are actually used? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200: > On 07/05/2013 04:44 AM, Hiroshi Doyu wrote: > > Register platform_devices to IOMMU dynamically via > > ops->{add,remove}_device(). > > > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > > > +/* > > + * ASID[0] for the system default > > + * ASID[1] for PPCS, which has SDMMC > > I have no idea what PPCS is. The patch description for 21/22 implies > much more than SDMMC is "in PPCS"... "PPCS" is all AHB bus children/(client). > > + * ASID[2][3].. open for drivers, first come, first served. > > + */ > > +enum { > > + SYSTEM_DEFAULT, > > + SYSTEM_PROTECTED, > > +}; > > Why hard-code this mapping? Can't devices be assigned to ASIDs based on > a DT property? I assume there's nothing in the SMMU HW that requires > specific ASIDs to be used? Right. Where should those policy be passed, board DT files? > I don't see anything in this series which implements the "open for > drivers" part of the description for ASID 2/3/... I expect that some smart HW(host1x) will use AS[2/3] for their smarter usage. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/29/2013 05:27 AM, Hiroshi Doyu wrote: > Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:17:21 +0200: > >> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote: >>> Register platform_devices to IOMMU dynamically via >>> ops->{add,remove}_device(). >> >>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c >> >>> +/* >>> + * ASID[0] for the system default >>> + * ASID[1] for PPCS, which has SDMMC >> >> I have no idea what PPCS is. The patch description for 21/22 implies >> much more than SDMMC is "in PPCS"... > > "PPCS" is all AHB bus children/(client). Just to be clear: I was hoping that the comment would be fixed. >>> + * ASID[2][3].. open for drivers, first come, first served. >>> + */ >>> +enum { >>> + SYSTEM_DEFAULT, >>> + SYSTEM_PROTECTED, >>> +}; >> >> Why hard-code this mapping? Can't devices be assigned to ASIDs based on >> a DT property? I assume there's nothing in the SMMU HW that requires >> specific ASIDs to be used? > > Right. Where should those policy be passed, board DT files? Is the HW module (== SW group?) to ASID mapping policy or is a particular mapping required by HW? Can the ASID mapping just happen dynamically in SW rather than DT dictating a particular mapping? I assume the mapping must be set up before any HW is used, and can't be modified later, so there's no much chance of deferring any policy decisions to user-space etc.? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 29 Jul 2013 19:54:09 +0200 Stephen Warren <swarren@wwwdotorg.org> wrote: ... > >> Why hard-code this mapping? Can't devices be assigned to ASIDs based on > >> a DT property? I assume there's nothing in the SMMU HW that requires > >> specific ASIDs to be used? > > > > Right. Where should those policy be passed, board DT files? > > Is the HW module (== SW group?) to ASID mapping policy or is a > particular mapping required by HW? only policy. But swgroup can have multiple HWs, they need to be set in the same swgroup. > Can the ASID mapping just happen dynamically in SW rather than DT > dictating a particular mapping? No info about ASID in DT, at least, right now. If a board DT file is the place to put policy, then it can be an option. > I assume the mapping must be set up before any HW is used, and can't be > modified later, so there's no much chance of deferring any policy > decisions to user-space etc.? I think that both cases are needed. Some traditional devices(MMC) can be changed later as it may have rootfs on it. For some smarter devices like host1x, it want to change ASID dynamically per request/context. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 1617c90..35f4a16 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -39,6 +39,7 @@ #include <asm/page.h> #include <asm/cacheflush.h> +#include <asm/dma-iommu.h> enum smmu_hwgrp { HWGRP_AFI, @@ -949,6 +950,40 @@ static void smmu_iommu_domain_destroy(struct iommu_domain *domain) dev_dbg(smmu->dev, "smmu_as@%p\n", as); } +/* + * ASID[0] for the system default + * ASID[1] for PPCS, which has SDMMC + * ASID[2][3].. open for drivers, first come, first served. + */ +enum { + SYSTEM_DEFAULT, + SYSTEM_PROTECTED, +}; + +static int smmu_iommu_add_device(struct device *dev) +{ + int err; + struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT]; + + if (!map) + goto out; + + err = arm_iommu_attach_device(dev, map); + if (err) { + dev_err(dev, "Failed to attach to map %p\n", map); + return err; + } +out: + dev_dbg(dev, "Attached to map %p\n", map); + return 0; +} + +static void smmu_iommu_remove_device(struct device *dev) +{ + dev_dbg(dev, "Detaching from map %p\n", to_dma_iommu_mapping(dev)); + arm_iommu_detach_device(dev); +} + static struct iommu_ops smmu_iommu_ops = { .domain_init = smmu_iommu_domain_init, .domain_destroy = smmu_iommu_domain_destroy, @@ -958,6 +993,8 @@ static struct iommu_ops smmu_iommu_ops = { .unmap = smmu_iommu_unmap, .iova_to_phys = smmu_iommu_iova_to_phys, .domain_has_cap = smmu_iommu_domain_has_cap, + .add_device = smmu_iommu_add_device, + .remove_device = smmu_iommu_remove_device, .pgsize_bitmap = SMMU_IOMMU_PGSIZES, };
Register platform_devices to IOMMU dynamically via ops->{add,remove}_device(). Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com> --- drivers/iommu/tegra-smmu.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)