Re: [UPDATED] [PATCH v4 3/7] [ppc] Process dynamic relocations for kernel
flat view
From: Segher Boessenkool <hidden>
Date: 2011-12-10 20:02:47
Hi Suzuki, Looks quite good, a few comments...
quoted hunk ↗ jump to hunk
+get_type: + /* r4 holds the relocation type */ + extrwi r4, r4, 8, 24 /* r4 = ((char*)r4)[3] */
This comment is confusing (only makes sense together with the lwz a long way up).
quoted hunk ↗ jump to hunk
+nxtrela: + /* + * We have to flush the modified instructions to the + * main storage from the d-cache. And also, invalidate the + * cached instructions in i-cache which has been modified. + * + * We delay the msync / isync operation till the end, since + * we won't be executing the modified instructions until + * we return from here. + */ + dcbst r4,r7 + icbi r4,r7
You still need a sync between these two. Without it, the icbi can complete before the dcbst for the same address does, which leaves room for an instruction fetch from that address to get old data.
quoted hunk ↗ jump to hunk
+ cmpwi r8, 0 /* relasz = 0 ? */ + ble done + add r9, r9, r6 /* move to next entry in the .rela table */ + subf r8, r6, r8 /* relasz -= relaent */ + b applyrela + +done: + msync /* Wait for the flush to finish */
The instruction is called "sync". msync is a BookE thing.
quoted hunk ↗ jump to hunk
next if (/R_PPC64_RELATIVE/ or /R_PPC64_NONE/ or /R_PPC64_ADDR64\s+mach_/); + next if (/R_PPC_ADDR16_LO/ or /R_PPC_ADDR16_HI/ or + /R_PPC_ADDR16_HA/ or /R_PPC_RELATIVE/);
Nothing new, but these should probably have \b or \s or just a space on each side. Segher