[Q] L1_CACHE_BYTES on flush_pfn_alias function.

[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:js07.lee@samsung.com] 
Sent: Tuesday, January 14, 2014 10:43 PM
To: 'catalin.marinas@arm.com'; 'linux-arm-kernel@lists.infradead.org'
Cc: 'linux@arm.linux.org.uk'
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?

Re: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

[Catalin Marinas]

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

RE: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

[Jungseung Lee]

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:catalin.marinas@arm.com] 
Sent: Saturday, January 25, 2014 12:43 AM
Cc: linux-arm-kernel@lists.infradead.org; linux@arm.linux.org.uk; linux-kernel@vger.kernel.org
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

Re: [Q] L1_CACHE_BYTES on flush_pfn_alias function.

[Catalin Marinas]

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