Hello,
On 2015-05-06 16:01, Robin Murphy wrote:
Hi Marek,
On 04/05/15 09:15, Marek Szyprowski wrote:
Some devices (like frame buffers) are enabled by bootloader and
configured
to perform DMA operations automatically (like displaying boot logo or
splash
screen). Such devices operate and perform DMA operation usually until
the
proper driver for them is loaded and probed. However before that
happens,
system usually loads IOMMU drivers and configures dma parameters for
each
device. When such initial configuration is created and enabled, it
usually
contains empty translation rules betweem IO address space and physical
memory, because no buffers nor memory regions have been requested by the
respective driver.
This patch adds support for "iommu-reserved-mapping", which can be used
to provide definitions for mappings that need to be created on system
boot to let such devices (enabled by bootloader) to operate properly
until respective driver is probed.
This appears to only work if you assume the driver is going to tear
down the existing domain entirely; what about drivers that don't
manage IOMMUs explicitly, or if there are multiple active devices
behind the same IOMMU which (in future) start out in the same default
domain? If any device is happy to remain in the default domain then it
would be nice to clear the reservations once they are no longer needed.
Right, this need to be somehow worked out, but frankly right now I don't
have
good idea which code should do it. The only idea that comes to my mind
is using
a BUS_NOTIFY_BOUND_DRIVER notifier, but I don't like this approach.
Could we not address the issue in a more robust way, like fleshing out
an implementation of the nascent IOMMU_DOMAIN_IDENTITY type, then just
flagging such devices to stipulate that their boot-time default domain
must be an identity-mapped one?
I'm open for any solution that will cover this case. Maybe my idea of iommu
reserved regions is a bit over-engineered? Maybe instead of defining
reserved
ranges it will be enough to add something like
'iommu-on-boot-identity-mapping'
property? This way one can later use it for IOMMU_DOMAIN_IDENTITY
approach or
something else what will be agreed?
Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
---
Documentation/devicetree/bindings/iommu/iommu.txt | 44 +++++++++
arch/arm/mm/dma-mapping.c | 112
++++++++++++++++++++++
2 files changed, 156 insertions(+)
[...]
@@ -2048,6 +2092,66 @@ void arm_iommu_detach_device(struct device *dev)
}
EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
+static int arm_iommu_add_reserved(struct device *dev,
+ struct dma_iommu_mapping *domain, phys_addr_t
phys,
+ dma_addr_t dma, size_t size)
+{
+ int ret;
+
+ ret = __reserve_iova(domain, dma, size);
+ if (ret) {
+ dev_err(dev, "failed to reserve mapping\n");
+ return -EINVAL;
+ }
+
+ ret = iommu_map(domain->domain, dma, phys, size, IOMMU_READ);
+ if (ret != 0) {
+ dev_err(dev, "create IOMMU mapping\n");
+ return ret;
+ }
+
+ dev_info(dev, "created reserved DMA mapping (%pa -> %pad, %zu
bytes)\n",
+ &phys, &dma, size);
+
+ return 0;
+}
+
+static int arm_iommu_init_reserved(struct device *dev,
+ struct dma_iommu_mapping *domain)
+{
+ const char *name = "iommu-reserved-mapping";
+ const __be32 *prop = NULL;
+ int len, naddr, nsize;
+ struct device_node *node = dev->of_node;
+ phys_addr_t phys;
+ dma_addr_t dma;
+ size_t size;
+
+ if (!node)
+ return 0;
+
+ naddr = of_n_addr_cells(node);
+ nsize = of_n_size_cells(node);
+
+ prop = of_get_property(node, name, &len);
+ if (!prop)
+ return 0;
+
+ len /= sizeof(u32);
+
+ if (len < 2 * naddr + nsize) {
+ dev_err(dev, "invalid length (%d cells) of %s
property\n",
+ len, name);
+ return -EINVAL;
+ }
+
+ phys = of_read_number(prop, naddr);
+ dma = of_read_number(prop + naddr, naddr);
+ size = of_read_number(prop + 2*naddr, nsize);
+
+ return arm_iommu_add_reserved(dev, domain, phys, dma, size);
> +}
I may be missing something, but I don't see how this can handle
multiple ranges for the same device as the binding says.
Right, loop is missing in the above calls.
static struct dma_map_ops *arm_get_iommu_dma_map_ops(bool coherent)
{
return coherent ? &iommu_coherent_ops : &iommu_ops;
@@ -2068,6 +2172,14 @@ static bool arm_setup_iommu_dma_ops(struct
device *dev, u64 dma_base, u64 size,
return false;
}
+ if (arm_iommu_init_reserved(dev, mapping) != 0) {
+ pr_warn("Failed to initialize reserved mapping for
device %s\n",
+ dev_name(dev));
+ __arm_iommu_detach_device(dev);
+ arm_iommu_release_mapping(mapping);
+ return false;
+ }
+
if (__arm_iommu_attach_device(dev, mapping)) {
pr_warn("Failed to attached device %s to
IOMMU_mapping\n",
dev_name(dev));
I'm hoping Joerg is still working on his default domain series,
because the domain creation in arch_setup_dma_ops turns out to be
horrible on a number of levels (like everything happening in the wrong
order for platform devices). If that doesn't negate this issue
entirely, it's going to significantly break this way of hooking up the
solution (depending on what the drivers do) - worth some
consideration, at least.
I would really like to have something working soon, it's been a lot of
discussion but still
very little of code that actually implements anything...
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
