[U-Boot] [PATCH v2 1/3] efi_loader: allow unaligned memory access

[Heinrich Schuchardt]

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.glpk at 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.

> 
> 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.

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.

That is why I used a weak function that does nothing if the CPU does not
support the flag.

Best regards

Heinrich

> 
> I basically want to make sure this is as explicit as it gets :).
> 
> 
> Alex
>