x86 PAT: track pfnmap mappings with remap_pfn_range and vm_insert_pfn
Subsystems (6)
cover x86 PAT: track pfnmap mappings with remap_pfn_range and vm_insert_pfn
(Sorry about the duplicates. Resending as the original mails did not make it to
the list due to some errors in my email header)
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 four 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.
PATCH 1/8 x86 PAT: store vm_pgoff for all linear_over_vma_region mappings
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 four 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.
--- include/linux/mm.h | 9 +++++++++ mm/memory.c | 7 +++---- 2 files changed, 12 insertions(+), 4 deletions(-)
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 ;
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 ); +}/* * vm_fault is filled by the the pagefault handler and passed to the vma's
PATCH 2/8 x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
In reply to Venkatesh Pallipadivm_insert_pfn() is not setting the VM_PFNMAP flag in vma. Fix that.
--- mm/memory.c | 2 ++ 1 file changed, 2 insertions(+)
mm/memory.c +2 -0
@@ -1444,6 +1444,8 @@ int vm_insert_pfn(struct vm_area_structif (addr < vma->vm_start || addr >= vma->vm_end)return -EFAULT ; + + vma->vm_flags |=VM_PFNMAP ;return insert_pfn (vma, addr, pfn, vma->vm_page_prot); }EXPORT_SYMBOL (vm_insert_pfn);
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
You have to be careful of this, because it can be called with mmap_sem held for read only. Hmm, I guess vm_insert_page is doing the same thing. Probably mostly works because all other modifiers of vm_flags are holding mmap_sem.
However, in some cases, code can do vm_insert_pfn and vm_insert_page (actually hmm, no vm_insert_mixed actually should cover most of those cases).
Still, I'd be much happier if we could make these into BUG_ON, and then teach callers to set it in their .mmap routines.
Quoted 24 lines · Venkatesh Pallipadi
vm_insert_pfn()is not setting theVM_PFNMAPflag in vma. Fix that.Signed-off-by Venkatesh Pallipadi <[email protected]>Signed-off-by Suresh Siddha <[email protected]>--- mm/memory.c | 2 ++ 1 file changed, 2 insertions(+)Index:
tip/mm/memory.c
===================================================================mm/memory.c +2
-0@@ -1444,6 +1444,8 @@ int vm_insert_pfn(struct vm_area_structif (addr < vma->vm_start || addr >= vma->vm_end)return -EFAULT ; + + vma->vm_flags |=VM_PFNMAP ;return insert_pfn (vma, addr, pfn, vma->vm_page_prot); }EXPORT_SYMBOL (vm_insert_pfn);Signature · 1 line
--
RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 11 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 3:23 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfn
Quoted 6 lines
You have to be careful of this, because it can be called with
mmap_semheld for read only. Hmm, I guessvm_insert_pageis doing the same thing.
Probably mostly works because all other modifiers ofvm_flagsare holdingmmap_sem.
Yes. I did the patch looking at vm_insert_page doing similar thing.
Quoted 7 lines
However, in some cases, code can do
vm_insert_pfnandvm_insert_page(actually hmm, novm_insert_mixedactually should cover most of those cases).Still, I'd be much happier if we could make these into
BUG_ON, and then teach callers to set it in their .mmap routines.
Actually, vm_insert_pfn() already has a BUG_ON() at the start for cases where neither (or both) MIXEDMAP and PFNMAP is not set. So, that should cover the case we are worried about it here and we can eliminate this patch altogether. Only part I am not sure about is why we are looking for MIXEDMAP here. Shouldn't they be using vm_insert_mixed instead?
Thanks,
Venki
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 25 lines · Venkatesh
Quoted 11 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 3:23 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 6 lines
You have to be careful of this, because it can be called with
mmap_semheld for read only. Hmm, I guessvm_insert_pageis doing the same thing.
Probably mostly works because all other modifiers ofvm_flagsare holdingmmap_sem.Yes. I did the patch looking at
vm_insert_pagedoing similar thing.Quoted 5 lines
However, in some cases, code can do
vm_insert_pfnandvm_insert_page(actually hmm, novm_insert_mixedactually should cover most of those cases).
Quoted 8 lines
Quoted 2 lines
Still, I'd be much happier if we could make these into
BUG_ON, and then teach callers to set it in their .mmap routines.Actually,
vm_insert_pfn()already has aBUG_ON()at the start for cases where neither (or both) MIXEDMAP and PFNMAP is not set. So, that should cover the case we are worried about it here and we can eliminate this patch altogether. Only part I am not sure about is why we are looking for MIXEDMAP here. Shouldn't they be usingvm_insert_mixedinstead?
They should, but it will do an inesrt_pfn in some cases, won't it?
RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 45 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 7:44 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 10 lines · Venkatesh
Quoted 10 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 3:23 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag in
vm_insert_pfnQuoted 2 lines
Quoted 2 lines
You have to be careful of this, because it can be called
with
mmap_semQuoted 11 lines
Quoted 5 lines
held for read only. Hmm, I guess
vm_insert_pageis doing the same thing.
Probably mostly works because all other modifiers ofvm_flagsare holdingmmap_sem.Yes. I did the patch looking at
vm_insert_pagedoing similar thing.Quoted 3 lines
However, in some cases, code can do
vm_insert_pfnandvm_insert_page(actually hmm, novm_insert_mixedactually should covermost of those
Quoted 3 lines
Quoted 3 lines
cases).
Still, I'd be much happier if we could make these into
BUG_ON, and thenQuoted 3 lines
Quoted 1 line
teach callers to set it in their .mmap routines.
Actually,
vm_insert_pfn()already has aBUG_ON()at the
Quoted 9 lines
start for cases
Quoted 1 line
where neither (or both) MIXEDMAP and PFNMAP is not set. So,
that should
Quoted 3 lines
cover the case we are worried about it here and we can eliminate this patch altogether. Only part I am not sure about is why we are looking for MIXEDMAP here. Shouldn't they be using
vm_insert_mixedinstead?They should, but it will do an
inesrt_pfnin some cases, won't it?
Yes. It does. But, it calls a lower level insert_pfn() function. The lower level insert_pfn() does not have any bug checks. But the higher level vm_insert_pfn() checks for PFNMAP or MIXEDMAP.
Thanks,
Venki
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 38 lines · Venkatesh
Quoted 38 lines
Quoted 8 lines
Quoted 8 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 3:23 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag in
vm_insert_pfnQuoted 2 lines
Quoted 2 lines
You have to be careful of this, because it can be called
with
mmap_semQuoted 11 lines
Quoted 5 lines
held for read only. Hmm, I guess
vm_insert_pageis doing the same thing.
Probably mostly works because all other modifiers ofvm_flagsare holdingmmap_sem.Yes. I did the patch looking at
vm_insert_pagedoing similar thing.Quoted 3 lines
However, in some cases, code can do
vm_insert_pfnandvm_insert_page(actually hmm, novm_insert_mixedactually should covermost of those
Quoted 3 lines
Quoted 3 lines
cases).
Still, I'd be much happier if we could make these into
BUG_ON, and thenQuoted 3 lines
Quoted 1 line
teach callers to set it in their .mmap routines.
Actually,
vm_insert_pfn()already has aBUG_ON()at thestart for cases
Quoted 1 line
where neither (or both) MIXEDMAP and PFNMAP is not set. So,
that should
Quoted 3 lines
cover the case we are worried about it here and we can eliminate this patch altogether. Only part I am not sure about is why we are looking for MIXEDMAP here. Shouldn't they be using
vm_insert_mixedinstead?
Quoted 6 lines
Quoted 2 lines
They should, but it will do an
inesrt_pfnin some cases, won't it?Yes. It does. But, it calls a lower level
insert_pfn()function. The lower levelinsert_pfn()does not have any bug checks. But the higher levelvm_insert_pfn()checks for PFNMAP or MIXEDMAP.
Yes, but is there anything extra you need to check for cache aliases in MIXEDMAP mappings?
RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 13 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Thursday, November 13, 2008 6:06 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 1 line · Venkatesh
Quoted 8 lines
Quoted 1 line
Yes. It does. But, it calls a lower level
insert_pfn()function. The lower
Quoted 2 lines
level
insert_pfn()does not have any bug checks. But the higher levelvm_insert_pfn()checks for PFNMAP or MIXEDMAP.Yes, but is there anything extra you need to check for cache aliases in MIXEDMAP mappings?
Yes. We need additional things to track MIXEDMAP and we are looking at that. But, that is slightly more trickier than the general PFNMAP case. And only in-tree user of MIXEDMAP is xip and that too it only uses it for regular WB mapping. So, we thought we should fix the more common case first here.
With MIXEDMAP there is no way whether to distinguish whether insert_pfn Or insert_page was used while looking at VMA. We can probably use PFNMAP in addition to MIXEDMAP to indicate that, which will make things easier. But, we are still looking at that and trying to understand the change implication.
Thanks,
Venki
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 28 lines · Venkatesh
Quoted 13 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Thursday, November 13, 2008 6:06 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 1 line · Venkatesh
Quoted 8 lines
Quoted 1 line
Yes. It does. But, it calls a lower level
insert_pfn()function. The lower
Quoted 2 lines
level
insert_pfn()does not have any bug checks. But the higher levelvm_insert_pfn()checks for PFNMAP or MIXEDMAP.Yes, but is there anything extra you need to check for cache aliases in MIXEDMAP mappings?
Yes. We need additional things to track MIXEDMAP and we are looking at that. But, that is slightly more trickier than the general PFNMAP case. And only in-tree user of MIXEDMAP is xip and that too it only uses it for regular WB mapping. So, we thought we should fix the more common case first here.
Quoted 3 lines
With MIXEDMAP there is no way whether to distinguish whether
insert_pfnOrinsert_pagewas used while looking at VMA. We can probably use PFNMAP in addition to MIXEDMAP to indicate that, which will make things easier.
It's difficult because it can have either method for a single VMA, and a given address in the vma may even change over time (not with current code in kernel AFAIKS, but AXFS eventually might get to that point).
Quoted 2 lines
But, we are still looking at that and trying to understand the change implication.
OK: now I understand correctly. Getting PFNMAP working is an important first step. I agree.
Thanks,
Nick
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 38 lines · Nick Piggin
Quoted 28 lines · Venkatesh
Quoted 13 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Thursday, November 13, 2008 6:06 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 1 line · Venkatesh
Quoted 8 lines
Quoted 1 line
Yes. It does. But, it calls a lower level
insert_pfn()function. The lower
Quoted 2 lines
level
insert_pfn()does not have any bug checks. But the higher levelvm_insert_pfn()checks for PFNMAP or MIXEDMAP.Yes, but is there anything extra you need to check for cache aliases in MIXEDMAP mappings?
Yes. We need additional things to track MIXEDMAP and we are looking at that. But, that is slightly more trickier than the general PFNMAP case. And only in-tree user of MIXEDMAP is xip and that too it only uses it for regular WB mapping. So, we thought we should fix the more common case first here.
Quoted 3 lines
With MIXEDMAP there is no way whether to distinguish whether
insert_pfnOrinsert_pagewas used while looking at VMA. We can probably use PFNMAP in addition to MIXEDMAP to indicate that, which will make things easier.It's difficult because it can have either method for a single VMA, and a given address in the vma may even change over time (not with current code in kernel AFAIKS, but AXFS eventually might get to that point).
Quoted 5 lines
Quoted 2 lines
But, we are still looking at that and trying to understand the change implication.
OK: now I understand correctly. Getting PFNMAP working is an important first step. I agree.
Venki, a patch logistics sidenote: the final mm/* bits of this patchset need acks from MM folks - Andrew, Nick or Hugh - we cannot just queue them up in the x86/pat tree without agreement from MM maintainers.
Ingo
RE: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 70 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Ingo Molnar
Sent: Tuesday, November 18, 2008 1:38 PM
To: Nick Piggin
Cc: Pallipadi, Venkatesh; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 11 lines · Nick Piggin
Quoted 10 lines · Venkatesh
Quoted 10 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Thursday, November 13, 2008 6:06 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag in
vm_insert_pfnQuoted 2 lines
Quoted 2 lines
Quoted 2 lines
On Thu, Nov 13, 2008 at 10:47:23AM -0800, Pallipadi,
Quoted 4 lines · Venkatesh
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
Yes. It does. But, it calls a lower level
insert_pfn()function. The lower
Quoted 1 line
level
insert_pfn()does not have any bug checks. Butthe higher level
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
vm_insert_pfn()checks for PFNMAP or MIXEDMAP.Yes, but is there anything extra you need to check for
cache aliases in
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
MIXEDMAP mappings?
Yes. We need additional things to track MIXEDMAP and we
are looking at that.
Quoted 1 line
Quoted 1 line
But, that is slightly more trickier than the general
PFNMAP case. And
Quoted 1 line
Quoted 1 line
only in-tree user of MIXEDMAP is xip and that too it only
uses it for
Quoted 1 line
Quoted 1 line
regular WB mapping. So, we thought we should fix the more
common case
Quoted 3 lines
Quoted 3 lines
first here.
With MIXEDMAP there is no way whether to distinguish
whether
insert_pfnQuoted 1 line
Quoted 1 line
Or
insert_pagewas used while looking at VMA. We canprobably use PFNMAP
Quoted 1 line
Quoted 1 line
in addition to MIXEDMAP to indicate that, which will make
things easier.
Quoted 2 lines
It's difficult because it can have either method for a
single VMA, and
Quoted 1 line
a given address in the vma may even change over time (not
with current
Quoted 4 lines
code in kernel AFAIKS, but AXFS eventually might get to that point).
Quoted 1 line
But, we are still looking at that and trying to understand
the change
Quoted 2 lines
Quoted 1 line
implication.
Quoted 9 lines
Quoted 1 line
OK: now I understand correctly. Getting PFNMAP working is an
important
Quoted 1 line
first step. I agree.
Venki, a patch logistics sidenote: the final
mm/*bits of this patchset need acks from MM folks - Andrew, Nick or Hugh - we cannot just queue them up in the x86/pat tree without agreement from MM maintainers.
OK. Feedback based on the discussions on this patchset, this particular patch set VM_PFNMAP flag in vm_insert_pfn
is not really needed and we should rather BUG_ON on that not being set. And there were few more comments on one other generic vm patch from Nick.
I will resend the entire patchset with changes soon and then we can work out the logistics.
Thanks,
Venki
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
Quoted 18 lines · Venkatesh
Quoted 13 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Ingo Molnar
Sent: Tuesday, November 18, 2008 1:38 PM
To: Nick Piggin
Cc: Pallipadi, Venkatesh; Thomas Gleixner; H Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 2/8] x86 PAT: setVM_PFNMAPflag invm_insert_pfnQuoted 4 lines
patchset need acks from MM folks - Andrew, Nick or Hugh - we cannot just queue them up in the x86/pat tree without agreement from MM maintainers.
Quoted 7 lines
OK. Feedback based on the discussions on this patchset, this particular patch set
VM_PFNMAPflag invm_insert_pfn
is not really needed and we should ratherBUG_ONon that not being set. And there were few more comments on one other generic vm patch from Nick.I will resend the entire patchset with changes soon and then we can work out the logistics.
So long as it has been reviewed by mm people (cc'ing linux-mm might be nice), I don't care so much how it makes its way upstream. If it was a lot of code, it might make sense to slowly work dependencies though various subsystems... but this isn't too much on the mm side, and it is primarily support code for x86 subsystem feature so I don't mind myself if it went via the x86 tree, with appropriate acks.
Re: [patch 2/8] x86 PAT: set VM_PFNMAP flag in vm_insert_pfn
In reply to Nick PigginQuoted 11 lines · Nick Piggin
You have to be careful of this, because it can be called with
mmap_semheld for read only. Hmm, I guessvm_insert_pageis doing the same thing. Probably mostly works because all other modifiers ofvm_flagsare holdingmmap_sem.However, in some cases, code can do
vm_insert_pfnandvm_insert_page(actually hmm, novm_insert_mixedactually should cover most of those cases).Still, I'd be much happier if we could make these into
BUG_ON, and then teach callers to set it in their .mmap routines.
Agreed. I don't think vm_insert_* is the right place to muck with that ..
Ben.
Quoted 31 lines · Venkatesh Pallipadi
Quoted 24 lines
vm_insert_pfn()is not setting theVM_PFNMAPflag in vma. Fix that.Signed-off-by Venkatesh Pallipadi <[email protected]>Signed-off-by Suresh Siddha <[email protected]>--- mm/memory.c | 2 ++ 1 file changed, 2 insertions(+)Index:
tip/mm/memory.c
===================================================================mm/memory.c +2
-0@@ -1444,6 +1444,8 @@ int vm_insert_pfn(struct vm_area_structif (addr < vma->vm_start || addr >= vma->vm_end)return -EFAULT ; + + vma->vm_flags |=VM_PFNMAP ;return insert_pfn (vma, addr, pfn, vma->vm_page_prot); }EXPORT_SYMBOL (vm_insert_pfn);Signature · 1 line
----
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
PATCH 3/8 x86 PAT: Add follow_pfnmp_pte routine to help tracking pfnmap pages
In reply to Venkatesh PallipadiAdd 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.
--- include/linux/mm.h | 3 +++ mm/memory.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)
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 */ +unsigned long 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,
mm/memory.c +43 -0
49 lines not on this page · raw message
Re: [patch 3/8] x86 PAT: Add follow_pfnmp_pte routine to help tracking pfnmap pages
Quoted 32 lines · Venkatesh Pallipadi
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:
tip/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 */ +unsigned long 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:
tip/mm/memory.c
===================================================================
mm/memory.c no_page_table: · line 1116
Quoted 6 lines
mm/memory.c +3
-0@@ -1111,6 +1111,49 @@ no_page_table:return page; } +unsigned long follow_pfnmap_pte (struct vm_area_struct *vma, +unsigned long address,pte_t *ret_ptep) +{
I think we'd typically return an int for functions like this. It probably wouldn't hurt to return more descriptive errors either. -EINVAL, perhaps, seems to fit best with what existing code there is doing.
Quoted 45 lines
mm/memory.c +40
-0+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 -1 ; +} +/* Can we do the FOLL_ANON optimization? */ static inline int use_zero_page (struct vm_area_struct *vma) {Signature · 1 line
--
RE: [patch 3/8] x86 PAT: Add follow_pfnmp_pte routine to help tracking pfnmap pages
Quoted 54 lines
-----Original Message-----
From: Nick Piggin [mailto:[email protected]]
Sent: Wednesday, November 12, 2008 3:27 PM
To: Pallipadi, Venkatesh
Cc: Ingo Molnar; Thomas Gleixner; H.Peter Anvin; Hugh Dickins; Roland Dreier; Jesse Barnes; Jeremy Fitzhardinge; Arjan van de Ven; [email protected]; Siddha, Suresh B
Subject: Re: [patch 3/8] x86 PAT: Addfollow_pfnmp_pteroutine to help tracking pfnmap pagesQuoted 2 lines · Venkatesh Pallipadi
Add a generic interface to follow pfn in a pfnmap vma range.
This is used by
Quoted 1 line
one of the subsequent x86 PAT related patch to keep track of
memory types
Quoted 8 lines
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+++++++++++++++++++++++++++++++++++++++++++
Quoted 5 lines
2 files changed, 46 insertions(+)Index:
tip/include/linux/mm.h
===================================================================
---tip.orig/include/linux/mm.h2008-11-0615:41:43.000000000 -0800
include/linux/mm.h struct page *follow_page(struct vm_area_ · line 1229
Quoted 9 lines
+++
tip/include/linux/mm.h2008-11-10 09:44:45.000000000 -0800include/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 */ +unsigned long 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,
Review struct page *follow_page(struct vm_area_ · line 1231
Quoted 2 lines
(quoted diff) +0
-0void *data); extern int apply_to_page_range(struct mm_struct *mm,unsigned long address,
Quoted 8 lines
Index:
tip/mm/memory.c
===================================================================mm/memory.c +0
-0@@ -1111,6 +1111,49 @@ no_page_table:return page; }
Quoted 9 lines
mm/memory.c no_page_table: · line 1116
Quoted 3 lines
mm/memory.c +3
-0+unsigned long follow_pfnmap_pte (struct vm_area_struct *vma, +unsigned long address,pte_t *ret_ptep) +{I think we'd typically return an int for functions like this. It probably wouldn't hurt to return more descriptive errors either.
-EINVAL, perhaps, seems to fit best with what existing code there is doing.
Yes. Will change the return type to int with more appropriate return values and update the patch.
Thanks,
Venki
PATCH 4/8 x86 PAT: hooks in generic vm code to help archs to track pfnmap regions
In reply to Venkatesh PallipadiIntroduce generic hooks in remap_pfn_range and vm_insert_pfn and corresponding copy and free routines with reserve and free tracking.
--- include/linux/mm.h | 6 +++++ mm/memory.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-)
mm/memory.c +53 -1
@@ -99,6 +99,28 @@ int randomize_va_space __read_mostly =2 ;#endif +#ifndef track_pfn_vma_new +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 +int track_pfn_vma_copy (struct vm_area_struct *vma) +{ +return 0 ; +} +#endif + +#ifndef untrack_pfn_vma +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 +691,16 @@ int copy_page_range(struct mm_struct *dsif (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 +947,9 @@ unsigned long unmap_vmas(struct mmu_gathif (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 +1508,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
@@ -1489,7 +1525,15 @@ int vm_insert_pfn(struct vm_area_structreturn -EFAULT ; vma->vm_flags |=VM_PFNMAP ; -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);
@@ -1627,6 +1671,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);
@@ -1638,6 +1686,10 @@ int remap_pfn_range(struct vm_area_strucif (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);
include/linux/mm.h +6 -0
@@ -155,6 +155,12 @@ static inline int is_pfn_mapping(structreturn (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
Andrew Morton
maintainerRe: [patch 4/8] x86 PAT: hooks in generic vm code to help archs to track pfnmap regions
Quoted 17 lines · On Wed, 12 Nov 2008 13:26:51 -0800 Venkatesh Pallipadi…
mm/memory.c +11
-0@@ -99,6 +99,28 @@ int randomize_va_space __read_mostly =2 ;#endif +#ifndef track_pfn_vma_new +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 +int track_pfn_vma_copy (struct vm_area_struct *vma) +{
mm/memory.c int randomize_va_space __read_mostly = · line 122
Quoted 10 lines
mm/memory.c +10
-0+ return 0; +} +#endif + +#ifndef untrack_pfn_vma +void untrack_pfn_vma (struct vm_area_struct *vma,unsigned long pfn, +unsigned long size) +{ +} +#endif
Using __weak would provide a somewhat neater result here.
RE: [patch 4/8] x86 PAT: hooks in generic vm code to help archs to track pfnmap regions
Quoted 38 lines
-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Tuesday, December 16, 2008 11:57 AM
To: Pallipadi, Venkatesh
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Pallipadi, Venkatesh; Siddha, Suresh B
Subject: Re: [patch 4/8] x86 PAT: hooks in generic vm code to help archs to track pfnmap regionsQuoted 21 lines · On Wed, 12 Nov 2008 13:26:51 -0800 Venkatesh Pallipadi…
mm/memory.c +15
-0@@ -99,6 +99,28 @@ int randomize_va_space __read_mostly =2 ;#endif +#ifndef track_pfn_vma_new +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 +int track_pfn_vma_copy (struct vm_area_struct *vma) +{ +return 0 ; +} +#endif +
Quoted 9 lines
mm/memory.c int randomize_va_space __read_mostly = · line 122
Quoted 6 lines
mm/memory.c +6
-0+#ifndef untrack_pfn_vma +void untrack_pfn_vma (struct vm_area_struct *vma,unsigned long pfn, +unsigned long size) +{ +} +#endif Using
__weakwould provide a somewhat neater result here.
Thought about that. But, then remembered the issues with gcc versions and __weak, as in here
http://lkml.org/lkml/2008/5/1/368
and decided to take the safer approach.
Thanks,
Venki
Andrew Morton
maintainerRe: [patch 4/8] x86 PAT: hooks in generic vm code to help archs to track pfnmap regions
Quoted 13 lines · Venkatesh
Quoted 13 lines
-----Original Message-----
From: Andrew Morton [mailto:[email protected]]
Sent: Tuesday, December 16, 2008 11:57 AM
To: Pallipadi, Venkatesh
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Pallipadi, Venkatesh; Siddha, Suresh B
Subject: Re: [patch 4/8] x86 PAT: hooks in generic vm code to help archs to track pfnmap regions
Your emails are getting mucked up.
Quoted 34 lines
Quoted 24 lines
Quoted 21 lines · On Wed, 12 Nov 2008 13:26:51 -0800 Venkatesh Pallipadi…
mm/memory.c +15
-0@@ -99,6 +99,28 @@ int randomize_va_space __read_mostly =2 ;#endif +#ifndef track_pfn_vma_new +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 +int track_pfn_vma_copy (struct vm_area_struct *vma) +{ +return 0 ; +} +#endif +Quoted 9 lines
mm/memory.c int randomize_va_space __read_mostly = · line 122
Quoted 6 lines
mm/memory.c +6
-0+#ifndef untrack_pfn_vma +void untrack_pfn_vma (struct vm_area_struct *vma,unsigned long pfn, +unsigned long size) +{ +} +#endif Using
__weakwould provide a somewhat neater result here.
Quoted 5 lines
Thought about that. But, then remembered the issues with gcc versions and
__weak, as in here
http://lkml.org/lkml/2008/5/1/368and decided to take the safer approach.
Yes, it is a concern.
PATCH 5/8 x86 PAT: Implement track/untrack of pfnmap regions for x86
In reply to Venkatesh PallipadiHookup remap_pfn_range and vm_insert_pfn and corresponding copy and free routines with reserve and free tracking.
reserve and free here only takes care of non RAM region mapping. For RAM region, driver should use set_memory_[uc|wc|wb] to set the cache type and then setup the mapping for user pte. We can bypass below reserve/free in that case.
--- arch/x86/include/asm/pgtable.h | 10 ++ arch/x86/mm/pat.c | 201 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+)
arch/x86/mm/pat.c +201 -0
@@ -596,6 +596,207 @@ void unmap_devmem(unsigned long pfn, unsfree_memtype (addr, addr + size); } +int reserve_pfn_range (u64 paddr,unsigned long size,pgprot_t vma_prot) +{ +unsigned long flags; +unsigned long want_flags = (pgprot_val (vma_prot) & _PAGE_CACHE_MASK); + +int is_ram =0 ; +int id_sz, ret; + + is_ram =pagerange_is_ram (paddr, paddr + size); + +if (is_ram !=0 ) { +/* +* For mapping RAM pages, drivers need to call +* set_memory_[uc|wc|wb] directly, for reserve and free, before +* setting up the PTE. +*/ +WARN_ON_ONCE (1 ); +return 0 ; + } + + ret =reserve_memtype (paddr, paddr + size, want_flags, &flags); +if (ret) +return ret; + +if (flags != want_flags) { +free_memtype (paddr, paddr + size); +printk (KERN_INFO +"%s:%d map pfn expected mapping type %s for %Lx-%Lx, got %s\n" , + current->comm, current->pid, +cattr_name (want_flags), + paddr, (unsigned long long )(paddr + size), +cattr_name (flags)); +return -EINVAL ; + } + +/* 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 ) { +free_memtype (paddr, paddr + size); +printk (KERN_INFO +"%s:%d reserve_pfn_range ioremap_change_attr failed %s " +"for %Lx-%Lx\n" , + current->comm, current->pid, +cattr_name (flags), + paddr, (unsigned long long )(paddr + size)); +return -EINVAL ; + } +return 0 ; +} + +void free_pfn_range (u64 paddr,unsigned long size) +{ +int is_ram; + + is_ram =pagerange_is_ram (paddr, paddr + size); +if (is_ram ==0 ) +free_memtype (paddr, paddr + size); +} + +int track_pfn_vma_copy (struct vm_area_struct *vma) +{ +int retval =0 ; +unsigned long i, j; +u64 paddr; +pgprot_t prot; +pte_t pte; +unsigned long vma_start = vma->vm_start; +unsigned long vma_end = vma->vm_end; +unsigned long vma_size = vma_end - vma_start; + +if (!pat_enabled) +return 0 ; + +if (is_linear_pfn_mapping (vma)) { +/* +* reserve the whole chunk starting from vm_pgoff, +* But, we have to get the protection from pte. +*/ +if (follow_pfnmap_pte (vma, vma_start, &pte)) { +WARN_ON_ONCE (1 ); +return -1 ; + } + prot =pte_pgprot (pte); + paddr = (u64 )vma->vm_pgoff <<PAGE_SHIFT ; +return reserve_pfn_range (paddr, vma_size, prot); + } + +/* reserve entire vma page by page, using pfn and prot from pte */ +for (i =0 ; i < vma_size; i +=PAGE_SIZE ) { +if (follow_pfnmap_pte (vma, vma_start + i, &pte)) +continue ; + + paddr =pte_pa (pte); + prot =pte_pgprot (pte); + retval =reserve_pfn_range (paddr,PAGE_SIZE , prot); +if (retval) + goto cleanup_ret; + } +return 0 ; + +cleanup_ret: +/* Reserve error: Cleanup partial reservation and return error */ +for (j =0 ; j < i; j +=PAGE_SIZE ) { +if (follow_pfnmap_pte (vma, vma_start + j, &pte)) +continue ; + + paddr =pte_pa (pte); +free_pfn_range (paddr,PAGE_SIZE ); + } + +return retval; +} + +int track_pfn_vma_new (struct vm_area_struct *vma,pgprot_t prot, +unsigned long pfn,unsigned long size) +{ +int retval =0 ; +unsigned long i, j; +u64 base_paddr; +u64 paddr; +unsigned long vma_start = vma->vm_start; +unsigned long vma_end = vma->vm_end; +unsigned long vma_size = vma_end - vma_start; + +if (!pat_enabled) +return 0 ; + +if (is_linear_pfn_mapping (vma)) { +/* reserve the whole chunk starting from vm_pgoff */ + paddr = (u64 )vma->vm_pgoff <<PAGE_SHIFT ; +return reserve_pfn_range (paddr, vma_size, prot); + } + +/* reserve page by page using pfn and size */ + base_paddr = (u64 )pfn <<PAGE_SHIFT ; +for (i =0 ; i < size; i +=PAGE_SIZE ) { + paddr = base_paddr + i; + retval =reserve_pfn_range (paddr,PAGE_SIZE , prot); +if (retval) + goto cleanup_ret; + } +return 0 ; + +cleanup_ret: +/* Reserve error: Cleanup partial reservation and return error */ +for (j =0 ; j < i; j +=PAGE_SIZE ) { + paddr = base_paddr + j; +free_pfn_range (paddr,PAGE_SIZE ); + } + +return retval; +} + +void untrack_pfn_vma (struct vm_area_struct *vma,unsigned long pfn, +unsigned long size) +{ +unsigned long i; +u64 paddr; +unsigned long vma_start = vma->vm_start; +unsigned long vma_end = vma->vm_end; +unsigned long vma_size = vma_end - vma_start; + +if (!pat_enabled) +return ; + +if (is_linear_pfn_mapping (vma)) { +/* free the whole chunk starting from vm_pgoff */ + paddr = (u64 )vma->vm_pgoff <<PAGE_SHIFT ; +free_pfn_range (paddr, vma_size); +return ; + } + +if (size !=0 && size != vma_size) { +/* free page by page, using pfn and size */ + + paddr = (u64 )pfn <<PAGE_SHIFT ; +for (i =0 ; i < size; i +=PAGE_SIZE ) { + paddr = paddr + i; +free_pfn_range (paddr,PAGE_SIZE ); + } + }else { +/* free entire vma, page by page, using the pfn from pte */ + +for (i =0 ; i < vma_size; i +=PAGE_SIZE ) { +pte_t pte; + +if (follow_pfnmap_pte (vma, vma_start + i, &pte)) +continue ; + + paddr =pte_pa (pte); +free_pfn_range (paddr,PAGE_SIZE ); + } + } +} +#if defined(CONFIG_DEBUG_FS ) && defined(CONFIG_X86_PAT )/* get Nth element of the linked list */
arch/x86/include/asm/pgtable.h +10 -0
@@ -219,6 +219,11 @@ static inline unsigned long pte_pfn(pte_return (pte_val (pte) &PTE_PFN_MASK ) >>PAGE_SHIFT ; } +static inline u64 pte_pa (pte_t pte) +{ +return pte_val (pte) &PTE_PFN_MASK ; +} +#define pte_page (pte) pfn_to_page(pte_pfn(pte))static inline int pmd_large (pmd_t pte)
@@ -328,6 +333,11 @@ static inline pgprot_t pgprot_modify(pgp#define canon_pgprot (p) __pgprot(pgprot_val(p) & __supported_pte_mask) +/* Indicate that x86 has its own track and untrack pfn vma functions */ +#define track_pfn_vma_new track_pfn_vma_new +#define track_pfn_vma_copy track_pfn_vma_copy +#define untrack_pfn_vma untrack_pfn_vma +#ifndef __ASSEMBLY__#define __HAVE_PHYS_MEM_ACCESS_PROTstruct file ;
Andrew Morton
maintainerRe: [patch 5/8] x86 PAT: Implement track/untrack of pfnmap regions for x86
Quoted 21 lines · On Wed, 12 Nov 2008 13:26:52 -0800 Venkatesh Pallipadi…
Hookup
remap_pfn_rangeandvm_insert_pfnand corresponding copy and free routines with reserve and free tracking.reserve and free here only takes care of non RAM region mapping. For RAM region, driver should use set_memory_[uc|wc|wb] to set the cache type and then setup the mapping for user pte. We can bypass below reserve/free in that case.
Signed-off-by Venkatesh Pallipadi <[email protected]>Signed-off-by Suresh Siddha <[email protected]>--- arch/x86/include/asm/pgtable.h | 10 ++ arch/x86/mm/pat.c | 201 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+)Index:
tip/arch/x86/mm/pat.c
===================================================================
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 599
Quoted 4 lines
arch/x86/mm/pat.c +1
-0@@ -596,6 +596,207 @@ void unmap_devmem(unsigned long pfn, unsfree_memtype (addr, addr + size); } +int reserve_pfn_range (u64 paddr,unsigned long size,pgprot_t vma_prot)
I don't think the kernel is improved by leaving all these things undocumented.
These are interfaces which other architectures might choose to implement, so a few words explaining why they exist, what they are supposed to do, etc wouldn't hurt.
It would certainly help code reviewers.
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 605
Quoted 6 lines
arch/x86/mm/pat.c +6
-0+{ +unsigned long flags; +unsigned long want_flags = (pgprot_val (vma_prot) & _PAGE_CACHE_MASK); + +int is_ram =0 ; +int id_sz, ret;
The newline in the middle of the definition of locals makes no sense.
Quoted 9 lines
arch/x86/mm/pat.c +9
-0+ is_ram =pagerange_is_ram (paddr, paddr + size); + +if (is_ram !=0 ) { +/* +* For mapping RAM pages, drivers need to call +* set_memory_[uc|wc|wb] directly, for reserve and free, before +* setting up the PTE. +*/ +WARN_ON_ONCE (1 );
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 625
Quoted 10 lines
arch/x86/mm/pat.c +10
-0+return 0 ; + } + + ret =reserve_memtype (paddr, paddr + size, want_flags, &flags); +if (ret) +return ret; + +if (flags != want_flags) { +free_memtype (paddr, paddr + size); + printk(KERN_INFO
Would KERN_ERR be more appropriate?
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 626
Quoted 2 lines
arch/x86/mm/pat.c +2
-0+"%s:%d map pfn expected mapping type %s for %Lx-%Lx, got %s\n" , + current->comm, current->pid,
get_task_comm() is more reliable, but it hardly matters here.
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 629
Quoted 2 lines
arch/x86/mm/pat.c +2
-0+cattr_name (want_flags), + paddr, (unsigned long long)(paddr + size),
Printing the u64 paddr without a cast is OK in arch code (we know that u64 is implemented as unsigned long long). But one might choose to cast it anyway, to set a good example.
Or one could not bother. But this code does it both ways!
Quoted 5 lines
arch/x86/mm/pat.c +5
-0+ cattr_name(flags)); +return -EINVAL ; + } + +/* Need to keep identity mapping in sync */
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 644
Quoted 10 lines
arch/x86/mm/pat.c +10
-0+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 ) { +free_memtype (paddr, paddr + size); + printk(KERN_INFO
KERN_ERR?
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 649
Quoted 5 lines
arch/x86/mm/pat.c +5
-0+"%s:%d reserve_pfn_range ioremap_change_attr failed %s " +"for %Lx-%Lx\n" , + current->comm, current->pid, +cattr_name (flags), + paddr, (unsigned long long)(paddr + size));
ditto
Quoted 41 lines
arch/x86/mm/pat.c +38
-0+return -EINVAL ; + } +return 0 ; +} + +void free_pfn_range (u64 paddr,unsigned long size) +{ +int is_ram; + + is_ram =pagerange_is_ram (paddr, paddr + size); +if (is_ram ==0 ) +free_memtype (paddr, paddr + size); +} +int track_pfn_vma_new (struct vm_area_struct *vma,pgprot_t prot, +unsigned long pfn,unsigned long size) +{ +int retval =0 ; +unsigned long i, j; +u64 base_paddr; +u64 paddr; +unsigned long vma_start = vma->vm_start; +unsigned long vma_end = vma->vm_end; +unsigned long vma_size = vma_end - vma_start; + +if (!pat_enabled) +return 0 ; + +if (is_linear_pfn_mapping (vma)) { +/* reserve the whole chunk starting from vm_pgoff */ + paddr = (u64 )vma->vm_pgoff <<PAGE_SHIFT ; +return reserve_pfn_range (paddr, vma_size, prot); + } + +/* reserve page by page using pfn and size */ + base_paddr = (u64 )pfn <<PAGE_SHIFT ; +for (i =0 ; i < size; i +=PAGE_SIZE ) { + paddr = base_paddr + i; + retval =reserve_pfn_range (paddr,PAGE_SIZE , prot);
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 752
Quoted 10 lines
arch/x86/mm/pat.c +10
-0+if (retval) + goto cleanup_ret; + } +return 0 ; + +cleanup_ret: +/* Reserve error: Cleanup partial reservation and return error */ +for (j =0 ; j < i; j +=PAGE_SIZE ) { + paddr = base_paddr + j; +free_pfn_range (paddr,PAGE_SIZE );
Could we simply do
free_pfn_range(base_paddr, i * PAGE_SIZE);
here?
If not, then perhaps add a helper function to do this, because that loop gets repeated in several places.
Quoted 8 lines
arch/x86/mm/pat.c +5
-0+ } + +return retval; +} +
Re: [patch 5/8] x86 PAT: Implement track/untrack of pfnmap regions for x86
Quoted 17 lines · Andrew Morton
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 599
Quoted 11 lines · Venkatesh Pallipadi
2 files changed, 211 insertions(+)Index:
tip/arch/x86/mm/pat.c
===================================================================arch/x86/mm/pat.c +1
-0@@ -596,6 +596,207 @@ void unmap_devmem(unsigned long pfn, unsfree_memtype (addr, addr + size); } +int reserve_pfn_range (u64 paddr,unsigned long size,pgprot_t vma_prot)I don't think the kernel is improved by leaving all these things undocumented.
Quoted 5 lines
These are interfaces which other architectures might choose to implement, so a few words explaining why they exist, what they are supposed to do, etc wouldn't hurt.
It would certainly help code reviewers.
Agreed. Will add documentation to the new functions here and update the patch.
Quoted 9 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 605
Quoted 7 lines
arch/x86/mm/pat.c +6
-0+{ +unsigned long flags; +unsigned long want_flags = (pgprot_val (vma_prot) & _PAGE_CACHE_MASK); + +int is_ram =0 ; +int id_sz, ret;The newline in the middle of the definition of locals makes no sense.
Will change.
Quoted 13 lines
Quoted 13 lines
arch/x86/mm/pat.c +13
-0+ is_ram =pagerange_is_ram (paddr, paddr + size); + +if (is_ram !=0 ) { +/* +* For mapping RAM pages, drivers need to call +* set_memory_[uc|wc|wb] directly, for reserve and free, before +* setting up the PTE. +*/ +WARN_ON_ONCE (1 ); +return 0 ; + } + + ret =reserve_memtype (paddr, paddr + size, want_flags, &flags);
Quoted 8 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 625
Quoted 6 lines
arch/x86/mm/pat.c +6
-0+if (ret) +return ret; + +if (flags != want_flags) { +free_memtype (paddr, paddr + size); + printk(KERN_INFOWould
KERN_ERRbe more appropriate?
OK.
Quoted 5 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 626
Quoted 3 lines
arch/x86/mm/pat.c +2
-0+"%s:%d map pfn expected mapping type %s for %Lx-%Lx, got %s\n" , + current->comm, current->pid,
get_task_comm()is more reliable, but it hardly matters here.
current->comm was used at other places in this file. I should probably change all of them as a separate cleanup patch.
Quoted 10 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 629
Quoted 3 lines
arch/x86/mm/pat.c +2
-0+cattr_name (want_flags), + paddr, (unsigned long long)(paddr + size),Printing the u64 paddr without a cast is OK in arch code (we know that u64 is implemented as unsigned long long). But one might choose to cast it anyway, to set a good example.
Or one could not bother. But this code does it both ways!
Will change.
Quoted 9 lines
Quoted 9 lines
arch/x86/mm/pat.c +9
-0+ cattr_name(flags)); +return -EINVAL ; + } + +/* Need to keep identity mapping in sync */ +if (paddr >=__pa (high_memory)) +return 0 ; + + id_sz = (__pa(high_memory) < paddr + size) ?
Quoted 8 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 644
Quoted 6 lines
arch/x86/mm/pat.c +6
-0+ __pa(high_memory) - paddr : +size ; + +if (ioremap_change_attr ((unsigned long )__va (paddr), id_sz, flags) <0 ) { +free_memtype (paddr, paddr + size); + printk(KERN_INFO
KERN_ERR?
OK.
Quoted 8 lines
arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 649
Quoted 6 lines
arch/x86/mm/pat.c +5
-0+"%s:%d reserve_pfn_range ioremap_change_attr failed %s " +"for %Lx-%Lx\n" , + current->comm, current->pid, +cattr_name (flags), + paddr, (unsigned long long)(paddr + size));ditto
Will do.
Quoted 55 lines
Quoted 42 lines
arch/x86/mm/pat.c +38
-0+return -EINVAL ; + } +return 0 ; +} + +void free_pfn_range (u64 paddr,unsigned long size) +{ +int is_ram; + + is_ram =pagerange_is_ram (paddr, paddr + size); +if (is_ram ==0 ) +free_memtype (paddr, paddr + size); +} +int track_pfn_vma_new (struct vm_area_struct *vma,pgprot_t prot, +unsigned long pfn,unsigned long size) +{ +int retval =0 ; +unsigned long i, j; +u64 base_paddr; +u64 paddr; +unsigned long vma_start = vma->vm_start; +unsigned long vma_end = vma->vm_end; +unsigned long vma_size = vma_end - vma_start; + +if (!pat_enabled) +return 0 ; + +if (is_linear_pfn_mapping (vma)) { +/* reserve the whole chunk starting from vm_pgoff */ + paddr = (u64 )vma->vm_pgoff <<PAGE_SHIFT ; +return reserve_pfn_range (paddr, vma_size, prot); + } + +/* reserve page by page using pfn and size */ + base_paddr = (u64 )pfn <<PAGE_SHIFT ; +for (i =0 ; i < size; i +=PAGE_SIZE ) { + paddr = base_paddr + i; + retval =reserve_pfn_range (paddr,PAGE_SIZE , prot);arch/x86/mm/pat.c void unmap_devmem(unsigned long pfn, uns · line 752
Quoted 10 lines
arch/x86/mm/pat.c +10
-0+if (retval) + goto cleanup_ret; + } +return 0 ; + +cleanup_ret: +/* Reserve error: Cleanup partial reservation and return error */ +for (j =0 ; j < i; j +=PAGE_SIZE ) { + paddr = base_paddr + j; +free_pfn_range (paddr,PAGE_SIZE );Could we simply do
Quoted 7 lines
free_pfn_range(base_paddr, i * PAGE_SIZE);here?
If not, then perhaps add a helper function to do this, because that loop gets repeated in several places.
free_pfn_range(base_paddr, i * PAGE_SIZE);
will not work as free should correspond to reserve granularity. I agree making this a function should make this cleaner. Will do that in the refresh of this patch.
Thanks,
Venki
PATCH 6/8 x86 PAT: change pgprot_noncached to uc_minus instead of strong uc
In reply to Venkatesh PallipadiMake pgprot_noncached uc_minus instead of strong UC. This will make pgprot_noncached to be in line with ioremap_nocache() and all the other APIs that map page uc_minus on uc request.
--- arch/x86/include/asm/pgtable.h | 8 ++++++++ arch/x86/include/asm/pgtable_32.h | 9 --------- arch/x86/include/asm/pgtable_64.h | 6 ------ 3 files changed, 8 insertions(+), 15 deletions(-)
arch/x86/include/asm/pgtable.h +8 -0
@@ -158,6 +158,14 @@#define PGD_IDENT_ATTR 0x001/* PRESENT (no other attributes) */ #endif +/* +* Macro to mark a page protection value as UC- +*/ +#define pgprot_noncached (prot) \ + ((boot_cpu_data.x86 > 3) \ + ? (__pgprot(pgprot_val(prot) | _PAGE_CACHE_UC_MINUS)) \ + : (prot)) +#ifndef __ASSEMBLY__/*
arch/x86/include/asm/pgtable_32.h +0 -9
@@ -107,15 +107,6 @@ extern unsigned long pg0[];#endif /* -* Macro to mark a page protection value as "uncacheable". -* On processors which do not support it, this is a no-op. -*/ -#define pgprot_noncached (prot) \ - ((boot_cpu_data.x86 > 3) \ - ? (__pgprot(pgprot_val(prot) | _PAGE_PCD | _PAGE_PWT)) \ - : (prot)) - -/* * Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to. */
arch/x86/include/asm/pgtable_64.h +0 -6
@@ -183,12 +183,6 @@ static inline int pmd_bad(pmd_t pmd)#define pages_to_mb (x) ((x) >> (20 - PAGE_SHIFT))/* FIXME: is this right? */ /* -* Macro to mark a page protection value as "uncacheable". -*/ -#define pgprot_noncached (prot) \ - (__pgprot(pgprot_val((prot)) | _PAGE_PCD | _PAGE_PWT)) - -/* * Conversion functions: convert a page and protection to a page entry, * and a page entry and page directory to the page they refer to. */
PATCH 7/8 x86 PAT: add pgprot_writecombine() interface for drivers
In reply to Venkatesh PallipadiAdd pgprot_writecombine. pgprot_writecombine will be aliased to pgprot_noncached when not supported by the architecture.
--- arch/x86/include/asm/pgtable.h | 3 +++ arch/x86/mm/pat.c | 8 ++++++++ include/asm-generic/pgtable.h | 4 ++++ 3 files changed, 15 insertions(+)
include/asm-generic/pgtable.h +4 -0
@@ -129,6 +129,10 @@ static inline void ptep_set_wrprotect(st#define move_pte (pte, prot, old_addr, new_addr) (pte)#endif +#ifndef pgprot_writecombine +#define pgprot_writecombine pgprot_noncached +#endif +/* * When walking page tables, get the address of the next boundary, * or the end address of the range if that comes earlier. Although no
arch/x86/include/asm/pgtable.h +3 -0
@@ -168,6 +168,9 @@#ifndef __ASSEMBLY__ +#define pgprot_writecombine pgprot_writecombine +extern pgprot_t pgprot_writecombine (pgprot_t prot); +/* * ZERO_PAGE is a global shared page that is always zero: used * for zero-mapped memory areas etc..
arch/x86/mm/pat.c +8 -0
@@ -797,6 +797,14 @@ void untrack_pfn_vma(struct vm_area_stru } } +pgprot_t pgprot_writecombine (pgprot_t prot) +{ +if (pat_enabled) +return __pgprot (pgprot_val (prot) | _PAGE_CACHE_WC); +else +return pgprot_noncached (prot); +} +#if defined(CONFIG_DEBUG_FS ) && defined(CONFIG_X86_PAT )/* get Nth element of the linked list */
PATCH 8/8 x86 PAT: update documentation to cover pgprot and remap_pfn related changes
In reply to Venkatesh PallipadiAdd documentation related to pgprot_* change.
--- Documentation/x86/pat.txt | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
Documentation/x86/pat.txt +24 -0
@@ -80,6 +80,30 @@ pci proc | -- | - | | | | ------------------------------------------------------------------- +Advanced APIs for drivers +------------------------- +A. Exporting pages to user with remap_pfn_range, io_remap_pfn_range, +vm_insert_pfn + +Drivers wanting to export some pages to userspace, do it by using mmap +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 +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 +wants to export 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. + + + Notes: -- in the above table mean "Not suggested usage for the API". Some of the --'s