2014-01-17 09:54:48

by Jungseung Lee

[permalink] [raw]
Subject: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

Hi,

Follow the mailing-list
http://comments.gmane.org/gmane.linux.ports.arm.omap/31686

>>Setting the L1 cache line size larger than it actually is should be safe.

the written code expected as L1_CACHE_BYTES should be real cache line size
has bug.
It looks like that flush_pfn_alias function should be fixed.

Anybody to have another opinion?

Cheers,
JS

-----Original Message-----
From: ?????? [mailto:[email protected]]
Sent: Tuesday, January 14, 2014 10:43 PM
To: '[email protected]'; '[email protected]'
Cc: '[email protected]'
Subject: Question on flush_pfn_alias function.

Dear Catalin,

I found below function and that clean and invalidate data cache range with
"mcrr"
The end address is end of page - L1_CACHE_BYTES (e.g. 32 , 64)

+static void flush_pfn_alias(unsigned long pfn, unsigned long vaddr) {
+ unsigned long to = ALIAS_FLUSH_START + (CACHE_COLOUR(vaddr) <<
+PAGE_SHIFT);
+
+ set_pte(TOP_PTE(to), pfn_pte(pfn, PAGE_KERNEL));
+ flush_tlb_kernel_page(to);
+
+ asm( "mcrr p15, 0, %1, %0, c14\n"
+ " mcrr p15, 0, %1, %0, c5\n"
+ :
+ : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES)
+ : "cc");
+}

However, follow the mail and current setting in vanilla kernel,
L1_CACHE_BYTES of Cortex A9 will be 64 not 32.
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/183316.html
I think that could be problem.

What is your opinion?


2014-01-24 15:43:56

by Catalin Marinas

[permalink] [raw]
Subject: Re: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

On Fri, Jan 17, 2014 at 09:54:42AM +0000, ������ wrote:
> Follow the mailing-list
> http://comments.gmane.org/gmane.linux.ports.arm.omap/31686
>
> >>Setting the L1 cache line size larger than it actually is should be safe.
>
> the written code expected as L1_CACHE_BYTES should be real cache line size
> has bug.
> It looks like that flush_pfn_alias function should be fixed.

Did you actually notice any problem with flushing some more bytes? It's
a clean+invalidate rather than invalidate, I don't see any problem.

--
Catalin

2014-01-26 05:13:47

by Jungseung Lee

[permalink] [raw]
Subject: RE: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

Not to flush some more bytes. In the scenario, they can *omit* to flush last 32 bytes.

L1_CACHE_BYTES = 64 (ARM v7, CA9)

asm( "mcrr p15, 0, %1, %0, c14\n"
" mcr p15, 0, %2, c7, c10, 4"
:
: "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
: "cc");
-----Original Message-----
From: Catalin Marinas [mailto:[email protected]]
Sent: Saturday, January 25, 2014 12:43 AM
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

On Fri, Jan 17, 2014 at 09:54:42AM +0000, wrote:
> Follow the mailing-list
> http://comments.gmane.org/gmane.linux.ports.arm.omap/31686
>
> >>Setting the L1 cache line size larger than it actually is should be safe.
>
> the written code expected as L1_CACHE_BYTES should be real cache line
> size has bug.
> It looks like that flush_pfn_alias function should be fixed.

Did you actually notice any problem with flushing some more bytes? It's a clean+invalidate rather than invalidate, I don't see any problem.

--
Catalin

2014-01-27 16:44:13

by Catalin Marinas

[permalink] [raw]
Subject: Re: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

Please do not top-post.

On Sun, Jan 26, 2014 at 05:13:43AM +0000, Jungseung Lee wrote:
> Not to flush some more bytes. In the scenario, they can *omit* to flush last 32 bytes.
>
> L1_CACHE_BYTES = 64 (ARM v7, CA9)
>
> asm( "mcrr p15, 0, %1, %0, c14\n"
> " mcr p15, 0, %2, c7, c10, 4"
> :
> : "r" (to), "r" (to + PAGE_SIZE - L1_CACHE_BYTES), "r" (zero)
> : "cc");

Ah, I got it now. I think this should be (to + PAGE_SIZE - 1). My
reading of the ARM ARM is that the bottom bits of the address are
ignored by mcrr.

--
Catalin