diff mbox

[1/7] arm: use generic fixmap.h

Message ID 1407353564-21478-2-git-send-email-keescook@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kees Cook Aug. 6, 2014, 7:32 p.m. UTC 
From: Mark Salter <msalter@redhat.com>

ARM is different from other architectures in that fixmap pages are indexed
with a positive offset from FIXADDR_START.  Other architectures index with
a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h
definitions, this patch redefines FIXADDR_TOP to be inclusive of the
useable range.  That is, FIXADDR_TOP is the virtual address of the topmost
fixed page.  The newly defined FIXADDR_END is the first virtual address
past the fixed mappings.

Signed-off-by: Mark Salter <msalter@redhat.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>
[update for "ARM: 8031/2: change fixmap mapping region to support 32 CPUs"]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/include/asm/fixmap.h | 27 +++++++++------------------
 arch/arm/mm/init.c            |  2 +-
 2 files changed, 10 insertions(+), 19 deletions(-)

Comments

Max Filippov Aug. 7, 2014, 3:15 p.m. UTC | #1 
Hi,

On Wed, Aug 6, 2014 at 11:32 PM, Kees Cook <keescook@chromium.org> wrote:
> ARM is different from other architectures in that fixmap pages are indexed
> with a positive offset from FIXADDR_START.  Other architectures index with
> a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h

Does anybody know if there's any reason why generic fixmap.h uses negative
offsets? It complicates things with no obvious benefit if you e.g. try to align
virtual address in the fixmap region with physical page color (that's why I've
switched xtensa to positive fixmap addressing in v3.17).
Rob Herring (Arm) Aug. 7, 2014, 3:22 p.m. UTC | #2 
On Thu, Aug 7, 2014 at 10:15 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Hi,
>
> On Wed, Aug 6, 2014 at 11:32 PM, Kees Cook <keescook@chromium.org> wrote:
>> ARM is different from other architectures in that fixmap pages are indexed
>> with a positive offset from FIXADDR_START.  Other architectures index with
>> a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h
>
> Does anybody know if there's any reason why generic fixmap.h uses negative
> offsets? It complicates things with no obvious benefit if you e.g. try to align
> virtual address in the fixmap region with physical page color (that's why I've
> switched xtensa to positive fixmap addressing in v3.17).

No, but each arch doing it differently is even more annoying.

Rob
Nicolas Pitre Aug. 7, 2014, 3:32 p.m. UTC | #3 
On Thu, 7 Aug 2014, Rob Herring wrote:

> On Thu, Aug 7, 2014 at 10:15 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Aug 6, 2014 at 11:32 PM, Kees Cook <keescook@chromium.org> wrote:
> >> ARM is different from other architectures in that fixmap pages are indexed
> >> with a positive offset from FIXADDR_START.  Other architectures index with
> >> a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h
> >
> > Does anybody know if there's any reason why generic fixmap.h uses negative
> > offsets? It complicates things with no obvious benefit if you e.g. try to align
> > virtual address in the fixmap region with physical page color (that's why I've
> > switched xtensa to positive fixmap addressing in v3.17).
> 
> No, but each arch doing it differently is even more annoying.

Why not switching everybody to positive offsets then?


Nicolas
Max Filippov Aug. 7, 2014, 3:42 p.m. UTC | #4 
On Thu, Aug 7, 2014 at 7:32 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Thu, 7 Aug 2014, Rob Herring wrote:
>
>> On Thu, Aug 7, 2014 at 10:15 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> > Hi,
>> >
>> > On Wed, Aug 6, 2014 at 11:32 PM, Kees Cook <keescook@chromium.org> wrote:
>> >> ARM is different from other architectures in that fixmap pages are indexed
>> >> with a positive offset from FIXADDR_START.  Other architectures index with
>> >> a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h
>> >
>> > Does anybody know if there's any reason why generic fixmap.h uses negative
>> > offsets? It complicates things with no obvious benefit if you e.g. try to align
>> > virtual address in the fixmap region with physical page color (that's why I've
>> > switched xtensa to positive fixmap addressing in v3.17).
>>
>> No, but each arch doing it differently is even more annoying.
>
> Why not switching everybody to positive offsets then?

I can cook a patch if people agree that that'd be good.
Mark Salter Aug. 7, 2014, 5:23 p.m. UTC | #5 
On Thu, 2014-08-07 at 19:42 +0400, Max Filippov wrote:
> On Thu, Aug 7, 2014 at 7:32 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Thu, 7 Aug 2014, Rob Herring wrote:
> >
> >> On Thu, Aug 7, 2014 at 10:15 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> >> > Hi,
> >> >
> >> > On Wed, Aug 6, 2014 at 11:32 PM, Kees Cook <keescook@chromium.org> wrote:
> >> >> ARM is different from other architectures in that fixmap pages are indexed
> >> >> with a positive offset from FIXADDR_START.  Other architectures index with
> >> >> a negative offset from FIXADDR_TOP.  In order to use the generic fixmap.h
> >> >
> >> > Does anybody know if there's any reason why generic fixmap.h uses negative
> >> > offsets? It complicates things with no obvious benefit if you e.g. try to align
> >> > virtual address in the fixmap region with physical page color (that's why I've
> >> > switched xtensa to positive fixmap addressing in v3.17).
> >>
> >> No, but each arch doing it differently is even more annoying.
> >
> > Why not switching everybody to positive offsets then?
> 
> I can cook a patch if people agree that that'd be good.
> 

I think that would be fine. I think x86 was first and used a negative
negative offset. Others that followed just copied that. When I did the
generic fixmap patch, using a negative offset was the natural thing to
do. Arm was only arch doing it differently.
diff mbox

Patch

diff --git a/arch/arm/include/asm/fixmap.h b/arch/arm/include/asm/fixmap.h
index 74124b0d0d79..190142d174ee 100644
--- a/arch/arm/include/asm/fixmap.h
+++ b/arch/arm/include/asm/fixmap.h
@@ -2,27 +2,18 @@ 
 #define _ASM_FIXMAP_H
 
 #define FIXADDR_START		0xffc00000UL
-#define FIXADDR_TOP		0xffe00000UL
-#define FIXADDR_SIZE		(FIXADDR_TOP - FIXADDR_START)
+#define FIXADDR_END		0xffe00000UL
+#define FIXADDR_TOP		(FIXADDR_END - PAGE_SIZE)
+#define FIXADDR_SIZE		(FIXADDR_END - FIXADDR_START)
 
 #define FIX_KMAP_NR_PTES	(FIXADDR_SIZE >> PAGE_SHIFT)
 
-#define __fix_to_virt(x)	(FIXADDR_START + ((x) << PAGE_SHIFT))
-#define __virt_to_fix(x)	(((x) - FIXADDR_START) >> PAGE_SHIFT)
+enum fixed_addresses {
+	FIX_KMAP_BEGIN,
+	FIX_KMAP_END = FIX_KMAP_NR_PTES - 1,
+	__end_of_fixed_addresses
+};
 
-extern void __this_fixmap_does_not_exist(void);
-
-static inline unsigned long fix_to_virt(const unsigned int idx)
-{
-	if (idx >= FIX_KMAP_NR_PTES)
-		__this_fixmap_does_not_exist();
-	return __fix_to_virt(idx);
-}
-
-static inline unsigned int virt_to_fix(const unsigned long vaddr)
-{
-	BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
-	return __virt_to_fix(vaddr);
-}
+#include <asm-generic/fixmap.h>
 
 #endif
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 659c75d808dc..ad82c05bfc3a 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -570,7 +570,7 @@  void __init mem_init(void)
 			MLK(DTCM_OFFSET, (unsigned long) dtcm_end),
 			MLK(ITCM_OFFSET, (unsigned long) itcm_end),
 #endif
-			MLK(FIXADDR_START, FIXADDR_TOP),
+			MLK(FIXADDR_START, FIXADDR_END),
 			MLM(VMALLOC_START, VMALLOC_END),
 			MLM(PAGE_OFFSET, (unsigned long)high_memory),
 #ifdef CONFIG_HIGHMEM