[PATCH] ARM: mvebu: drop pointless check for coherency_base

[PATCH] ARM: mvebu: drop pointless check for coherency_base

[Ard Biesheuvel]

The MMU off code path in ll_get_coherency_base() attempts to decide
whether the coherency fabric is mapped by testing the value of
coherency_base, which carries its virtual address if its mapped, and
0x0 otherwise.

However, what the code actually does is take the virtual address of
the coherency_base symbol, and compare it with 0x0, which are never
equal, and so the branch is never taken. In fact, with the MMU off,
dereferencing the VA of coherency_base is not possible to begin with,
nor can its value be relied upon with the MMU off since it is not
cleaned to the Dcache as is done with coherency_phys_base in
armada_370_coherency_init().

Instead, the value of coherency_phys_base is returned, which results
in the correct behavior since it will be 0x0 as well if the coherency
fabric is not mapped, and it is accessible with the MMU off. So just
drop the comparison and the branch.

Fixes: 30cdef97107370a7 ("ARM: mvebu: make the coherency_ll.S functions work with no coherency fabric")
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
Found by accident. Build tested only.

 arch/arm/mach-mvebu/coherency_ll.S | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
index 2d962fe48821..a3a64bf97250 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -35,13 +35,8 @@ ENTRY(ll_get_coherency_base)
 
 	/*
 	 * MMU is disabled, use the physical address of the coherency
-	 * base address. However, if the coherency fabric isn't mapped
-	 * (i.e its virtual address is zero), it means coherency is
-	 * not enabled, so we return 0.
+	 * base address, (or 0x0 if the coherency fabric is not mapped)
 	 */
-	ldr	r1, =coherency_base
-	cmp	r1, #0
-	beq	2f
 	adr	r1, 3f
 	ldr	r3, [r1]
 	ldr	r1, [r1, r3]
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: mvebu: drop pointless check for coherency_base

[Gregory CLEMENT]

Hi Ard,

> The MMU off code path in ll_get_coherency_base() attempts to decide
> whether the coherency fabric is mapped by testing the value of
> coherency_base, which carries its virtual address if its mapped, and
> 0x0 otherwise.
>
> However, what the code actually does is take the virtual address of
> the coherency_base symbol, and compare it with 0x0, which are never
> equal, and so the branch is never taken. In fact, with the MMU off,
> dereferencing the VA of coherency_base is not possible to begin with,
> nor can its value be relied upon with the MMU off since it is not
> cleaned to the Dcache as is done with coherency_phys_base in
> armada_370_coherency_init().
>
> Instead, the value of coherency_phys_base is returned, which results
> in the correct behavior since it will be 0x0 as well if the coherency
> fabric is not mapped, and it is accessible with the MMU off. So just
> drop the comparison and the branch.

Thanks for this patch. I needed to refresh my memories about this
subject and I agree with your explanation. I applied it on mvebu/fixes.

Gregory

>
> Fixes: 30cdef97107370a7 ("ARM: mvebu: make the coherency_ll.S functions work with no coherency fabric")
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory CLEMENT <gregory.clement@bootlin.com>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> Found by accident. Build tested only.
>
>  arch/arm/mach-mvebu/coherency_ll.S | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/coherency_ll.S b/arch/arm/mach-mvebu/coherency_ll.S
> index 2d962fe48821..a3a64bf97250 100644
> --- a/arch/arm/mach-mvebu/coherency_ll.S
> +++ b/arch/arm/mach-mvebu/coherency_ll.S
> @@ -35,13 +35,8 @@ ENTRY(ll_get_coherency_base)
>  
>  	/*
>  	 * MMU is disabled, use the physical address of the coherency
> -	 * base address. However, if the coherency fabric isn't mapped
> -	 * (i.e its virtual address is zero), it means coherency is
> -	 * not enabled, so we return 0.
> +	 * base address, (or 0x0 if the coherency fabric is not mapped)
>  	 */
> -	ldr	r1, =coherency_base
> -	cmp	r1, #0
> -	beq	2f
>  	adr	r1, 3f
>  	ldr	r3, [r1]
>  	ldr	r1, [r1, r3]
> -- 
> 2.17.1
>

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel