[PATCH] arm64: mm: use ubfm for dcache_line_size

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Jingoo Han]

Use 'ubfm' for the bitfield move instruction; thus, single
instruction can be used instead of two instructions, when
getting the minimum D-cache line size from CTR_EL0 register.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 arch/arm64/mm/proc-macros.S |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
index 8957b82..c31f41e 100644
--- a/arch/arm64/mm/proc-macros.S
+++ b/arch/arm64/mm/proc-macros.S
@@ -38,8 +38,7 @@
  */
 	.macro	dcache_line_size, reg, tmp
 	mrs	\tmp, ctr_el0			// read CTR
-	lsr	\tmp, \tmp, #16
-	and	\tmp, \tmp, #0xf		// cache line size encoding
+	ubfm	\tmp, \tmp, #0x16, 0x19		// cache line size encoding
 	mov	\reg, #4			// bytes per word
 	lsl	\reg, \reg, \tmp		// actual cache line size
 	.endm
-- 
1.7.10.4

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Will Deacon]

On Fri, Jan 17, 2014 at 08:04:32AM +0000, Jingoo Han wrote:
> Use 'ubfm' for the bitfield move instruction; thus, single
> instruction can be used instead of two instructions, when
> getting the minimum D-cache line size from CTR_EL0 register.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  arch/arm64/mm/proc-macros.S |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> index 8957b82..c31f41e 100644
> --- a/arch/arm64/mm/proc-macros.S
> +++ b/arch/arm64/mm/proc-macros.S
> @@ -38,8 +38,7 @@
>   */
>  	.macro	dcache_line_size, reg, tmp
>  	mrs	\tmp, ctr_el0			// read CTR
> -	lsr	\tmp, \tmp, #16
> -	and	\tmp, \tmp, #0xf		// cache line size encoding
> +	ubfm	\tmp, \tmp, #0x16, 0x19		// cache line size encoding

0x16 and 0x19. Are you sure?

You can also grep for other occurences of this pattern and change those too
(pgtable stuff in head.S, clidr in cache.S).

Will

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Ard Biesheuvel]

On 17 January 2014 09:04, Jingoo Han <jg1.han@samsung.com> wrote:
> Use 'ubfm' for the bitfield move instruction; thus, single
> instruction can be used instead of two instructions, when
> getting the minimum D-cache line size from CTR_EL0 register.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  arch/arm64/mm/proc-macros.S |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> index 8957b82..c31f41e 100644
> --- a/arch/arm64/mm/proc-macros.S
> +++ b/arch/arm64/mm/proc-macros.S
> @@ -38,8 +38,7 @@
>   */
>         .macro  dcache_line_size, reg, tmp
>         mrs     \tmp, ctr_el0                   // read CTR
> -       lsr     \tmp, \tmp, #16
> -       and     \tmp, \tmp, #0xf                // cache line size encoding
> +       ubfm    \tmp, \tmp, #0x16, 0x19         // cache line size encoding

Even if the # is optional for immediates these days, perhaps it is
better to adhere to a single style in one line?

-- 
Ard.

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Will Deacon]

On Fri, Jan 17, 2014 at 11:22:27AM +0000, Ard Biesheuvel wrote:
> On 17 January 2014 09:04, Jingoo Han <jg1.han@samsung.com> wrote:
> > Use 'ubfm' for the bitfield move instruction; thus, single
> > instruction can be used instead of two instructions, when
> > getting the minimum D-cache line size from CTR_EL0 register.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  arch/arm64/mm/proc-macros.S |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> > index 8957b82..c31f41e 100644
> > --- a/arch/arm64/mm/proc-macros.S
> > +++ b/arch/arm64/mm/proc-macros.S
> > @@ -38,8 +38,7 @@
> >   */
> >         .macro  dcache_line_size, reg, tmp
> >         mrs     \tmp, ctr_el0                   // read CTR
> > -       lsr     \tmp, \tmp, #16
> > -       and     \tmp, \tmp, #0xf                // cache line size encoding
> > +       ubfm    \tmp, \tmp, #0x16, 0x19         // cache line size encoding
> 
> Even if the # is optional for immediates these days, perhaps it is
> better to adhere to a single style in one line?

In ARM assembler syntax, the preferred prefix for decimal immediates is '#'
not 0x ;)

Will

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Jingoo Han]

> -----Original Message-----
> From: Will Deacon [mailto:will.deacon at arm.com]
> Sent: Friday, January 17, 2014 8:13 PM
> To: Jingoo Han
> Cc: Catalin Marinas; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] arm64: mm: use ubfm for dcache_line_size
> 
> On Fri, Jan 17, 2014 at 08:04:32AM +0000, Jingoo Han wrote:
> > Use 'ubfm' for the bitfield move instruction; thus, single
> > instruction can be used instead of two instructions, when
> > getting the minimum D-cache line size from CTR_EL0 register.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  arch/arm64/mm/proc-macros.S |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> > index 8957b82..c31f41e 100644
> > --- a/arch/arm64/mm/proc-macros.S
> > +++ b/arch/arm64/mm/proc-macros.S
> > @@ -38,8 +38,7 @@
> >   */
> >  	.macro	dcache_line_size, reg, tmp
> >  	mrs	\tmp, ctr_el0			// read CTR
> > -	lsr	\tmp, \tmp, #16
> > -	and	\tmp, \tmp, #0xf		// cache line size encoding
> > +	ubfm	\tmp, \tmp, #0x16, 0x19		// cache line size encoding
> 
> 0x16 and 0x19. Are you sure?

(+cc Ard Biesheuvel)

Oh, it's my mistake.
CTR_EL0[19:16] is a proper bit-field; it means 'DminLine'.
So, I will fit it as below:

ubfm    \tmp, \tmp, #16, #19

> 
> You can also grep for other occurences of this pattern and change
> those too (pgtable stuff in head.S, clidr in cache.S).

OK, I will change others of this pattern.
I appreciate your comments. :-)

Best regards,
Jingoo Han

[PATCH] arm64: mm: use ubfm for dcache_line_size

[Jingoo Han]

On Friday, January 17, 2014 8:13 PM, Will Deacon wrote:
> On Fri, Jan 17, 2014 at 08:04:32AM +0000, Jingoo Han wrote:
> > Use 'ubfm' for the bitfield move instruction; thus, single
> > instruction can be used instead of two instructions, when
> > getting the minimum D-cache line size from CTR_EL0 register.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  arch/arm64/mm/proc-macros.S |    3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/mm/proc-macros.S b/arch/arm64/mm/proc-macros.S
> > index 8957b82..c31f41e 100644
> > --- a/arch/arm64/mm/proc-macros.S
> > +++ b/arch/arm64/mm/proc-macros.S
> > @@ -38,8 +38,7 @@
> >   */
> >  	.macro	dcache_line_size, reg, tmp
> >  	mrs	\tmp, ctr_el0			// read CTR
> > -	lsr	\tmp, \tmp, #16
> > -	and	\tmp, \tmp, #0xf		// cache line size encoding
> > +	ubfm	\tmp, \tmp, #0x16, 0x19		// cache line size encoding
> 
> 0x16 and 0x19. Are you sure? 
> 
> You can also grep for other occurences of this pattern and change those too
> (pgtable stuff in head.S, clidr in cache.S).

I will not change those things, due to the following reasons.
So, I will send v3 patch that fixes only dcache_line_size in
proc-macros.S.

1. pgtable stuff in head.S
'ubfx' can be used instead of 'ubfm'. Also, additional definition
such as 'PGDIR_WIDTH' is necessary.

./arch/arm64/kernel/head.S
@@ -371,8 +371,7 @@ ENDPROC(__calc_phys_offset)
  * Corrupts:   tmp1, tmp2
  */
        .macro  create_pgd_entry, pgd, tbl, virt, tmp1, tmp2
-       lsr     \tmp1, \virt, #PGDIR_SHIFT
-       and     \tmp1, \tmp1, #PTRS_PER_PGD - 1 // PGD index
+       ubfx    \tmp1, \virt, #PGDIR_SHIFT, #PGDIR_WIDTH        // PGD index

./arch/arm64/include/asm/pgtable-2level-hwdef.h
@@ -32,6 +32,7 @@
+#define PGDIR_WIDTH            13

./arch/arm64/include/asm/pgtable-3level-hwdef.h
@@ -32,6 +32,7 @@
+#define PGDIR_WIDTH            9


2. clidr in cache.S
'ubfm' can be used; however, additional 'lsl' is necessary.

ENTRY(__flush_dcache_all)
        dsb     sy                              // ensure ordering with previous memory accesses
        mrs     x0, clidr_el1                   // read clidr
-       and     x3, x0, #0x7000000              // extract loc from clidr
-       lsr     x3, x3, #23                     // left align loc bit field
+       ubfm    x3, x0, #24, #26
+       lsl     x3, x3, #1
        cbz     x3, finished                    // if loc is 0, then no need to clean
        mov     x10, #0                         // start clean at cache level 0

Best regards,
Jingoo Han