On 04/04/2018 09:14 PM, Heinrich Schuchardt wrote:
On 04/04/2018 06:11 PM, Alexander Graf wrote:
On 04.04.18 17:10, Heinrich Schuchardt wrote:
On 04/04/2018 02:32 PM, Alexander Graf wrote:
On 03.04.18 21:59, Heinrich Schuchardt wrote:
The UEFI spec mandates that unaligned memory access should be enabled if
supported by the CPU architecture.
This patch adds an empty weak function unaligned_access() that can be
overridden by an architecture specific routine.
Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
---
cmd/bootefi.c | 13 +++++++++++++
include/asm-generic/unaligned.h | 3 +++
2 files changed, 16 insertions(+)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index ba258ac9f3..412e6519ba 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -18,6 +18,7 @@
#include <memalign.h>
#include <asm/global_data.h>
#include <asm-generic/sections.h>
+#include <asm-generic/unaligned.h>
#include <linux/linkage.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -89,6 +90,15 @@ out:
return ret;
}
+/*
+ * Allow unaligned memory access.
+ *
+ * This routine is overridden by architectures providing this feature.
+ */
+void __weak allow_unaligned(void)
+{
+}
+
I'd prefer to guard the body of the function with an #ifdef CONFIG_ARM
so everyone knows why it's there. Then call straight into a function
provided in the ARM core code:
The same visibility can be achieved with a comment.
It's not as obvious. The default really should be to map memory as
cached and allow for unaligned accesses.
static void allow_unaligned(void)
{
/* On ARM we prohibit unaligned accesses by default, enable them here */
#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64) &&
!defined(CONFIG_CPU_V7M)
mmu_enable_unaligned();
#endif
}
RISC-V also supports traps for unaligned access.
Just because it supports them doesn't mean we should use them. AArch64
also allows for unaligned access traps and I went through great length
to refactor the mm code in U-Boot to ensure that we do not trap.
Using architecture specific flags outside arch/ is a mess.
We should not commit new sins but eliminate the existing deviations.
And then in arch/arm/lib/cache-cp15.c:
void mmu_enable_unaligned(void)
{
set_cr(get_cr() & ~CR_A);
}
For some ARM architecture versions the bit is reserved and not used for
unaligned access. No clue what this function would do in this case.
Do you have pointers? Anything defined in the UEFI spec should have the bit.
UEFI spec 2.7:
<cite>2.3.5 AArch32 Platforms
In addition, the invoking OSs can assume that unaligned access support
is enabled if it is present in the processor.</cite>
So the UEFI spec foresees processors supporting unaligned memory access
and others that do not support it.
I think the only corner case is Cortex-M, but that's not terribly high
up on my priority list. And if that requires everything to be aligned,
so be it.
That is why I used a weak function that does nothing if the CPU does not
support the flag.
Any platform that uses cache-cp15 also supports the CR_A bit FWIW. So it
really belongs there.>
And again, I do not want to prettify a special hack for a broken
architecture. People should see warts when they're there.
The real fix IMHO is to enable aligned accesses always, like we do on
any sane architecture.
Is this a typo: "enable aligned accesses"?
Aligned access is always enabled. It is unaligned access that currently
is not enabled in U-Boot.
Yes, enable unaligned accesses of course :).
Albert, this is your call I think. Would you be heavily opposed to
Heinrich's initial patch? It really is the best option IMHO:
https://patchwork.ozlabs.org/patch/893014/
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
