x86 PAT: track pfnmap mappings with remap_pfn_range vm_insert_pfn - v3

Drivers use mmap followed by pgprot_* and remap_pfn_range or vm_insert_pfn, in order to export reserved memory to userspace. Currently, such mappings are not tracked and hence not kept consistent with other mappings (/dev/mem, pci resource, ioremap) for the sme memory, that may exist in the system.

The following patchset adds x86 PAT attribute tracking and untracking for pfnmap related APIs.

First three patches in the patchset are changing the generic mm code to fit in this tracking. Last four patches are x86 specific to make things work with x86 PAT code. The patchset aso introduces pgprot_writecombine interface, which gives writecombine mapping when enabled, falling back to pgprot_noncached otherwise.

This patch:

While working on x86 PAT, we faced some hurdles with trackking remap_pfn_range() regions, as we do not have any information to say whether that PFNMAP mapping is linear for the entire vma range or it is smaller granularity regions within the vma.

A simple solution to this is to use vm_pgoff as an indicator for linear mapping over the vma region. Currently, remap_pfn_range only sets vm_pgoff for COW mappings. Below patch changes the logic and sets the vm_pgoff irrespective of COW. This will still not be enough for the case where pfn is zero (vma region mapped to physical address zero). But, for all the other cases, we can look at pfnmap VMAs and say whether the mappng is for the entire vma region or not.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 include/linux/mm.h |    9 +++++++++
 mm/memory.c        |    7 +++----
 2 files changed, 12 insertions(+), 4 deletions(-)

Index: linux-2.6/mm/memory.c
===================================================================

mm/memory.c +3 -4

@@ -1575,11 +1575,10 @@ int remap_pfn_range(struct vm_area_struc
 	 * behaviour that some programs depend on. We mark the "original"
 	 * un-COW'ed pages by matching them up with "vma->vm_pgoff".
 	 */
-	if (is_cow_mapping(vma->vm_flags)) {
-		if (addr != vma->vm_start || end != vma->vm_end)
-			return -EINVAL;
+	if (addr == vma->vm_start && end == vma->vm_end)
 		vma->vm_pgoff = pfn;
-	}
+	else if (is_cow_mapping(vma->vm_flags))
+		return -EINVAL;
 
 	vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP;
 

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +2 -0

@@ -145,6 +145,15 @@ extern pgprot_t protection_map[16];
 #define FAULT_FLAG_WRITE	0x01	/* Fault was a write access */
 #define FAULT_FLAG_NONLINEAR	0x02	/* Fault was via a nonlinear mapping */
 
+static inline int is_linear_pfn_mapping(struct vm_area_struct *vma)
+{

Quoted 75 lines · On Thu, Dec 18, 2008 at 11:41:27AM -0800, [email protected] wrote:

Drivers use mmap followed by pgprot_* and remap_pfn_range or vm_insert_pfn, in order to export reserved memory to userspace. Currently, such mappings are not tracked and hence not kept consistent with other mappings (/dev/mem, pci resource, ioremap) for the sme memory, that may exist in the system.

The following patchset adds x86 PAT attribute tracking and untracking for pfnmap related APIs.

First three patches in the patchset are changing the generic mm code to fit in this tracking. Last four patches are x86 specific to make things work with x86 PAT code. The patchset aso introduces pgprot_writecombine interface, which gives writecombine mapping when enabled, falling back to pgprot_noncached otherwise.

This patch:

While working on x86 PAT, we faced some hurdles with trackking remap_pfn_range() regions, as we do not have any information to say whether that PFNMAP mapping is linear for the entire vma range or it is smaller granularity regions within the vma.

A simple solution to this is to use vm_pgoff as an indicator for linear mapping over the vma region. Currently, remap_pfn_range only sets vm_pgoff for COW mappings. Below patch changes the logic and sets the vm_pgoff irrespective of COW. This will still not be enough for the case where pfn is zero (vma region mapped to physical address zero). But, for all the other cases, we can look at pfnmap VMAs and say whether the mappng is for the entire vma region or not.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 include/linux/mm.h |    9 +++++++++
 mm/memory.c        |    7 +++----
 2 files changed, 12 insertions(+), 4 deletions(-)

Index: linux-2.6/mm/memory.c
===================================================================

mm/memory.c +3 -4

@@ -1575,11 +1575,10 @@ int remap_pfn_range(struct vm_area_struc
 	 * behaviour that some programs depend on. We mark the "original"
 	 * un-COW'ed pages by matching them up with "vma->vm_pgoff".
 	 */
-	if (is_cow_mapping(vma->vm_flags)) {
-		if (addr != vma->vm_start || end != vma->vm_end)
-			return -EINVAL;
+	if (addr == vma->vm_start && end == vma->vm_end)
 		vma->vm_pgoff = pfn;
-	}
+	else if (is_cow_mapping(vma->vm_flags))
+		return -EINVAL;
 
 	vma->vm_flags |= VM_IO | VM_RESERVED | VM_PFNMAP;
 

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +9 -0

@@ -145,6 +145,15 @@ extern pgprot_t protection_map[16];
 #define FAULT_FLAG_WRITE	0x01	/* Fault was a write access */
 #define FAULT_FLAG_NONLINEAR	0x02	/* Fault was via a nonlinear mapping */
 
+static inline int is_linear_pfn_mapping(struct vm_area_struct *vma)
+{
+	return ((vma->vm_flags & VM_PFNMAP) && vma->vm_pgoff);
+}
+
+static inline int is_pfn_mapping(struct vm_area_struct *vma)
+{
+	return (vma->vm_flags & VM_PFNMAP);
+}
 

include/linux/mm.h extern pgprot_t protection_map[16]; · line 159

Quoted 2 lines

include/linux/mm.h +0 -0

 /*
  * vm_fault is filled by the the pagefault handler and passed to the vma's

This is fine by me, however:
1. Can you add some comments to say "this is not for core vm but for pat, oh and a pgoff of zero is not going to work".

2. Can you please justify to me (or the changelog) roughly why PAT wants to know if the mapping is linear or not? Presumably it has to handle both types? If performance wasn't an issue, then you could manually scan the ptes to verify (which would solve your zero-offset bug). etc.

Quoted 4 lines · Nick Piggin

This is fine by me, however:
1. Can you add some comments to say "this is not for core vm but for pat, oh and a pgoff of zero is not going to work".

OK. Will add comments about both the points.

Quoted 4 lines

2. Can you please justify to me (or the changelog) roughly why PAT wants to know if the mapping is linear or not? Presumably it has to handle both types? If performance wasn't an issue, then you could manually scan the ptes to verify (which would solve your zero-offset bug). etc.

The main reason is performance. If we know it is linear, we can track the entire region as one block and do the reserve free for entire region. But, if it is not linear, then we have to reserve memtype of physical addresses page by page. This will not be optimal as it will result in reserve and free becoming slower. Almost all users that we find in kernel today (atleast in x86) are all linear.

Add a generic interface to follow pfn in a pfnmap vma range. This is used by one of the subsequent x86 PAT related patch to keep track of memory types for vma regions across vma copy and free.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 include/linux/mm.h |    3 +++
 mm/memory.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +3 -0

@@ -1223,6 +1223,9 @@ struct page *follow_page(struct vm_area_
 #define FOLL_GET	0x04	/* do get_page on page */
 #define FOLL_ANON	0x08	/* give ZERO_PAGE if no pgtable */
 
+int follow_pfnmap_pte(struct vm_area_struct *vma,
+				unsigned long address, pte_t *ret_ptep);
+
 typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
 			void *data);
 extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,

Index: linux-2.6/mm/memory.c
===================================================================

mm/memory.c +43 -0

@@ -1111,6 +1111,49 @@ no_page_table:
 	return page;
 }
 
+int follow_pfnmap_pte(struct vm_area_struct *vma, unsigned long address,
+			pte_t *ret_ptep)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *ptep, pte;
+	spinlock_t *ptl;
+	struct page *page;
+	struct mm_struct *mm = vma->vm_mm;
+
+	if (!is_pfn_mapping(vma))
+		goto err;
+
+	page = NULL;
+	pgd = pgd_offset(mm, address);
+	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+		goto err;
+
+	pud = pud_offset(pgd, address);
+	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+		goto err;
+
+	pmd = pmd_offset(pud, address);
+	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+		goto err;
+
+	ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
+
+	pte = *ptep;
+	if (!pte_present(pte))
+		goto err_unlock;
+
+	*ret_ptep = pte;
+	pte_unmap_unlock(ptep, ptl);
+	return 0;
+
+err_unlock:
+	pte_unmap_unlock(ptep, ptl);
+err:
+	return -EINVAL;
+}
+
 /* Can we do the FOLL_ANON optimization? */
 static inline int use_zero_page(struct vm_area_struct *vma)
 {

Signature · 1 line

-- 

Quoted 4 lines · On Thu, Dec 18, 2008 at 11:41:28AM -0800, [email protected] wrote:

Add a generic interface to follow pfn in a pfnmap vma range. This is used by one of the subsequent x86 PAT related patch to keep track of memory types for vma regions across vma copy and free.

Quoted 2 lines

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

Can you please reuse follow_phys for this? (preferably use the same API even if it requires some modification, otherwise if not possible, then at least can you implement a common core for both APIs).

Introduce generic hooks in remap_pfn_range and vm_insert_pfn and corresponding copy and free routines with reserve and free tracking.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 include/linux/mm.h |    6 ++++
 mm/memory.c        |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 1 deletion(-)

Index: linux-2.6/mm/memory.c
===================================================================

mm/memory.c +75 -1

@@ -99,6 +99,50 @@ int randomize_va_space __read_mostly =
 					2;
 #endif
 
+#ifndef track_pfn_vma_new
+/*
+ * Interface that can be used by architecture code to keep track of
+ * memory type of pfn mappings (remap_pfn_range, vm_insert_pfn)
+ *
+ * track_pfn_vma_new is called when a _new_ pfn mapping is being established
+ * for physical range indicated by pfn and size.
+ */
+int track_pfn_vma_new(struct vm_area_struct *vma, pgprot_t prot,
+			unsigned long pfn, unsigned long size)
+{
+	return 0;
+}
+#endif
+
+#ifndef track_pfn_vma_copy
+/*
+ * Interface that can be used by architecture code to keep track of
+ * memory type of pfn mappings (remap_pfn_range, vm_insert_pfn)
+ *
+ * track_pfn_vma_copy is called when vma that is covering the pfnmap gets
+ * copied through copy_page_range().
+ */
+int track_pfn_vma_copy(struct vm_area_struct *vma)
+{
+	return 0;
+}
+#endif
+
+#ifndef untrack_pfn_vma
+/*
+ * Interface that can be used by architecture code to keep track of
+ * memory type of pfn mappings (remap_pfn_range, vm_insert_pfn)
+ *
+ * untrack_pfn_vma is called while unmapping a pfnmap for a region.
+ * untrack can be called for a specific region indicated by pfn and size or
+ * can be for the entire vma (in which case size can be zero).
+ */
+void untrack_pfn_vma(struct vm_area_struct *vma, unsigned long pfn,
+			unsigned long size)
+{
+}
+#endif
+
 static int __init disable_randmaps(char *s)
 {
 	randomize_va_space = 0;

@@ -669,6 +713,16 @@ int copy_page_range(struct mm_struct *ds
 	if (is_vm_hugetlb_page(vma))
 		return copy_hugetlb_page_range(dst_mm, src_mm, vma);
 
+	if (is_pfn_mapping(vma)) {
+		/*
+		 * We do not free on error cases below as remove_vma
+		 * gets called on error from higher level routine
+		 */
+		ret = track_pfn_vma_copy(vma);
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * We need to invalidate the secondary MMU mappings only when
 	 * there could be a permission downgrade on the ptes of the

@@ -915,6 +969,9 @@ unsigned long unmap_vmas(struct mmu_gath
 		if (vma->vm_flags & VM_ACCOUNT)
 			*nr_accounted += (end - start) >> PAGE_SHIFT;
 
+		if (is_pfn_mapping(vma))
+			untrack_pfn_vma(vma, 0, 0);
+
 		while (start != end) {
 			if (!tlb_start_valid) {
 				tlb_start = start;

@@ -1473,6 +1530,7 @@ out:
 int vm_insert_pfn(struct vm_area_struct *vma, unsigned long addr,
 			unsigned long pfn)
 {
+	int ret;
 	/*
 	 * Technically, architectures with pte_special can avoid all these
 	 * restrictions (same for remap_pfn_range).  However we would like

@@ -1487,7 +1545,15 @@ int vm_insert_pfn(struct vm_area_struct 
 
 	if (addr < vma->vm_start || addr >= vma->vm_end)
 		return -EFAULT;
-	return insert_pfn(vma, addr, pfn, vma->vm_page_prot);
+	if (track_pfn_vma_new(vma, vma->vm_page_prot, pfn, PAGE_SIZE))
+		return -EINVAL;
+
+  	ret = insert_pfn(vma, addr, pfn, vma->vm_page_prot);
+
+	if (ret)
+		untrack_pfn_vma(vma, pfn, PAGE_SIZE);
+
+	return ret;
 }
 EXPORT_SYMBOL(vm_insert_pfn);
 

@@ -1625,6 +1691,10 @@ 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)
+		return -EINVAL;
+
 	BUG_ON(addr >= end);
 	pfn -= addr >> PAGE_SHIFT;
 	pgd = pgd_offset(mm, addr);

@@ -1636,6 +1706,10 @@ int remap_pfn_range(struct vm_area_struc
 		if (err)
 			break;
 	} while (pgd++, addr = next, addr != end);
+
+	if (err)
+		untrack_pfn_vma(vma, pfn, PAGE_ALIGN(size));
+
 	return err;
 }
 EXPORT_SYMBOL(remap_pfn_range);

Index: linux-2.6/include/linux/mm.h
===================================================================

include/linux/mm.h +6 -0

@@ -155,6 +155,12 @@ static inline int is_pfn_mapping(struct 
 	return (vma->vm_flags & VM_PFNMAP);
 }
 
+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,
+				unsigned long size);
+
 /*
  * vm_fault is filled by the the pagefault handler and passed to the vma's
  * ->fault function. The vma's ->fault is responsible for returning a bitmask

Signature · 1 line

-- 

Quoted 2 lines · On Thu, Dec 18, 2008 at 11:41:29AM -0800, [email protected] wrote:

Introduce generic hooks in remap_pfn_range and vm_insert_pfn and corresponding copy and free routines with reserve and free tracking.

These should be inline so that they can be folded out (I'm sure gcc with -Os and "optimize" inlining will do something stupid here). Also, the normal way to add such arch hooks is to put the default into asm-generic and have other archs include it... that would be nicer than sticking it into mm/memory.c wouldn't it?

Sigh, fork/exit paths slow down yet again. But oh well. Maybe can you add some branch hints?

Hookup remap_pfn_range and vm_insert_pfn and corresponding copy and free routines with reserve and free tracking.

And that's the end of the mm/ code... yeah the mm/ parts seem OK to me, with my minor comments addressed I would ack them all.

Quoted 2 lines · Nick Piggin

And that's the end of the mm/ code... yeah the mm/ parts seem OK to me, with my minor comments addressed I would ack them all.

Great!

Venki, can this be done with incremental patches, or would that be too messy?

Quoted 4 lines · H. Peter Anvin

Quoted 2 lines · Nick Piggin

And that's the end of the mm/ code... yeah the mm/ parts seem OK to me, with my minor comments addressed I would ack them all.

Quoted 4 lines

Great!

Venki, can this be done with incremental patches, or would that be too messy?

would be nice to have them as deltas. The patches are pushed out into tip/master already - and we'll push it towards linux-next once Nick's concerns have been addressed.

[email protected] wrote:

(quoted diff) +3 -0

+Advanced APIs for drivers
+-------------------------
+A. Exporting pages to user with remap_pfn_range, io_remap_pfn_range,
 
                    to users

or to userspace

(quoted diff) +3 -0

+vm_insert_pfn
+
+Drivers wanting to export some pages to userspace, do it by using mmap

Drop comma.

(quoted diff) +5 -0

+interface and a combination of
+1) pgprot_noncached()
+2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn()
+
+With pat support, a new API pgprot_writecombine is being added. So, driver can

s/pat/PAT/
s/driver/drivers/ or s/driver/a driver/

(quoted diff) +6 -0

+continue to use the above sequence, with either pgprot_noncached() or
+pgprot_writecombine() in step 1, followed by step 2.
+
+In addition, step 2 internally tracks the region as UC or WC in memtype
+list in order to ensure no conflicting mapping.
+

Quoted 36 lines · Randy Dunlap

[email protected] wrote:

Documentation/x86/pat.txt +3 -0

+Advanced APIs for drivers
+-------------------------
+A. Exporting pages to user with remap_pfn_range, io_remap_pfn_range,
 
                    to users

or to userspace

Documentation/x86/pat.txt +3 -0

+vm_insert_pfn
+
+Drivers wanting to export some pages to userspace, do it by using mmap

Drop comma.

Documentation/x86/pat.txt +5 -0

+interface and a combination of
+1) pgprot_noncached()
+2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn()
+
+With pat support, a new API pgprot_writecombine is being added. So, driver can

s/pat/PAT/
s/driver/drivers/ or s/driver/a driver/

Documentation/x86/pat.txt +8 -0

+continue to use the above sequence, with either pgprot_noncached() or
+pgprot_writecombine() in step 1, followed by step 2.
+
+In addition, step 2 internally tracks the region as UC or WC in memtype
+list in order to ensure no conflicting mapping.
+
+Note that this set of APIs only work with IO (non RAM) regions. If driver
 
                                 works with IO (non-RAM) regions. If a driver
 
+wants to export RAM region, it has to do set_memory_uc() or set_memory_wc()

Documentation/x86/pat.txt pci proc | -- | - · line 103

Quoted 6 lines

export a RAM region,

Documentation/x86/pat.txt +2 -0

+as step 0 above and also track the usage of those pages and use set_memory_wb()
+before the page is freed to free pool.

refreshed patch below

Thanks,
Venki

Add documentation related to pgprot_* change.

Signed-off-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Suresh Siddha <[email protected]>

---
 Documentation/x86/pat.txt |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Index: linux-2.6/Documentation/x86/pat.txt
===================================================================

Documentation/x86/pat.txt +1 -0

@@ -80,6 +80,30 @@ pci proc               |    --    |    -
                        |          |            |                  |
 -------------------------------------------------------------------
 
+Advanced APIs for drivers

Documentation/x86/pat.txt +3 -0

+wants to export a RAM region, it has to do set_memory_uc() or set_memory_wc()
+as step 0 above and also track the usage of those pages and use set_memory_wb()
+before the page is freed to free pool.

-----Original Message-----
From: Randy Dunlap [mailto:[email protected]]
Sent: Thursday, December 18, 2008 1:53 PM
To: Pallipadi, Venkatesh
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Siddha, Suresh B
Subject: Re: [patch 7/7] x86 PAT: update documentation to cover pgprot and remap_pfn related changes - v3

Quoted 6 lines · Venkatesh

Documentation/x86/pat.txt pci proc | -- | -

Quoted 5 lines · Randy Dunlap

[email protected] wrote:

Documentation/x86/pat.txt +3 -0

+Advanced APIs for drivers
+-------------------------
+A. Exporting pages to user with remap_pfn_range, 

io_remap_pfn_range,

Quoted 7 lines

Documentation/x86/pat.txt pci proc | -- | -

Quoted 7 lines

to users
or to userspace

Documentation/x86/pat.txt +3 -0

+vm_insert_pfn
+
+Drivers wanting to export some pages to userspace, do it 

by using mmap

Quoted 8 lines

Documentation/x86/pat.txt pci proc | -- | -

Quoted 8 lines

Drop comma.

Documentation/x86/pat.txt +5 -0

+interface and a combination of
+1) pgprot_noncached()
+2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn()
+
+With pat support, a new API pgprot_writecombine is being 

added. So, driver can

Quoted 5 lines

Quoted 5 lines

s/pat/PAT/
s/driver/drivers/ or s/driver/a driver/

+continue to use the above sequence, with either

pgprot_noncached() or

Quoted 3 lines

Documentation/x86/pat.txt pci proc | -- | -

Quoted 3 lines

Documentation/x86/pat.txt +3 -0

+pgprot_writecombine() in step 1, followed by step 2.
+
+In addition, step 2 internally tracks the region as UC or 

WC in memtype

Quoted 3 lines

Documentation/x86/pat.txt pci proc | -- | -

Quoted 3 lines

Documentation/x86/pat.txt +3 -0

+list in order to ensure no conflicting mapping.
+
+Note that this set of APIs only work with IO (non RAM) 

regions. If driver

Quoted 2 lines

Quoted 2 lines

works with IO (non-RAM)

regions. If a driver

Quoted 2 lines

Quoted 2 lines

+wants to export RAM region, it has to do set_memory_uc()

or set_memory_wc()

Quoted 4 lines

Quoted 4 lines

export a RAM region,

+as step 0 above and also track the usage of those pages

and use set_memory_wb()

Quoted 5 lines

Quoted 3 lines

+before the page is freed to free pool.

refreshed patch below

Some of them were changed, others not changed, with no explanation... More changes below:

v3: Patches updated based on Andrew's comments on the earlier version.

Drivers use mmap followed by pgprot_* and remap_pfn_range or vm_insert_pfn, in order to export reserved memory to userspace. Currently, such mappings are not tracked and hence not kept consistent with other mappings (/dev/mem, pci resource, ioremap) for the sme memory, that may exist in the system.

The following patchset adds x86 PAT attribute tracking and untracking for pfnmap related APIs.

First three patches in the patchset are changing the generic mm code to fit in this tracking. Last four patches are x86 specific to make things work with x86 PAT code. The patchset aso introduces pgprot_writecombine interface, which gives writecombine mapping when enabled, falling back to pgprot_noncached otherwise.