[PATCH 0/2] Minor fixes for CCMs

[PATCH 0/2] Minor fixes for CCMs

[Alexey Brodkin]

It turned out current implementation of CCM support doesn't work at all.
There're 2 isseus:
 * Data/code which is supposed to be in DCCM or ICCM accordingly gets
   merged in common .data and .text sections so CCMs won't be used
 * Kerenl will panic on early boot because comparison of CCM sizes
   is implemented incorrectly

This short series fixes both issues above so CCMs are usable again.
And both patches indeed should be back-ported to stable kernels.

Alexey Brodkin (2):
  arc: rename xCCM sections so they are not merged in global .data/.text
  arc: Fix xCCM size check

 arch/arc/include/asm/linkage.h | 6 +++---
 arch/arc/kernel/setup.c        | 4 ++--
 arch/arc/kernel/vmlinux.lds.S  | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH 1/2] arc: rename xCCM sections so they are not merged in global .data/.text

[Alexey Brodkin]

If Linux kernel is compiled with "-ffunction-sections" each function is placed in
its own section named ".text.function_name". This is required for
discarding of not-used functions during final linkage. But in the end
all ".text.XXX" sections are merged in the global ".text" section of
vmlinux Elf.

The same happens with data sections when "-fdata-sections" are used.

That means our ".data.arcfp" and ".text.arcfp" sections get silently
merged in global ".data" and ".text" sections even though we want to
put them in separate memory regions in case ICCM and/or DCCM exist in
the ARC core.

Solution is as simple as addition of one extra period in section name.

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Igor Guryanov <guryanov@synopsys.com>
Cc: stable@vger.kernel.org
---
 arch/arc/include/asm/linkage.h | 6 +++---
 arch/arc/kernel/vmlinux.lds.S  | 8 ++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arc/include/asm/linkage.h b/arch/arc/include/asm/linkage.h
index b29f1a9fd6f7..3a5f13d65ee1 100644
--- a/arch/arc/include/asm/linkage.h
+++ b/arch/arc/include/asm/linkage.h
@@ -28,7 +28,7 @@
 /* annotation for data we want in DCCM - if enabled in .config */
 .macro ARCFP_CODE
 #ifdef CONFIG_ARC_HAS_ICCM
-	.section .text.arcfp, "ax",@progbits
+	.section .text..arcfp, "ax",@progbits
 #else
 	.section .text, "ax",@progbits
 #endif
@@ -47,13 +47,13 @@
 #else	/* !__ASSEMBLY__ */
 
 #ifdef CONFIG_ARC_HAS_ICCM
-#define __arcfp_code __attribute__((__section__(".text.arcfp")))
+#define __arcfp_code __attribute__((__section__(".text..arcfp")))
 #else
 #define __arcfp_code __attribute__((__section__(".text")))
 #endif
 
 #ifdef CONFIG_ARC_HAS_DCCM
-#define __arcfp_data __attribute__((__section__(".data.arcfp")))
+#define __arcfp_data __attribute__((__section__(".data..arcfp")))
 #else
 #define __arcfp_data __attribute__((__section__(".data")))
 #endif
diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
index f35ed578e007..f69ae479ee73 100644
--- a/arch/arc/kernel/vmlinux.lds.S
+++ b/arch/arc/kernel/vmlinux.lds.S
@@ -37,8 +37,8 @@ SECTIONS
 	}
 
 #ifdef CONFIG_ARC_HAS_ICCM
-	.text.arcfp : {
-		*(.text.arcfp)
+	.text..arcfp : {
+		*(.text..arcfp)
 		. = ALIGN(CONFIG_ARC_ICCM_SZ * 1024);
 	}
 #endif
@@ -151,8 +151,8 @@ SECTIONS
 #ifdef CONFIG_ARC_HAS_DCCM
 	. = CONFIG_ARC_DCCM_BASE;
 	__arc_dccm_base = .;
-	.data.arcfp : {
-		*(.data.arcfp)
+	.data..arcfp : {
+		*(.data..arcfp)
 	}
 	. = ALIGN(CONFIG_ARC_DCCM_SZ * 1024);
 #endif
-- 
2.7.4

Re: [PATCH 1/2] arc: rename xCCM sections so they are not merged in global .data/.text

[Vineet Gupta]

On 12/22/2016 06:09 AM, Alexey Brodkin wrote:
> If Linux kernel is compiled with "-ffunction-sections" each function is placed in
> its own section named ".text.function_name". This is required for
> discarding of not-used functions during final linkage. But in the end
> all ".text.XXX" sections are merged in the global ".text" section of
> vmlinux Elf.
>
> The same happens with data sections when "-fdata-sections" are used.
>
> That means our ".data.arcfp" and ".text.arcfp" sections get silently
> merged in global ".data" and ".text" sections even though we want to
> put them in separate memory regions in case ICCM and/or DCCM exist in
> the ARC core.
>
> Solution is as simple as addition of one extra period in section name.
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Igor Guryanov <guryanov@synopsys.com>
> Cc: stable@vger.kernel.org

For stable backport -I think there is no point in CC'ing them initially.
Better to make a note in the patch and conclude at the time of patch review and I
can in the end add the CC tag so this gets pulled automatically w/o CC'ing the
stable mailing list at all !

> ---
>  arch/arc/include/asm/linkage.h | 6 +++---
>  arch/arc/kernel/vmlinux.lds.S  | 8 ++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arc/include/asm/linkage.h b/arch/arc/include/asm/linkage.h
> index b29f1a9fd6f7..3a5f13d65ee1 100644
> --- a/arch/arc/include/asm/linkage.h
> +++ b/arch/arc/include/asm/linkage.h
> @@ -28,7 +28,7 @@
>  /* annotation for data we want in DCCM - if enabled in .config */
>  .macro ARCFP_CODE
>  #ifdef CONFIG_ARC_HAS_ICCM
> -	.section .text.arcfp, "ax",@progbits
> +	.section .text..arcfp, "ax",@progbits

Why not turn this around and call it arcfp.text and arcfp.data etc - just like
done for other special sections such as sched, cpuidle, lock... Just so we are
consistent with normal kernel convention !

>  #else
>  	.section .text, "ax",@progbits
>  #endif
> @@ -47,13 +47,13 @@
>  #else	/* !__ASSEMBLY__ */
>  
>  #ifdef CONFIG_ARC_HAS_ICCM
> -#define __arcfp_code __attribute__((__section__(".text.arcfp")))
> +#define __arcfp_code __attribute__((__section__(".text..arcfp")))
>  #else
>  #define __arcfp_code __attribute__((__section__(".text")))
>  #endif
>  
>  #ifdef CONFIG_ARC_HAS_DCCM
> -#define __arcfp_data __attribute__((__section__(".data.arcfp")))
> +#define __arcfp_data __attribute__((__section__(".data..arcfp")))
>  #else
>  #define __arcfp_data __attribute__((__section__(".data")))
>  #endif
> diff --git a/arch/arc/kernel/vmlinux.lds.S b/arch/arc/kernel/vmlinux.lds.S
> index f35ed578e007..f69ae479ee73 100644
> --- a/arch/arc/kernel/vmlinux.lds.S
> +++ b/arch/arc/kernel/vmlinux.lds.S
> @@ -37,8 +37,8 @@ SECTIONS
>  	}
>  
>  #ifdef CONFIG_ARC_HAS_ICCM
> -	.text.arcfp : {
> -		*(.text.arcfp)
> +	.text..arcfp : {
> +		*(.text..arcfp)
>  		. = ALIGN(CONFIG_ARC_ICCM_SZ * 1024);
>  	}
>  #endif
> @@ -151,8 +151,8 @@ SECTIONS
>  #ifdef CONFIG_ARC_HAS_DCCM
>  	. = CONFIG_ARC_DCCM_BASE;
>  	__arc_dccm_base = .;
> -	.data.arcfp : {
> -		*(.data.arcfp)
> +	.data..arcfp : {
> +		*(.data..arcfp)
>  	}
>  	. = ALIGN(CONFIG_ARC_DCCM_SZ * 1024);
>  #endif

Re: [PATCH 1/2] arc: rename xCCM sections so they are not merged in global .data/.text

[Alexey Brodkin]

Hi Vineet,

On Thu, 2016-12-22 at 16:25 -0800, Vineet Gupta wrote:
> On 12/22/2016 06:09 AM, Alexey Brodkin wrote:
> > diff --git a/arch/arc/include/asm/linkage.h b/arch/arc/include/asm/linkage.h
> > index b29f1a9fd6f7..3a5f13d65ee1 100644
> > --- a/arch/arc/include/asm/linkage.h
> > +++ b/arch/arc/include/asm/linkage.h
> > @@ -28,7 +28,7 @@
> >  /* annotation for data we want in DCCM - if enabled in .config */
> >  .macro ARCFP_CODE
> >  #ifdef CONFIG_ARC_HAS_ICCM
> > -	.section .text.arcfp, "ax",@progbits
> > +	.section .text..arcfp, "ax",@progbits
> 
> Why not turn this around and call it arcfp.text and arcfp.data etc - just like
> done for other special sections such as sched, cpuidle, lock... Just so we are
> consistent with normal kernel convention !

Right that looks much better!
Will do so in v2.

-Alexey

[PATCH 2/2] arc: Fix xCCM size check

[Alexey Brodkin]

CONFIG_ARC_ICCM_SZ in menuconfig is specified in kB while
"cpu->Xccm.sz" contains value in bytes thus direct comparison fails
leading to boot-time panic like that:
----------------------->8---------------------
IDENTITY        : ARCVER [0x52] ARCNUM [0x1] CHIPID [ 0x0]
processor [1]   : ARC HS38 R2.1 (ARCv2 ISA)
Timers          : Timer0 Timer1 Local-64-bit-Ctr (not used)
ISA Extn        : atomic ll64 unalign (not used)
                : mpy[opt 9] div_rem norm barrel-shift swap minmax swape
BPU             : full match, cache:2048, Predict Table:16384
MMU [v0]        : 0k PAGE, JTLB 0 (0x0), uDTLB 0, uITLB 0
I-Cache         : N/A
D-Cache         : N/A
Peripherals     : 0xf0000000, IO-Coherency (disabled)
Vector Table    : 0x80000000
FPU             : SP DP
DEBUG           : ActionPoint smaRT RTT
Extn [CCM]      : DCCM @ e0000000, 256 KB / ICCM: @ 60000000, 256 KB
OS ABI [v4]     : 64-bit data any register aligned
Extn [SMP]      : ARConnect (v2): 2 cores with IPI IDU DEBUG GFRC
Kernel panic - not syncing: Linux built with incorrect DCCM Size

---[ end Kernel panic - not syncing: Linux built with incorrect DCCM Size
----------------------->8---------------------

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: Igor Guryanov <guryanov@synopsys.com>
Cc: stable@vger.kernel.org
---
 arch/arc/kernel/setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index ee574f37f365..601dab6fe5e0 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -333,12 +333,12 @@ static void arc_chk_core_config(void)
 	if ((unsigned int)__arc_dccm_base != cpu->dccm.base_addr)
 		panic("Linux built with incorrect DCCM Base address\n");
 
-	if (CONFIG_ARC_DCCM_SZ != cpu->dccm.sz)
+	if (CONFIG_ARC_DCCM_SZ != TO_KB(cpu->dccm.sz))
 		panic("Linux built with incorrect DCCM Size\n");
 #endif
 
 #ifdef CONFIG_ARC_HAS_ICCM
-	if (CONFIG_ARC_ICCM_SZ != cpu->iccm.sz)
+	if (CONFIG_ARC_ICCM_SZ != TO_KB(cpu->iccm.sz))
 		panic("Linux built with incorrect ICCM Size\n");
 #endif
 
-- 
2.7.4

Re: [PATCH 2/2] arc: Fix xCCM size check

[Vineet Gupta]

On 12/22/2016 06:09 AM, Alexey Brodkin wrote:
> CONFIG_ARC_ICCM_SZ in menuconfig is specified in kB while
> "cpu->Xccm.sz" contains value in bytes thus direct comparison fails
> leading to boot-time panic like that:
> ----------------------->8---------------------
> IDENTITY        : ARCVER [0x52] ARCNUM [0x1] CHIPID [ 0x0]
> processor [1]   : ARC HS38 R2.1 (ARCv2 ISA)
> Timers          : Timer0 Timer1 Local-64-bit-Ctr (not used)
> ISA Extn        : atomic ll64 unalign (not used)
>                 : mpy[opt 9] div_rem norm barrel-shift swap minmax swape
> BPU             : full match, cache:2048, Predict Table:16384
> MMU [v0]        : 0k PAGE, JTLB 0 (0x0), uDTLB 0, uITLB 0
> I-Cache         : N/A
> D-Cache         : N/A
> Peripherals     : 0xf0000000, IO-Coherency (disabled)
> Vector Table    : 0x80000000
> FPU             : SP DP
> DEBUG           : ActionPoint smaRT RTT
> Extn [CCM]      : DCCM @ e0000000, 256 KB / ICCM: @ 60000000, 256 KB
> OS ABI [v4]     : 64-bit data any register aligned
> Extn [SMP]      : ARConnect (v2): 2 cores with IPI IDU DEBUG GFRC
> Kernel panic - not syncing: Linux built with incorrect DCCM Size
>
> ---[ end Kernel panic - not syncing: Linux built with incorrect DCCM Size
> ----------------------->8---------------------
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> Cc: Igor Guryanov <guryanov@synopsys.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/arc/kernel/setup.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index ee574f37f365..601dab6fe5e0 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -333,12 +333,12 @@ static void arc_chk_core_config(void)
>  	if ((unsigned int)__arc_dccm_base != cpu->dccm.base_addr)
>  		panic("Linux built with incorrect DCCM Base address\n");
>  
> -	if (CONFIG_ARC_DCCM_SZ != cpu->dccm.sz)
> +	if (CONFIG_ARC_DCCM_SZ != TO_KB(cpu->dccm.sz))

Could we just avoid this existing TO_KB non sense in multiple places by keeping
the *ccm.sz unit consistent with CONFIG_ARC_*CCM_SZ ?
so
-            cpu->iccm.sz = 4096 << iccm.sz;    /* 8K to 512K */
+            cpu->iccm.sz = 4 << iccm.sz;    /* 8K to 512K */

Since we only want to keep ccm size to kb granularity

And while at it, rename @sz placeholder in bcr_(i|d)ccm_arc(v2,compact) to sz_k

-Vineet

>  		panic("Linux built with incorrect DCCM Size\n");
>  #endif
>  
>  #ifdef CONFIG_ARC_HAS_ICCM
> -	if (CONFIG_ARC_ICCM_SZ != cpu->iccm.sz)
> +	if (CONFIG_ARC_ICCM_SZ != TO_KB(cpu->iccm.sz))
>  		panic("Linux built with incorrect ICCM Size\n");
>  #endif
>  

Re: [PATCH 2/2] arc: Fix xCCM size check

[Alexey Brodkin]

Hi Vineet,

On Thu, 2016-12-22 at 16:34 -0800, Vineet Gupta wrote:
> On 12/22/2016 06:09 AM, Alexey Brodkin wrote:
> > 
> > CONFIG_ARC_ICCM_SZ in menuconfig is specified in kB while
> > "cpu->Xccm.sz" contains value in bytes thus direct comparison fails
> > leading to boot-time panic like that:
> > ----------------------->8---------------------
> > IDENTITY        : ARCVER [0x52] ARCNUM [0x1] CHIPID [ 0x0]
> > processor [1]   : ARC HS38 R2.1 (ARCv2 ISA)
> > Timers          : Timer0 Timer1 Local-64-bit-Ctr (not used)
> > ISA Extn        : atomic ll64 unalign (not used)
> >                 : mpy[opt 9] div_rem norm barrel-shift swap minmax swape
> > BPU             : full match, cache:2048, Predict Table:16384
> > MMU [v0]        : 0k PAGE, JTLB 0 (0x0), uDTLB 0, uITLB 0
> > I-Cache         : N/A
> > D-Cache         : N/A
> > Peripherals     : 0xf0000000, IO-Coherency (disabled)
> > Vector Table    : 0x80000000
> > FPU             : SP DP
> > DEBUG           : ActionPoint smaRT RTT
> > Extn [CCM]      : DCCM @ e0000000, 256 KB / ICCM: @ 60000000, 256 KB
> > OS ABI [v4]     : 64-bit data any register aligned
> > Extn [SMP]      : ARConnect (v2): 2 cores with IPI IDU DEBUG GFRC
> > Kernel panic - not syncing: Linux built with incorrect DCCM Size
> > 
> > ---[ end Kernel panic - not syncing: Linux built with incorrect DCCM Size
> > ----------------------->8---------------------
> > 
> > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> > Cc: Igor Guryanov <guryanov@synopsys.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  arch/arc/kernel/setup.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> > index ee574f37f365..601dab6fe5e0 100644
> > --- a/arch/arc/kernel/setup.c
> > +++ b/arch/arc/kernel/setup.c
> > @@ -333,12 +333,12 @@ static void arc_chk_core_config(void)
> >  	if ((unsigned int)__arc_dccm_base != cpu->dccm.base_addr)
> >  		panic("Linux built with incorrect DCCM Base address\n");
> >  
> > -	if (CONFIG_ARC_DCCM_SZ != cpu->dccm.sz)
> > +	if (CONFIG_ARC_DCCM_SZ != TO_KB(cpu->dccm.sz))
> 
> Could we just avoid this existing TO_KB non sense in multiple places by keeping
> the *ccm.sz unit consistent with CONFIG_ARC_*CCM_SZ ?
> so
> -            cpu->iccm.sz = 4096 << iccm.sz;    /* 8K to 512K */
> +            cpu->iccm.sz = 4 << iccm.sz;    /* 8K to 512K */
> 
> Since we only want to keep ccm size to kb granularity
> 
> And while at it, rename @sz placeholder in bcr_(i|d)ccm_arc(v2,compact) to sz_k

Well when I started to look at that I understood that in case of ARCv2 smallest
xCCM is 512 bytes which won't fit in our 1kB grained solution.

So it looks like instead of moving to "sz_k" we have to stay with what we have
and moreover add support of that 512 byte corner-case becase TO_KB(512) = 0.

-Alexey

Re: [PATCH 0/2] Minor fixes for CCMs

[Vineet Gupta]

On 12/22/2016 06:09 AM, Alexey Brodkin wrote:
> It turned out current implementation of CCM support doesn't work at all.
> There're 2 isseus:
>  * Data/code which is supposed to be in DCCM or ICCM accordingly gets
>    merged in common .data and .text sections so CCMs won't be used
>  * Kerenl will panic on early boot because comparison of CCM sizes
>    is implemented incorrectly

FWIW, for current hsdk bringup I won't recommend to switch them on just yet !

>
> This short series fixes both issues above so CCMs are usable again.
> And both patches indeed should be back-ported to stable kernels.
>
> Alexey Brodkin (2):
>   arc: rename xCCM sections so they are not merged in global .data/.text
>   arc: Fix xCCM size check
>
>  arch/arc/include/asm/linkage.h | 6 +++---
>  arch/arc/kernel/setup.c        | 4 ++--
>  arch/arc/kernel/vmlinux.lds.S  | 8 ++++----
>  3 files changed, 9 insertions(+), 9 deletions(-)
>