[U-Boot] [PATCH v7 1/2] armv8: Support loading 32-bit OS in AArch32 execution state

[Ryan Harkin]

Hi Alison,

On 7 November 2016 at 02:21, Alison Wang <alison.wang at 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