| Message ID | adak5ddf8co.fsf@cisco.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | a880e7623397bcb44877b012cd65baa11ad1bbf8 |
| Headers | show |
On Sep 15, 2008, at 3:43 PM, Roland Dreier wrote: > Commit 8b150478 ("ppc: make phys_mem_access_prot() work with pfns > instead of addresses") fixed page_is_ram() in arch/ppc to avoid > overflow > for addresses above 4G on 32-bit kernels. However arch/powerpc's > page_is_ram() is missing the same fix -- it computes a physical > address > by doing pfn << PAGE_SHIFT, which overflows if pfn corresponds to a > page > above 4G. > > In particular this causes pages above 4G to be mapped with the wrong > caching attribute; for example many ppc440-based SoCs have PCI space > above 4G, and mmap()ing MMIO space may end up with a mapping that has > caching enabled. > > Fix this by working with the pfn and avoiding the conversion to > physical address that causes the overflow. This patch compares the > pfn to max_pfn, which is a semantic change from the old code -- that > code compared the physical address to high_memory, which corresponds > to max_low_pfn. However, I think that was is another bug, since > highmem pages are still RAM. > > Reported-by: vb <vb@vsbe.com> > Signed-off-by: Roland Dreier <rolandd@cisco.com> > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > Paul, didn't see this in your list... please add for 2.6.28. > > arch/powerpc/mm/mem.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c > index 1c93c25..98d7bf9 100644 > --- a/arch/powerpc/mm/mem.c > +++ b/arch/powerpc/mm/mem.c > @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long > vaddr) > > int page_is_ram(unsigned long pfn) > { > - unsigned long paddr = (pfn << PAGE_SHIFT); > - > #ifndef CONFIG_PPC64 /* XXX for now */ > - return paddr < __pa(high_memory); > + return pfn < max_pfn; > #else > + unsigned long paddr = (pfn << PAGE_SHIFT); seems like this could be a phys_addr_t > > int i; > for (i=0; i < lmb.memory.cnt; i++) { > unsigned long base; - k
> > #ifndef CONFIG_PPC64 /* XXX for now */ > > - return paddr < __pa(high_memory); > > + return pfn < max_pfn; > > #else > > + unsigned long paddr = (pfn << PAGE_SHIFT); > > seems like this could be a phys_addr_t Yes, it could I guess, but that would be an unrelated change, and I'm not sure there's much point given this is in 64-bit-only code. - R.
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index 1c93c25..98d7bf9 100644 --- a/arch/powerpc/mm/mem.c +++ b/arch/powerpc/mm/mem.c @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr) int page_is_ram(unsigned long pfn) { - unsigned long paddr = (pfn << PAGE_SHIFT); - #ifndef CONFIG_PPC64 /* XXX for now */ - return paddr < __pa(high_memory); + return pfn < max_pfn; #else + unsigned long paddr = (pfn << PAGE_SHIFT); int i; for (i=0; i < lmb.memory.cnt; i++) { unsigned long base;
Commit 8b150478 ("ppc: make phys_mem_access_prot() work with pfns instead of addresses") fixed page_is_ram() in arch/ppc to avoid overflow for addresses above 4G on 32-bit kernels. However arch/powerpc's page_is_ram() is missing the same fix -- it computes a physical address by doing pfn << PAGE_SHIFT, which overflows if pfn corresponds to a page above 4G. In particular this causes pages above 4G to be mapped with the wrong caching attribute; for example many ppc440-based SoCs have PCI space above 4G, and mmap()ing MMIO space may end up with a mapping that has caching enabled. Fix this by working with the pfn and avoiding the conversion to physical address that causes the overflow. This patch compares the pfn to max_pfn, which is a semantic change from the old code -- that code compared the physical address to high_memory, which corresponds to max_low_pfn. However, I think that was is another bug, since highmem pages are still RAM. Reported-by: vb <vb@vsbe.com> Signed-off-by: Roland Dreier <rolandd@cisco.com> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>