diff mbox

[1/4] ARM: mmu: decouple VECTORS_BASE from Kconfig

Message ID 20170118203739.6400-1-afzal.mohd.ma@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

afzal mohammed Jan. 18, 2017, 8:37 p.m. UTC
For MMU configurations, VECTORS_BASE is always 0xffff0000, a macro
definition will suffice.

Once exception address is handled dynamically for no-MMU also (this
would involve taking care of region setup too), VECTORS_BASE can be
removed from Kconfig.

Suggested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---

Though there was no build error without inclusion of asm/memory.h, to
be on the safer side it has been added, to reduce chances of build
breakage in random configurations.

 arch/arm/include/asm/memory.h  | 2 ++
 arch/arm/mach-berlin/platsmp.c | 3 ++-
 arch/arm/mm/dump.c             | 5 +++--
 arch/arm/mm/init.c             | 4 ++--
 4 files changed, 9 insertions(+), 5 deletions(-)

Comments

afzal mohammed Jan. 19, 2017, 1:21 p.m. UTC | #1
+ Marvell Berlin SoC maintainers - Sebastian, Jisheng

On Thu, Jan 19, 2017 at 02:07:39AM +0530, afzal mohammed wrote:
> For MMU configurations, VECTORS_BASE is always 0xffff0000, a macro
> definition will suffice.
> 
> Once exception address is handled dynamically for no-MMU also (this
> would involve taking care of region setup too), VECTORS_BASE can be
> removed from Kconfig.
> 
> Suggested-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
> ---
> 
> Though there was no build error without inclusion of asm/memory.h, to
> be on the safer side it has been added, to reduce chances of build
> breakage in random configurations.
> 
>  arch/arm/include/asm/memory.h  | 2 ++
>  arch/arm/mach-berlin/platsmp.c | 3 ++-
>  arch/arm/mm/dump.c             | 5 +++--
>  arch/arm/mm/init.c             | 4 ++--
>  4 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..9cc9f1dbc88e 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -83,6 +83,8 @@
>  #define IOREMAP_MAX_ORDER	24
>  #endif
>  
> +#define VECTORS_BASE		0xffff0000
> +
>  #else /* CONFIG_MMU */
>  
>  /*
> diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
> index 93f90688db18..578d41031abf 100644
> --- a/arch/arm/mach-berlin/platsmp.c
> +++ b/arch/arm/mach-berlin/platsmp.c
> @@ -15,6 +15,7 @@
>  
>  #include <asm/cacheflush.h>
>  #include <asm/cp15.h>
> +#include <asm/memory.h>
>  #include <asm/smp_plat.h>
>  #include <asm/smp_scu.h>
>  
> @@ -75,7 +76,7 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
>  	if (!cpu_ctrl)
>  		goto unmap_scu;
>  
> -	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
> +	vectors_base = ioremap(VECTORS_BASE, SZ_32K);
>  	if (!vectors_base)
>  		goto unmap_scu;
>  
> diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
> index 9fe8e241335c..21192d6eda40 100644
> --- a/arch/arm/mm/dump.c
> +++ b/arch/arm/mm/dump.c
> @@ -18,6 +18,7 @@
>  #include <linux/seq_file.h>
>  
>  #include <asm/fixmap.h>
> +#include <asm/memory.h>
>  #include <asm/pgtable.h>
>  
>  struct addr_marker {
> @@ -31,8 +32,8 @@ static struct addr_marker address_markers[] = {
>  	{ 0,			"vmalloc() Area" },
>  	{ VMALLOC_END,		"vmalloc() End" },
>  	{ FIXADDR_START,	"Fixmap Area" },
> -	{ CONFIG_VECTORS_BASE,	"Vectors" },
> -	{ CONFIG_VECTORS_BASE + PAGE_SIZE * 2, "Vectors End" },
> +	{ VECTORS_BASE,	"Vectors" },
> +	{ VECTORS_BASE + PAGE_SIZE * 2, "Vectors End" },
>  	{ -1,			NULL },
>  };
>  
> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
> index 370581aeb871..cf47f86f79ed 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> @@ -27,6 +27,7 @@
>  #include <asm/cp15.h>
>  #include <asm/mach-types.h>
>  #include <asm/memblock.h>
> +#include <asm/memory.h>
>  #include <asm/prom.h>
>  #include <asm/sections.h>
>  #include <asm/setup.h>
> @@ -521,8 +522,7 @@ void __init mem_init(void)
>  			"      .data : 0x%p" " - 0x%p" "   (%4td kB)\n"
>  			"       .bss : 0x%p" " - 0x%p" "   (%4td kB)\n",
>  
> -			MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
> -				(PAGE_SIZE)),
> +			MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
>  #ifdef CONFIG_HAVE_TCM
>  			MLK(DTCM_OFFSET, (unsigned long) dtcm_end),
>  			MLK(ITCM_OFFSET, (unsigned long) itcm_end),
> -- 
> 2.11.0
kernel test robot Jan. 19, 2017, 2:07 p.m. UTC | #2
Hi afzal,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc4 next-20170119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/afzal-mohammed/ARM-mmu-decouple-VECTORS_BASE-from-Kconfig/20170119-171424
config: arm-stm32_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

Note: the linux-review/afzal-mohammed/ARM-mmu-decouple-VECTORS_BASE-from-Kconfig/20170119-171424 HEAD dd110acc21d40c8f50374de1e500a091d14f29c8 builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   arch/arm/mm/init.c: In function 'mem_init':
>> arch/arm/mm/init.c:525:11: error: 'VECTORS_BASEUL' undeclared (first use in this function)
       MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
              ^
   arch/arm/mm/init.c:501:19: note: in definition of macro 'MLK'
    #define MLK(b, t) b, t, ((t) - (b)) >> 10
                      ^
>> include/uapi/linux/const.h:20:18: note: in expansion of macro '__AC'
    #define _AC(X,Y) __AC(X,Y)
                     ^~~~
>> arch/arm/include/asm/memory.h:29:15: note: in expansion of macro '_AC'
    #define UL(x) _AC(x, UL)
                  ^~~
>> include/linux/printk.h:297:36: note: in expansion of macro 'UL'
     printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
                                       ^~~~~~~~~~~
>> arch/arm/mm/init.c:505:2: note: in expansion of macro 'pr_notice'
     pr_notice("Virtual kernel memory layout:\n"
     ^~~~~~~~~
   arch/arm/mm/init.c:525:11: note: each undeclared identifier is reported only once for each function it appears in
       MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
              ^
   arch/arm/mm/init.c:501:19: note: in definition of macro 'MLK'
    #define MLK(b, t) b, t, ((t) - (b)) >> 10
                      ^
>> include/uapi/linux/const.h:20:18: note: in expansion of macro '__AC'
    #define _AC(X,Y) __AC(X,Y)
                     ^~~~
>> arch/arm/include/asm/memory.h:29:15: note: in expansion of macro '_AC'
    #define UL(x) _AC(x, UL)
                  ^~~
>> include/linux/printk.h:297:36: note: in expansion of macro 'UL'
     printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
                                       ^~~~~~~~~~~
>> arch/arm/mm/init.c:505:2: note: in expansion of macro 'pr_notice'
     pr_notice("Virtual kernel memory layout:\n"
     ^~~~~~~~~

vim +/VECTORS_BASEUL +525 arch/arm/mm/init.c

   499		mem_init_print_info(NULL);
   500	
   501	#define MLK(b, t) b, t, ((t) - (b)) >> 10
   502	#define MLM(b, t) b, t, ((t) - (b)) >> 20
   503	#define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), SZ_1K)
   504	
 > 505		pr_notice("Virtual kernel memory layout:\n"
   506				"    vector  : 0x%08lx - 0x%08lx   (%4ld kB)\n"
   507	#ifdef CONFIG_HAVE_TCM
   508				"    DTCM    : 0x%08lx - 0x%08lx   (%4ld kB)\n"
   509				"    ITCM    : 0x%08lx - 0x%08lx   (%4ld kB)\n"
   510	#endif
   511				"    fixmap  : 0x%08lx - 0x%08lx   (%4ld kB)\n"
   512				"    vmalloc : 0x%08lx - 0x%08lx   (%4ld MB)\n"
   513				"    lowmem  : 0x%08lx - 0x%08lx   (%4ld MB)\n"
   514	#ifdef CONFIG_HIGHMEM
   515				"    pkmap   : 0x%08lx - 0x%08lx   (%4ld MB)\n"
   516	#endif
   517	#ifdef CONFIG_MODULES
   518				"    modules : 0x%08lx - 0x%08lx   (%4ld MB)\n"
   519	#endif
   520				"      .text : 0x%p" " - 0x%p" "   (%4td kB)\n"
   521				"      .init : 0x%p" " - 0x%p" "   (%4td kB)\n"
   522				"      .data : 0x%p" " - 0x%p" "   (%4td kB)\n"
   523				"       .bss : 0x%p" " - 0x%p" "   (%4td kB)\n",
   524	
 > 525				MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
   526	#ifdef CONFIG_HAVE_TCM
   527				MLK(DTCM_OFFSET, (unsigned long) dtcm_end),
   528				MLK(ITCM_OFFSET, (unsigned long) itcm_end),

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Russell King (Oracle) Jan. 19, 2017, 2:24 p.m. UTC | #3
On Thu, Jan 19, 2017 at 02:07:39AM +0530, afzal mohammed wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..9cc9f1dbc88e 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -83,6 +83,8 @@
>  #define IOREMAP_MAX_ORDER	24
>  #endif
>  
> +#define VECTORS_BASE		0xffff0000

This should be UL(0xffff0000)

> -			MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
> -				(PAGE_SIZE)),
> +			MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),

which means you don't need it here, which will then fix the build error
reported by the 0-day builder.
afzal mohammed Jan. 20, 2017, 4:06 p.m. UTC | #4
Hi,

On Thu, Jan 19, 2017 at 02:24:24PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 19, 2017 at 02:07:39AM +0530, afzal mohammed wrote:

> > +++ b/arch/arm/include/asm/memory.h

> > +#define VECTORS_BASE		0xffff0000
> 
> This should be UL(0xffff0000)

> > -			MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
> > -				(PAGE_SIZE)),
> > +			MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
> 
> which means you don't need it here, which will then fix the build error
> reported by the 0-day builder.

Seems there is some confusion here,

VECTORS_BASE definition above in memory.h is enclosed within
CONFIG_MMU. Robot used a no-MMU defconfig, it didn't get a
VECTORS_BASE definition at this patch, causing the build error. Our
dear robot mentioned that my HEAD didn't break build, but
bisectability is broken at this point.

With "PATCH 3/4 ARM: nommu: display vectors base", the above is
changed to
        #ifdef CONFIG_MMU
                MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
        #else
                ...
        #endif
thus making the series build again for no-MMU

One option to keep bisectability would be to squash this with PATCH
3/4, but i think a better & natural solution would be define
VECTORS_BASE outside of
        #ifdef CONFIG_MMU
        ...
        #else
        ...
        #endif
and then in PATCH 3/4, move VECTORS_BASE to be inside
        #ifdef CONFIG_MMU
        ...
        #else

Regards
afzal
afzal mohammed Jan. 22, 2017, 3:27 a.m. UTC | #5
Hi,

On Thu, Jan 19, 2017 at 02:24:24PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 19, 2017 at 02:07:39AM +0530, afzal mohammed wrote:

> > +#define VECTORS_BASE		0xffff0000
> 
> This should be UL(0xffff0000)

This has been taken care in v2.

Regards
afzal
diff mbox

Patch

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 76cbd9c674df..9cc9f1dbc88e 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -83,6 +83,8 @@ 
 #define IOREMAP_MAX_ORDER	24
 #endif
 
+#define VECTORS_BASE		0xffff0000
+
 #else /* CONFIG_MMU */
 
 /*
diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
index 93f90688db18..578d41031abf 100644
--- a/arch/arm/mach-berlin/platsmp.c
+++ b/arch/arm/mach-berlin/platsmp.c
@@ -15,6 +15,7 @@ 
 
 #include <asm/cacheflush.h>
 #include <asm/cp15.h>
+#include <asm/memory.h>
 #include <asm/smp_plat.h>
 #include <asm/smp_scu.h>
 
@@ -75,7 +76,7 @@  static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
 	if (!cpu_ctrl)
 		goto unmap_scu;
 
-	vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
+	vectors_base = ioremap(VECTORS_BASE, SZ_32K);
 	if (!vectors_base)
 		goto unmap_scu;
 
diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 9fe8e241335c..21192d6eda40 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -18,6 +18,7 @@ 
 #include <linux/seq_file.h>
 
 #include <asm/fixmap.h>
+#include <asm/memory.h>
 #include <asm/pgtable.h>
 
 struct addr_marker {
@@ -31,8 +32,8 @@  static struct addr_marker address_markers[] = {
 	{ 0,			"vmalloc() Area" },
 	{ VMALLOC_END,		"vmalloc() End" },
 	{ FIXADDR_START,	"Fixmap Area" },
-	{ CONFIG_VECTORS_BASE,	"Vectors" },
-	{ CONFIG_VECTORS_BASE + PAGE_SIZE * 2, "Vectors End" },
+	{ VECTORS_BASE,	"Vectors" },
+	{ VECTORS_BASE + PAGE_SIZE * 2, "Vectors End" },
 	{ -1,			NULL },
 };
 
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 370581aeb871..cf47f86f79ed 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -27,6 +27,7 @@ 
 #include <asm/cp15.h>
 #include <asm/mach-types.h>
 #include <asm/memblock.h>
+#include <asm/memory.h>
 #include <asm/prom.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
@@ -521,8 +522,7 @@  void __init mem_init(void)
 			"      .data : 0x%p" " - 0x%p" "   (%4td kB)\n"
 			"       .bss : 0x%p" " - 0x%p" "   (%4td kB)\n",
 
-			MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
-				(PAGE_SIZE)),
+			MLK(UL(VECTORS_BASE), UL(VECTORS_BASE) + (PAGE_SIZE)),
 #ifdef CONFIG_HAVE_TCM
 			MLK(DTCM_OFFSET, (unsigned long) dtcm_end),
 			MLK(ITCM_OFFSET, (unsigned long) itcm_end),