8xx: avoid icbi misbehaviour in __flush_dcache_icache_phys
PATCH 1/1 8xx: avoid icbi misbehaviour in __flush_dcache_icache_phys
Anton,
Can you please test the following which is equivalent to your=20 suggest changes, the only difference being the location, contained=20 inside flush_dcache_icache_page(). After confirmation that it works we can send up to the kernel maintainers.
We should still try to understand why this is happening, possibly=20 matching it to a published CPU errata bug, or inform Freescale otherwise.
I've written that entry down in the 8xx TODO list.
Thanks!
-------------
On 8xx, in the case where a pagefault happens for a process who's not the owner of the vma in question (ptrace for instance), the flush=20 operation is performed via the physical address.
Unfortunately, that results in a strange, unexplainable "icbi"=20 instruction fault, most likely due to a CPU bug (see oops below).
Avoid that by flushing the page via its kernel virtual address.=20
Oops: kernel access of bad area, sig: 11 [#2] NIP: C000543C LR: C000B060 SP: C0F35DF0 REGS: c0f35d40 TRAP: 0300 Not tai=
nted
MSR: 00009022 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 10
DAR: 00000010, DSISR: C2000000
TASK =3D c0ea8430[761] 'gdbserver' THREAD: c0f34000
Last syscall: 26
GPR00: 00009022 C0F35DF0 C0EA8430 00F59000 00000100 FFFFFFFF 00F58000 00000001
GPR08: C021DAEF C0270000 00009032 C0270000 22044024 10025428 01000800 00000001
GPR16: 007FFF3F 00000001 00000000 7FBC6AC0 00F61022 00000001 C0839300 C01E0000
GPR24: 00CD0889 C082F568 3000AC18 C02A7A00 C0EA15C8 00F588A9 C02ACB00 C02ACB00
NIP [c000543c] __flush_dcache_icache_phys+0x38/0x54
LR [c000b060] flush_dcache_icache_page+0x20/0x30
Call trace: [c000b154] update_mmu_cache+0x7c/0xa4 [c005ae98] do_wp_page+0x460/0x5ec [c005c8a0] handle_mm_fault+0x7cc/0x91c [c005ccec] get_user_pages+0x2fc/0x65c [c0027104] access_process_vm+0x9c/0x1d4 [c00076e0] sys_ptrace+0x240/0x4a4 [c0002bd0] ret_from_syscall+0x0/0x44
From: Anton W=F6llert <[email protected]>=20
arch/ppc/mm/init.c renamed from arch/ppc/mm/init.c.orig +3 -0
@@ -577,6 +577,9 @@#ifdef CONFIG_BOOKE __flush_dcache_icache (kmap (page));kunmap (page); +#elif CONFIG_8xx +/* On 8xx there is no need to kmap since highmem is not supported */ + __flush_dcache_icache(page_address(page));=20 #else__flush_dcache_icache_phys (page_to_pfn (page) <<PAGE_SHIFT );#endif
Quoted 2 lines · Marcelo Tosatti
Can you please test the following which is equivalent to your suggest changes, the only difference being the location, contained inside
flush_dcache_icache_page(). After confirmation that it works we can send up to the kernel maintainers.
of course, i can do that :).
Quoted 2 lines
We should still try to understand why this is happening, possibly matching it to a published CPU errata bug, or inform Freescale otherwise.
i think so too. as far as i understand it, the thing with the previous __flush_dcache_icache&tlbie patch was that __flush_dcache_icache should try to call dcbst on every address on a cache-line-boundary that could reference the page to give the cpu a hint, so that the next write-access to this page will be more effective, right? but while trying to do the dcbst, the TLB is looked up for the addresses for a faster translation. but here the old TLB-Entry exists, that says invalid Page or something like that, right? So it should be invalidated before, right?
Question, normally the dcbst should cause a TLB-Error-Exception, that
than should do reload the TLB-Entry automatically, doesn't this work because of the dcb*-errata (btw. where can i found that?). And the __flush_dcache_icache should do a dcbst 4096/16=256 times, but there are just 64 cache-lines à 16Byte (1Kb DCache), so shouldn't be 3/4 of that routine redundant? and also, as it seems to me, the whole dcache will be *flushed* or better dcbst'd trough this routine. is that right?
please correct me!
so, now the __flush_dcache_icache_phys(). this is just called, when the memory-mapping of the vma is not same as the one of that from the current process. this should be the case with ptrace. couldn't now be done the same as above? maybe no because the address is the virtual address as seen from the ptraced process? and so that will *flush* the pages from the ptraced process (that doesn't exists yet?, because they were just created for the tracing process?). so the pte, that was created for the tracing process is used. from that, the physical address is calculated (how is this done, whats the magic about the pfn-stuff etc, i didn't really understand that :( ). now __flush_dcache_icache_phys is used, because the [d|i]cache uses physical address tags and so the data-mmu could be turned off to flush these entries.
so to understand the things further, i have to understand which addresses are used with dcbst and icbi. there is page=pfn_to_page(pte_pfn(pte)) and (page_to_pfn(page) << PAGE_SHIFT). but what are these values or where do they point. any hints to the pfn&pte stuff? unfortunately i didn't have much time the last days to understand this more deeply...
Quoted 5 lines
On 8xx, in the case where a pagefault happens for a process who's not
the owner of the vma in question (ptrace for instance), the flush operation is performed via the physical address.Unfortunately, that results in a strange, unexplainable "icbi" instruction fault, most likely due to a CPU bug (see oops below).
where do you know, that "icbi" is the bad?
Anton
Hi Anton,
Quoted 8 lines · On Thu, Jul 21, 2005 at 01:34:53AM +0200, Anton W=F6llert wrote:
Quoted 4 lines · Marcelo Tosatti
Can you please test the following which is equivalent to your=20 suggest changes, the only difference being the location, contained=20 inside
flush_dcache_icache_page(). After confirmation that it works we can send up to the kernel maintainers.=20
of course, i can do that :).
=20
Quoted 6 lines
Quoted 3 lines
We should still try to understand why this is happening, possibly=20 matching it to a published CPU errata bug, or inform Freescale otherwise.
=20
i think so too. as far as i understand it, the thing with the previous=20 __flush_dcache_icache&tlbie patch was that__flush_dcache_icacheshould=
=20
Quoted 2 lines
try to call dcbst on every address on a cache-line-boundary that could=20 reference the page to give the cpu a hint, so that the next write-acces=
s=20
Quoted 2 lines
to this page will be more effective, right? but while trying to do the=20 dcbst, the TLB is looked up for the addresses for a faster translation.=
=20
Quoted 2 lines
but here the old TLB-Entry exists, that says invalid Page or something=20 like that, right? So it should be invalidated before, right?
Actually the dcbst instruction flushes data from cache to main memory. Quoting MPC860UM.pdf:
Data Cache Block Store (dcbst)
The effective address is computed, translated, and checked for protection violations as defined in the PowerPC architecture. This instruction is=20 treated as a load with respect to address translation and memory protecti= on.
If the address hits in the cache and the cache block is in the unmodified-valid state, no action is taken. If the address hits in the ca= che
and the cache block is in the modified-valid state, the modified block is= =20
written back to memory and the cache block is placed in the unmodified-va= lid
state.
Quoted 2 lines
Question, normally the dcbst should cause a TLB-Error-Exception,=20 that than should do reload the TLB-Entry automatically, doesn't this w=
ork=20
Quoted 1 line
because of the dcb*-errata (btw. where can i found that?).=20
I dont think that this one bug which the TLB invalidate worksaround, name= ly=20
dcbst misbehaviour with invalid (zeroed) TLB, is covered by Freescale errata (I'm looking at Errata revision A.2).
Quoted 1 line
And the
__flush_dcache_icacheshould do a dcbst 4096/16=3D256 times, bu=
t there are=20
Quoted 1 line
just 64 cache-lines =E0 16Byte (1Kb DCache), so shouldn't be 3/4 of tha=
t=20
Quoted 1 line
routine redundant? and also, as it seems to me, the whole dcache will b=
e=20
Quoted 2 lines
*flushed* or better dcbst'd trough this routine. is that right? please correct me!
See the dcbst description above, the cache line entry is only flushed if the address hits in cache _and_ is the modified state.
Quoted 1 line
so, now the
__flush_dcache_icache_phys(). this is just called, when the=
=20
Quoted 6 lines
memory-mapping of the vma is not same as the one of that from the=20 current process. this should be the case with ptrace. couldn't now be=20 done the same as above? maybe no because the address is the virtual=20 address as seen from the ptraced process? and so that will *flush* the=20 pages from the ptraced process (that doesn't exists yet?, because they=20 were just created for the tracing process?).=20
Since the 8xx cache is physically indexed and tagged, you can flush memor= y
by either its virtual address or its physical address - virtual accesses=20 need to perform v->p translation with help from the MMU.
On the ptrace case the kernel must do a physical flush because the ptrace= ing=20
process does not have an equivalent virtual->physical mapping in its own=20 address space. ie. equivalent virtual addresses from the ptracing and=20 ptrace'd processes are obviously not physically equivalent.=20
The change you proposed flushes the page via its kernel virtual map:=20 page_address(page), which is similar to doing a physical flush. Only=20 difference being that on 8xx "icbi" goes nuts when using the physical=20 flush, as you reported.
Quoted 1 line
so the pte, that was created for the tracing process is used. from that=
,=20
Quoted 2 lines
the physical address is calculated (how is this done, whats the magic=20 about the pfn-stuff etc, i didn't really understand that :( ).
Actually the page being faulted in by the ptrace'ing process is a page of the _ptraced_ process, that is, one process is faulting in pages=20 of another process.
Quoted 1 line
now
__flush_dcache_icache_physis used, because the [d|i]cache uses ph=
ysical=20
Quoted 1 line
address tags and so the data-mmu could be turned off to flush these ent=
ries.
Quoted 3 lines
so to understand the things further, i have to understand which=20 addresses are used with dcbst and icbi. there is=20 page=3Dpfn_to_page(pte_pfn(pte)) and (
page_to_pfn(page)<< PAGE_SHIFT).=
=20
Quoted 3 lines
but what are these values or where do they point. any hints to the=20 pfn&pte stuff? unfortunately i didn't have much time the last days to=20 understand this more deeply...=20
PFN =3D Page Frame Number. This value is the page identifier by page size=
units.=20
page_to_pfn(page) << PAGE_SHIFT means:
Get the pfn of the page and multiply it by page size ("<< PAGE_SHIFT").=20
Returns address of page in bytes, not in page units.
Quoted 8 lines
Quoted 6 lines
On 8xx, in the case where a pagefault happens for a process who's not the owner of the vma in question (ptrace for instance), the flush=20 operation is performed via the physical address.
Unfortunately, that results in a strange, unexplainable "icbi"=20 instruction fault, most likely due to a CPU bug (see oops below).
=20
where do you know, that "icbi" is the bad?
What instruction resides at __flush_dcache_icache_phys+0x38 ?=20
Quoted 1 line
From what I could see that is "icbi".