[PATCH v2] efi: introduce efi_arch_flush_dcache_area

[PATCH v2] efi: introduce efi_arch_flush_dcache_area

[Stefano Stabellini]

Objects loaded by FileHandle->Read need to be flushed from dcache,
otherwise copy_from_paddr will read stale data when copying the kernel,
causing a failure to boot.

Introduce efi_arch_flush_dcache_area and call it from read_file.

This commit introduces no functional changes on x86.

Reported-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
CC: Mark Rutland <mark.rutland@arm.com>
CC: ian.campbell@citrix.com
CC: JBeulich@suse.com

---

Changed in v2:
- code style
- remove misplaced "else"
---
 xen/arch/arm/efi/efi-boot.h |    7 +++++++
 xen/arch/x86/efi/efi-boot.h |    2 ++
 xen/common/efi/boot.c       |    1 +
 3 files changed, 10 insertions(+)

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 6a12d91..e427e5f 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -9,6 +9,7 @@
 #include <asm/smp.h>
 
 void noreturn efi_xen_start(void *fdt_ptr, uint32_t fdt_size);
+void __flush_dcache_area(const void *vaddr, unsigned long size);
 
 #define DEVICE_TREE_GUID \
 {0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0}}
@@ -571,6 +572,12 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
                                        EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info)
 {
 }
+
+static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size)
+{
+    __flush_dcache_area(vaddr, size);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 2dd69f6..4c7f383 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -640,6 +640,8 @@ static bool_t __init efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
     return 1; /* x86 always uses a config file */
 }
 
+static void efi_arch_flush_dcache_area(const void *vaddr, UINTN size) { }
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 75a939f..be5ed8e 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -527,6 +527,7 @@ static bool_t __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
         PrintErr(L" failed for ");
         PrintErrMesg(name, ret);
     }
+    efi_arch_flush_dcache_area(file->ptr, file->size);
 
     return 1;
 }
-- 
1.7.10.4

Re: [PATCH v2] efi: introduce efi_arch_flush_dcache_area

[Ian Campbell]

On Tue, 2015-09-08 at 11:38 +0100, Stefano Stabellini wrote:
> Objects loaded by FileHandle->Read need to be flushed from dcache,
> otherwise copy_from_paddr will read stale data when copying the kernel,
> causing a failure to boot.
> 
> Introduce efi_arch_flush_dcache_area and call it from read_file.
> 
> This commit introduces no functional changes on x86.
> 
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Release-acked-by: Wei Liu <wei.liu2@citrix.com>
> CC: Mark Rutland <mark.rutland@arm.com>
> CC: ian.campbell@citrix.com
> CC: JBeulich@suse.com
> 

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH v2] efi: introduce efi_arch_flush_dcache_area

[Julien Grall]

Hi Stefano,

On 08/09/15 11:38, Stefano Stabellini wrote:
> Objects loaded by FileHandle->Read need to be flushed from dcache,
> otherwise copy_from_paddr will read stale data when copying the kernel,
> causing a failure to boot.

It would have been nice to have some explanation why we can read stale
data in the commit message (see [1]).

It would avoid to look at the ML in the future to understand why this
patch is here.

> Introduce efi_arch_flush_dcache_area and call it from read_file.
> 
> This commit introduces no functional changes on x86.

[1] http://www.gossamer-threads.com/lists/xen/devel/396892#396892

Regards,

-- 
Julien Grall