Hello Peng, (cutting a bit through the previous mails quoting)
> >This, in turn, leads to new questions: > > > >1. How is this PSCI code put in place? Is it bundled with the image, > > with a specificy copy routine which puts it in place then locks the > Yeah. > > memory range against writing? Or is it loaded in place by some other > > agent than U-Boot, and how does this agent know what to put where? > > In arch/arm/cpu/armv7/virt-v7.c, relocate_secure_section() does this job. > > CONFIG_ARMV7_SECURE_BASE is the location that secure code will be copied > to. To ARMV7, PSCI code is bundled with the uboot image. Hmm, the 'relocate' part of the function name is a somewhat non-optimal choice, since the routine just copies data without modifying it. Looking at relocate_secure_section(), I see that it it is called long after the U-Boot code was relocated -- actually, it is called during bootm. This means that for any one of the other boards around that seem to use PSCI, either "their" PSCI code does not have any relocations (which I doubt), or it has but they don't cause any data abort when done by the relocate_code() routine. So what's the difference between those and your board? My bet is that their CONFIG_ARMV7_SECURE_BASE is not write-protected at the time the relocate_code() routine executes (whereas on your board it is), and will be unlocked in some way by the time relocate_secure_section() executes. I'm not saying that the correct solution would be to enable write access to CONFIG_ARMV7_SECURE_BASE before running relocate_code(), because doing that would be obviously wrong: relocate_code() would try to fix code which is not there yet. I'm just saying that's what actually happens, and if I am right, then it confirms that the correct fix is to not let relocations to the secure stay in the U-Boot ELF, or better yet, to not let them happen to boot [pun half-intended]. > CONFIG_ARMV7_SECURE_BASE is the location that secure code will be copied > to. To ARMV7, PSCI code is bundled with the uboot image. > > > > >2. Does this code and the relocatable image refer to symbols of one > > another, > Yeah. There is asm function reference. You can see arch/arm/cpu/armv7/psci.S Looking at this file, I suspect that actually, PSCI is called through a software interrupt / exception vector, not through a function name, and the only symbolic relationship is via __secure_start and __secure_end -- and those must (and will) be correctly relocated along with the U-Boot image. > >or do they interact through an IPC independent of their > > respective location (in which case, one wonders why they are built > > into a single ELF file)? > > This comes a question, why PSCI framework intergated into U-Boot? I have no > idea. There could have been alternative designs, indeed. Here is the design now, so let's handle it. > >More generally... which target do you see this problem on? Reproducing > >the build myself would save both you and me some time, I guess. :) > > Build target: mx7dsabresd > > Needs to apply the three patches: > https://patchwork.ozlabs.org/patch/527040/ > https://patchwork.ozlabs.org/patch/527038/ > https://patchwork.ozlabs.org/patch/527039/ > > There is a small difference from my previous log, since my previous log use > 0x180000 as the secure address, but the patches use 0x900000 as the secure > address. But sure there will be relocation entry there. Thanks. I'll try and play with compiler / linker options, but from my own experience, this can be tricky. Meanwhile, I suggest you for for the not-too-quick-and-not-too-dirty linker script solution. > >Do you need these relocation records? If not, then just append the > >'*(.rel._secure*) input section spec (with a suitable comment) to > >the already existing DISCARD output section. By the way, doesn't this > >linker script change alone fix the data abort? > > Yeah. Since the relocation entry for secure section will be stored > in rel.secure section. And this section will not be touched in relocate.S Ok, so that's our first clean, linker-script-based, solution confirmed. > >Still, if there *are* relocation records emitted, that's because the > >toolchain considered them necessary -- IOW, it was (wrongly) told the > >PSCI code must be relocatable [or the code really is relocatable and we > >just haven't not hit a bug about this yet]. > > The PSCI code is bundled with the u-boot image. But it's running address > is not same with u-boot. In my example, u-boot is compiled with starting > address 0x87800000, but to PSCI, it's running address is 0x180000. > relocate_secure_section is just copy the psci code from uboot to 0x180000. > > compliation address is same with running address, to PSCI part. (I understand that. My question was not "why is PSCI built for its run-time address?" but "Since PSCI is built for its run-time address, and since there is little dependency on how come it is built as relocatable?") > >Testing every single relocation record target against __image_copy_start > >and __image_copy_end would be rather costly in the tight loop of the > >relocation routine, which should run as fast as possible. > > > >There is at least one fix to your problem which removes the out-of- > >bounds relocation records, and quite probably one better fix which > >prevents generating them in the first place. > > > >Therefore, instead of the currently proposed patch which increases the > >size (very slightly) and boot time (statistically in O(n) proportion) > >of the image, I prefer one of the two alternative solutions which > >decrease the image size (by removing useless relocation records) and > >leave the boot time unchanged (since the fix is at build time only). > > Agree. OK. So, for now, I suggest that you: - resubmit v2 of this series with patch 1/3 reworked into a linker script change rather than a relocate routine change (and collect the secure relocation records input sections into the DISCARD output section rather than into a purposeless non-discarded section); - make sure that you CC: the maintainers for the other boards which use PSCI and explicitly ask them to test the change on their boards and give their "Tested-by". It seems like the list of candidates for testing includes jetson-tk1, ls1021aqds, ls1021atwr and all sun[678]i boards, but I am by no means a PSCI specialist, so anyone feel free to correct me about this list. > Regards, > Peng. Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
