Patchwork [v2,21/22] iommu/tegra: smmu: Support Multiple ASID

login
register
mail settings
Submitter Hiroshi Doyu
Date July 5, 2013, 10:44 a.m.
Message ID <1373021097-32420-22-git-send-email-hdoyu@nvidia.com>
Download mbox | patch
Permalink /patch/257095/
State Superseded, archived
Headers show

Comments

Hiroshi Doyu - July 5, 2013, 10:44 a.m.
Support Multiple Address Space(AS). Tegra SMMU can have multiple
ASes. We reserve 2 of them for static assignment, AS[0] for system
default, AS[1] for AHB clusters as protected domain from others, where
there are many traditional pheripheral devices like USB, SD/MMC. They
should be isolated from some smart devices like host1x for system
robustness. Even if smart devices behaves wrongly, the traditional
devices(SD/MMC, USB) wouldn't be affected, and the system could
continue most likely.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
Stephen Warren - July 18, 2013, 8:43 p.m.
On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> Support Multiple Address Space(AS). Tegra SMMU can have multiple
> ASes. We reserve 2 of them for static assignment, AS[0] for system
> default, AS[1] for AHB clusters as protected domain from others, where
> there are many traditional pheripheral devices like USB, SD/MMC. They
> should be isolated from some smart devices like host1x for system
> robustness. Even if smart devices behaves wrongly, the traditional
> devices(SD/MMC, USB) wouldn't be affected, and the system could
> continue most likely.

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

>  static int smmu_iommu_add_device(struct device *dev)
>  {
>  	int err;
> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> +	u64 swgroup;
> +	struct dma_iommu_mapping *map = NULL;
> +
> +	swgroup = smmu_of_get_memory_client(dev);
> +	switch (swgroup) {
> +	case TEGRA_SWGROUP_BIT(PPCS):
> +		map = smmu_handle->map[SYSTEM_PROTECTED];
> +		break;
> +	default:
> +		map = smmu_handle->map[SYSTEM_DEFAULT];

I already mentioned this, but just for completeness: What is
smmu->num_as <= SYSTEM_DEFAULT?
--
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
Hiroshi Doyu - July 29, 2013, 10:31 a.m.
Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:

> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
> > Support Multiple Address Space(AS). Tegra SMMU can have multiple
> > ASes. We reserve 2 of them for static assignment, AS[0] for system
> > default, AS[1] for AHB clusters as protected domain from others, where
> > there are many traditional pheripheral devices like USB, SD/MMC. They
> > should be isolated from some smart devices like host1x for system
> > robustness. Even if smart devices behaves wrongly, the traditional
> > devices(SD/MMC, USB) wouldn't be affected, and the system could
> > continue most likely.
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> >  static int smmu_iommu_add_device(struct device *dev)
> >  {
> >  	int err;
> > -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
> > +	u64 swgroup;
> > +	struct dma_iommu_mapping *map = NULL;
> > +
> > +	swgroup = smmu_of_get_memory_client(dev);
> > +	switch (swgroup) {
> > +	case TEGRA_SWGROUP_BIT(PPCS):
> > +		map = smmu_handle->map[SYSTEM_PROTECTED];
> > +		break;
> > +	default:
> > +		map = smmu_handle->map[SYSTEM_DEFAULT];
> 
> I already mentioned this, but just for completeness: What is
> smmu->num_as <= SYSTEM_DEFAULT?

I think that this belongs to the system operation policy. Which H/W
should be configured to which Address Space(AS). This put all AHB
clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
USB and SD/MMC so that they should be kept separated from the smart
IOMMU clients like host1x.

Is there any place to configure this kind of board specific policy
rather than here?
--
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 - July 29, 2013, 5:41 p.m.
On 07/29/2013 04:31 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Thu, 18 Jul 2013 22:43:06 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Support Multiple Address Space(AS). Tegra SMMU can have multiple
>>> ASes. We reserve 2 of them for static assignment, AS[0] for system
>>> default, AS[1] for AHB clusters as protected domain from others, where
>>> there are many traditional pheripheral devices like USB, SD/MMC. They
>>> should be isolated from some smart devices like host1x for system
>>> robustness. Even if smart devices behaves wrongly, the traditional
>>> devices(SD/MMC, USB) wouldn't be affected, and the system could
>>> continue most likely.
>>
>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>
>>>  static int smmu_iommu_add_device(struct device *dev)
>>>  {
>>>  	int err;
>>> -	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
>>> +	u64 swgroup;
>>> +	struct dma_iommu_mapping *map = NULL;
>>> +
>>> +	swgroup = smmu_of_get_memory_client(dev);
>>> +	switch (swgroup) {
>>> +	case TEGRA_SWGROUP_BIT(PPCS):
>>> +		map = smmu_handle->map[SYSTEM_PROTECTED];
>>> +		break;
>>> +	default:
>>> +		map = smmu_handle->map[SYSTEM_DEFAULT];
>>
>> I already mentioned this, but just for completeness: What is
>> smmu->num_as <= SYSTEM_DEFAULT?
> 
> I think that this belongs to the system operation policy. Which H/W
> should be configured to which Address Space(AS). This put all AHB
> clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> USB and SD/MMC so that they should be kept separated from the smart
> IOMMU clients like host1x.
> 
> Is there any place to configure this kind of board specific policy
> rather than here?

I'm not sure that answers the question I asked.

I mean that the driver expects that two AS always exist;
SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
IIRC defined by the DT content. What if the DT doesn't define two ASs?
Shouldn't there at least be an error-check for this case, so the driver
doesn't just blindly continue and access smmu_handle->map[1] when the
map[] array only has 1 entry?

--
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
Hiroshi Doyu - July 30, 2013, 5:22 a.m.
On Mon, 29 Jul 2013 19:41:23 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:
...
> > I think that this belongs to the system operation policy. Which H/W
> > should be configured to which Address Space(AS). This put all AHB
> > clients(PPCS) into AS[1](SYSTEM_PROTECTED), and the rest into
> > AS[0](SYSTEM_DEFAULT). AHB clients are mainly traditional H/Ws like
> > USB and SD/MMC so that they should be kept separated from the smart
> > IOMMU clients like host1x.
> > 
> > Is there any place to configure this kind of board specific policy
> > rather than here?
> 
> I'm not sure that answers the question I asked.
> 
> I mean that the driver expects that two AS always exist;
> SYSTEM_PROTECTED and SYSTEM_DEFAULT. However, the set of extant ASs is
> IIRC defined by the DT content. What if the DT doesn't define two ASs?
> Shouldn't there at least be an error-check for this case, so the driver
> doesn't just blindly continue and access smmu_handle->map[1] when the
> map[] array only has 1 entry?

OK, now I got it.

We can create multiple maps on the same AS in theory. So the limitation
is only for how many maps S/W creaed. This can/should be checked only
within kernel side. No constraints from DT.
--
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

Patch

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 8a9434e..1945815 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -904,7 +904,18 @@  enum {
 static int smmu_iommu_add_device(struct device *dev)
 {
 	int err;
-	struct dma_iommu_mapping *map = smmu_handle->map[SYSTEM_DEFAULT];
+	u64 swgroup;
+	struct dma_iommu_mapping *map = NULL;
+
+	swgroup = smmu_of_get_memory_client(dev);
+	switch (swgroup) {
+	case TEGRA_SWGROUP_BIT(PPCS):
+		map = smmu_handle->map[SYSTEM_PROTECTED];
+		break;
+	default:
+		map = smmu_handle->map[SYSTEM_DEFAULT];
+		break;
+	}
 
 	if (!map)
 		goto out;
@@ -915,7 +926,7 @@  static int smmu_iommu_add_device(struct device *dev)
 		return err;
 	}
 out:
-	dev_dbg(dev, "Attached to map %p\n", map);
+	dev_dbg(dev, "Attached to map %p, swgroup:%08llx\n", map, swgroup);
 	return 0;
 }