Thank you Matt very much for the code review! Pls. see comments inline.
On 5/27/2015 5:31 AM, Matt Fleming wrote:
On Mon, 04 May, at 02:02:14PM, Jonathan (Zhixiong) Zhang wrote:
From: "Jonathan (Zhixiong) Zhang" <zjzhang@xxxxxxxxxxxxxx>
Both x86 and ia64 implemented efi_mem_attributs function, which is architecture
agnositc. This function is moved to efi subsystem.
efi_remap() function is added. If EFI memmap feature is enabled, and if a
memory region has attribute of EFI_MEMORY_UC, map it as uncached.
---
This patch was tested on an arm64 platform. It was built on x86 platform.
Signed-off-by: Jonathan (Zhixiong) Zhang <zjzhang@xxxxxxxxxxxxxx>
---
arch/ia64/kernel/efi.c | 11 -----------
arch/x86/platform/efi/efi.c | 18 ------------------
drivers/firmware/efi/efi.c | 27 +++++++++++++++++++++++++++
include/linux/efi.h | 1 +
4 files changed, 28 insertions(+), 29 deletions(-)
>> <snipped>
This should be split into two patches, one to remove the duplicate
efi_mem_attributes() and the other to create the new efi_ioremap()
function.
Makes sense, will do.
+void __iomem *efi_remap(phys_addr_t phys_addr, size_t size)
+{
+ if (efi_enabled(EFI_MEMMAP) &&
+ (efi_mem_attributes(phys_addr) & EFI_MEMORY_UC))
+ return ioremap(phys_addr, size);
+ else
+ return ioremap_cache(phys_addr, size);
+}
Note that on x86 we don't leave the EFI memmap mapped throughout
runtime, it gets unmapped in efi_free_boot_services().
Which means that the second patch in this series isn't going to work
correctly if an error is reported after the kernel has finished booting.
It looks like arm64 leaves the EFI memmap mapped at runtime, right?
Correct, only x86 unmaps EFI memmap since efi_free_boot_services() is
not implemented for other architectures, as shown in include/linux/efi.h .
With x86 arch, efi_unmap_memmap() is called in efi_free_boot_services()
to unmap EFI memmap, this function is defined in
arch/x86/platform/efi/efi.c. It clears EFI_MEMMAP bit in efi flag.
Therefore in this case, efi_enabled(EFI_MEMMAP) returns false for x86
platforms with EFI memory map support, after kernel finished booting;
efi_remap() function in turn maps the memory region as cached.
Therefore, I believe the second patch in this series will work correctly
for above situation as well. How do you think?
--
Jonathan (Zhixiong) Zhang
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
