Hi Joerg, Thank you for your quick review.
From: Joerg Roedel <joerg.roedel@xxxxxxx>
Subject: Re: [PATCH v2 2/2] [RFC] ARM: IOMMU: Tegra30: iommu_ops for SMMU driver
Date: Fri, 16 Dec 2011 16:39:04 +0100
Message-ID: <20111216153904.GC29877@xxxxxxx>
> On Thu, Dec 15, 2011 at 03:11:30PM +0200, Hiroshi DOYU wrote:
> > +static int smmu_iommu_attach_dev(struct iommu_domain *domain,
> > + struct device *dev)
> > +{
> > + struct smmu_as *as = domain->priv;
> > + struct smmu_client *client, *c;
> > + u32 map;
> > + int err;
> > +
> > + client = kmalloc(sizeof(*c), GFP_KERNEL);
> > + if (!client)
> > + return -ENOMEM;
> > + client->dev = dev;
> > + client->as = as;
> > + map = (unsigned long)dev->platform_data;
> > + if (!map)
> > + return -EINVAL;
> > +
> > + err = smmu_client_enable_hwgrp(client, map);
> > + if (err)
> > + goto err_hwgrp;
> > +
> > + spin_lock(&as->client_lock);
> > + list_for_each_entry(c, &as->client, list) {
> > + if (c->dev == dev) {
> > + pr_err("%s is already attached\n", dev_name(dev));
> > + err = -EINVAL;
> > + goto err_client;
> > + }
> > + }
> > + list_add(&client->list, &as->client);
> > + spin_unlock(&as->client_lock);
> > +
> > + /*
> > + * Reserve "page zero" for AVP vectors using a common dummy
> > + * page.
> > + */
> > + if (map & HWG_AVPC) {
> > + struct page *page;
> > +
> > + page = as->smmu->avp_vector_page;
> > + __smmu_iommu_map_pfn(as, 0, page_to_pfn(page));
> > +
> > + pr_info("Reserve \"page zero\" for AVP vectors using a common dummy\n");
> > + }
> > +
> > + pr_debug("Attached %s\n", dev_name(dev));
> > + return 0;
> > +err_client:
> > + smmu_client_disable_hwgrp(client);
> > + spin_unlock(&as->client_lock);
> > +err_hwgrp:
> > + kfree(client);
> > + return err;
> > +}
>
> Hmm, I have a question about that. Reading the code it looks like your
> SMMU exists per pheripheral device
A single SMMU is shared with multiple peripheral devices, a single
SMMU has multiple ASIDs. ASID is used per group of peripheral
devices. These peripheral groups can be configured by the above
"hwgrp"/"map" passed via platform_data. The above "struct device"
represents a groupd of peripheral devices, a kind of virtual device.
The above "struct device" ~= a group of peripheral devices.
> and the SMMU hardware supports multiple address spaces per device,
> right?
Yes, where "device" is a gropud of peripheral devices.
> The domains are implemented for one address-space.
Yes.
> So is it right that a device can have multiple
> address-spaces?
No, at least, the following code prevents if a peripheral is already
assigned ASID.
smmu_client_enable_hwgrp():
....
+ for_each_set_bit(i, &map, BITS_PER_LONG) {
+ offs = HWGRP_ASID_REG(i);
+ val = smmu_read(smmu, offs);
+ if (on) {
+ if (WARN_ON(val & mask))
+ goto err_hw_busy;
> If so, what kind of devices do you bind to the domains
> then. I doesn't make sense to bind whole peripheral devices in this
> case.
Here, the above "struct device" ~= a group of any peripheral
devices. Those groups are configurable.
In my simple DMA API test(not posted), the above hwgrp/map is configured as below:
146 static int __init dmaapi_test_init(void)
147 {
148 int i;
149 struct dma_iommu_mapping *map;
150
151 map = arm_iommu_create_mapping(IOVA_START, IOVA_SIZE, 0);
152 BUG_ON(!map);
153 pr_debug("Allocate IOVA: %08x-%08x\n", map->base, map->base + IOVA_SIZE);
154
155 for (i = 0; i < ARRAY_SIZE(dmaapi_dummy_device); i++) {
156 int err;
157 struct platform_device *pdev = &dmaapi_dummy_device[i];
158
159 pdev->dev.platform_data = (void *)dummy_hwgrp_map[i];
160 err = platform_device_register(pdev);
161 BUG_ON(err);
162
163 err = arm_iommu_attach_device(&pdev->dev, map);
164 BUG_ON(err);
165 pr_debug("IOMMU API: Attached to %s\n", dev_name(&pdev->dev));
166 }
So peripheral devices can be divided into multiple groups, and each
each group represents struct device with hwgrp/map info in
platform_data.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
