Skip to contentSkip to conversation

arm: msm: Add MSM IOMMU support.

PATCH Superseded Stepan Moskovchenko 2 participants 5 messages Aug 10, 2010

Subsystems (3)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 arm: msm: Add MSM IOMMU support.

Add support for the IOMMUs found on the upcoming Qualcomm MSM8x60 chips. These IOMMUs allow virtualization of the address space used by most of the multimedia cores on these chips. The driver implements the kernel's IOMMU API.

Fixed in v3: style, types for ioremapped addresses, use of const, failure paths, cache flushing functions, unnecessary initialization, ordering of spinlocks, unnecessary casting, and other things.

Also, iova_to_phys now returns 0 on any error, to make this implementation consistent with how other IOMMU drivers handle this case.

---
 arch/arm/mach-msm/include/mach/iommu.h         |  102 ++
 arch/arm/mach-msm/include/mach/iommu_hw-8xxx.h | 1864 ++++++++++++++++++++++++
 arch/arm/mach-msm/iommu.c                      |  601 ++++++++
 3 files changed, 2567 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-msm/include/mach/iommu.h
 create mode 100644 arch/arm/mach-msm/include/mach/iommu_hw-8xxx.h
 create mode 100644 arch/arm/mach-msm/iommu.c
arch/arm/mach-msm/include/mach/iommu.h new file +102 -0
new file mode 100644
index 0000000..66138aa
@@ -0,0 +1,102 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#ifndef MSM_IOMMU_H
+#define MSM_IOMMU_H
+
+#include <linux/interrupt.h>
+
+/* Maximum number of Machine IDs that we are allowing to be mapped to the same
+ * context bank. The number of MIDs mapped to the same CB does not affect
+ * performance, but there is a practical limit on how many distinct MIDs may
+ * be present. These mappings are typically determined at design time and are
+ * not expected to change at run time.
+ */
+#define MAX_NUM_MIDS	16
+
+/**
+ * struct msm_iommu_dev - a single IOMMU hardware instance
+ * name		Human-readable name given to this IOMMU HW instance
+ * clk_rate	Rate to set for this IOMMU's clock, if applicable to this
+ *		particular IOMMU. 0 means don't set a rate.
+ *
+ */
+struct msm_iommu_dev {
+	const char *name;
+	unsigned long clk_rate;
+};
+
+/**
+ * struct msm_iommu_ctx_dev - an IOMMU context bank instance
+ * name		Human-readable name given to this context bank
+ * num		Index of this context bank within the hardware
+ * mids		List of Machine IDs that are to be mapped into this context
+ *		bank, terminated by -1. The MID is a set of signals on the
+ *		AXI bus that identifies the function associated with a specific
+ *		memory request. (See ARM spec).
+ */
+struct msm_iommu_ctx_dev {
+	const char *name;
+	int num;
+	int mids[MAX_NUM_MIDS];
+};
+
+
+/**
+ * struct msm_iommu_drvdata - A single IOMMU hardware instance
+ * @base:	IOMMU config port base address (VA)
+ * @irq:	Interrupt number
+  *
+ * A msm_iommu_drvdata holds the global driver data about a single piece
+ * of an IOMMU hardware instance.
+ */
+struct msm_iommu_drvdata {
+	void __iomem *base;
+	int irq;
+};
+
+/**
+ * struct msm_iommu_ctx_drvdata - an IOMMU context bank instance
+ * @num:		Hardware context number of this context
+ * @pdev:		Platform device associated wit this HW instance
+ * @attached_elm:	List element for domains to track which devices are
+ *			attached to them
+ *
+ * A msm_iommu_ctx_drvdata holds the driver data for a single context bank
+ * within each IOMMU hardware instance
+ */
+struct msm_iommu_ctx_drvdata {
+	int num;
+	struct platform_device *pdev;
+	struct list_head attached_elm;
+};
+
+/*
+ * Look up an IOMMU context device by its context name. NULL if none found.
+ * Useful for testing and drivers that do not yet fully have IOMMU stuff in
+ * their platform devices.
+ */
+struct device *msm_iommu_get_ctx(const char *ctx_name);
+
+/*
+ * Interrupt handler for the IOMMU context fault interrupt. Hooking the
+ * interrupt is not supported in the API yet, but this will print an error
+ * message and dump useful IOMMU registers.
+ */
+irqreturn_t msm_iommu_fault_handler(int irq, void *dev_id);
+
+#endif
arch/arm/mach-msm/include/mach/iommu_hw-8xxx.h new file +1864 -0

1,864 lines not on this page · raw message

arch/arm/mach-msm/iommu.c new file +601 -0

601 lines not on this page · raw message

--
1.7.0.2
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

#1 lore raw

arch/arm/mach-msm/iommu.c

Quoted 10 lines · Stepan Moskovchenko

Add support for the IOMMUs found on the upcoming Qualcomm MSM8x60 chips. These IOMMUs allow virtualization of the address space used by most of the multimedia cores on these chips. The driver implements the kernel's IOMMU API.

arch/arm/mach-msm/iommu.c new file +0 -0
new file mode 100644
index 0000000..b22aecd
@@ -0,0 +1,601 @@

Why use both pr_err and printk(KERN_ERR ?

Quoted 19 lines
arch/arm/mach-msm/iommu.c new file +19 -0
+#define pr_fmt(fmt) "%s %i " fmt, __func__, __LINE__
+		pr_err("bad size: %d\n", len);
+		pr_err("null page table\n");
+			pr_err("could not allocate second level table\n");
+		pr_err("null page table\n");
+		pr_err("first level PTE is 0\n");
+	printk(KERN_ERR "FAR    = %08x    PAR    = %08x\n",
+	printk(KERN_ERR "FSR    = %08x [%s%s%s%s%s%s%s%s%s%s]\n", fsr,
+	printk(KERN_ERR "FSYNR0 = %08x    FSYNR1 = %08x\n",
+	printk(KERN_ERR "TTBR0  = %08x    TTBR1  = %08x\n",
+	printk(KERN_ERR "SCTLR  = %08x    ACTLR  = %08x\n",
+	printk(KERN_ERR "PRRR   = %08x    NMRR   = %08x\n",
+		pr_err("Invalid device ID in context interrupt handler\n");
+	printk(KERN_ERR "===== WOAH! =====\n");
+	printk(KERN_ERR "Unexpected IOMMU page fault!\n");
+	printk(KERN_ERR "base = %08x\n", (unsigned int) base);
+			printk(KERN_ERR "Fault occurred in context %d.\n", i);
+			printk(KERN_ERR "Interesting registers:\n");
+			printk(KERN_ERR "\n");
#2 lore raw
Quoted 1 line

Why use both pr_err and printk(KERN_ERR ?

I had been using pr_err for printing badness messages internal to the driver, but I guess their functionality is largely paralleled by return error codes. I had been using printk(KERN_ERR for messages that have been caused by an event that is more external to the driver and its callers (like the data for the page fault, which is most likely to be caused by a misprogrammed client). pr_err gives other information like function name, line number, etc, which would be noise when looking at an iommu page fault. And, I guess if the pr_err stuff got turned off, you would want the page fault data to still be printed... but that's kind of a minor point.

I am actually considering doing away with all the pr_err lines entirely because they typically also result in returning -Ewhatever and other IOMMU drivers don't give this level of error reporting. But, they may be helpful to the users of the API. What are people's thoughts on this?

Thanks
Steve

Quoted 27 lines
Quoted 9 lines
(quoted diff) +9 -0
+#define pr_fmt(fmt) "%s %i " fmt, __func__, __LINE__
+		pr_err("bad size: %d\n", len);
+		pr_err("null page table\n");
+			pr_err("could not allocate second level table\n");
+		pr_err("null page table\n");
+		pr_err("first level PTE is 0\n");
+	printk(KERN_ERR "FAR    = %08x    PAR    = %08x\n",
+	printk(KERN_ERR "FSR    = %08x [%s%s%s%s%s%s%s%s%s%s]\n", fsr,
+	printk(KERN_ERR "FSYNR0 = %08x    FSYNR1 = %08x\n",

Review

Quoted 10 lines
(quoted diff) +10 -0
+	printk(KERN_ERR "TTBR0  = %08x    TTBR1  = %08x\n",
+	printk(KERN_ERR "SCTLR  = %08x    ACTLR  = %08x\n",
+	printk(KERN_ERR "PRRR   = %08x    NMRR   = %08x\n",
+		pr_err("Invalid device ID in context interrupt handler\n");
+	printk(KERN_ERR "===== WOAH! =====\n");
+	printk(KERN_ERR "Unexpected IOMMU page fault!\n");
+	printk(KERN_ERR "base = %08x\n", (unsigned int) base);
+			printk(KERN_ERR "Fault occurred in context %d.\n", i);
+			printk(KERN_ERR "Interesting registers:\n");
+			printk(KERN_ERR "\n");

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 4 lines · stepanm at codeaurora.org

I am actually considering doing away with all the pr_err lines entirely because they typically also result in returning -Ewhatever and other IOMMU drivers don't give this level of error reporting. But, they may be helpful to the users of the API. What are people's thoughts on this?

I believe the current uses of pr_err in this module are unnecessary or could be converted to pr_debug.

I think __func__/line aren't particularly useful.

I prefer log messages prefixed with the module name.

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

and I'd convert all the current pr_err to pr_debug and convert the printk(KERN_ERR to pr_err(

cheers, Joe

#4 lore raw
Quoted 6 lines · On Mon, 2010-08-09 at 20:42 -0700, [email protected] wrote:

I believe the current uses of pr_err in this module are unnecessary or could be converted to pr_debug.

I think __func__/line aren't particularly useful.

Quoted 8 lines

I prefer log messages prefixed with the module name.

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

and I'd convert all the current pr_err to pr_debug and convert the printk(KERN_ERR to pr_err(

cheers, Joe

Thanks. Fixed in v4, which will go up in a few minutes.