Message-Id: <20131205115931.6922FC407BC@trevor.secretlab.ca>
Date:	Thu, 05 Dec 2013 11:59:31 +0000
From:	Grant Likely <grant.likely@...aro.org>
To:	Leif Lindholm <leif.lindholm@...aro.org>,
	linux-arm-kernel@...ts.infradead.org, linux-efi@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux@....linux.org.uk
Cc:	matt.fleming@...el.com, roy.franz@...aro.org, msalter@...hat.com,
	patches@...aro.org, linaro-uefi@...aro.org, mark.rutland@....com,
	Leif Lindholm <leif.lindholm@...aro.org>
Subject: Re: [PATCH v3 2/3] arm: Add [U]EFI runtime services support

On Thu, 28 Nov 2013 16:41:22 +0000, Leif Lindholm <leif.lindholm@...aro.org> wrote:
> This patch implements basic support for UEFI runtime services in the
> ARM architecture - a requirement for using efibootmgr to read and update
> the system boot configuration.
> 
> It uses the generic configuration table scanning to populate ACPI and
> SMBIOS pointers.
> 
> Changes since v2:
> - Updated FDT bindings.
> - Preserve regions marked RESERVED (but don't map them).
> - Rename 'efi' -> 'uefi' within this new port (leaving core code as is).
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@...aro.org>

Hi Leif,

I've made a bunch of comments below. I've got concerns about the amount
of casting going on in this code, but the rest are pretty minor

Reviewed-by: Grant Likely <grant.likely@...aro.org>

> ---
>  arch/arm/Kconfig            |   15 ++
>  arch/arm/include/asm/uefi.h |   22 ++
>  arch/arm/kernel/Makefile    |    2 +
>  arch/arm/kernel/setup.c     |    6 +
>  arch/arm/kernel/uefi.c      |  469 +++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/uefi_phys.S |   59 ++++++
>  include/linux/efi.h         |    2 +-
>  7 files changed, 574 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/include/asm/uefi.h
>  create mode 100644 arch/arm/kernel/uefi.c
>  create mode 100644 arch/arm/kernel/uefi_phys.S
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 78a79a6a..db8d212 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1853,6 +1853,19 @@ config EARLY_IOREMAP
>  	  the same virtual memory range as kmap so all early mappings must
>  	  be unapped before paging_init() is called.
>  
> +config EFI
> +	bool "UEFI runtime service support"
> +	depends on OF && !CPU_BIG_ENDIAN
> +	select UCS2_STRING
> +	select EARLY_IOREMAP
> +	---help---
> +	  This enables the kernel to use UEFI runtime services that are
> +	  available (such as the UEFI variable services).
> +
> +	  This option is only useful on systems that have UEFI firmware.
> +	  However, even with this option, the resultant kernel will
> +	  continue to boot on non-UEFI platforms.
> +
>  config SECCOMP
>  	bool
>  	prompt "Enable seccomp to safely compute untrusted bytecode"
> @@ -2272,6 +2285,8 @@ source "net/Kconfig"
>  
>  source "drivers/Kconfig"
>  
> +source "drivers/firmware/Kconfig"
> +
>  source "fs/Kconfig"
>  
>  source "arch/arm/Kconfig.debug"
> diff --git a/arch/arm/include/asm/uefi.h b/arch/arm/include/asm/uefi.h
> new file mode 100644
> index 0000000..519ca18
> --- /dev/null
> +++ b/arch/arm/include/asm/uefi.h
> @@ -0,0 +1,22 @@
> +#ifndef _ASM_ARM_EFI_H
> +#define _ASM_ARM_EFI_H
> +
> +#include <asm/mach/map.h>
> +
> +extern int uefi_memblock_arm_reserve_range(void);
> +
> +typedef efi_status_t uefi_phys_call_t(u32 memory_map_size,
> +				      u32 descriptor_size,
> +				      u32 descriptor_version,
> +				      efi_memory_desc_t *dsc,
> +				      efi_set_virtual_address_map_t *f);
> +
> +extern efi_status_t uefi_phys_call(u32, u32, u32, efi_memory_desc_t *,
> +				   efi_set_virtual_address_map_t *);
> +
> +#define uefi_remap(cookie, size) __arm_ioremap((cookie), (size), MT_MEMORY)
> +#define uefi_ioremap(cookie, size) __arm_ioremap((cookie), (size), MT_DEVICE)
> +#define uefi_unmap(cookie) __arm_iounmap((cookie))
> +#define uefi_iounmap(cookie) __arm_iounmap((cookie))
> +
> +#endif /* _ASM_ARM_EFI_H */
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index a30fc9b..736cce4 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -98,4 +98,6 @@ obj-y				+= psci.o
>  obj-$(CONFIG_SMP)		+= psci_smp.o
>  endif
>  
> +obj-$(CONFIG_EFI)		+= uefi.o uefi_phys.o
> +
>  extra-y := $(head-y) vmlinux.lds
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 04c1757..9d44edd 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -30,6 +30,7 @@
>  #include <linux/bug.h>
>  #include <linux/compiler.h>
>  #include <linux/sort.h>
> +#include <linux/efi.h>
>  
>  #include <asm/unified.h>
>  #include <asm/cp15.h>
> @@ -57,6 +58,7 @@
>  #include <asm/unwind.h>
>  #include <asm/memblock.h>
>  #include <asm/virt.h>
> +#include <asm/uefi.h>
>  
>  #include "atags.h"
>  
> @@ -898,6 +900,10 @@ void __init setup_arch(char **cmdline_p)
>  	sanity_check_meminfo();
>  	arm_memblock_init(&meminfo, mdesc);
>  
> +#ifdef CONFIG_EFI
> +	uefi_memblock_arm_reserve_range();
> +#endif
> +

#ifdef block in code? You know better. :-) Make an empty stub in the
header files instead.

>  	paging_init(mdesc);
>  	request_standard_resources(mdesc);
>  
> diff --git a/arch/arm/kernel/uefi.c b/arch/arm/kernel/uefi.c
> new file mode 100644
> index 0000000..f771026
> --- /dev/null
> +++ b/arch/arm/kernel/uefi.c
> @@ -0,0 +1,469 @@
> +/*
> + * Based on Unified Extensible Firmware Interface Specification version 2.3.1
> + *
> + * Copyright (C) 2013  Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/efi.h>
> +#include <linux/export.h>
> +#include <linux/memblock.h>
> +#include <linux/of.h>
> +#include <linux/of_fdt.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/idmap.h>
> +#include <asm/tlbflush.h>
> +#include <asm/uefi.h>
> +
> +struct efi_memory_map memmap;
> +
> +static efi_runtime_services_t *runtime;
> +
> +static phys_addr_t uefi_system_table;
> +static phys_addr_t uefi_boot_mmap;
> +static u32 uefi_boot_mmap_size;
> +static u32 uefi_mmap_desc_size;
> +static u32 uefi_mmap_desc_ver;
> +
> +static unsigned long arm_uefi_facility;
> +
> +/*
> + * If you're planning to wire up a debugger and debug the UEFI side ...
> + */
> +#undef KEEP_ALL_REGIONS
> +
> +/*
> + * If you need to (temporarily) support buggy firmware.
> + */
> +#define KEEP_BOOT_SERVICES_REGIONS
> +
> +/*
> + * Returns 1 if 'facility' is enabled, 0 otherwise.
> + */
> +int efi_enabled(int facility)
> +{
> +	return test_bit(facility, &arm_uefi_facility) != 0;
> +}
> +EXPORT_SYMBOL(efi_enabled);
> +
> +static int uefi_debug __initdata;
> +static int __init uefi_debug_setup(char *str)
> +{
> +	uefi_debug = 1;
> +
> +	return 0;
> +}
> +early_param("uefi_debug", uefi_debug_setup);
> +
> +static struct {
> +	const char *name;
> +	const char *propname;
> +	int numcells;
> +	void *target;
> +} dt_params[] = {
> +	{
> +		"UEFI System Table", "linux,uefi-system-table",
> +		2, &uefi_system_table
> +	}, {
> +		"UEFI mmap address", "linux,uefi-mmap-start",
> +		2, &uefi_boot_mmap
> +	}, {
> +		"UEFI mmap size", "linux,uefi-mmap-size",
> +		1, &uefi_boot_mmap_size
> +	}, {
> +		"UEFI mmap descriptor size", "linux,uefi-mmap-desc-size",
> +		1, &uefi_mmap_desc_size
> +	}, {
> +		"UEFI mmap descriptor version", "linux,uefi-mmap-desc-ver",
> +		1, &uefi_mmap_desc_ver
> +	}
> +};
> +
> +static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> +				       int depth, void *data)
> +{
> +	void *prop;
> +	int i;
> +
> +	if (depth != 1 ||
> +	    (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
> +		return 0;
> +
> +	pr_info("Getting UEFI parameters from FDT.\n");
> +
> +	for (i = 0; i < sizeof(dt_params)/sizeof(*dt_params); i++) {
> +		prop = of_get_flat_dt_prop(node, dt_params[i].propname, NULL);
> +		if (!prop)
> +			return 0;
> +		if (dt_params[i].numcells == 1) {
> +			u32 *target = dt_params[i].target;
> +			*target = of_read_ulong(prop, 1);
> +		} else {
> +			u64 *target = dt_params[i].target;
> +			*target = of_read_number(prop, 2);
> +		}
> +
> +		if (uefi_debug)
> +			pr_info("  %s @ 0x%08x\n", dt_params[i].name,
> +				*((u32 *)dt_params[i].target));
> +	}
> +
> +	return 1;
> +}
> +
> +static int __init uefi_init(void)
> +{
> +	efi_char16_t *c16;
> +	char vendor[100] = "unknown";
> +	int i, retval;
> +
> +	efi.systab = early_memremap(uefi_system_table,
> +				    sizeof(efi_system_table_t));
> +
> +	/*
> +	 * Verify the UEFI System Table
> +	 */
> +	if (efi.systab == NULL)
> +		panic("Whoa! Can't find UEFI system table.\n");
> +	if (efi.systab->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
> +		panic("Whoa! UEFI system table signature incorrect\n");

Do you really want to panic here? The kernel may be able to continue
with the data in the DT. I would move these tests into
uefi_memblock_arm_reserve_range() and let it bail out doing nothing if
the tests fail (although it is good to print a warning).

> +	if ((efi.systab->hdr.revision >> 16) == 0)
> +		pr_warn("Warning: UEFI system table version %d.%02d, expected 1.00 or greater\n",
> +			efi.systab->hdr.revision >> 16,
> +			efi.systab->hdr.revision & 0xffff);
> +
> +	/* Show what we know for posterity */
> +	c16 = (efi_char16_t *)early_memremap(efi.systab->fw_vendor,
> +					    sizeof(vendor));

If you have to resort to a cast, then you're doing it wrong.
early_memremap() should already return a void*. Does that not work here?

> +	if (c16) {
> +		for (i = 0; i < (int) sizeof(vendor) - 1 && *c16; ++i)
> +			vendor[i] = c16[i];
> +		vendor[i] = '\0';
> +	}

We need a better parser here, or at least filter out non-printable
characters. The best would be a conversion to UTF-8 I think (not that
we're doing that on the rest of UEFI code right now, so I would put this
as a followup item).

> +
> +	pr_info("UEFI v%u.%.02u by %s\n",
> +		efi.systab->hdr.revision >> 16,
> +		efi.systab->hdr.revision & 0xffff, vendor);
> +
> +	retval = efi_config_init(NULL);
> +	if (retval == 0)
> +		set_bit(EFI_CONFIG_TABLES, &arm_uefi_facility);
> +
> +	early_iounmap(c16, sizeof(vendor));
> +	early_iounmap(efi.systab,  sizeof(efi_system_table_t));
> +
> +	return retval;
> +}
> +
> +static __init int is_discardable_region(efi_memory_desc_t *md)
> +{
> +#ifdef KEEP_ALL_REGIONS
> +	return 0;
> +#endif
> +
> +	if (md->attribute & EFI_MEMORY_RUNTIME)
> +		return 0;
> +
> +	switch (md->type) {
> +#ifdef KEEP_BOOT_SERVICES_REGIONS
> +	case EFI_BOOT_SERVICES_CODE:
> +	case EFI_BOOT_SERVICES_DATA:
> +#endif

Having the #ifdef in code is gross. I would leave the block in place,
but make the return value configured by a #define value.

ie.

#define DISCARD_BOOT_SERVICES_REGIONS 1 /* Can be set to 0 */

...

	case EFI_BOOT_SERVICES_CODE:
	case EFI_BOOT_SERVICES_DATA:
		return DISCARD_BOOT_SERVICES_REGIONS;

> +	/* Keep tables around for any future kexec operations */
> +	case EFI_ACPI_RECLAIM_MEMORY:
> +		return 0;
> +	/* Preserve */
> +	case EFI_RESERVED_TYPE:
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static __initdata struct {
> +	u32 type;
> +	const char *name;
> +}  memory_type_name_map[] = {
> +	{EFI_RESERVED_TYPE, "reserved"},
> +	{EFI_LOADER_CODE, "loader code"},
> +	{EFI_LOADER_DATA, "loader data"},
> +	{EFI_BOOT_SERVICES_CODE, "boot services code"},
> +	{EFI_BOOT_SERVICES_DATA, "boot services data"},
> +	{EFI_RUNTIME_SERVICES_CODE, "runtime services code"},
> +	{EFI_RUNTIME_SERVICES_DATA, "runtime services data"},
> +	{EFI_CONVENTIONAL_MEMORY, "conventional memory"},
> +	{EFI_UNUSABLE_MEMORY, "unusable memory"},
> +	{EFI_ACPI_RECLAIM_MEMORY, "ACPI reclaim memory"},
> +	{EFI_ACPI_MEMORY_NVS, "ACPI memory nvs"},
> +	{EFI_MEMORY_MAPPED_IO, "memory mapped I/O"},
> +	{EFI_MEMORY_MAPPED_IO_PORT_SPACE, "memory mapped I/O port space"},
> +	{EFI_PAL_CODE, "pal code"},
> +	{EFI_MAX_MEMORY_TYPE, NULL},
> +};

Can this be in common code?

> +
> +static __init void remove_sections(phys_addr_t addr, unsigned long size)
> +{
> +	unsigned long section_offset;
> +	unsigned long num_sections;
> +
> +	section_offset = addr - (addr & SECTION_MASK);
> +	num_sections = size / SECTION_SIZE;
> +	if (size % SECTION_SIZE)
> +		num_sections++;
> +
> +	memblock_remove(addr - section_offset, num_sections * SECTION_SIZE);
> +}
> +
> +static __init int remove_regions(void)
> +{
> +	efi_memory_desc_t *md;
> +	int count = 0;
> +	void *p, *e;
> +
> +	if (uefi_debug)
> +		pr_info("Processing UEFI memory map:\n");
> +
> +	memmap.phys_map = early_memremap((phys_addr_t)(u32) uefi_boot_mmap,
> +					 uefi_boot_mmap_size);

Double casting? Why isn't only "(phys_addr_t)" sufficient? In fact,
uefi_boot_mmap is already phys_addr_t. Why is the cast needed at all?

> +	if (!memmap.phys_map)
> +		return 1;
> +
> +	p = memmap.phys_map;
> +	e = (void *)((u32)p + uefi_boot_mmap_size);
> +	for (; p < e; p += uefi_mmap_desc_size) {

Again, the casting looks wrong. p is a void pointer. Adding
uefi_boot_mmap_size without casting should be just fine. I would do the
following to be more readable:

e = memmap.phys_map + uefi_boot_mmap_size;
for (p = memmap.phys_map; p < e; p += uefi_mmap_desc_size)

> +		md = p;
> +		if (is_discardable_region(md))
> +			continue;
> +
> +		if (uefi_debug)
> +			pr_info("  %8llu pages @ %016llx (%s)\n",
> +				md->num_pages, md->phys_addr,
> +				memory_type_name_map[md->type].name);
> +
> +		if (md->type != EFI_MEMORY_MAPPED_IO) {
> +			remove_sections(md->phys_addr,
> +					md->num_pages * PAGE_SIZE);
> +			count++;
> +		}
> +		memmap.nr_map++;
> +	}
> +
> +	early_iounmap(memmap.phys_map, uefi_boot_mmap_size);
> +
> +	if (uefi_debug)
> +		pr_info("%d regions preserved.\n", memmap.nr_map);
> +
> +	return 0;
> +}
> +
> +int __init uefi_memblock_arm_reserve_range(void)
> +{
> +	if (!of_scan_flat_dt(fdt_find_uefi_params, NULL))
> +		return 0;
> +
> +	set_bit(EFI_BOOT, &arm_uefi_facility);
> +
> +	uefi_init();
> +
> +	remove_regions();
> +
> +	return 0;
> +}
> +
> +/*
> + * Disable instrrupts, enable idmap and disable caches.
> + */
> +static void __init phys_call_prologue(void)
> +{
> +	local_irq_disable();
> +
> +	/* Take out a flat memory mapping. */
> +	setup_mm_for_reboot();
> +
> +	/* Clean and invalidate caches */
> +	flush_cache_all();
> +
> +	/* Turn off caching */
> +	cpu_proc_fin();
> +
> +	/* Push out any further dirty data, and ensure cache is empty */
> +	flush_cache_all();
> +}
> +
> +/*
> + * Restore original memory map and re-enable interrupts.
> + */
> +static void __init phys_call_epilogue(void)
> +{
> +	static struct mm_struct *mm = &init_mm;
> +
> +	/* Restore original memory mapping */
> +	cpu_switch_mm(mm->pgd, mm);
> +
> +	/* Flush branch predictor and TLBs */
> +	local_flush_bp_all();
> +#ifdef CONFIG_CPU_HAS_ASID
> +	local_flush_tlb_all();
> +#endif

Empty stub please

> +
> +	local_irq_enable();
> +}
> +
> +static int __init remap_region(efi_memory_desc_t *md, efi_memory_desc_t *entry)
> +{
> +	u64 va;
> +	u64 paddr;
> +	u64 size;
> +
> +	*entry = *md;
> +	paddr = entry->phys_addr;
> +	size = entry->num_pages << EFI_PAGE_SHIFT;
> +
> +	/*
> +	 * Map everything writeback-capable as coherent memory,
> +	 * anything else as device.
> +	 */
> +	if (md->attribute & EFI_MEMORY_WB)
> +		va = (u64)((u32)uefi_remap(paddr, size) & 0xffffffffUL);
> +	else
> +		va = (u64)((u32)uefi_ioremap(paddr, size) & 0xffffffffUL);

Casting? Is it appropriate to throw away the upper 32 bits? On a large
memory system could that mean aliasing high regions into low?

> +	if (!va)
> +		return 0;
> +	entry->virt_addr = va;
> +
> +	if (uefi_debug)
> +		pr_info("  %016llx-%016llx => 0x%08x : (%s)\n",
> +			paddr, paddr + size - 1, (u32)va,
> +			md->attribute &  EFI_MEMORY_WB ? "WB" : "I/O");
> +
> +	return 1;
> +}
> +
> +static int __init remap_regions(void)
> +{
> +	void *p, *next;
> +	efi_memory_desc_t *md;
> +
> +	memmap.phys_map = uefi_remap(uefi_boot_mmap, uefi_boot_mmap_size);
> +	if (!memmap.phys_map)
> +		return 0;
> +	memmap.map_end = (void *)memmap.phys_map + uefi_boot_mmap_size;
> +	memmap.desc_size = uefi_mmap_desc_size;
> +	memmap.desc_version = uefi_mmap_desc_ver;
> +
> +	/* Allocate space for the physical region map */
> +	memmap.map = kzalloc(memmap.nr_map * memmap.desc_size, GFP_KERNEL);
> +	if (!memmap.map)
> +		return 0;
> +
> +	next = memmap.map;
> +	for (p = memmap.phys_map; p < memmap.map_end; p += memmap.desc_size) {
> +		md = p;
> +		if (is_discardable_region(md) || md->type == EFI_RESERVED_TYPE)
> +			continue;
> +
> +		if (!remap_region(p, next))
> +			return 0;
> +
> +		next += memmap.desc_size;
> +	}
> +
> +	memmap.map_end = next;
> +	efi.memmap = &memmap;
> +
> +	uefi_unmap(memmap.phys_map);
> +	memmap.phys_map = efi_lookup_mapped_addr(uefi_boot_mmap);
> +	efi.systab = efi_lookup_mapped_addr(uefi_system_table);
> +	if (efi.systab)
> +		set_bit(EFI_SYSTEM_TABLES, &arm_uefi_facility);
> +	/*
> +	 * efi.systab->runtime is a 32-bit pointer to something guaranteed by
> +	 * the UEFI specification to be 1:1 mapped in a 4GB address space.
> +	 */
> +	runtime = efi_lookup_mapped_addr((u32)efi.systab->runtime);
> +
> +	return 1;
> +}
> +
> +
> +/*
> + * This function switches the UEFI runtime services to virtual mode.
> + * This operation must be performed only once in the system's lifetime,
> + * including any kecec calls.
> + *
> + * This must be done with a 1:1 mapping. The current implementation
> + * resolves this by disabling the MMU.
> + */
> +efi_status_t  __init phys_set_virtual_address_map(u32 memory_map_size,
> +						  u32 descriptor_size,
> +						  u32 descriptor_version,
> +						  efi_memory_desc_t *dsc)
> +{
> +	uefi_phys_call_t *phys_set_map;
> +	efi_status_t status;
> +
> +	phys_call_prologue();
> +
> +	phys_set_map = (void *)(unsigned long)virt_to_phys(uefi_phys_call);
> +
> +	/* Called with caches disabled, returns with caches enabled */
> +	status = phys_set_map(memory_map_size, descriptor_size,
> +			      descriptor_version, dsc,
> +			      efi.set_virtual_address_map)
> +;
> +	phys_call_epilogue();
> +
> +	return status;
> +}
> +
> +/*
> + * Called explicitly from init/mm.c
> + */
> +void __init efi_enter_virtual_mode(void)
> +{
> +	efi_status_t status;
> +
> +	if (!efi_enabled(EFI_BOOT)) {
> +		pr_info("UEFI services will not be available.\n");
> +		return;
> +	}
> +
> +	pr_info("Remapping and enabling UEFI services.\n");
> +
> +	/* Map the regions we memblock_remove:d earlier into kernel
> +	   address space */
> +	if (!remap_regions()) {
> +		pr_info("Failed to remap UEFI regions - runtime services will not be available.\n");
> +		return;
> +	}
> +
> +	/* Call SetVirtualAddressMap with the physical address of the map */
> +	efi.set_virtual_address_map =
> +		(efi_set_virtual_address_map_t *)
> +		runtime->set_virtual_address_map;

I don't think the cast is necessary.

> +	memmap.phys_map =
> +		(efi_memory_desc_t *)(u32) __virt_to_phys((u32)memmap.map);

casting?

> +
> +	status = phys_set_virtual_address_map(memmap.nr_map * memmap.desc_size,
> +					      memmap.desc_size,
> +					      memmap.desc_version,
> +					      memmap.phys_map);
> +
> +	if (status != EFI_SUCCESS) {
> +		pr_info("Failed to set UEFI virtual address map!\n");
> +		return;
> +	}
> +
> +	/* Set up function pointers for efivars */
> +	efi.get_variable = (efi_get_variable_t *)runtime->get_variable;
> +	efi.get_next_variable =
> +		(efi_get_next_variable_t *)runtime->get_next_variable;
> +	efi.set_variable = (efi_set_variable_t *)runtime->set_variable;
> +	set_bit(EFI_RUNTIME_SERVICES, &arm_uefi_facility);

ditto

> +}
> diff --git a/arch/arm/kernel/uefi_phys.S b/arch/arm/kernel/uefi_phys.S
> new file mode 100644
> index 0000000..e9a1542
> --- /dev/null
> +++ b/arch/arm/kernel/uefi_phys.S
> @@ -0,0 +1,59 @@
> +/*
> + * arch/arm/kernel/uefi_phys.S
> + *
> + * Copyright (C) 2013  Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +#define PAR_MASK 0xfff
> +
> +	.text
> +@ uefi_phys_call(a, b, c, d, *f)
> +	.align  5
> +        .pushsection    .idmap.text, "ax"
> +ENTRY(uefi_phys_call)
> +	@ Save physical context
> +	mov	r12, sp
> +	push	{r4-r5, r12, lr}
> +
> +	@ Extract function pointer (don't write r12 beyond this)
> +	ldr	r12, [sp, #16]
> +
> +	@ Convert sp to 32-bit physical
> +	mov	lr, sp
> +	ldr	r4, =PAR_MASK
> +	and	r5, lr, r4			@ Extract lower 12 bits of sp
> +	mcr	p15, 0, lr, c7, c8, 1		@ Write VA -> ATS1CPW
> +	mrc	p15, 0, lr, c7, c4, 0		@ Physical Address Register
> +	mvn	r4, r4
> +	and	lr, lr, r4			@ Clear lower 12 bits of PA
> +	add	lr, lr, r5			@ Calculate phys sp
> +	mov	sp, lr				@ Update
> +
> +	@ Disable MMU
> +        mrc     p15, 0, lr, c1, c0, 0           @ ctrl register
> +        bic     lr, lr, #0x1                    @ ...............m
> +        mcr     p15, 0, lr, c1, c0, 0           @ disable MMU
> +	isb

Nit: whitespace (tabs vs. spaces)

> +
> +	@ Make call
> +	blx	r12
> +
> +	pop	{r4-r5, r12, lr}
> +
> +	@ Enable MMU + Caches
> +        mrc     p15, 0, r1, c1, c0, 0		@ ctrl register
> +        orr     r1, r1, #0x1000			@ ...i............
> +        orr     r1, r1, #0x0005			@ .............c.m
> +        mcr     p15, 0, r1, c1, c0, 0		@ enable MMU
> +	isb

Nit: whitespace

> +
> +	@ Restore virtual sp and return
> +	mov	sp, r12
> +	bx	lr
> +ENDPROC(uefi_phys_call)
> +        .popsection
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index bc5687d..ebb34ee 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -655,7 +655,7 @@ extern int __init efi_setup_pcdp_console(char *);
>  #define EFI_64BIT		5	/* Is the firmware 64-bit? */
>  
>  #ifdef CONFIG_EFI
> -# ifdef CONFIG_X86
> +# if defined(CONFIG_X86) || defined(CONFIG_ARM)

ARM64 will need this too. Can we instead create a new empty config to
test here? CONFIG_EFI_ENABLED_ARCH perhaps?

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists