From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Fri, 19 Jul 2019 20:36:19 +0200 Subject: [U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi In-Reply-To: <7e8d03a8-e8a4-bd73-a329-cda5faf7e0a4@gmx.de> References: <20190208184650.22765-1-xypron.glpk@gmx.de> <20190208184650.22765-4-xypron.glpk@gmx.de> <20190718060016.GA73593@largo.jsg.id.au> <5438c1db9647d8f4@bloch.sibelius.xs4all.nl> <20190718091644.GA18133@largo.jsg.id.au> <94f74740-6541-cdba-ff0d-34b2de4f3aca@csgraf.de> <5438c3eaeb24c83d@bloch.sibelius.xs4all.nl> <1cbc0442-6b04-7d78-b29b-2a70f440dd7d@gmx.de> <20190719061405.GD66880@largo.jsg.id.au> <7e8d03a8-e8a4-bd73-a329-cda5faf7e0a4@gmx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On 7/19/19 7:52 PM, Heinrich Schuchardt wrote: > On 7/19/19 8:14 AM, Jonathan Gray wrote: >> On Fri, Jul 19, 2019 at 07:43:17AM +0200, Heinrich Schuchardt wrote: >>> On 7/19/19 1:07 AM, Mark Kettenis wrote: >>>>> From: Alexander Graf >>>>> Date: Thu, 18 Jul 2019 20:50:39 +0200 >>>>> >>>>> On 18.07.19 19:33, Heinrich Schuchardt wrote: >>>>>> On 7/18/19 11:16 AM, Jonathan Gray wrote: >>>>>>> On Thu, Jul 18, 2019 at 10:39:57AM +0200, Mark Kettenis wrote: >>>>>>>>> Date: Thu, 18 Jul 2019 16:00:16 +1000 >>>>>>>>> From: Jonathan Gray >>>>>>>>> >>>>>>>>> On Fri, Feb 08, 2019 at 07:46:49PM +0100, Heinrich Schuchardt >>>>>>>>> wrote: >>>>>>>>>> Remove the duplicate code in efi_do_enter() and use >>>>>>>>>> efi_start_image() to >>>>>>>>>> start the image invoked by the bootefi command. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Heinrich Schuchardt >>>>>>>>>> --- >>>>>>>>>> v2 >>>>>>>>>>   ????????use EFI_CALL >>>>>>>>> >>>>>>>>> This commit broke booting OpenBSD/armv7 kernels on mx6cuboxi with >>>>>>>>> U-Boot >>>>>>>>> releases after 2019.01.?? 2019.04 works if this commit is >>>>>>>>> reverted. >>>>>>>>> With >>>>>>>>> 2019.07 there are conflicts trying to revert it and it is still >>>>>>>>> broken >>>>>>>>> as released. >>>>>>>>> >>>>>>>>> f69d63fae281ba98c3d063097cf4e95d17f3754d is the first bad commit >>>>>>>>> commit f69d63fae281ba98c3d063097cf4e95d17f3754d >>>>>>>>> Author: Heinrich Schuchardt >>>>>>>>> Date:???? Wed Dec 26 13:28:09 2018 +0100 >>>>>>>>> >>>>>>>>>   ???????? efi_loader: use efi_start_image() for bootefi >>>>>>>>> >>>>>>>>>   ???????? Remove the duplicate code in efi_do_enter() and use >>>>>>>>> efi_start_image() to >>>>>>>>>   ???????? start the image invoked by the bootefi command. >>>>>>>>> >>>>>>>>>   ???????? Signed-off-by: Heinrich Schuchardt >>>>>>>>> >>>>>>>>>   ?? cmd/bootefi.c???????????????????????????????? | 22 >>>>>>>>> +--------------------- >>>>>>>>>   ?? include/efi_loader.h?????????????????? |?? 4 ++++ >>>>>>>>>   ?? lib/efi_loader/efi_boottime.c |?? 6 +++--- >>>>>>>>>   ?? 3 files changed, 8 insertions(+), 24 deletions(-) >>>>>>>> >>>>>>>> Hi Jonathan, >>>>>>>> >>>>>>>> With this commit the OpenBSD bootloader (an EFI application) still >>>>>>>> boots, but the loaded OpenBSD kernel doesn't isn't it? >>>>>>> >>>>>>> Yes, when it fails the last thing on serial is: >>>>>>> >>>>>>> ## Starting EFI application at 12000000 ... >>>>>>>>> OpenBSD/armv7 BOOTARM 1.3 >>>>>>> boot> >>>>>>> booting sd0a:/bsd: 4572484+689312+238360+561608 >>>>>>> [298268+120+314400+278666]=0x0 >>>>>>> >>>>>>>> >>>>>>>> I bet the problem here is that efi_start_image() sets >>>>>>>> efi_is_direct_boot to false, which means that efi_exit_caches() >>>>>>>> which >>>>>>>> runs as a result of calling ExitBootServices() no longer >>>>>>>> flushes/disables the caches on 32-bit ARM. >>>>>>> >>>>>>> Indeed, removing 'efi_is_direct_boot = false;' from >>>>>>> efi_start_image() allows me to boot multiuser on cubox with 2019.07. >>>>> >>>>> >>>>> Heinrich asked me to clarify again in the mail thread. >>>>> >>>>> The UEFI spec says that caches on 32bit ARM are supposed to be >>>>> *enabled* >>>>> always. That means throughout boot time, as well as after >>>>> ExitBootServices(). So the behavior with efi_is_direct_boot = false is >>>>> the expected behavior according to the spec. >>>>> >>>>> The reason we have the hack above in the tree is that grub had a nasty >>>>> deficiency on 32bit ARM where it called into the Linux kernel using >>>>> the >>>>> legacy boot protocol which again expected caches to be disabled. >>>>> >>>>> The problem you're facing here sounds to me like a bug in the OpenBSD >>>>> loader that resembles the bug in grub. Instead of calling into a BSD >>>>> kernel code path that expects caches to already be enabled, it expects >>>>> to come up with caches disabled. >>>> >>>> I don't think we assume that the kernel is called with the >>>> architecturally defined caches disabled.  As far as I know, OpenBSD >>>> works fine with a recent U-Boot on Cortex-A7 systems that don't have >>>> an non-architectural L2 cache. >>>> >>>>> You *could* solve this generically by adding functionality akin to >>>>> clean_up_before_linux() inside the BSD loader. The downside of that is >>>>> that the code would be platform specific, as L2 caches used to be >>>>> enabled/disabled via MMIOs and not via architected registers in the >>>>> old >>>>> days. >>>> >>>> Messing with the non-architectural L2 cache is someting we defenitely >>>> don't want to do in the OpenBSD bootloader and kernel bootstrap code. >>>> We have a single bootloader/kernel that works on all supported armv7 >>>> systems! >>>> >>>> I quoted UEFI 2.5 upthread, but 2.8 retains the same text in section >>>> 2.3.5: >>>> >>>>     Implementations of boot services will enable architecturally >>>>     manageable caches and TLBs i.e., those that can be managed directly >>>>     using CP15 operations using mechanisms and procedures defined in >>>> the >>>>     ARM Architecture Reference Manual. They should not enable caches >>>>     requiring platform information to manage or invoke >>>> non-architectural >>>>     cache/TLB lockdown mechanisms >>>> >>>> So I'd say the problem here is that U-Boot passes control to the EFI >>>> payload with the L2 cache enabled. >>>> >>> >>> Hello Mark, >>> >>> your last observation concerned an i.MX6 board. Does this processor have >>> any cache that falls under the cited paragraph? A link to the processor >>> documentation would be helpful. >>> >>> Best regards >>> >>> Heinrich >> >> i.MX6 has a PL310 L2 cache which has it's own TRM from Arm. >> NXP now requires an account to download IMX6DQRM.pdf sadly. >> >> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0246c/DDI0246C_l2cc_pl310_r2p0_trm.pdf >> >> "The cache controller is configured using memory-mapped registers, >> rather than using CP15 instructions" >> >> u-boot/include/configs/mx6_common.h:#define CONFIG_SYS_L2_PL310 >> > > mx6cuboxi_defconfig has CONFIG_SYS_L2CACHE_OFF=N. > > Doesn't this mean that we conform to the UEFI specification? Jonathan can you, please, test booting OpenBSD with CONFIG_SYS_L2CACHE_OFF=Y. Best regards Heinrich