Hi Alison, On 7 November 2016 at 02:21, Alison Wang <alison.w...@nxp.com> wrote: >> 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. >
That sounds perfect. > If you all agree with this re-order, I will send out the v8 patch includes the > first, second and third patches. > I haven't tried to understand this code, so I can't say if that order will work or not, but I'm very happy to test it if you produce a series like this. Regards, Ryan. > > Best Regards, > Alison Wang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot