[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Vinayak Kale]

Add DSB after icache flush operation.

Signed-off-by: Vinayak Kale <vkale@apm.com>
---
 arch/arm64/include/asm/cacheflush.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index fea9ee3..88932498 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
 static inline void __flush_icache_all(void)
 {
 	asm("ic	ialluis");
+	dsb();
 }
 
 #define flush_dcache_mmap_lock(mapping) \
-- 
1.7.9.5

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Will Deacon]

Hi Vinayak,

On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush operation.

Please elaborate a bit on what this achieves (i.e. completion of the
maintenance operation).

> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
>  arch/arm64/include/asm/cacheflush.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
> index fea9ee3..88932498 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
>  static inline void __flush_icache_all(void)
>  {
>  	asm("ic	ialluis");

This needs a "memory" clobber to prevent re-ordering by GCC. We should
probably check the rest of the code for other occurrences of this too.

> +	dsb();

Can you make a corresponding change for arch/arm/ too, please? I think we're
missing the barrier there as well.

Will

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Catalin Marinas]

On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush operation.
> 
> Signed-off-by: Vinayak Kale <vkale@apm.com>

I think we should also mention that this function is used for user
addresses and an ISB is not required because of an exception return
before executing user instructions.

-- 
Catalin

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Vinayak Kale]

On Mon, Jan 27, 2014 at 5:46 PM, Will Deacon <will.deacon@arm.com> wrote:
> Hi Vinayak,
>
> On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
>> Add DSB after icache flush operation.
>
> Please elaborate a bit on what this achieves (i.e. completion of the
> maintenance operation).

Okay.

>
>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>> ---
>>  arch/arm64/include/asm/cacheflush.h |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
>> index fea9ee3..88932498 100644
>> --- a/arch/arm64/include/asm/cacheflush.h
>> +++ b/arch/arm64/include/asm/cacheflush.h
>> @@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
>>  static inline void __flush_icache_all(void)
>>  {
>>       asm("ic ialluis");
>
> This needs a "memory" clobber to prevent re-ordering by GCC. We should
> probably check the rest of the code for other occurrences of this too.

Okay.

>
>> +     dsb();
>
> Can you make a corresponding change for arch/arm/ too, please? I think we're
> missing the barrier there as well.

I could have, but I don't have hardware to test it on.

>
> Will

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Vinayak Kale]

On Mon, Jan 27, 2014 at 5:55 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
>> Add DSB after icache flush operation.
>>
>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>
> I think we should also mention that this function is used for user
> addresses and an ISB is not required because of an exception return
> before executing user instructions.
Okay, will mention this. Thanks.
>
> --
> Catalin

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Vinayak Kale]

Add DSB after icache flush to complete the cache maintenance operation.
The function __flush_icache_all() is used only for user space mappings
and an ISB is not required because of an exception return before executing
user instructions. An exception return would behave like an ISB.

Signed-off-by: Vinayak Kale <vkale@apm.com>
---
 arch/arm64/include/asm/cacheflush.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index fea9ee3..88932498 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
 static inline void __flush_icache_all(void)
 {
 	asm("ic	ialluis");
+	dsb();
 }
 
 #define flush_dcache_mmap_lock(mapping) \
-- 
1.7.9.5

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Will Deacon]

On Wed, Feb 05, 2014 at 09:34:36AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush to complete the cache maintenance operation.
> The function __flush_icache_all() is used only for user space mappings
> and an ISB is not required because of an exception return before executing
> user instructions. An exception return would behave like an ISB.
> 
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
>  arch/arm64/include/asm/cacheflush.h |    1 +
>  1 file changed, 1 insertion(+)

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH] arm64: add DSB after icache flush in __flush_icache_all()

[Catalin Marinas]

On Wed, Feb 05, 2014 at 09:34:36AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush to complete the cache maintenance operation.
> The function __flush_icache_all() is used only for user space mappings
> and an ISB is not required because of an exception return before executing
> user instructions. An exception return would behave like an ISB.
> 
> Signed-off-by: Vinayak Kale <vkale@apm.com>

Thanks. Applied.

-- 
Catalin