[PATCH] vexpress_ca9x4: Disable EFI_LOADER on this platform

[Tom Rini]

On Sat, Jul 12, 2025 at 01:56:45PM +0200, Mark Kettenis wrote:
> > Date: Sat, 12 Jul 2025 12:35:44 +0200
> > From: Mark Kettenis <mark.kettenis at xs4all.nl>
> > 
> > > From: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > Date: Sat, 12 Jul 2025 09:28:07 +0300
> > 
> > Hi Ilias, Heinrich and Tom,
> > 
> > As you probably know, OpenBSD/armv7 relies on EFI_LOAD, so disabling
> > EFI_LOADER on a platform means you can't boot OpenBSD on it.  Ilias
> > and I talked about this a while ago.  We talked about telling QEMU to
> > use an older version of the architecture that didn't do this alignment
> > check, but that may have been based on a misunderstanding of the
> > changes in QEMU.
> > 
> > At the time I didn't look any further into this, but I did now.  And
> > I'm confused because...
> > 
> > > Hi Heinrich
> > > 
> > > On Fri, 11 Jul 2025 at 22:35, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > > >
> > > > Am 11. Juli 2025 20:55:12 MESZ schrieb Tom Rini <trini at konsulko.com>:
> > > > >As part of upgrading to QEMU 10.0.0 we discovered that the EFI loader
> > > > >tests would no longer pass CI, on this specific platform, while still
> > > > >passing on real hardware. Upon further investigation it turns out that
> > > > >on ARMv7 we rely on some undefined behavior with respect to enabling
> > > > >unaligned access. And while this is seemingly fine on real hardware,
> > > > >QEMU is now correct enough in implementation to no longer allow this. In
> > > > >order to not rely on this undefined behavior we would need to implement
> > > > >some MMU configuration (and then ensure proper tear-down possibly).
> > > > >
> > > > >At the moment, the most reasonable path forward seems to be to document
> > > > >this and disable the support here.
> > > >
> > > > Where will we document the issue?
> > > 
> > > I think we can add it on the EFI documentation? IIRC we only enable
> > > the i-cache not the mmu + d-cache. I can double check and send a
> > > patch.
> > 
> > There is code in U-Boot to enable the MMU and the D-cache.  The code
> > turns on the MMU gets turned on when we turn on the D-cache, which
> > happens when we call dcache_enable().  That function typically gets
> > called from the board.c file and the one used by vexpress_ca9x4
> > doesn't seem to do that.  But other ARMv7 targets do.  For example in
> > arch/arm/mach-sunxi/board.c we have:
> > 
> > #if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) && defined(CONFIG_CPU_V7A)
> > void enable_caches(void)
> > {
> >         /* Enable D-cache. I-cache is already enabled in start.S */
> >         dcache_enable();
> > }
> > #endif
> > 
> > Note the CONFIG_CPU_V7A there.  This means that for the Allwinner
> > ARMv7 SoCs, which typically use Cortex-A7 cores that do include the
> > virtualization extensions and therefore should implement the stricter
> > unaligned access checks, we do enable the MMU.
> > 
> > > >  Does it imply that we should discourage EFI use on all ARMv7 systems?
> > 
> > So no, it does *not* imply we should discourage EFI use on all ARMv7
> > systems.
> > 
> > > The spec says the MMU must be enabled, so I think yes. I thought about
> > > disabling it, but that's too strict.
> > > Perhaps someone that uses v7 and *needs* EFI can look at enabling the
> > > MMU with a simple identity mapping.
> > 
> > That should be as simple the diff below.  I've not verified if that
> > fixes the QEMU issue, but I strongly suspect it will.  Whether this
> > works on actual vexpress hardware is a different issue though.
> 
> Fixes the issue with QEMU 9.2.2.  Before it would reset, now it boots
> into the OpenBSD installer just fine.
> 
> > diff --git a/board/armltd/vexpress/vexpress_common.c b/board/armltd/vexpress/vexpress_common.c
> > index 6c374e25e32..3833af59b09 100644
> > --- a/board/armltd/vexpress/vexpress_common.c
> > +++ b/board/armltd/vexpress/vexpress_common.c
> > @@ -165,3 +165,11 @@ void smp_set_core_boot_addr(unsigned long addr, int corenr)
> >  	writel(addr, CONFIG_SYSFLAGS_ADDR);
> >  }
> >  #endif
> > +
> > +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
> > +void enable_caches(void)
> > +{
> > +	/* Enable D-cache. I-cache is already enabled in start.S */
> > +	dcache_enable();
> > +}
> > +#endif

Thanks Mark! I was hoping this would spur some further discussion and
perhaps a patch. Please post this formal patch and I'll go reject the
patch I posted now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250712/0ad777a1/attachment.sig>