On 11/04/2016 10:12 AM, Alexander Graf wrote:
On 04/11/2016 17:08, york sun wrote:
On 11/04/2016 09:53 AM, Alexander Graf wrote:
On 04/11/2016 16:43, york sun wrote:
On 11/04/2016 09:32 AM, Ryan Harkin wrote:
>> >> Yes, with the attached patch on top of your original 2 patches, >> everything works again. I tested on FVP Foundation and AEMv8 >> models and Juno R0, R1 and R2. >> >> I don't think it would be good to stack these three patches the >> way they are presented in the upstream tree because it would
not
>> be bisect-able. Some re-work or re-ordering would be needed. >> >> Note: I haven't attempted to understand what any of this code
is
>> doing, I'm just testing it with my standard boot flow to make >> sure nothing is broken for me. > > Ryan, > > I support Alison's patch order for her 32-bit patch sets. This > feature doesn't exist before her first set. It is functional if > you run U-Boot at EL3 after the first patch.
Which I don't do. I follow the boot flow recommended by ARM and it doesn't work for that setup, which I don't think is the right thing to do.
> It gets EL2 working after the 2nd set. If there is room to > clarify in the commit message, please kindly suggest. >
Well, I'm not the maintainer of the tree, but I wouldn't want to have a tree that wasn't bootable at any point in the patch
sequence.
That's generally unacceptable on most projects I work on.
Keeping
the tree bisect-able to prove which commit caused a problem is considered to be a valuable tool.
Ryan,
Thanks for sharing your concern. I support git-bisect. It is valuable, no doubt. Let me try to understand the issue here. Without Alison's patches, everything boots OK. With her first set,
does something break?
Yes, with the patches booting 64bit Linux with U-Boot running in
EL2
breaks according to Ryan.
My understanding is 32-bit OS can boot. If existing 64-bit OS fails, then she needs to fix it.
That's his point :). And I concur.
Thanks for the confirmation.
(btw, you guys really should start thinking about following the ARM recommended boot model. It's pretty cumbersome to do everything different just for NXP)
If you are referring the trusted firmware, we are following that direction. Just not fully up yet on some platform.
It is definitely not our intention to be cumbersome. Please point
out
where it went sideway beside the trusted firmware.
Basically it boils down to the fact that you are the only platform that runs U-Boot in EL3 :).
If you want to keep the memory initialization inside of U-Boot, I think that's great. But you could either split that into SPL/EL2 or degrade yourself into EL2 as soon as possible by calling into an EL3
firmware.
Whether you build that firmware as part of U-Boot (the stock one
could
be very trivial) or externally is not really too much of a problem.
Things like Alison's patches could then do a simple PSCI call into said EL3 firmware to call into 32bit code in EL2 for example.
Basically I agree with you. U-Boot will run at EL2 as soon as we have the trusted firmware in place.
[Alison Wang] Thanks for all your comments.
For the issue about the tree would not be bisect-able, I have a solution. Actually it is the root cause that 64-bit kernel could not boot up when U-Boot is running in EL2. I will move these codes from the third patch to the first patch.
ENTRY(armv8_switch_to_el2) switch_el x5, 1f, 0f, 0f -0: ret + /* + * x3 is kernel entry point or switch_to_el1 + * if CONFIG_ARMV8_SWITCH_TO_EL1 is defined. + * When running in EL2 now, jump to the + * address saved in x3. + */ +0: br x3 1: armv8_switch_to_el2_m x3, x4, x5 ENDPROC(armv8_switch_to_el2)
ENTRY(armv8_switch_to_el1) switch_el x5, 0f, 1f, 0f -0: ret + + /* + * x3 is kernel entry point. When running in EL1 + * now, jump to the address saved in x3. + */ +0: br x3 1: armv8_switch_to_el1_m x3, x4, x5 ENDPROC(armv8_switch_to_el1)
With this re-order, the bitsect issue will be fixed and there is not a point that kernel could not boot up.
If you all agree with this re-order, I will send out the v8 patch includes the first, second and third patches.
Best Regards, Alison Wang