Skip to contentSkip to conversation

iommu-PMEN_REG boot up support

PATCH In review Mark Gross 2 participants 4 messages Oct 26, 2007 – Oct 30, 2007

Subsystems (2)
Documentation PCI
lore
Share & tools
lore

Mark Gross

author
#0 lore raw

PATCH 1/1 iommu-PMEN_REG boot up support

The following patch clears the portect memory region enable bit at boot time by default. It also provides a kernel parrameter for disabling this behavior and leave the PMEN_REG untouched if so wanted.

If the boot loader or platform has protected memory regions enabled at boot time it could prevent DMA's from happening as drivers are loaded and used.

Singed-off-by mark gross <[email protected]>

--mgross

drivers/pci/intel-iommu.c +27 -0
Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================
@@ -55,6 +55,7 @@
 static int dmar_disabled;
 static int __initdata dmar_map_gfx = 1;
 static int dmar_forcedac;
+static int clear_pmen_epm = 1;
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
 static DEFINE_SPINLOCK(device_domain_lock);

@@ -76,8 +77,14 @@
 			printk (KERN_INFO
 				"Intel-IOMMU: Forcing DAC for PCI devices\n");
 			dmar_forcedac = 1;
+		} else if (!strncmp(str, "no_epm_clear", 12)) {
+			printk(KERN_INFO
+				"Intel-IOMMU: subress clearing of Enable "
+				"Protected Memory bit\n");
+			clear_pmen_epm = 0;
 		}
 
+
 		str += strcspn(str, ",");
 		while (*str == ',')
 			str++;

@@ -692,6 +699,23 @@
 		DMA_TLB_PSI_FLUSH, non_present_entry_flush);
 }
 
+static void iommu_disable_protect_mem_regions(struct intel_iommu *iommu)
+{
+	u32 pmen;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iommu->register_lock, flags);
+	pmen = readl(iommu->reg + DMAR_PMEN_REG);
+	pmen &= ~DMA_PMEN_EPM;
+	writel(pmen, iommu->reg + DMAR_PMEN_REG);
+
+	/* wait for the protected region status bit to clear */
+	IOMMU_WAIT_OP(iommu, DMAR_PMEN_REG,
+		readl, (pmen & DMA_PMEN_PRS), pmen);
+
+	spin_unlock_irqrestore(&iommu->register_lock, flags);
+}
+
 static int iommu_enable_translation(struct intel_iommu *iommu)
 {
 	u32 sts;

@@ -1731,6 +1755,9 @@
 		iommu_flush_context_global(iommu, 0);
 		iommu_flush_iotlb_global(iommu, 0);
 
+		if (clear_pmen_epm)
+			iommu_disable_protect_mem_regions(iommu);
+
 		ret = iommu_enable_translation(iommu);
 		if (ret)
 			goto error;
drivers/pci/intel-iommu.h +4 -0
Index: linux-2.6/drivers/pci/intel-iommu.h
===================================================================
@@ -126,6 +126,10 @@
 #define DMA_TLB_IH_NONLEAF (((u64)1) << 6)
 #define DMA_TLB_MAX_SIZE (0x3f)
 
+/* PMEN_REG */
+#define DMA_PMEN_EPM (((u32)1)<<31)
+#define DMA_PMEN_PRS (((u32)1)<<0)
+
 /* GCMD_REG */
 #define DMA_GCMD_TE (((u32)1) << 31)
 #define DMA_GCMD_SRTP (((u32)1) << 30)
Documentation/kernel-parameters.txt +4 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -789,6 +789,10 @@
 			than 32 bit addressing. The default is to look
 			for translation below 32 bit and if not available
 			then look in the higher range.
+		no_epm_clear
+			This will override the default of disabling protected
+			memory regions from DMA to leave the PMEM_REG settings
+			as they where set up by the boot loader / platform.
 
 	io7=		[HW] IO7 for Marvel based alpha systems
 			See comment before marvel_specify_io7 in
#1 lore raw
Quoted 8 lines · Mark Gross

The following patch clears the portect memory region enable bit at boot time by default. It also provides a kernel parrameter for disabling this behavior and leave the PMEN_REG untouched if so wanted.

If the boot loader or platform has protected memory regions enabled at boot time it could prevent DMA's from happening as drivers are loaded and used.

What's the value of adding a command line option for this? Under what circumstances would we want to not clear this bit?

drivers/pci/intel-iommu.c line 84

Quoted 5 lines
drivers/pci/intel-iommu.c +5 -0
+		} else if (!strncmp(str, "no_epm_clear", 12)) {
+			printk(KERN_INFO
+				"Intel-IOMMU: subress clearing of Enable "
+				"Protected Memory bit\n");
+			clear_pmen_epm = 0;

`suppress', I assume. Rest looks fine, if the configuration option is really needed.

Cheers,
Muli

Signature · 4 lines
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007
author
#2 lore raw
Quoted 7 lines · Muli Ben-Yehuda
Quoted 5 lines · Mark Gross

The following patch clears the portect memory region enable bit at boot time by default. It also provides a kernel parrameter for disabling this behavior and leave the PMEN_REG untouched if so wanted.

Quoted 6 lines
Quoted 3 lines

If the boot loader or platform has protected memory regions enabled at boot time it could prevent DMA's from happening as drivers are loaded and used.

What's the value of adding a command line option for this? Under what circumstances would we want to not clear this bit?

umm, /me asks around and finds out that at this time there isn't much point to having a command line to preserve any protected areas set up at boot time.

Quoted 10 lines

Review

Quoted 6 lines
(quoted diff) +5 -0
+		} else if (!strncmp(str, "no_epm_clear", 12)) {
+			printk(KERN_INFO
+				"Intel-IOMMU: subress clearing of Enable "
+				"Protected Memory bit\n");
+			clear_pmen_epm = 0;

`suppress', I assume. Rest looks fine, if the configuration option is really needed.

Quoted 8 lines

Cheers,
Muli

Signature · 5 lines
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007
-
Signed-off-by mark gross <[email protected]>

--mgross

drivers/pci/intel-iommu.c +19 -0
Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================
@@ -692,6 +692,23 @@
 		DMA_TLB_PSI_FLUSH, non_present_entry_flush);
 }
 
+static void iommu_disable_protect_mem_regions(struct intel_iommu *iommu)
+{
+	u32 pmen;
+	unsigned long flags;
+
+	spin_lock_irqsave(&iommu->register_lock, flags);
+	pmen = readl(iommu->reg + DMAR_PMEN_REG);
+	pmen &= ~DMA_PMEN_EPM;
+	writel(pmen, iommu->reg + DMAR_PMEN_REG);
+
+	/* wait for the protected region status bit to clear */
+	IOMMU_WAIT_OP(iommu, DMAR_PMEN_REG,
+		readl, (pmen & DMA_PMEN_PRS), pmen);
+
+	spin_unlock_irqrestore(&iommu->register_lock, flags);
+}
+
 static int iommu_enable_translation(struct intel_iommu *iommu)
 {
 	u32 sts;

@@ -1731,6 +1748,8 @@
 		iommu_flush_context_global(iommu, 0);
 		iommu_flush_iotlb_global(iommu, 0);
 
+		iommu_disable_protect_mem_regions(iommu);
+
 		ret = iommu_enable_translation(iommu);
 		if (ret)
 			goto error;
drivers/pci/intel-iommu.h +4 -0
Index: linux-2.6/drivers/pci/intel-iommu.h
===================================================================
@@ -126,6 +126,10 @@
 #define DMA_TLB_IH_NONLEAF (((u64)1) << 6)
 #define DMA_TLB_MAX_SIZE (0x3f)
 
+/* PMEN_REG */
+#define DMA_PMEN_EPM (((u32)1)<<31)
+#define DMA_PMEN_PRS (((u32)1)<<0)
+
 /* GCMD_REG */
 #define DMA_GCMD_TE (((u32)1) << 31)
 #define DMA_GCMD_SRTP (((u32)1) << 30)
Acked-by
#3 lore raw
Quoted 6 lines · Mark Gross
Quoted 2 lines

What's the value of adding a command line option for this? Under what circumstances would we want to not clear this bit?

umm, /me asks around and finds out that at this time there isn't much point to having a command line to preserve any protected areas set up at boot time.

The society for controlled proliferation of command line options thanks you.

Quoted 1 line
Signed-off-by mark gross <[email protected]>
Acked-by Muli Ben-Yehuda <[email protected]>

Cheers,
Muli

Signature · 4 lines
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007