On Fri, Oct 14, 2022 at 09:55:59AM -0600, Simon Glass wrote:
Hi,
On Fri, 14 Oct 2022 at 04:38, Abdellatif El Khlifi abdellatif.elkhlifi@arm.com wrote:
On Thu, Sep 29, 2022 at 12:32:42PM +0300, Ilias Apalodimas wrote:
Hi Abdellatif,
--- a/arch/arm/cpu/armv8/cache.S +++ b/arch/arm/cpu/armv8/cache.S @@ -3,6 +3,9 @@
- (C) Copyright 2013
- David Feng fenghua@phytium.com.cn
- (C) Copyright 2022 ARM Limited
- Abdellatif El Khlifi abdellatif.elkhlifi@arm.com
*/
- This file is based on sample code from ARMv8 ARM.
@@ -21,7 +24,11 @@
- x1: 0 clean & invalidate, 1 invalidate only
- x2~x9: clobbered
*/ +#ifdef CONFIG_EFI_LOADER +.pushsection .text.efi_runtime, "ax"
Maybe we discussed this in the past and I forgot, but why would you need __asm_dcache_level, __asm_dcache_all, __asm_invalidate_dcache_alla etc in the runtime section ?
Because cache invalidation needs to be done at ffa_mm_communicate() level (v4 patchset). That code is runtime compatible so all the called functions must be under .text.efi_runtime
However, since we agreed to drop EFI runtime support from the patchset, v6 patchset takes care of that.
+#else .pushsection .text.__asm_dcache_level, "ax" +#endif ENTRY(__asm_dcache_level) lsl x12, x0, #1 msr csselr_el1, x12 /* select cache level */ @@ -65,7 +72,11 @@ ENDPROC(__asm_dcache_level)
- flush or invalidate all data cache by SET/WAY.
*/ +#ifdef CONFIG_EFI_LOADER +.pushsection .text.efi_runtime, "ax" +#else .pushsection .text.__asm_dcache_all, "ax" +#endif ENTRY(__asm_dcache_all) mov x1, x0 dsb sy @@ -109,7 +120,11 @@ ENTRY(__asm_flush_dcache_all) ENDPROC(__asm_flush_dcache_all) .popsection
+#ifdef CONFIG_EFI_LOADER +.pushsection .text.efi_runtime, "ax" +#else .pushsection .text.__asm_invalidate_dcache_all, "ax" +#endif ENTRY(__asm_invalidate_dcache_all) mov x0, #0x1 b __asm_dcache_all @@ -182,7 +197,11 @@ ENTRY(__asm_invalidate_icache_all) ENDPROC(__asm_invalidate_icache_all) .popsection
+#ifdef CONFIG_EFI_LOADER +.pushsection .text.efi_runtime, "ax" +#else .pushsection .text.__asm_invalidate_l3_dcache, "ax" +#endif WEAK(__asm_invalidate_l3_dcache) mov x0, #0 /* return status as success */ ret diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c index e4736e5643..45f57372c2 100644 --- a/arch/arm/cpu/armv8/cache_v8.c +++ b/arch/arm/cpu/armv8/cache_v8.c @@ -5,10 +5,14 @@
- (C) Copyright 2016
- Alexander Graf agraf@suse.de
- (C) Copyright 2022 ARM Limited
*/
- Abdellatif El Khlifi abdellatif.elkhlifi@arm.com
#include <common.h> #include <cpu_func.h> +#include <efi_loader.h> #include <hang.h> #include <log.h> #include <asm/cache.h> @@ -445,7 +449,7 @@ __weak void mmu_setup(void) /*
- Performs a invalidation of the entire data cache at all levels
*/ -void invalidate_dcache_all(void) +void __efi_runtime invalidate_dcache_all(void) { __asm_invalidate_dcache_all(); __asm_invalidate_l3_dcache(); diff --git a/include/mm_communication.h b/include/mm_communication.h index e65fbde60d..fe9104c56d 100644 --- a/include/mm_communication.h
[...]
- always begin with efi_mm_communicate_header.
*/ -struct __packed efi_mm_communicate_header { +struct efi_mm_communicate_header { efi_guid_t header_guid; size_t message_len; u8 data[]; @@ -145,7 +150,7 @@ struct smm_variable_communicate_header {
- Defined in EDK2 as SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE.
*/ -struct smm_variable_access { +struct __packed smm_variable_access {
You are randomly adding and deleting __packed cwin both structs. But you can't do that. Those structs are defined in StandAloneMM. This is the reason each struct description has the corresponding EDK2 definition.
Thanks for the comment.
However, we are not setting randomly the __packed keyword. There is a good reason for that. It has been explained before in this reply [1]. Basically it's because of data padding issues breaking the communication between u-boot and secure world (Optee).
When upgrading Optee to v3.18, no issues anymore.
The __packed changes have been dropped in patchset v6.
That is the Linux mailing list. I cannot see any reason to add
Thanks Simon. The link above is not part of the linux mailing list. It points to the mirror of the u-boot mailing list under lore.kernel.org
__packed to this struct as it is already set up that way.
Also efi_mm_communicate_header is really long. I suggest efi_mm_hdr or efi_mm_comms_hdr
Why are you using SMM on ARM? Isn't that an Intel thing?
[..]
Regards, SImon