On 12/11/11 01:32, Segher Boessenkool wrote:
Hi Suzuki,

Looks quite good, a few comments...

+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).

Agree, will fix them.

+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.

Ok.
+ 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.

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.
Will fix this too. Also will include the R_PPC_NONE to the list
of valid relocations.

Thanks
Suzuki




Segher

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to