From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Gray Date: Fri, 19 Jul 2019 16:14:05 +1000 Subject: [U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi In-Reply-To: <1cbc0442-6b04-7d78-b29b-2a70f440dd7d@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> Message-ID: <20190719061405.GD66880@largo.jsg.id.au> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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