<apalos>
Tartarus: I did look a bit befora I responded to xypron
<apalos>
As far as I can tell disabling unaligned access was how the code was commited
<apalos>
IOW git show 0b02b184003e6 has that UA disabled,
matthias_bgg has joined #u-boot
<apalos>
I guess the reasoning is that "dont enable UA on platforms that support it because that might break other code"
<apalos>
but it's 2023 we got aCI
<apalos>
and the compiler should generate proper code to begin with,
<apalos>
OTOH I dont know we we have it disabled to begin with :>, I just tried to limit my change as much as possible
ldevulder has quit [Quit: Leaving]
<Tartarus>
apalos: OK, poke around git log in some other places? We've had long threads about how we should / shouldn't default to handling unaligned access stuff
<Tartarus>
And, now I'm confused by what you're saying you're seeing
<Tartarus>
To be clear, UEFI requires what, exactly, again, here on 32bit ARM?
<xypron>
Tartarus: The UEFI spec requires that unaligned access is supported. Our UEFI code needs it too.
<xypron>
Tartarus: You discovered that with LTO we have a CI problem on qemu_arm_defconfig. It is due to a linker issue with weak functions.
<xypron>
Ilias tries to avoid that weak function by enabling unaligned access in start.S.
<apalos>
Tartarus: when compiling with LTO, the linker manages to replace an asm defined function that enables UA with the _weak version of it
<Tartarus>
Right, yes, I'm just trying to refresh my memory on where we set everything else to
<apalos>
Which is empty. So we end up starting EFI with UA disabled and we blow up in colorful ways
<Tartarus>
Which, ah, right, was the middle ground of 32bit ARM does -mno-unaligned-access (and UEFI has to step-around this for spec reasons), but we also don't globally pass -mno-unaligned-access as that made more work for everything else than was required
<Tartarus>
So yes, we disable unaligned access on 32bit ARM normally
<Tartarus>
As a matter of routine
<xypron>
With apalos' patch it will be enabled for EFI enabled systems on 32bit ARM.
<Tartarus>
So, wrong path then
<Tartarus>
We only want to turn it on for as long as EFI needs it on
<Tartarus>
Not always
<apalos>
Tartarus: the question here is *why* we disable UA and use -mno-unaligned-access
<apalos>
Aren't we better of turning it on on archs that support it ?
<xypron>
Tartarus: currently we enable unaligned access in efi_init_early() which is reached before the CLI.
<apalos>
In principle ti would make sense to force byte access on CPUs that dont support unaligned accesses overall. Unless I am missing any historical bits
<xypron>
I guess apalos would not have to enable it in SPL.
<Tartarus>
Oh wow
<Tartarus>
So we just ignored the hours and hours of debate that got us to where we were before
<apalos>
xypron: I think it's not enabled in SPL
<Tartarus>
It's supposed to not be enabled
<Tartarus>
until something like bootefi enables it
<Tartarus>
We debated this to near-death ages ago
<apalos>
Tartarus: ok, i'll go do some digging then
<Tartarus>
It's why, I think anyhow, Albert stopped doing U-Boot work
<Tartarus>
So I am _unhappy_ we just ignored all of that for EFI
<Tartarus>
as I assumed it was just enabling it, during bootef
<Tartarus>
*bootefi
<Tartarus>
not just the regular boot
<apalos>
no, it's enabbling it on start.S (it's on the commit log)
<xypron>
apalos: you are right CONFIG_IS_ENABLED() would expand CONFIG_EFI_LOADER to CONFIG_SPL_EFI_LOADER which is not enabled.
<xypron>
Tartarus: we have capsule updates before the CLI. We have integrated EFI and dm block devices.
<xypron>
So we run through EFI code quite early.
<Tartarus>
Yes, but it's only EFI payloads that _need_ the flag enabled, yes?
<apalos>
xypron: he has a point though, although EFI_LOADER is default y not everyone boots with EFI
<Tartarus>
Since we build with -mno-unaligned-access the compiler isn't breaking things behind our back
<apalos>
but I still like to dig around and understand *why* we have it disabled it to begin with
<Tartarus>
But grub.efi or whatever quite well can, which is fine for it
<Tartarus>
apalos: the tl;dr; is that the world of U-Boot is not only ARM that supports unaligned gracefully (on modern procs), so, we don't enable fixups like that when we do not have to, so that generically broken code will be fixed
<apalos>
Tartarus: fwiw the kernel only really enables no-unaligned-access with mmu off
<apalos>
Tartarus: yea but I am only enabling this for Arm though
<Tartarus>
Yes, but you run generic code paths on ARM, and finding and fixing unaligned stuff this way via ARM means it's also found and fixed on less popular platforms
<apalos>
and the CI tests a lot more than Arm nowadays.
<Tartarus>
Rather than papered over
<apalos>
So we should be able to catch that code when we compile for something else
<Tartarus>
There's long-ass threads about this
<apalos>
right I get the point, but I dont really agree. I mean the reasoning of fixing more platforms is fine
<Tartarus>
yes, that's a more modern example of the why, xypron, thanks
<apalos>
But we can try and do it more generically, rather than prohibitng platforms that can do it, to do it
<Tartarus>
It's been debated to death, and at this point the "fun" moving forward is 64bit arm where it's a non-issue
<Tartarus>
So I'd _really_ like to go and re-work things such that again, we only turn this feature on, on 32bit ARM, at the last possible moment, for the EFI payload case
<Tartarus>
Please do go and re-read all the history, but I'm fairly firm in my position here
<apalos>
ok will do, thanks
<Tartarus>
And if anything, the example thread above is more relevant today than it was then
monstr has quit [Remote host closed the connection]
stefanro has quit [Quit: Leaving.]
d-s-e has quit [Quit: Konversation terminated!]
goliath has quit [Quit: SIGSEGV]
___nick___ has joined #u-boot
vagrantc has joined #u-boot
guillaume_g has quit [Quit: Konversation terminated!]