[patch 0/6] PAT fixes/cleanup following the pfnmap changes

[patch 0/6] PAT fixes/cleanup following the pfnmap changes

[venkatesh.pallipadi]

Individual patch header has descriptions.

-- 

[patch 1/6] x86 PAT: remove PFNMAP type on track_pfn_vma_new() error

[venkatesh.pallipadi]

[-- Attachment #1: pfn_cleaup_fix_02.patch --]
[-- Type: text/plain, Size: 1293 bytes --]

On track_pfn_vma_new() failure, reset the vm_flags so that there will be
no second cleanup happening when upper level routines call unmap_vmas().

This patch fixes part of the bug reported here
http://marc.info/?l=linux-kernel&m=123108883716357&w=2

Specifically the error message
X:5010 freeing invalid memtype d0000000-d0101000
Is due to multiple frees on error path, will not happen with the patch below.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 mm/memory.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: tip/mm/memory.c
===================================================================
--- tip.orig/mm/memory.c	2009-01-06 11:14:35.000000000 -0800
+++ tip/mm/memory.c	2009-01-06 13:44:57.000000000 -0800
@@ -1605,8 +1605,14 @@ int remap_pfn_range(struct vm_area_struc
 	vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP;
 
 	err = track_pfn_vma_new(vma, prot, pfn, PAGE_ALIGN(size));
-	if (err)
+	if (err) {
+		/*
+		 * To indicate that track_pfn related cleanup is not
+		 * needed from higher level routine calling unmap_vmas
+		 */
+		vma->vm_flags &= ~(VM_IO | VM_RESERVED | VM_PFNMAP);
 		return -EINVAL;
+	}
 
 	BUG_ON(addr >= end);
 	pfn -= addr >> PAGE_SHIFT;

-- 

[patch 2/6] x86 PAT: consolidate old memtype new memtype check into a function

[venkatesh.pallipadi]

[-- Attachment #1: cleanup_allowed_memtype_change.patch --]
[-- Type: text/plain, Size: 2448 bytes --]

Move the new memtype old memtype allowed check to header so that is can be
shared by other users. Subsequent patch uses this in pat.c in remap_pfn_range()
code path. No functionality change in this patch.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/include/asm/pgtable.h |   19 +++++++++++++++++++
 arch/x86/pci/i386.c            |   12 +-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

Index: linux-2.6/arch/x86/include/asm/pgtable.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/pgtable.h	2009-01-07 15:00:34.000000000 -0800
+++ linux-2.6/arch/x86/include/asm/pgtable.h	2009-01-07 16:03:25.000000000 -0800
@@ -341,6 +341,25 @@ static inline pgprot_t pgprot_modify(pgp
 
 #define canon_pgprot(p) __pgprot(pgprot_val(p) & __supported_pte_mask)
 
+static inline int is_new_memtype_allowed(unsigned long flags,
+						unsigned long new_flags)
+{
+	/*
+	 * Certain new memtypes are not allowed with certain
+	 * requested memtype:
+	 * - request is uncached, return cannot be write-back
+	 * - request is write-combine, return cannot be write-back
+	 */
+	if ((flags == _PAGE_CACHE_UC_MINUS &&
+	     new_flags == _PAGE_CACHE_WB) ||
+	    (flags == _PAGE_CACHE_WC &&
+	     new_flags == _PAGE_CACHE_WB)) {
+		return 0;
+	}
+
+	return 1;
+}
+
 #ifndef __ASSEMBLY__
 /* Indicate that x86 has its own track and untrack pfn vma functions */
 #define __HAVE_PFNMAP_TRACKING
Index: linux-2.6/arch/x86/pci/i386.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/i386.c	2009-01-07 15:00:34.000000000 -0800
+++ linux-2.6/arch/x86/pci/i386.c	2009-01-07 16:03:20.000000000 -0800
@@ -314,17 +314,7 @@ int pci_mmap_page_range(struct pci_dev *
 		return retval;
 
 	if (flags != new_flags) {
-		/*
-		 * Do not fallback to certain memory types with certain
-		 * requested type:
-		 * - request is uncached, return cannot be write-back
-		 * - request is uncached, return cannot be write-combine
-		 * - request is write-combine, return cannot be write-back
-		 */
-		if ((flags == _PAGE_CACHE_UC_MINUS &&
-		     (new_flags == _PAGE_CACHE_WB)) ||
-		    (flags == _PAGE_CACHE_WC &&
-		     new_flags == _PAGE_CACHE_WB)) {
+		if (!is_new_memtype_allowed(flags, new_flags)) {
 			free_memtype(addr, addr+len);
 			return -EINVAL;
 		}

-- 

[patch 3/6] x86 PAT: change track_pfn_vma_new to take pgprot_t pointer param

[venkatesh.pallipadi]

[-- Attachment #1: change_track_pfn_vma_new_param.patch --]
[-- Type: text/plain, Size: 4172 bytes --]

Change the protection parameter for track_pfn_vma_new() into a pgprot_t pointer.
Subsequent patch changes the x86 PAT handling to return a compatible
memtype in pgprot_t, if what was requested cannot be allowed due to conflicts.
No fuctionality change in this patch.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/mm/pat.c             |    6 +++---
 include/asm-generic/pgtable.h |    4 ++--
 mm/memory.c                   |    7 ++++---
 3 files changed, 9 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/mm/pat.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/pat.c	2009-01-07 15:00:34.000000000 -0800
+++ linux-2.6/arch/x86/mm/pat.c	2009-01-07 16:17:47.000000000 -0800
@@ -741,7 +741,7 @@ cleanup_ret:
  * Note that this function can be called with caller trying to map only a
  * subrange/page inside the vma.
  */
-int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t prot,
+int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t *prot,
 			unsigned long pfn, unsigned long size)
 {
 	int retval = 0;
@@ -758,14 +758,14 @@ int track_pfn_vma_new(struct vm_area_str
 	if (is_linear_pfn_mapping(vma)) {
 		/* reserve the whole chunk starting from vm_pgoff */
 		paddr = (resource_size_t)vma->vm_pgoff << PAGE_SHIFT;
-		return reserve_pfn_range(paddr, vma_size, prot);
+		return reserve_pfn_range(paddr, vma_size, *prot);
 	}
 
 	/* reserve page by page using pfn and size */
 	base_paddr = (resource_size_t)pfn << PAGE_SHIFT;
 	for (i = 0; i < size; i += PAGE_SIZE) {
 		paddr = base_paddr + i;
-		retval = reserve_pfn_range(paddr, PAGE_SIZE, prot);
+		retval = reserve_pfn_range(paddr, PAGE_SIZE, *prot);
 		if (retval)
 			goto cleanup_ret;
 	}
Index: linux-2.6/include/asm-generic/pgtable.h
===================================================================
--- linux-2.6.orig/include/asm-generic/pgtable.h	2009-01-07 15:00:35.000000000 -0800
+++ linux-2.6/include/asm-generic/pgtable.h	2009-01-07 16:12:23.000000000 -0800
@@ -301,7 +301,7 @@ static inline void ptep_modify_prot_comm
  * track_pfn_vma_new is called when a _new_ pfn mapping is being established
  * for physical range indicated by pfn and size.
  */
-static inline int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t prot,
+static inline int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t *prot,
 					unsigned long pfn, unsigned long size)
 {
 	return 0;
@@ -332,7 +332,7 @@ static inline void untrack_pfn_vma(struc
 {
 }
 #else
-extern int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t prot,
+extern int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t *prot,
 				unsigned long pfn, unsigned long size);
 extern int track_pfn_vma_copy(struct vm_area_struct *vma);
 extern void untrack_pfn_vma(struct vm_area_struct *vma, unsigned long pfn,
Index: linux-2.6/mm/memory.c
===================================================================
--- linux-2.6.orig/mm/memory.c	2009-01-07 15:35:17.000000000 -0800
+++ linux-2.6/mm/memory.c	2009-01-07 16:16:10.000000000 -0800
@@ -1444,6 +1444,7 @@ int vm_insert_pfn(struct vm_area_struct 
 			unsigned long pfn)
 {
 	int ret;
+	pgprot_t pgprot = vma->vm_page_prot;
 	/*
 	 * Technically, architectures with pte_special can avoid all these
 	 * restrictions (same for remap_pfn_range).  However we would like
@@ -1458,10 +1459,10 @@ int vm_insert_pfn(struct vm_area_struct 
 
 	if (addr < vma->vm_start || addr >= vma->vm_end)
 		return -EFAULT;
-	if (track_pfn_vma_new(vma, vma->vm_page_prot, pfn, PAGE_SIZE))
+	if (track_pfn_vma_new(vma, &pgprot, pfn, PAGE_SIZE))
 		return -EINVAL;
 
-	ret = insert_pfn(vma, addr, pfn, vma->vm_page_prot);
+	ret = insert_pfn(vma, addr, pfn, pgprot);
 
 	if (ret)
 		untrack_pfn_vma(vma, pfn, PAGE_SIZE);
@@ -1604,7 +1605,7 @@ int remap_pfn_range(struct vm_area_struc
 
 	vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP;
 
-	err = track_pfn_vma_new(vma, prot, pfn, PAGE_ALIGN(size));
+	err = track_pfn_vma_new(vma, &prot, pfn, PAGE_ALIGN(size));
 	if (err) {
 		/*
 		 * To indicate that track_pfn related cleanup is not

-- 

[patch 4/6] x86 PAT: return compatible mapping to remap_pfn_range callers

[venkatesh.pallipadi]

[-- Attachment #1: allow_non_exact_memtype_in_track_pfn_vma_new.patch --]
[-- Type: text/plain, Size: 4506 bytes --]

Change x86 PAT code to return compatible memtype if the exact memtype that
was requested in remap_pfn_rage and friends is not available due to some
conflict.

This is done by returning the compatible type in pgprot parameter of
track_pfn_vma_new(), and the caller uses that memtype for page table.

Note that track_pfn_vma_copy() which is basically called during fork gets the
prot from existing page table and should not have any conflict. Hence we use
strict memtype check there and do not allow compatible memtypes.

This patch fixes the bug reported here
http://marc.info/?l=linux-kernel&m=123108883716357&w=2

Specifically the error message
X:5010 map pfn expected mapping type write-back for d0000000-d0101000, 
got write-combining


Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/mm/pat.c |   43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

Index: linux-2.6/arch/x86/mm/pat.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/pat.c	2009-01-07 16:17:47.000000000 -0800
+++ linux-2.6/arch/x86/mm/pat.c	2009-01-07 16:35:57.000000000 -0800
@@ -601,12 +601,13 @@ void unmap_devmem(unsigned long pfn, uns
  * Reserved non RAM regions only and after successful reserve_memtype,
  * this func also keeps identity mapping (if any) in sync with this new prot.
  */
-static int reserve_pfn_range(u64 paddr, unsigned long size, pgprot_t vma_prot)
+static int reserve_pfn_range(u64 paddr, unsigned long size, pgprot_t *vma_prot,
+				int strict_prot)
 {
 	int is_ram = 0;
 	int id_sz, ret;
 	unsigned long flags;
-	unsigned long want_flags = (pgprot_val(vma_prot) & _PAGE_CACHE_MASK);
+	unsigned long want_flags = (pgprot_val(*vma_prot) & _PAGE_CACHE_MASK);
 
 	is_ram = pagerange_is_ram(paddr, paddr + size);
 
@@ -625,15 +626,24 @@ static int reserve_pfn_range(u64 paddr, 
 		return ret;
 
 	if (flags != want_flags) {
-		free_memtype(paddr, paddr + size);
-		printk(KERN_ERR
-		"%s:%d map pfn expected mapping type %s for %Lx-%Lx, got %s\n",
-			current->comm, current->pid,
-			cattr_name(want_flags),
-			(unsigned long long)paddr,
-			(unsigned long long)(paddr + size),
-			cattr_name(flags));
-		return -EINVAL;
+		if (strict_prot || !is_new_memtype_allowed(want_flags, flags)) {
+			free_memtype(paddr, paddr + size);
+			printk(KERN_ERR "%s:%d map pfn expected mapping type %s"
+				" for %Lx-%Lx, got %s\n",
+				current->comm, current->pid,
+				cattr_name(want_flags),
+				(unsigned long long)paddr,
+				(unsigned long long)(paddr + size),
+				cattr_name(flags));
+			return -EINVAL;
+		}
+		/*
+		 * We allow returning different type than the one requested in
+		 * non strict case.
+		 */
+		*vma_prot = __pgprot((pgprot_val(*vma_prot) &
+				      (~_PAGE_CACHE_MASK)) |
+				     flags);
 	}
 
 	/* Need to keep identity mapping in sync */
@@ -689,6 +699,7 @@ int track_pfn_vma_copy(struct vm_area_st
 	unsigned long vma_start = vma->vm_start;
 	unsigned long vma_end = vma->vm_end;
 	unsigned long vma_size = vma_end - vma_start;
+	pgprot_t pgprot;
 
 	if (!pat_enabled)
 		return 0;
@@ -702,7 +713,8 @@ int track_pfn_vma_copy(struct vm_area_st
 			WARN_ON_ONCE(1);
 			return -EINVAL;
 		}
-		return reserve_pfn_range(paddr, vma_size, __pgprot(prot));
+		pgprot = __pgprot(prot);
+		return reserve_pfn_range(paddr, vma_size, &pgprot, 1);
 	}
 
 	/* reserve entire vma page by page, using pfn and prot from pte */
@@ -710,7 +722,8 @@ int track_pfn_vma_copy(struct vm_area_st
 		if (follow_phys(vma, vma_start + i, 0, &prot, &paddr))
 			continue;
 
-		retval = reserve_pfn_range(paddr, PAGE_SIZE, __pgprot(prot));
+		pgprot = __pgprot(prot);
+		retval = reserve_pfn_range(paddr, PAGE_SIZE, &pgprot, 1);
 		if (retval)
 			goto cleanup_ret;
 	}
@@ -758,14 +771,14 @@ int track_pfn_vma_new(struct vm_area_str
 	if (is_linear_pfn_mapping(vma)) {
 		/* reserve the whole chunk starting from vm_pgoff */
 		paddr = (resource_size_t)vma->vm_pgoff << PAGE_SHIFT;
-		return reserve_pfn_range(paddr, vma_size, *prot);
+		return reserve_pfn_range(paddr, vma_size, prot, 0);
 	}
 
 	/* reserve page by page using pfn and size */
 	base_paddr = (resource_size_t)pfn << PAGE_SHIFT;
 	for (i = 0; i < size; i += PAGE_SIZE) {
 		paddr = base_paddr + i;
-		retval = reserve_pfn_range(paddr, PAGE_SIZE, *prot);
+		retval = reserve_pfn_range(paddr, PAGE_SIZE, prot, 0);
 		if (retval)
 			goto cleanup_ret;
 	}

-- 

[patch 5/6] x86 PAT: ioremap_wc should take resource_size_t parameter

[venkatesh.pallipadi]

[-- Attachment #1: ioremap_wc_resource_size_change.patch --]
[-- Type: text/plain, Size: 1638 bytes --]

ioremap_wc was taking in unsigned long parameter, where as it should take 64
bit resource_size_t parameter like other ioremap variants.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/include/asm/io.h |    2 +-
 arch/x86/mm/ioremap.c     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/include/asm/io.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/io.h	2009-01-09 11:23:10.000000000 -0800
+++ linux-2.6/arch/x86/include/asm/io.h	2009-01-09 13:21:58.000000000 -0800
@@ -91,7 +91,7 @@ extern void unxlate_dev_mem_ptr(unsigned
 
 extern int ioremap_change_attr(unsigned long vaddr, unsigned long size,
 				unsigned long prot_val);
-extern void __iomem *ioremap_wc(unsigned long offset, unsigned long size);
+extern void __iomem *ioremap_wc(resource_size_t offset, unsigned long size);
 
 /*
  * early_ioremap() and early_iounmap() are for temporary early boot-time
Index: linux-2.6/arch/x86/mm/ioremap.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/ioremap.c	2009-01-09 11:23:10.000000000 -0800
+++ linux-2.6/arch/x86/mm/ioremap.c	2009-01-09 13:21:58.000000000 -0800
@@ -367,7 +367,7 @@ EXPORT_SYMBOL(ioremap_nocache);
  *
  * Must be freed with iounmap.
  */
-void __iomem *ioremap_wc(unsigned long phys_addr, unsigned long size)
+void __iomem *ioremap_wc(resource_size_t phys_addr, unsigned long size)
 {
 	if (pat_enabled)
 		return __ioremap_caller(phys_addr, size, _PAGE_CACHE_WC,

-- 

[patch 6/6] x86 PAT: Remove CPA WARN_ON for zero pte

[venkatesh.pallipadi]

[-- Attachment #1: function_kernel_map_sync_memtype.patch --]
[-- Type: text/plain, Size: 7525 bytes --]

The logic to find whether identity map exists or not using
high_memory or max_low_pfn_mapped/max_pfn_mapped are not complete
as the memory withing the range may not be mapped if there is a
unusable hole in e820.

Specifically, on my test system I started seeing these warnings with
tools like hwinfo, acpidump trying to map ACPI region.

[   27.400018] ------------[ cut here ]------------
[   27.400344] WARNING: at /home/venkip/src/linus/linux-2.6/arch/x86/mm/pageattr.c:560 __change_page_attr_set_clr+0xf3/0x8b8()
[   27.400821] Hardware name: X7DB8
[   27.401070] CPA: called for zero pte. vaddr = ffff8800cff6a000 cpa->vaddr = ffff8800cff6a000
[   27.401569] Modules linked in:
[   27.401882] Pid: 4913, comm: dmidecode Not tainted 2.6.28-05716-gfe0bdec #586
[   27.402141] Call Trace:
[   27.402488]  [<ffffffff80237c21>] warn_slowpath+0xd3/0x10f
[   27.402749]  [<ffffffff80274ade>] ? find_get_page+0xb3/0xc9
[   27.403028]  [<ffffffff80274a2b>] ? find_get_page+0x0/0xc9
[   27.403333]  [<ffffffff80226425>] __change_page_attr_set_clr+0xf3/0x8b8
[   27.403628]  [<ffffffff8028ec99>] ? __purge_vmap_area_lazy+0x192/0x1a1
[   27.403883]  [<ffffffff8028eb52>] ? __purge_vmap_area_lazy+0x4b/0x1a1
[   27.404172]  [<ffffffff80290268>] ? vm_unmap_aliases+0x1ab/0x1bb
[   27.404512]  [<ffffffff80290105>] ? vm_unmap_aliases+0x48/0x1bb
[   27.404766]  [<ffffffff80226d28>] change_page_attr_set_clr+0x13e/0x2e6
[   27.405026]  [<ffffffff80698fa7>] ? _spin_unlock+0x26/0x2a
[   27.405292]  [<ffffffff80227e6a>] ? reserve_memtype+0x19b/0x4e3
[   27.405590]  [<ffffffff80226ffd>] _set_memory_wb+0x22/0x24
[   27.405844]  [<ffffffff80225d28>] ioremap_change_attr+0x26/0x28
[   27.406097]  [<ffffffff80228355>] reserve_pfn_range+0x1a3/0x235
[   27.406427]  [<ffffffff80228430>] track_pfn_vma_new+0x49/0xb3
[   27.406686]  [<ffffffff80286c46>] remap_pfn_range+0x94/0x32c
[   27.406940]  [<ffffffff8022878d>] ? phys_mem_access_prot_allowed+0xb5/0x1a8
[   27.407209]  [<ffffffff803e9bf4>] mmap_mem+0x75/0x9d
[   27.407523]  [<ffffffff8028b3b4>] mmap_region+0x2cf/0x53e
[   27.407776]  [<ffffffff8028b8cc>] do_mmap_pgoff+0x2a9/0x30d
[   27.408034]  [<ffffffff8020f4a4>] sys_mmap+0x92/0xce
[   27.408339]  [<ffffffff8020b65b>] system_call_fastpath+0x16/0x1b
[   27.408614] ---[ end trace 4b16ad70c09a602d ]---
[   27.408871] dmidecode:4913 reserve_pfn_range ioremap_change_attr failed write-back for cff6a000-cff6b000

This is wih track_pfn_vma_new trying to keep identity map in sync.
The address cff6a000 is the ACPI region according to e820.

[    0.000000] BIOS-provided physical RAM map:
[    0.000000]  BIOS-e820: 0000000000000000 - 000000000009c000 (usable)
[    0.000000]  BIOS-e820: 000000000009c000 - 00000000000a0000 (reserved)
[    0.000000]  BIOS-e820: 00000000000cc000 - 00000000000d0000 (reserved)
[    0.000000]  BIOS-e820: 00000000000e4000 - 0000000000100000 (reserved)
[    0.000000]  BIOS-e820: 0000000000100000 - 00000000cff60000 (usable)
[    0.000000]  BIOS-e820: 00000000cff60000 - 00000000cff69000 (ACPI data)
[    0.000000]  BIOS-e820: 00000000cff69000 - 00000000cff80000 (ACPI NVS)
[    0.000000]  BIOS-e820: 00000000cff80000 - 00000000d0000000 (reserved)
[    0.000000]  BIOS-e820: 00000000e0000000 - 00000000f0000000 (reserved)
[    0.000000]  BIOS-e820: 00000000fec00000 - 00000000fec10000 (reserved)
[    0.000000]  BIOS-e820: 00000000fee00000 - 00000000fee01000 (reserved)
[    0.000000]  BIOS-e820: 00000000ff000000 - 0000000100000000 (reserved)
[    0.000000]  BIOS-e820: 0000000100000000 - 0000000230000000 (usable)

And is not mapped as per init_memory_mapping.

[    0.000000] init_memory_mapping: 0000000000000000-00000000cff60000
[    0.000000] init_memory_mapping: 0000000100000000-0000000230000000

We can add logic to check for this. But, there can also be other holes in
identity map when we have 1GB of aligned reserved space in e820.

Patch below handles this by removing the WARN_ON and returning a specific
error value (EFAULT) to indicate that the address does not have any
identity mapping. The code that tries to keep identity map in sync can ignore
this error, with other callers of cpa still getting error here.
 
Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

---
 arch/x86/mm/pageattr.c |   10 ++++++----
 arch/x86/mm/pat.c      |   45 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 17 deletions(-)

Index: linux-2.6/arch/x86/mm/pat.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/pat.c	2009-01-09 13:19:52.000000000 -0800
+++ linux-2.6/arch/x86/mm/pat.c	2009-01-09 14:00:20.000000000 -0800
@@ -505,6 +505,35 @@ static inline int range_is_allowed(unsig
 }
 #endif /* CONFIG_STRICT_DEVMEM */
 
+/*
+ * Change the memory type for the physial address range in kernel identity
+ * mapping space if that range is a part of identity map.
+ */
+static int kernel_map_sync_memtype(u64 base, unsigned long size,
+					unsigned long flags)
+{
+	unsigned long id_sz;
+	int ret;
+
+	if (!pat_enabled || base >= __pa(high_memory))
+		return 0;
+
+	id_sz = (__pa(high_memory) < base + size) ?
+						__pa(high_memory) - base :
+						size;
+
+	ret = ioremap_change_attr((unsigned long)__va(base), id_sz, flags);
+	/*
+	 * -EFAULT return means that the addr was not valid and did not have
+	 * any identity mapping. That case is a success for
+	 * kernel_map_sync_memtype.
+	 */
+	if (ret == -EFAULT)
+		ret = 0;
+
+	return ret;
+}
+
 int phys_mem_access_prot_allowed(struct file *file, unsigned long pfn,
 				unsigned long size, pgprot_t *vma_prot)
 {
@@ -555,9 +584,7 @@ int phys_mem_access_prot_allowed(struct 
 	if (retval < 0)
 		return 0;
 
-	if (((pfn < max_low_pfn_mapped) ||
-	     (pfn >= (1UL<<(32 - PAGE_SHIFT)) && pfn < max_pfn_mapped)) &&
-	    ioremap_change_attr((unsigned long)__va(offset), size, flags) < 0) {
+	if (kernel_map_sync_memtype(offset, size, flags)) {
 		free_memtype(offset, offset + size);
 		printk(KERN_INFO
 		"%s:%d /dev/mem ioremap_change_attr failed %s for %Lx-%Lx\n",
@@ -605,7 +632,7 @@ static int reserve_pfn_range(u64 paddr, 
 				int strict_prot)
 {
 	int is_ram = 0;
-	int id_sz, ret;
+	int ret;
 	unsigned long flags;
 	unsigned long want_flags = (pgprot_val(*vma_prot) & _PAGE_CACHE_MASK);
 
@@ -646,15 +673,7 @@ static int reserve_pfn_range(u64 paddr, 
 				     flags);
 	}
 
-	/* Need to keep identity mapping in sync */
-	if (paddr >= __pa(high_memory))
-		return 0;
-
-	id_sz = (__pa(high_memory) < paddr + size) ?
-				__pa(high_memory) - paddr :
-				size;
-
-	if (ioremap_change_attr((unsigned long)__va(paddr), id_sz, flags) < 0) {
+	if (kernel_map_sync_memtype(paddr, size, flags)) {
 		free_memtype(paddr, paddr + size);
 		printk(KERN_ERR
 			"%s:%d reserve_pfn_range ioremap_change_attr failed %s "
Index: linux-2.6/arch/x86/mm/pageattr.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/pageattr.c	2009-01-09 13:19:52.000000000 -0800
+++ linux-2.6/arch/x86/mm/pageattr.c	2009-01-09 13:54:46.000000000 -0800
@@ -555,10 +555,12 @@ repeat:
 	if (!pte_val(old_pte)) {
 		if (!primary)
 			return 0;
-		WARN(1, KERN_WARNING "CPA: called for zero pte. "
-		       "vaddr = %lx cpa->vaddr = %lx\n", address,
-		       *cpa->vaddr);
-		return -EINVAL;
+
+		/*
+		 *  Special error value returned, indicating that the mapping
+		 * did not exist at this address.
+		 */
+		return -EFAULT;
 	}
 
 	if (level == PG_LEVEL_4K) {

-- 

Re: [patch 0/6] PAT fixes/cleanup following the pfnmap changes

[Kevin Winchester]

venkatesh.pallipadi@intel.com wrote:
> Individual patch header has descriptions.
> 

$ git am Patches
Applying: x86 PAT: change track_pfn_vma_new to take pgprot_t pointer param
error: patch failed: mm/memory.c:1604
error: mm/memory.c: patch does not apply
Patch failed at 0001.
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".


These do not apply for me to latest Linus git, which is the tree where I experienced the problem.  Is there some way to fuzz them to get them to apply?

-- 
Kevin Winchester

Re: [patch 0/6] PAT fixes/cleanup following the pfnmap changes

[Ingo Molnar]

* Kevin Winchester <kjwinchester@gmail.com> wrote:

> venkatesh.pallipadi@intel.com wrote:
> > Individual patch header has descriptions.
> > 
> 
> $ git am Patches
> Applying: x86 PAT: change track_pfn_vma_new to take pgprot_t pointer param
> error: patch failed: mm/memory.c:1604
> error: mm/memory.c: patch does not apply
> Patch failed at 0001.
> When you have resolved this problem run "git am --resolved".
> If you would prefer to skip this patch, instead run "git am --skip".
> To restore the original branch and stop patching run "git am --abort".
> 
> These do not apply for me to latest Linus git, which is the tree where I 
> experienced the problem.  Is there some way to fuzz them to get them to 
> apply?

Could you check tip/master:

  http://people.redhat.com/mingo/tip.git/README

do you still get the messages? [ Btw., did you also experience any 3D 
performance problems that are solved via Venki's fixes in tip/master? ]

	Ingo

Re: [patch 0/6] PAT fixes/cleanup following the pfnmap changes

[Kevin Winchester]

Ingo Molnar wrote:
> * Kevin Winchester <kjwinchester@gmail.com> wrote:
> 
>> venkatesh.pallipadi@intel.com wrote:
>>> Individual patch header has descriptions.
>>>
>> $ git am Patches
>> Applying: x86 PAT: change track_pfn_vma_new to take pgprot_t pointer param
>> error: patch failed: mm/memory.c:1604
>> error: mm/memory.c: patch does not apply
>> Patch failed at 0001.
>> When you have resolved this problem run "git am --resolved".
>> If you would prefer to skip this patch, instead run "git am --skip".
>> To restore the original branch and stop patching run "git am --abort".
>>
>> These do not apply for me to latest Linus git, which is the tree where I 
>> experienced the problem.  Is there some way to fuzz them to get them to 
>> apply?
> 
> Could you check tip/master:
> 
>   http://people.redhat.com/mingo/tip.git/README
> 
> do you still get the messages? [ Btw., did you also experience any 3D 
> performance problems that are solved via Venki's fixes in tip/master? ]
> 

I just checked tip/master, and I do still get the messages:

[   66.895807] X:5022 map pfn expected mapping type write-back for d0000000-d0101000, got write-combining
[   66.895873] X:5022 freeing invalid memtype d0000000-d0101000

As for 3D performance issues, glxgears without any other applications running gives:

    457 frames in 5.0 seconds = 91.312 FPS
    475 frames in 5.0 seconds = 94.887 FPS
    478 frames in 5.0 seconds = 95.411 FPS

I can't say for sure, but I expect those numbers should be much higher for a 3D accelerated radeon card (even if it is a little old):

    01:00.0 VGA compatible controller: ATI Technologies Inc M9+ 5C61 [Radeon Mobility 9200 (AGP)] (rev 01)

glxinfo gives:

    direct rendering: Yes


Anything else I can try?

-- 
Kevin Winchester

Re: [patch 0/6] PAT fixes/cleanup following the pfnmap changes

[Kevin Winchester]

Ingo Molnar wrote:
> * Kevin Winchester <kjwinchester@gmail.com> wrote:
> 
>> venkatesh.pallipadi@intel.com wrote:
>>> Individual patch header has descriptions.
>>>
>> $ git am Patches
>> Applying: x86 PAT: change track_pfn_vma_new to take pgprot_t pointer param
>> error: patch failed: mm/memory.c:1604
>> error: mm/memory.c: patch does not apply
>> Patch failed at 0001.
>> When you have resolved this problem run "git am --resolved".
>> If you would prefer to skip this patch, instead run "git am --skip".
>> To restore the original branch and stop patching run "git am --abort".
>>
>> These do not apply for me to latest Linus git, which is the tree where I 
>> experienced the problem.  Is there some way to fuzz them to get them to 
>> apply?
> 
> Could you check tip/master:
> 
>   http://people.redhat.com/mingo/tip.git/README
> 
> do you still get the messages? [ Btw., did you also experience any 3D 
> performance problems that are solved via Venki's fixes in tip/master? ]
> 

(I had noted in another email that tip/master still had the problem.)

However, with latest Linus' git tree (2.6.29-rc1-00224-ga652504),
I no longer see the error messages I had noted before.  Instead I now see:

[  475.132031] agpgart-amd64 0000:00:00.0: AGP 3.0 bridge
[  475.132056] agpgart-amd64 0000:00:00.0: putting AGP V3 device into 8x mode
[  475.132123] pci 0000:01:00.0: putting AGP V3 device into 8x mode
[  475.502856] [drm:radeon_do_init_cp] *ERROR* could not find ioremap agp regions!

Is this related?  I added Dave Airlie to the CC since perhaps he would be the one
to ask about it?

Thanks,

-- 
Kevin Winchester