> Date: Sat, 12 Jul 2025 09:48:47 -0600
> From: Tom Rini <tr...@konsulko.com>
> 
> 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.kette...@xs4all.nl>
> > > 
> > > > From: Ilias Apalodimas <ilias.apalodi...@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.g...@gmx.de> 
> > > > wrote:
> > > > >
> > > > > Am 11. Juli 2025 20:55:12 MESZ schrieb Tom Rini <tr...@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.

Done.  I also noticed that the vexpress_ca9x4 board was added back
specifically for use with QEMU.  So any concerns about real hardware
are probably unwarranted.

Cheers,

Mark

Reply via email to