Skip to contentSkip to conversation

intel-iommu fixes

PATCH In review Al Viro 4 participants 4 messages Oct 29, 2007 – Nov 16, 2007 PCI

lore
Share & tools
lore

Al Viro

author
#0 lore raw

PATCH 1/1 intel-iommu fixes

* off by one in dmar_get_fault_reason() (maximal index in
array is ARRAY_SIZE()-1, not ARRAY_SIZE())
* NULL noise removal
* __iomem annotation fix

---

drivers/pci/intel-iommu.c +3 -4
index 0c4ab3b..9b35259 100644
@@ -745,7 +745,7 @@ static char *fault_reason_strings[] =
 	"non-zero reserved fields in PTE",
 	"Unknown"
 };
-#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings)
+#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings) - 1
 
 char *dmar_get_fault_reason(u8 fault_reason)
 {
@@ -995,7 +995,6 @@ static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
 	return iommu;
 error_unmap:
 	iounmap(iommu->reg);
-	iommu->reg = 0;
 error:
 	kfree(iommu);
 	return NULL;
@@ -1808,7 +1807,7 @@ get_valid_domain_for_dev(struct pci_dev *pdev)
 	if (!domain) {
 		printk(KERN_ERR
 			"Allocating domain for %s failed", pci_name(pdev));
-		return 0;
+		return NULL;
 	}
 
 	/* make sure context mapping is ok */
@@ -1818,7 +1817,7 @@ get_valid_domain_for_dev(struct pci_dev *pdev)
 			printk(KERN_ERR
 				"Domain context map for %s failed",
 				pci_name(pdev));
-			return 0;
+			return NULL;
 		}
 	}
 
drivers/pci/intel-iommu.h +1 -1
index ee88dd2..459ad1f 100644
@@ -58,7 +58,7 @@
 		hi = readl(dmar + reg + 4); \
 		(((u64) hi) << 32) + lo; })
 */
-static inline u64 dmar_readq(void *addr)
+static inline u64 dmar_readq(void __iomem *addr)
 {
 	u32 lo, hi;
 	lo = readl(addr);
#1 lore raw
Quoted 12 lines · Al Viro

* off by one in dmar_get_fault_reason() (maximal index in
array is ARRAY_SIZE()-1, not ARRAY_SIZE())
* NULL noise removal
* __iomem annotation fix

---

drivers/pci/intel-iommu.c +0 -0
index 0c4ab3b..9b35259 100644

drivers/pci/intel-iommu.c static char *fault_reason_strings[] = · line 748

Quoted 5 lines
drivers/pci/intel-iommu.c +1 -1
@@ -745,7 +745,7 @@ static char *fault_reason_strings[] =
 	"non-zero reserved fields in PTE",
 	"Unknown"
 };
-#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings)
+#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings) - 1

This define now should really have ()'s around its value now that it is an expression to avoid potential problems at its reference sites.

<snip>

Signature · 1 line
Mark Rustad, [email protected]
#2 lore raw
In reply to Al Viro
Quoted 12 lines · Al Viro

* off by one in dmar_get_fault_reason() (maximal index in
array is ARRAY_SIZE()-1, not ARRAY_SIZE())
* NULL noise removal
* __iomem annotation fix

---

drivers/pci/intel-iommu.c +0 -0
index 0c4ab3b..9b35259 100644

drivers/pci/intel-iommu.c static char *fault_reason_strings[] = · line 748

Quoted 5 lines
drivers/pci/intel-iommu.c +1 -1
@@ -745,7 +745,7 @@ static char *fault_reason_strings[] =
 	"non-zero reserved fields in PTE",
 	"Unknown"
 };
-#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings)
+#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings) - 1

Probably should be
+#define MAX_FAULT_REASON_IDX (ARRAY_SIZE(fault_reason_strings) - 1)

Quoted 34 lines
drivers/pci/intel-iommu.c +2 -3
 char *dmar_get_fault_reason(u8 fault_reason)
 {
@@ -995,7 +995,6 @@ static struct intel_iommu *alloc_iommu(struct dmar_drhd_unit *drhd)
 	return iommu;
 error_unmap:
 	iounmap(iommu->reg);
-	iommu->reg = 0;
 error:
 	kfree(iommu);
 	return NULL;
@@ -1808,7 +1807,7 @@ get_valid_domain_for_dev(struct pci_dev *pdev)
 	if (!domain) {
 		printk(KERN_ERR
 			"Allocating domain for %s failed", pci_name(pdev));
-		return 0;
+		return NULL;
 	}
 
 	/* make sure context mapping is ok */
@@ -1818,7 +1817,7 @@ get_valid_domain_for_dev(struct pci_dev *pdev)
 			printk(KERN_ERR
 				"Domain context map for %s failed",
 				pci_name(pdev));
-			return 0;
+			return NULL;
 		}
 	}
 
drivers/pci/intel-iommu.h +0 -0
index ee88dd2..459ad1f 100644

drivers/pci/intel-iommu.h line 64

Quoted 8 lines
drivers/pci/intel-iommu.h +1 -1
@@ -58,7 +58,7 @@
 		hi = readl(dmar + reg + 4); \
 		(((u64) hi) << 32) + lo; })
 */
-static inline u64 dmar_readq(void *addr)
+static inline u64 dmar_readq(void __iomem *addr)
 {
 	u32 lo, hi;
 	lo = readl(addr);

Looks good to me.

ACK --mgross

#3 lore raw
In reply to Al Viro

drivers/pci/intel-iommu.c static char *fault_reason_strings[] = · line 748

Quoted 2 lines · Al Viro
drivers/pci/intel-iommu.c +1 -1
-#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings)
+#define MAX_FAULT_REASON_IDX 	ARRAY_SIZE(fault_reason_strings) - 1

The macro is unsafe without yet another ()s around it. --
vda