public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
@ 2007-10-25  6:57 Huang, Ying
  2007-10-25 16:09 ` Thomas Gleixner
  2007-10-25 17:01 ` Eric W. Biederman
  0 siblings, 2 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-25  6:57 UTC (permalink / raw)
  To: akpm, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan
  Cc: linux-kernel

This patch adds basic runtime services support for EFI x86_64
system. The main file of the patch is the addition of efi.c for
x86_64. This file is modeled after the EFI IA32 avatar. EFI runtime
services initialization are implemented in efi.c. Some x86_64
specifics are worth noting here. On x86_64, parameters passed to UEFI
firmware services need to follow the UEFI calling convention. For this
purpose, a set of functions named lin2win<x> (<x> is the number of
parameters) are implemented. EFI function calls are wrapped before
calling the firmware service.

Signed-off-by: Chandramouli Narayanan <mouli@linux.intel.com>
Signed-off-by: Huang Ying <ying.huang@intel.com>

---

 arch/x86/kernel/Makefile_64       |    1 
 arch/x86/kernel/efi_64.c          |  593 ++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/efi_callwrap_64.S |   69 ++++
 arch/x86/kernel/setup_64.c        |   15 
 arch/x86_64/Kconfig               |   11 
 include/asm-x86/bootparam.h       |    5 
 include/asm-x86/efi_64.h          |    8 
 include/asm-x86/eficallwrap_64.h  |   33 ++
 include/asm-x86/fixmap_64.h       |    3 
 9 files changed, 735 insertions(+), 3 deletions(-)

Index: linux-2.6.24-rc1/include/asm-x86/eficallwrap_64.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.24-rc1/include/asm-x86/eficallwrap_64.h	2007-10-25 13:58:18.000000000 +0800
@@ -0,0 +1,33 @@
+/*
+ *  Copyright (C) 2007 Intel Corp
+ *	Bibo Mao <bibo.mao@intel.com>
+ *	Huang Ying <ying.huang@intel.com>
+ *
+ *  Function calling ABI conversion from SYSV to Windows for x86_64
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#ifndef __ASM_X86_64_EFICALLWRAP_H
+#define __ASM_X86_64_EFICALLWRAP_H
+
+extern efi_status_t lin2win0(void *fp);
+extern efi_status_t lin2win1(void *fp, u64 arg1);
+extern efi_status_t lin2win2(void *fp, u64 arg1, u64 arg2);
+extern efi_status_t lin2win3(void *fp, u64 arg1, u64 arg2, u64 arg3);
+extern efi_status_t lin2win4(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+extern efi_status_t lin2win5(void *fp, u64 arg1, u64 arg2, u64 arg3,
+			     u64 arg4, u64 arg5);
+extern efi_status_t lin2win6(void *fp, u64 arg1, u64 arg2, u64 arg3,
+			     u64 arg4, u64 arg5, u64 arg6);
+
+#endif
Index: linux-2.6.24-rc1/arch/x86/kernel/efi_64.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.24-rc1/arch/x86/kernel/efi_64.c	2007-10-25 14:51:41.000000000 +0800
@@ -0,0 +1,593 @@
+/*
+ * Extensible Firmware Interface
+ *
+ * Based on Extensible Firmware Interface Specification version 1.0
+ *
+ * Copyright (C) 1999 VA Linux Systems
+ * Copyright (C) 1999 Walt Drummond <drummond@valinux.com>
+ * Copyright (C) 1999-2002 Hewlett-Packard Co.
+ *	David Mosberger-Tang <davidm@hpl.hp.com>
+ *	Stephane Eranian <eranian@hpl.hp.com>
+ * Copyright (C) 2005-2008 Intel Co.
+ *	Fenghua Yu <fenghua.yu@intel.com>
+ *	Bibo Mao <bibo.mao@intel.com>
+ *	Chandramouli Narayanan <mouli@linux.intel.com>
+ *	Huang Ying <ying.huang@intel.com>
+ *
+ * Code to convert EFI to E820 map has been implemented in elilo bootloader
+ * based on a EFI patch by Edgar Hucek. Based on the E820 map, the page table
+ * is setup appropriately for EFI runtime code.
+ * - mouli 06/14/2007.
+ *
+ * All EFI Runtime Services are not implemented yet as EFI only
+ * supports physical mode addressing on SoftSDV. This is to be fixed
+ * in a future version.  --drummond 1999-07-20
+ *
+ * Implemented EFI runtime services and virtual mode calls.  --davidm
+ *
+ * Goutham Rao: <goutham.rao@intel.com>
+ *	Skip non-WB memory and ignore empty memory ranges.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/types.h>
+#include <linux/time.h>
+#include <linux/spinlock.h>
+#include <linux/bootmem.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/efi.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include <linux/reboot.h>
+
+#include <asm/setup.h>
+#include <asm/bootparam.h>
+#include <asm/page.h>
+#include <asm/e820.h>
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+#include <asm/cacheflush.h>
+#include <asm/proto.h>
+#include <asm/eficallwrap_64.h>
+#include <asm/efi_64.h>
+#include <asm/time_64.h>
+
+int efi_enabled;
+EXPORT_SYMBOL(efi_enabled);
+
+struct efi efi;
+EXPORT_SYMBOL(efi);
+
+struct efi_memory_map memmap;
+
+struct efi efi_phys __initdata;
+static efi_system_table_t efi_systab __initdata;
+static unsigned long efi_flags __initdata;
+/* efi_lock protects efi physical mode call */
+static __initdata DEFINE_SPINLOCK(efi_lock);
+static pgd_t save_pgd __initdata;
+static int noefi_time __initdata;
+
+static int __init setup_noefi(char *arg)
+{
+	efi_enabled = 0;
+	return 0;
+}
+early_param("noefi", setup_noefi);
+
+static int __init setup_noefi_time(char *arg)
+{
+	noefi_time = 1;
+	return 0;
+}
+early_param("noefi_time", setup_noefi_time);
+
+static efi_status_t _efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
+{
+	return lin2win2((void *)efi.systab->runtime->get_time,
+			(u64)tm, (u64)tc);
+}
+
+static efi_status_t _efi_set_time(efi_time_t *tm)
+{
+	return lin2win1((void *)efi.systab->runtime->set_time, (u64)tm);
+}
+
+static efi_status_t _efi_get_wakeup_time(efi_bool_t *enabled,
+					 efi_bool_t *pending,
+					 efi_time_t *tm)
+{
+	return lin2win3((void *)efi.systab->runtime->get_wakeup_time,
+			(u64)enabled, (u64)pending, (u64)tm);
+}
+
+static efi_status_t _efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
+{
+	return lin2win2((void *)efi.systab->runtime->set_wakeup_time,
+			(u64)enabled, (u64)tm);
+}
+
+static efi_status_t _efi_get_variable(efi_char16_t *name,
+				      efi_guid_t *vendor,
+				      u32 *attr,
+				      unsigned long *data_size,
+				      void *data)
+{
+	return lin2win5((void *)efi.systab->runtime->get_variable,
+			(u64)name, (u64)vendor, (u64)attr,
+			(u64)data_size, (u64)data);
+}
+
+static efi_status_t _efi_get_next_variable(unsigned long *name_size,
+					   efi_char16_t *name,
+					   efi_guid_t *vendor)
+{
+	return lin2win3((void *)efi.systab->runtime->get_next_variable,
+			(u64)name_size, (u64)name, (u64)vendor);
+}
+
+static efi_status_t _efi_set_variable(
+			efi_char16_t *name, efi_guid_t *vendor,
+			u64 attr, u64 data_size, void *data)
+{
+	return lin2win5((void *)efi.systab->runtime->set_variable,
+			(u64)name, (u64)vendor, (u64)attr,
+			(u64)data_size, (u64)data);
+}
+
+static efi_status_t _efi_get_next_high_mono_count(u32 *count)
+{
+	return lin2win1((void *)efi.systab->runtime->get_next_high_mono_count,
+			(u64)count);
+}
+
+static efi_status_t _efi_reset_system(int reset_type,
+				      efi_status_t status,
+				      unsigned long data_size,
+				      efi_char16_t *data)
+{
+	return lin2win4((void *)efi.systab->runtime->reset_system,
+			(u64)reset_type, (u64)status,
+			(u64)data_size, (u64)data);
+}
+
+static efi_status_t _efi_set_virtual_address_map(
+	unsigned long memory_map_size,
+	unsigned long descriptor_size,
+	u32 descriptor_version,
+	efi_memory_desc_t *virtual_map)
+{
+	return lin2win4((void *)efi.systab->runtime->set_virtual_address_map,
+			(u64)memory_map_size, (u64)descriptor_size,
+			(u64)descriptor_version, (u64)virtual_map);
+}
+
+static void __init early_mapping_set_exec(unsigned long start,
+					  unsigned long end,
+					  int executable)
+{
+	pte_t *kpte;
+
+	while (start < end) {
+		kpte = lookup_address((unsigned long)__va(start));
+		BUG_ON(!kpte);
+		if (executable)
+			set_pte(kpte, pte_mkexec(*kpte));
+		else
+			set_pte(kpte, __pte((pte_val(*kpte) | _PAGE_NX) & \
+					    __supported_pte_mask));
+		if (pte_huge(*kpte))
+			start = (start + PMD_SIZE) & PMD_MASK;
+		else
+			start = (start + PAGE_SIZE) & PAGE_MASK;
+	}
+}
+
+static void __init early_runtime_code_mapping_set_exec(int executable)
+{
+	efi_memory_desc_t *md;
+	void *p;
+
+	/* Make EFI runtime service code area executable */
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		md = p;
+		if (md->type == EFI_RUNTIME_SERVICES_CODE) {
+			unsigned long end;
+			end = md->phys_addr + (md->num_pages << PAGE_SHIFT);
+			early_mapping_set_exec(md->phys_addr, end, executable);
+		}
+	}
+}
+
+static void __init efi_call_phys_prelog(void) __acquires(efi_lock)
+{
+	unsigned long vaddress;
+
+	/*
+	 * Lock sequence is different from normal case because
+	 * efi_flags is global
+	 */
+	spin_lock(&efi_lock);
+	local_irq_save(efi_flags);
+	early_runtime_code_mapping_set_exec(1);
+	vaddress = (unsigned long)__va(0x0UL);
+	pgd_val(save_pgd) = pgd_val(*pgd_offset_k(0x0UL));
+	set_pgd(pgd_offset_k(0x0UL), *pgd_offset_k(vaddress));
+	global_flush_tlb();
+}
+
+static void __init efi_call_phys_epilog(void) __releases(efi_lock)
+{
+	/*
+	 * After the lock is released, the original page table is restored.
+	 */
+	set_pgd(pgd_offset_k(0x0UL), save_pgd);
+	early_runtime_code_mapping_set_exec(0);
+	global_flush_tlb();
+	local_irq_restore(efi_flags);
+	spin_unlock(&efi_lock);
+}
+
+static efi_status_t __init phys_efi_set_virtual_address_map(
+	unsigned long memory_map_size,
+	unsigned long descriptor_size,
+	u32 descriptor_version,
+	efi_memory_desc_t *virtual_map)
+{
+	efi_status_t status;
+
+	efi_call_phys_prelog();
+	status = lin2win4((void *)efi_phys.set_virtual_address_map,
+			  (u64)memory_map_size, (u64)descriptor_size,
+			  (u64)descriptor_version, (u64)virtual_map);
+	efi_call_phys_epilog();
+	return status;
+}
+
+static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
+					     efi_time_cap_t *tc)
+{
+	efi_status_t status;
+
+	efi_call_phys_prelog();
+	status = lin2win2((void *)efi_phys.get_time, (u64)tm, (u64)tc);
+	efi_call_phys_epilog();
+	return status;
+}
+
+/*
+ * To call this function, the irq must be disabled and rtc_lock in
+ * time_64.c must be held.
+ */
+int efi_set_rtc_mmss(unsigned long nowtime)
+{
+	int real_seconds, real_minutes;
+	efi_status_t 	status;
+	efi_time_t 	eft;
+	efi_time_cap_t 	cap;
+
+	status = efi.get_time(&eft, &cap);
+	if (status != EFI_SUCCESS) {
+		printk(KERN_ERR "Oops: efitime: can't read time!\n");
+		return -1;
+	}
+
+	real_seconds = nowtime % 60;
+	real_minutes = nowtime / 60;
+	if (((abs(real_minutes - eft.minute) + 15)/30) & 1)
+		real_minutes += 30;
+	real_minutes %= 60;
+	eft.minute = real_minutes;
+	eft.second = real_seconds;
+
+	status = efi.set_time(&eft);
+	if (status != EFI_SUCCESS) {
+		printk(KERN_ERR "Oops: efitime: can't write time!\n");
+		return -1;
+	}
+	return 0;
+}
+
+/*
+ * To call this function, the irq must be disabled and rtc_lock in
+ * time_64.c must be held.
+ */
+unsigned long efi_get_time(void)
+{
+	efi_status_t status;
+	efi_time_t eft;
+	efi_time_cap_t cap;
+
+	status = efi.get_time(&eft, &cap);
+	if (status != EFI_SUCCESS)
+		printk(KERN_ERR "Oops: efitime: can't read time!\n");
+
+	return mktime(eft.year, eft.month, eft.day, eft.hour,
+		      eft.minute, eft.second);
+}
+
+void __init efi_reserve_bootmem(void)
+{
+	reserve_bootmem_generic((unsigned long)memmap.phys_map,
+				memmap.nr_map * memmap.desc_size);
+}
+
+void __init efi_init(void)
+{
+	efi_config_table_t *config_tables;
+	efi_runtime_services_t *runtime;
+	efi_char16_t *c16;
+	char vendor[100] = "unknown";
+	int i = 0;
+
+	memset(&efi, 0, sizeof(efi));
+	memset(&efi_phys, 0, sizeof(efi_phys));
+
+	efi_phys.systab = (efi_system_table_t *)
+		(boot_params.efi_info.efi_systab |
+		 ((__u64)boot_params.efi_info.efi_systab_hi<<32));
+	memmap.phys_map = (void *)
+		(boot_params.efi_info.efi_memmap |
+		 ((__u64)boot_params.efi_info.efi_memmap_hi<<32));
+	memmap.nr_map = boot_params.efi_info.efi_memmap_size /
+		boot_params.efi_info.efi_memdesc_size;
+	memmap.desc_version = boot_params.efi_info.efi_memdesc_version;
+	memmap.desc_size = boot_params.efi_info.efi_memdesc_size;
+
+	efi.systab = early_ioremap((unsigned long)efi_phys.systab,
+				   sizeof(efi_system_table_t));
+	memcpy(&efi_systab, efi.systab, sizeof(efi_system_table_t));
+	efi.systab = &efi_systab;
+	/*
+	 * Verify the EFI Table
+	 */
+	if (efi.systab->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE)
+		printk(KERN_ERR "Woah! EFI system table "
+		       "signature incorrect\n");
+	if ((efi.systab->hdr.revision >> 16) == 0)
+		printk(KERN_ERR "Warning: EFI 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 = early_ioremap(efi.systab->fw_vendor, 2);
+	if (!probe_kernel_address(c16, i)) {
+		for (i = 0; i < sizeof(vendor) && *c16; ++i)
+			vendor[i] = *c16++;
+		vendor[i] = '\0';
+	}
+
+	printk(KERN_INFO "EFI v%u.%.02u by %s \n",
+	       efi.systab->hdr.revision >> 16,
+	       efi.systab->hdr.revision & 0xffff, vendor);
+
+	/*
+	 * Let's see what config tables the firmware passed to us.
+	 */
+	config_tables = early_ioremap(
+		efi.systab->tables,
+		efi.systab->nr_tables * sizeof(efi_config_table_t));
+	if (config_tables == NULL)
+		printk(KERN_ERR "Could not map EFI Configuration Table!\n");
+
+	printk(KERN_INFO);
+	for (i = 0; i < efi.systab->nr_tables; i++) {
+		if (!efi_guidcmp(config_tables[i].guid, MPS_TABLE_GUID)) {
+			efi.mps = config_tables[i].table;
+			printk(" MPS=0x%lx ", config_tables[i].table);
+		} else if (!efi_guidcmp(config_tables[i].guid,
+					ACPI_20_TABLE_GUID)) {
+			efi.acpi20 = config_tables[i].table;
+			printk(" ACPI 2.0=0x%lx ", config_tables[i].table);
+		} else if (!efi_guidcmp(config_tables[i].guid,
+					ACPI_TABLE_GUID)) {
+			efi.acpi = config_tables[i].table;
+			printk(" ACPI=0x%lx ", config_tables[i].table);
+		} else if (!efi_guidcmp(config_tables[i].guid,
+					SMBIOS_TABLE_GUID)) {
+			efi.smbios = config_tables[i].table;
+			printk(" SMBIOS=0x%lx ", config_tables[i].table);
+		} else if (!efi_guidcmp(config_tables[i].guid,
+					HCDP_TABLE_GUID)) {
+			efi.hcdp = config_tables[i].table;
+			printk(" HCDP=0x%lx ", config_tables[i].table);
+		} else if (!efi_guidcmp(config_tables[i].guid,
+					UGA_IO_PROTOCOL_GUID)) {
+			efi.uga = config_tables[i].table;
+			printk(" UGA=0x%lx ", config_tables[i].table);
+		}
+	}
+	printk("\n");
+
+	/*
+	 * Check out the runtime services table. We need to map
+	 * the runtime services table so that we can grab the physical
+	 * address of several of the EFI runtime functions, needed to
+	 * set the firmware into virtual mode.
+	 */
+	runtime = early_ioremap((unsigned long)efi.systab->runtime,
+				sizeof(efi_runtime_services_t));
+	if (runtime != NULL) {
+		/*
+		 * We will only need *early* access to the following
+		 * two EFI runtime services before set_virtual_address_map
+		 * is invoked.
+		 */
+		efi_phys.get_time = (efi_get_time_t *)runtime->get_time;
+		efi_phys.set_virtual_address_map =
+			(efi_set_virtual_address_map_t *)runtime->set_virtual_address_map;
+		/*
+		 * Make efi_get_time can be called before entering
+		 * virtual mode.
+		 */
+		efi.get_time = phys_efi_get_time;
+	} else
+		printk(KERN_ERR "Could not map the EFI runtime service "
+		       "table!\n");
+
+	/* Map the EFI memory map */
+	memmap.map = __va((unsigned long)memmap.phys_map);
+	memmap.map_end = memmap.map + (memmap.nr_map * memmap.desc_size);
+	if (memmap.desc_size != sizeof(efi_memory_desc_t))
+		printk(KERN_WARNING "Kernel-defined memdesc"
+		       "doesn't match the one from EFI!\n");
+
+	/* Setup for EFI runtime service */
+	reboot_type = BOOT_EFI;
+
+	if (!noefi_time) {
+		get_wallclock = efi_get_time;
+		set_wallclock = efi_set_rtc_mmss;
+	}
+}
+
+static void __init runtime_code_page_mkexec(void)
+{
+	efi_memory_desc_t *md;
+	void *p;
+
+	/* Make EFI runtime service code area executable */
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		md = p;
+		if (md->type == EFI_RUNTIME_SERVICES_CODE)
+			change_page_attr_addr(md->virt_addr,
+					      md->num_pages,
+					      PAGE_KERNEL_EXEC);
+	}
+	global_flush_tlb();
+}
+
+static void __iomem * __init efi_ioremap(unsigned long offset,
+					 unsigned long size)
+{
+	static unsigned pages_mapped;
+	unsigned long last_addr;
+	unsigned i, pages;
+
+	last_addr = offset + size - 1;
+	offset &= PAGE_MASK;
+	pages = (PAGE_ALIGN(last_addr) - offset) >> PAGE_SHIFT;
+	if (pages_mapped + pages > MAX_EFI_IO_PAGES)
+		return NULL;
+
+	for (i = 0; i < pages; i++) {
+		set_fixmap_nocache(FIX_EFI_IO_MAP_FIRST_PAGE - pages_mapped,
+				   offset);
+		offset += PAGE_SIZE;
+		pages_mapped++;
+	}
+
+	return (void __iomem *)__fix_to_virt(FIX_EFI_IO_MAP_FIRST_PAGE - \
+					     (pages_mapped - pages));
+}
+
+/*
+ * This function will switch the EFI runtime services to virtual mode.
+ * Essentially, look through the EFI memmap and map every region that
+ * has the runtime attribute bit set in its memory descriptor and update
+ * that memory descriptor with the virtual address obtained from ioremap().
+ * This enables the runtime services to be called without having to
+ * thunk back into physical mode for every invocation.
+ */
+void __init efi_enter_virtual_mode(void)
+{
+	efi_memory_desc_t *md;
+	efi_status_t status;
+	unsigned long end;
+	void *p;
+
+	efi.systab = NULL;
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		md = p;
+		if (!(md->attribute & EFI_MEMORY_RUNTIME))
+			continue;
+		if (md->attribute & EFI_MEMORY_WB)
+			md->virt_addr = (unsigned long)__va(md->phys_addr);
+		else if (md->attribute & (EFI_MEMORY_UC | EFI_MEMORY_WC))
+			md->virt_addr = (unsigned long)
+				efi_ioremap(md->phys_addr,
+					    md->num_pages << EFI_PAGE_SHIFT);
+		end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT);
+		if ((md->phys_addr <= (unsigned long)efi_phys.systab) &&
+		    ((unsigned long)efi_phys.systab < end))
+			efi.systab = (efi_system_table_t *)
+				(md->virt_addr - md->phys_addr +
+				 (unsigned long)efi_phys.systab);
+	}
+
+	BUG_ON(!efi.systab);
+
+	status = phys_efi_set_virtual_address_map(
+		memmap.desc_size * memmap.nr_map,
+		memmap.desc_size,
+		memmap.desc_version,
+		memmap.phys_map);
+
+	if (status != EFI_SUCCESS) {
+		printk(KERN_ALERT "You are screwed! "
+		       "Unable to switch EFI into virtual mode "
+		       "(status=%lx)\n", status);
+		panic("EFI call to SetVirtualAddressMap() failed!");
+	}
+
+	/*
+	 * Now that EFI is in virtual mode, update the function
+	 * pointers in the runtime service table to the new virtual addresses.
+	 *
+	 * Call EFI services through wrapper functions.
+	 */
+	efi.get_time = (efi_get_time_t *)_efi_get_time;
+	efi.set_time = (efi_set_time_t *)_efi_set_time;
+	efi.get_wakeup_time = (efi_get_wakeup_time_t *)_efi_get_wakeup_time;
+	efi.set_wakeup_time = (efi_set_wakeup_time_t *)_efi_set_wakeup_time;
+	efi.get_variable = (efi_get_variable_t *)_efi_get_variable;
+	efi.get_next_variable = (efi_get_next_variable_t *)
+		_efi_get_next_variable;
+	efi.set_variable = (efi_set_variable_t *)_efi_set_variable;
+	efi.get_next_high_mono_count = (efi_get_next_high_mono_count_t *)
+		_efi_get_next_high_mono_count;
+	efi.reset_system = (efi_reset_system_t *)_efi_reset_system;
+	efi.set_virtual_address_map = (efi_set_virtual_address_map_t *)
+		_efi_set_virtual_address_map;
+
+	runtime_code_page_mkexec();
+}
+
+/*
+ * Convenience functions to obtain memory types and attributes
+ */
+u32 efi_mem_type(unsigned long phys_addr)
+{
+	efi_memory_desc_t *md;
+	void *p;
+
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		md = p;
+		if ((md->phys_addr <= phys_addr) &&
+		    (phys_addr < (md->phys_addr +
+				  (md->num_pages << EFI_PAGE_SHIFT))))
+			return md->type;
+	}
+	return 0;
+}
+
+u64 efi_mem_attributes(unsigned long phys_addr)
+{
+	efi_memory_desc_t *md;
+	void *p;
+
+	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
+		md = p;
+		if ((md->phys_addr <= phys_addr) &&
+		    (phys_addr < (md->phys_addr +
+				  (md->num_pages << EFI_PAGE_SHIFT))))
+			return md->attribute;
+	}
+	return 0;
+}
Index: linux-2.6.24-rc1/arch/x86/kernel/efi_callwrap_64.S
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.24-rc1/arch/x86/kernel/efi_callwrap_64.S	2007-10-25 13:58:18.000000000 +0800
@@ -0,0 +1,69 @@
+/*
+ * linux/arch/x86_64/kernel/efi_callwrap.S -- Function calling ABI
+ *	conversion from SYSV to Windows for x86_64
+ *
+ * Copyright (C) 2007 Intel Corp
+ *	Bibo Mao <bibo.mao@intel.com>
+ *	Huang Ying <ying.huang@intel.com>
+ */
+
+#include <linux/linkage.h>
+
+ENTRY(lin2win0)
+	subq $40, %rsp
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win1)
+	subq $40, %rsp
+	mov  %rsi, %rcx
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win2)
+	subq $40, %rsp
+	mov  %rsi, %rcx
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win3)
+	subq $40, %rsp
+	mov  %rcx, %r8
+	mov  %rsi, %rcx
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win4)
+	subq $40, %rsp
+	mov %r8, %r9
+	mov %rcx, %r8
+	mov %rsi, %rcx
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win5)
+	subq $40, %rsp
+	mov %r9, 32(%rsp)
+	mov %r8, %r9
+	mov %rcx, %r8
+	mov %rsi, %rcx
+	call *%rdi
+	addq $40, %rsp
+	ret
+
+ENTRY(lin2win6)
+	subq $56, %rsp
+	mov 56+8(%rsp), %rax
+	mov %r9, 32(%rsp)
+	mov %rax, 40(%rsp)
+	mov %r8, %r9
+	mov %rcx, %r8
+	mov %rsi, %rcx
+	call *%rdi
+	addq $56, %rsp
+	ret
Index: linux-2.6.24-rc1/arch/x86/kernel/setup_64.c
===================================================================
--- linux-2.6.24-rc1.orig/arch/x86/kernel/setup_64.c	2007-10-25 13:58:14.000000000 +0800
+++ linux-2.6.24-rc1/arch/x86/kernel/setup_64.c	2007-10-25 13:58:18.000000000 +0800
@@ -39,6 +39,7 @@
 #include <linux/dmi.h>
 #include <linux/dma-mapping.h>
 #include <linux/ctype.h>
+#include <linux/efi.h>
 
 #include <asm/mtrr.h>
 #include <asm/uaccess.h>
@@ -269,6 +270,11 @@
 	rd_prompt = ((boot_params.hdr.ram_size & RAMDISK_PROMPT_FLAG) != 0);
 	rd_doload = ((boot_params.hdr.ram_size & RAMDISK_LOAD_FLAG) != 0);
 #endif
+#ifdef CONFIG_EFI
+	if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
+		     "EFIL", 4))
+		efi_enabled = 1;
+#endif
 	setup_memory_region();
 	copy_edd();
 
@@ -308,6 +314,8 @@
 	discover_ebda();
 
 	init_memory_mapping(0, (end_pfn_map << PAGE_SHIFT));
+	if (efi_enabled)
+		efi_init();
 
 	dmi_scan_machine();
 
@@ -379,6 +387,10 @@
         */
        acpi_reserve_bootmem();
 #endif
+
+	if (efi_enabled)
+		efi_reserve_bootmem();
+
 	/*
 	 * Find and reserve possible boot-time SMP configuration:
 	 */
@@ -448,7 +460,8 @@
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
-	conswitchp = &vga_con;
+	if (!efi_enabled || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY))
+		conswitchp = &vga_con;
 #elif defined(CONFIG_DUMMY_CONSOLE)
 	conswitchp = &dummy_con;
 #endif
Index: linux-2.6.24-rc1/include/asm-x86/fixmap_64.h
===================================================================
--- linux-2.6.24-rc1.orig/include/asm-x86/fixmap_64.h	2007-10-25 13:58:14.000000000 +0800
+++ linux-2.6.24-rc1/include/asm-x86/fixmap_64.h	2007-10-25 13:58:18.000000000 +0800
@@ -15,6 +15,7 @@
 #include <asm/apicdef.h>
 #include <asm/page.h>
 #include <asm/vsyscall.h>
+#include <asm/efi_64.h>
 
 /*
  * Here we define all the compile-time 'special' virtual
@@ -41,6 +42,8 @@
 	FIX_APIC_BASE,	/* local (CPU) APIC) -- required for SMP or not */
 	FIX_IO_APIC_BASE_0,
 	FIX_IO_APIC_BASE_END = FIX_IO_APIC_BASE_0 + MAX_IO_APICS-1,
+	FIX_EFI_IO_MAP_LAST_PAGE,
+	FIX_EFI_IO_MAP_FIRST_PAGE = FIX_EFI_IO_MAP_LAST_PAGE+MAX_EFI_IO_PAGES-1,
 	__end_of_fixed_addresses
 };
 
Index: linux-2.6.24-rc1/arch/x86_64/Kconfig
===================================================================
--- linux-2.6.24-rc1.orig/arch/x86_64/Kconfig	2007-10-25 13:58:14.000000000 +0800
+++ linux-2.6.24-rc1/arch/x86_64/Kconfig	2007-10-25 13:58:18.000000000 +0800
@@ -269,6 +269,17 @@
 	depends on SMP && !MK8
 	default y
 
+config EFI
+	bool "EFI runtime service support (EXPERIMENTAL)"
+	---help---
+	  This enables the kernel to use any EFI runtime services that are
+	  available (such as the EFI variable services).
+	  This option is only useful on systems that have EFI firmware
+	  and will result in a kernel image that is ~8k larger. However,
+	  even with this option, the resultant kernel should continue to
+	  boot on existing non-EFI platforms. For more information on
+	  how to set up [U]EFI64 system, see Documentation/x86_64/uefi.txt.
+
 config MATH_EMULATION
 	bool
 
Index: linux-2.6.24-rc1/arch/x86/kernel/Makefile_64
===================================================================
--- linux-2.6.24-rc1.orig/arch/x86/kernel/Makefile_64	2007-10-25 13:58:14.000000000 +0800
+++ linux-2.6.24-rc1/arch/x86/kernel/Makefile_64	2007-10-25 13:58:18.000000000 +0800
@@ -33,6 +33,7 @@
 obj-$(CONFIG_X86_VSMP)		+= vsmp_64.o
 obj-$(CONFIG_K8_NB)		+= k8.o
 obj-$(CONFIG_AUDIT)		+= audit_64.o
+obj-$(CONFIG_EFI)		+= efi_64.o efi_callwrap_64.o
 
 obj-$(CONFIG_MODULES)		+= module_64.o
 obj-$(CONFIG_PCI)		+= early-quirks.o
Index: linux-2.6.24-rc1/include/asm-x86/bootparam.h
===================================================================
--- linux-2.6.24-rc1.orig/include/asm-x86/bootparam.h	2007-10-25 13:58:14.000000000 +0800
+++ linux-2.6.24-rc1/include/asm-x86/bootparam.h	2007-10-25 13:58:18.000000000 +0800
@@ -54,13 +54,14 @@
 };
 
 struct efi_info {
-	__u32 _pad1;
+	__u32 efi_loader_signature;
 	__u32 efi_systab;
 	__u32 efi_memdesc_size;
 	__u32 efi_memdesc_version;
 	__u32 efi_memmap;
 	__u32 efi_memmap_size;
-	__u32 _pad2[2];
+	__u32 efi_systab_hi;
+	__u32 efi_memmap_hi;
 };
 
 /* The so-called "zeropage" */
Index: linux-2.6.24-rc1/include/asm-x86/efi_64.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.24-rc1/include/asm-x86/efi_64.h	2007-10-25 13:58:18.000000000 +0800
@@ -0,0 +1,8 @@
+#ifndef __ASM_X86_64_EFI_H
+#define __ASM_X86_64_EFI_H
+
+#define MAX_EFI_IO_PAGES	100
+
+extern void efi_reserve_bootmem(void);
+
+#endif

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25  6:57 [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support Huang, Ying
@ 2007-10-25 16:09 ` Thomas Gleixner
  2007-10-25 16:28   ` H. Peter Anvin
                     ` (3 more replies)
  2007-10-25 17:01 ` Eric W. Biederman
  1 sibling, 4 replies; 32+ messages in thread
From: Thomas Gleixner @ 2007-10-25 16:09 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Thu, 25 Oct 2007, Huang, Ying wrote:

> This patch adds basic runtime services support for EFI x86_64
> system. The main file of the patch is the addition of efi.c for
> x86_64. This file is modeled after the EFI IA32 avatar.

modeled means copied and modified, right?

This is wrong. I compared efi_32.c and efi_64.c and a large amount of
the code is simply the same. The small details can be sorted out by
two sets of macros/inline functions easily.

Please fix this up.

> EFI runtime
> services initialization are implemented in efi.c. Some x86_64
> specifics are worth noting here. On x86_64, parameters passed to UEFI
> firmware services need to follow the UEFI calling convention. For this
> purpose, a set of functions named lin2win<x> (<x> is the number of
> parameters) are implemented. EFI function calls are wrapped before
> calling the firmware service.

Why needs this to be called lin2win? We do not call Windows, we call
EFI services, so please use a naming convention which is related to
the functionality of the code.

> + *
> + *  Function calling ABI conversion from SYSV to Windows for x86_64

Again, these are wrappers to access EFI and not Windows.

       tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:09 ` Thomas Gleixner
@ 2007-10-25 16:28   ` H. Peter Anvin
  2007-10-25 16:55   ` Eric W. Biederman
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 16:28 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Huang, Ying, Andrew Morton, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

Thomas Gleixner wrote:
> 
>> EFI runtime
>> services initialization are implemented in efi.c. Some x86_64
>> specifics are worth noting here. On x86_64, parameters passed to UEFI
>> firmware services need to follow the UEFI calling convention. For this
>> purpose, a set of functions named lin2win<x> (<x> is the number of
>> parameters) are implemented. EFI function calls are wrapped before
>> calling the firmware service.
> 
> Why needs this to be called lin2win? We do not call Windows, we call
> EFI services, so please use a naming convention which is related to
> the functionality of the code.
> 

Well, presumably EFI inherited the calling convention (as well as 
another bunch of stupidity) from that corner.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:09 ` Thomas Gleixner
  2007-10-25 16:28   ` H. Peter Anvin
@ 2007-10-25 16:55   ` Eric W. Biederman
  2007-10-25 16:56     ` Arjan van de Ven
  2007-10-25 17:06     ` Andi Kleen
  2007-10-26  1:03   ` Huang, Ying
  2007-10-26  3:36   ` Huang, Ying
  3 siblings, 2 replies; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 16:55 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Huang, Ying, Andrew Morton, H. Peter Anvin, Ingo Molnar,
	Andi Kleen, Chandramouli Narayanan, LKML, Arjan van de Ven

Thomas Gleixner <tglx@linutronix.de> writes:

> On Thu, 25 Oct 2007, Huang, Ying wrote:
>
>> This patch adds basic runtime services support for EFI x86_64
>> system. The main file of the patch is the addition of efi.c for
>> x86_64. This file is modeled after the EFI IA32 avatar.
>
> modeled means copied and modified, right?
>
> This is wrong. I compared efi_32.c and efi_64.c and a large amount of
> the code is simply the same. The small details can be sorted out by
> two sets of macros/inline functions easily.
>
> Please fix this up.

I don't think there is a compelling case for us to use any efi
services at this time so unification should be easy:
rm arch/x86/kernel/efi_32.c arch/x86/kernel/efi_stub_32.c

Especially for accessing the real time clock that has a well
defined hardware interface going through efi an additional
software emulation layer looks like asking for trouble.

Eric




^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:55   ` Eric W. Biederman
@ 2007-10-25 16:56     ` Arjan van de Ven
  2007-10-25 17:05       ` H. Peter Anvin
  2007-10-25 17:06       ` Eric W. Biederman
  2007-10-25 17:06     ` Andi Kleen
  1 sibling, 2 replies; 32+ messages in thread
From: Arjan van de Ven @ 2007-10-25 16:56 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Thomas Gleixner, Huang, Ying, Andrew Morton, H. Peter Anvin,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

On Thu, 25 Oct 2007 10:55:44 -0600
ebiederm@xmission.com (Eric W. Biederman) wrote:

> I don't think there is a compelling case for us to use any efi
> services at this time

I would almost agree with this if it wasn't for the 1 call that OS
installers need to tell EFI about bootloader stuff; I've cc'd Peter
Jones since he'll know better what OS installers need; if they don't
need it after all...

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25  6:57 [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support Huang, Ying
  2007-10-25 16:09 ` Thomas Gleixner
@ 2007-10-25 17:01 ` Eric W. Biederman
  2007-10-26  1:17   ` Huang, Ying
  1 sibling, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 17:01 UTC (permalink / raw)
  To: Huang, Ying
  Cc: akpm, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Andi Kleen,
	Chandramouli Narayanan, linux-kernel

"Huang, Ying" <ying.huang@intel.com> writes:

> +static void __init efi_call_phys_prelog(void) __acquires(efi_lock)
> +{
> +	unsigned long vaddress;
> +
> +	/*
> +	 * Lock sequence is different from normal case because
> +	 * efi_flags is global
> +	 */
> +	spin_lock(&efi_lock);
> +	local_irq_save(efi_flags);
> +	early_runtime_code_mapping_set_exec(1);
> +	vaddress = (unsigned long)__va(0x0UL);
> +	pgd_val(save_pgd) = pgd_val(*pgd_offset_k(0x0UL));
> +	set_pgd(pgd_offset_k(0x0UL), *pgd_offset_k(vaddress));
> +	global_flush_tlb();
> +}
> +
> +static void __init efi_call_phys_epilog(void) __releases(efi_lock)
> +{
> +	/*
> +	 * After the lock is released, the original page table is restored.
> +	 */
> +	set_pgd(pgd_offset_k(0x0UL), save_pgd);
> +	early_runtime_code_mapping_set_exec(0);
> +	global_flush_tlb();
> +	local_irq_restore(efi_flags);
> +	spin_unlock(&efi_lock);
> +}
> +
> +static efi_status_t __init phys_efi_set_virtual_address_map(
> +	unsigned long memory_map_size,
> +	unsigned long descriptor_size,
> +	u32 descriptor_version,
> +	efi_memory_desc_t *virtual_map)
> +{
> +	efi_status_t status;
> +
> +	efi_call_phys_prelog();
> +	status = lin2win4((void *)efi_phys.set_virtual_address_map,
> +			  (u64)memory_map_size, (u64)descriptor_size,
> +			  (u64)descriptor_version, (u64)virtual_map);
> +	efi_call_phys_epilog();
> +	return status;
> +}

So you still have this piece of code which makes a kernel using
efi not compatible with kexec.  But you are still supporting a physical
call mode for efi.  If you are going to do this can we please just
remove the hacks that make the EFI physical call mode early boot only
and just always use that mode.  Depending on weird call once functions
like efi_set_virtual_address_map makes me very uncomfortable.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:56     ` Arjan van de Ven
@ 2007-10-25 17:05       ` H. Peter Anvin
  2007-10-25 17:39         ` Eric W. Biederman
  2007-10-25 17:06       ` Eric W. Biederman
  1 sibling, 1 reply; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 17:05 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Eric W. Biederman, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

Arjan van de Ven wrote:
> On Thu, 25 Oct 2007 10:55:44 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
> 
>> I don't think there is a compelling case for us to use any efi
>> services at this time
> 
> I would almost agree with this if it wasn't for the 1 call that OS
> installers need to tell EFI about bootloader stuff; I've cc'd Peter
> Jones since he'll know better what OS installers need; if they don't
> need it after all...
> 

Well, the original motivation for all of this was to enable 
implementation of a EFI framebuffer (UGA/GOP).  Now, you can say what 
you want about EFI (and I definitely have my opinion on it), but that 
seems legitimate to me.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:55   ` Eric W. Biederman
  2007-10-25 16:56     ` Arjan van de Ven
@ 2007-10-25 17:06     ` Andi Kleen
  2007-10-25 17:08       ` H. Peter Anvin
  1 sibling, 1 reply; 32+ messages in thread
From: Andi Kleen @ 2007-10-25 17:06 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Thomas Gleixner, Huang, Ying, Andrew Morton, H. Peter Anvin,
	Ingo Molnar, Chandramouli Narayanan, LKML, Arjan van de Ven


> Especially for accessing the real time clock that has a well
> defined hardware interface going through efi an additional
> software emulation layer looks like asking for trouble.

I agree it's pointless for the hardware clock, but EFI 
also offers services to write some data to the CMOS RAM
which could be very useful to save oops data over reboot.
I don't think this can be done safely otherwise 
without BIOS cooperation.

-Andi

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:56     ` Arjan van de Ven
  2007-10-25 17:05       ` H. Peter Anvin
@ 2007-10-25 17:06       ` Eric W. Biederman
  2007-10-26  1:28         ` Huang, Ying
  1 sibling, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 17:06 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Thomas Gleixner, Huang, Ying, Andrew Morton, H. Peter Anvin,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

Arjan van de Ven <arjan@infradead.org> writes:

> On Thu, 25 Oct 2007 10:55:44 -0600
> ebiederm@xmission.com (Eric W. Biederman) wrote:
>
>> I don't think there is a compelling case for us to use any efi
>> services at this time
>
> I would almost agree with this if it wasn't for the 1 call that OS
> installers need to tell EFI about bootloader stuff; I've cc'd Peter
> Jones since he'll know better what OS installers need; if they don't
> need it after all...

Yes.  I think that is usage of the variable service.  Although
I don't know if that is actually needed.

Support for the variable service is not implemented in this
patchset.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:06     ` Andi Kleen
@ 2007-10-25 17:08       ` H. Peter Anvin
  2007-10-25 17:30         ` Eric W. Biederman
  0 siblings, 1 reply; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 17:08 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Eric W. Biederman, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Chandramouli Narayanan, LKML, Arjan van de Ven

Andi Kleen wrote:
>> Especially for accessing the real time clock that has a well
>> defined hardware interface going through efi an additional
>> software emulation layer looks like asking for trouble.
> 
> I agree it's pointless for the hardware clock, but EFI 
> also offers services to write some data to the CMOS RAM
> which could be very useful to save oops data over reboot.
> I don't think this can be done safely otherwise 
> without BIOS cooperation.
> 

The ability to scurry away even a small amount of data without relying 
on the disk system is highly desirable.  Think next-boot type information.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:08       ` H. Peter Anvin
@ 2007-10-25 17:30         ` Eric W. Biederman
  2007-10-26  2:12           ` Huang, Ying
  0 siblings, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 17:30 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Andi Kleen, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Chandramouli Narayanan, LKML, Arjan van de Ven

"H. Peter Anvin" <hpa@zytor.com> writes:

> Andi Kleen wrote:
>>> Especially for accessing the real time clock that has a well
>>> defined hardware interface going through efi an additional
>>> software emulation layer looks like asking for trouble.
>>
>> I agree it's pointless for the hardware clock, but EFI also offers services to
>> write some data to the CMOS RAM
>> which could be very useful to save oops data over reboot.
>> I don't think this can be done safely otherwise without BIOS cooperation.
>>
>
> The ability to scurry away even a small amount of data without relying on the
> disk system is highly desirable.  Think next-boot type information.

Yes.  If that were to be the justifying case and if that was what
the code was implementing I could see the point.

However this point was made in an earlier review.  This point
was already been made, and still this patchset doesn't
include that functionality and it still includes the code
to disable direct hardware access for no seemingly sane
reason.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:05       ` H. Peter Anvin
@ 2007-10-25 17:39         ` Eric W. Biederman
  2007-10-25 17:51           ` H. Peter Anvin
  0 siblings, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 17:39 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

"H. Peter Anvin" <hpa@zytor.com> writes:

> Arjan van de Ven wrote:
>> On Thu, 25 Oct 2007 10:55:44 -0600
>> ebiederm@xmission.com (Eric W. Biederman) wrote:
>>
>>> I don't think there is a compelling case for us to use any efi
>>> services at this time
>>
>> I would almost agree with this if it wasn't for the 1 call that OS
>> installers need to tell EFI about bootloader stuff; I've cc'd Peter
>> Jones since he'll know better what OS installers need; if they don't
>> need it after all...
>>
>
> Well, the original motivation for all of this was to enable implementation of a
> EFI framebuffer (UGA/GOP).  Now, you can say what you want about EFI (and I
> definitely have my opinion on it), but that seems legitimate to me.

To be very clear.  I think we need the EFI boot parameters but
we certainly don't runtime services to implement an EFI framebuffer.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:39         ` Eric W. Biederman
@ 2007-10-25 17:51           ` H. Peter Anvin
  2007-10-25 18:04             ` Eric W. Biederman
  0 siblings, 1 reply; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 17:51 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

Eric W. Biederman wrote:
>>>
>> Well, the original motivation for all of this was to enable implementation of a
>> EFI framebuffer (UGA/GOP).  Now, you can say what you want about EFI (and I
>> definitely have my opinion on it), but that seems legitimate to me.
> 
> To be very clear.  I think we need the EFI boot parameters but
> we certainly don't runtime services to implement an EFI framebuffer.
> 

Ying claimed that GOP requires EFI runtime services.  Is that not true?

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:51           ` H. Peter Anvin
@ 2007-10-25 18:04             ` Eric W. Biederman
  2007-10-25 20:36               ` H. Peter Anvin
  0 siblings, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 18:04 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

"H. Peter Anvin" <hpa@zytor.com> writes:

> Eric W. Biederman wrote:
>>>>
>>> Well, the original motivation for all of this was to enable implementation of
> a
>>> EFI framebuffer (UGA/GOP).  Now, you can say what you want about EFI (and I
>>> definitely have my opinion on it), but that seems legitimate to me.
>>
>> To be very clear.  I think we need the EFI boot parameters but
>> we certainly don't runtime services to implement an EFI framebuffer.
>>
>
> Ying claimed that GOP requires EFI runtime services.  Is that not true?

None of the EFI framebuffer patches that I saw used EFI runtime services.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 18:04             ` Eric W. Biederman
@ 2007-10-25 20:36               ` H. Peter Anvin
  2007-10-25 22:20                 ` Eric W. Biederman
  2007-10-26  2:14                 ` Huang, Ying
  0 siblings, 2 replies; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 20:36 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

Eric W. Biederman wrote:
>>>
>> Ying claimed that GOP requires EFI runtime services.  Is that not true?
> 
> None of the EFI framebuffer patches that I saw used EFI runtime services.
> 

Ying, could you please clarify this situation?

(Eric: do note that there are two EFI framebuffer standard, UGA and GOP. 
  Apparently UGA is obsolete and we have always been at war with GOP at 
the moment.)

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 20:36               ` H. Peter Anvin
@ 2007-10-25 22:20                 ` Eric W. Biederman
  2007-10-25 22:29                   ` H. Peter Anvin
  2007-10-26  2:14                 ` Huang, Ying
  1 sibling, 1 reply; 32+ messages in thread
From: Eric W. Biederman @ 2007-10-25 22:20 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

"H. Peter Anvin" <hpa@zytor.com> writes:

> Eric W. Biederman wrote:
>>>>
>>> Ying claimed that GOP requires EFI runtime services.  Is that not true?
>>
>> None of the EFI framebuffer patches that I saw used EFI runtime services.
>>
>
> Ying, could you please clarify this situation?
>
> (Eric: do note that there are two EFI framebuffer standard, UGA and
> GOP. Apparently UGA is obsolete and we have always been at war with GOP at the
> moment.)


Peter please look back in your email archives to yesterday and
see Ying's patch:

[PATCH 1/2 -v2 resend] x86_64 EFI boot support: EFI frame buffer driver

All of the data the GOP needs is acquired through the a query made
by the bootloader and passed through screen info.

Eric

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 22:20                 ` Eric W. Biederman
@ 2007-10-25 22:29                   ` H. Peter Anvin
  2007-10-26  2:31                     ` Huang, Ying
  0 siblings, 1 reply; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-25 22:29 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arjan van de Ven, Thomas Gleixner, Huang, Ying, Andrew Morton,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

Eric W. Biederman wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
>> Eric W. Biederman wrote:
>>>> Ying claimed that GOP requires EFI runtime services.  Is that not true?
>>> None of the EFI framebuffer patches that I saw used EFI runtime services.
>>>
>> Ying, could you please clarify this situation?
>>
>> (Eric: do note that there are two EFI framebuffer standard, UGA and
>> GOP. Apparently UGA is obsolete and we have always been at war with GOP at the
>> moment.)
> 
> Peter please look back in your email archives to yesterday and
> see Ying's patch:
> 
> [PATCH 1/2 -v2 resend] x86_64 EFI boot support: EFI frame buffer driver
> 
> All of the data the GOP needs is acquired through the a query made
> by the bootloader and passed through screen info.
> 

Then I fully agree with your assessment.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:09 ` Thomas Gleixner
  2007-10-25 16:28   ` H. Peter Anvin
  2007-10-25 16:55   ` Eric W. Biederman
@ 2007-10-26  1:03   ` Huang, Ying
  2007-10-26  8:48     ` Thomas Gleixner
  2007-10-26 11:31     ` Alan Cox
  2007-10-26  3:36   ` Huang, Ying
  3 siblings, 2 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  1:03 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Thu, 2007-10-25 at 18:09 +0200, Thomas Gleixner wrote:
> > EFI runtime
> > services initialization are implemented in efi.c. Some x86_64
> > specifics are worth noting here. On x86_64, parameters passed to UEFI
> > firmware services need to follow the UEFI calling convention. For this
> > purpose, a set of functions named lin2win<x> (<x> is the number of
> > parameters) are implemented. EFI function calls are wrapped before
> > calling the firmware service.
> 
> Why needs this to be called lin2win? We do not call Windows, we call
> EFI services, so please use a naming convention which is related to
> the functionality of the code.
> 
> > + *
> > + *  Function calling ABI conversion from SYSV to Windows for x86_64
> 
> Again, these are wrappers to access EFI and not Windows.

EFI uses the Windows x86_64 calling convention. The lin2win may be a
more general naming convention that can be used for some other code (the
NDISwrapper?) in the future. Do you agree?

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:01 ` Eric W. Biederman
@ 2007-10-26  1:17   ` Huang, Ying
  0 siblings, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  1:17 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: akpm, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Andi Kleen,
	Chandramouli Narayanan, linux-kernel

On Thu, 2007-10-25 at 11:01 -0600, Eric W. Biederman wrote:
> > +static efi_status_t __init phys_efi_set_virtual_address_map(
> > +	unsigned long memory_map_size,
> > +	unsigned long descriptor_size,
> > +	u32 descriptor_version,
> > +	efi_memory_desc_t *virtual_map)
> > +{
> > +	efi_status_t status;
> > +
> > +	efi_call_phys_prelog();
> > +	status = lin2win4((void *)efi_phys.set_virtual_address_map,
> > +			  (u64)memory_map_size, (u64)descriptor_size,
> > +			  (u64)descriptor_version, (u64)virtual_map);
> > +	efi_call_phys_epilog();
> > +	return status;
> > +}
> 
> So you still have this piece of code which makes a kernel using
> efi not compatible with kexec.  But you are still supporting a physical
> call mode for efi.  If you are going to do this can we please just
> remove the hacks that make the EFI physical call mode early boot only
> and just always use that mode.  Depending on weird call once functions
> like efi_set_virtual_address_map makes me very uncomfortable.

The kexec issue is solved as that of IA-64. The EFI runtime code and
data memory area is mapped with identity mapping, so they will have same
virtual address across kexec. The memory mapped IO memory area used by
EFI runtime services is mapped with fixmap, so they will have same
virtual address across kexec too. And the efi_set_virtual_address_map
runtime service function will be skipped in the kexeced kernel (set to
"nop" in /sbin/kexec). So the kexeced kernel can use the virtual mode of
EFI from kernel bootstrap on.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:06       ` Eric W. Biederman
@ 2007-10-26  1:28         ` Huang, Ying
  0 siblings, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  1:28 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Arjan van de Ven, Thomas Gleixner, Andrew Morton, H. Peter Anvin,
	Ingo Molnar, Andi Kleen, Chandramouli Narayanan, LKML, pjones

On Thu, 2007-10-25 at 11:06 -0600, Eric W. Biederman wrote:
> Arjan van de Ven <arjan@infradead.org> writes:
> 
> > On Thu, 25 Oct 2007 10:55:44 -0600
> > ebiederm@xmission.com (Eric W. Biederman) wrote:
> >
> >> I don't think there is a compelling case for us to use any efi
> >> services at this time
> >
> > I would almost agree with this if it wasn't for the 1 call that OS
> > installers need to tell EFI about bootloader stuff; I've cc'd Peter
> > Jones since he'll know better what OS installers need; if they don't
> > need it after all...
> 
> Yes.  I think that is usage of the variable service.  Although
> I don't know if that is actually needed.
> 
> Support for the variable service is not implemented in this
> patchset.

Support for the variable service has been implemented in this patchset.
The interfaces are:

efi.get_variable
efi.get_next_variable
efi.set_variable

And a sysfs interface (/sys/firmware/efi/vars) is provided in
drivers/firmware/efivars.c, which depends on this patchset to work.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 17:30         ` Eric W. Biederman
@ 2007-10-26  2:12           ` Huang, Ying
  0 siblings, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  2:12 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: H. Peter Anvin, Andi Kleen, Thomas Gleixner, Andrew Morton,
	Ingo Molnar, Chandramouli Narayanan, LKML, Arjan van de Ven

On Thu, 2007-10-25 at 11:30 -0600, Eric W. Biederman wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> > Andi Kleen wrote:
> >>> Especially for accessing the real time clock that has a well
> >>> defined hardware interface going through efi an additional
> >>> software emulation layer looks like asking for trouble.
> >>
> >> I agree it's pointless for the hardware clock, but EFI also offers services to
> >> write some data to the CMOS RAM
> >> which could be very useful to save oops data over reboot.
> >> I don't think this can be done safely otherwise without BIOS cooperation.
> >>
> >
> > The ability to scurry away even a small amount of data without relying on the
> > disk system is highly desirable.  Think next-boot type information.
> 
> Yes.  If that were to be the justifying case and if that was what
> the code was implementing I could see the point.
> 
> However this point was made in an earlier review.  This point
> was already been made, and still this patchset doesn't
> include that functionality and it still includes the code
> to disable direct hardware access for no seemingly sane
> reason.

The EFI variable runtime service is included in this patchset.

The EFI time runtime service is selectable via kernel command line
parameter now. If it is desired, it can be disabled by default, and only
enabled if specified in kernel command line parameter. I think the time
runtime service may be useful if the underlying hardware is changed
silently by some vendor.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 20:36               ` H. Peter Anvin
  2007-10-25 22:20                 ` Eric W. Biederman
@ 2007-10-26  2:14                 ` Huang, Ying
  1 sibling, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  2:14 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Eric W. Biederman, Arjan van de Ven, Thomas Gleixner,
	Andrew Morton, Ingo Molnar, Andi Kleen, Chandramouli Narayanan,
	LKML, pjones

On Thu, 2007-10-25 at 13:36 -0700, H. Peter Anvin wrote:
> Eric W. Biederman wrote:
> >>>
> >> Ying claimed that GOP requires EFI runtime services.  Is that not true?
> > 
> > None of the EFI framebuffer patches that I saw used EFI runtime services.
> > 
> 
> Ying, could you please clarify this situation?
> 
> (Eric: do note that there are two EFI framebuffer standard, UGA and GOP. 
>   Apparently UGA is obsolete and we have always been at war with GOP at 
> the moment.)

EFI framebuffer doesn't depend on EFI runtime service. It only depends
on kernel boot parameters (screen_info).

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 22:29                   ` H. Peter Anvin
@ 2007-10-26  2:31                     ` Huang, Ying
  0 siblings, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  2:31 UTC (permalink / raw)
  To: H. Peter Anvin, Eric W. Biederman, Andi Kleen, Thomas Gleixner
  Cc: Arjan van de Ven, Andrew Morton, Ingo Molnar,
	Chandramouli Narayanan, LKML, pjones

On Thu, 2007-10-25 at 15:29 -0700, H. Peter Anvin wrote:
> Eric W. Biederman wrote:
> > "H. Peter Anvin" <hpa@zytor.com> writes:
> > 
> >> Eric W. Biederman wrote:
> >>>> Ying claimed that GOP requires EFI runtime services.  Is that not true?
> >>> None of the EFI framebuffer patches that I saw used EFI runtime services.
> >>>
> >> Ying, could you please clarify this situation?
> >>
> >> (Eric: do note that there are two EFI framebuffer standard, UGA and
> >> GOP. Apparently UGA is obsolete and we have always been at war with GOP at the
> >> moment.)
> > 
> > Peter please look back in your email archives to yesterday and
> > see Ying's patch:
> > 
> > [PATCH 1/2 -v2 resend] x86_64 EFI boot support: EFI frame buffer driver
> > 
> > All of the data the GOP needs is acquired through the a query made
> > by the bootloader and passed through screen info.
> > 
> 
> Then I fully agree with your assessment.

EFI framebuffer doesn't depend on EFI runtime service.

But EFI variable service depends on EFI runtime service, and most people
think it is useful. It can be used to:

- Provide a standard method to communicate with BIOS, such as specifying
the boot device or bootloader for the next boot.
- Provide a standard method to write the OOPS information to flash.

To improve the reliability of OOPS information writing, the virtual mode
of EFI should be used. And through mapping all memory area used by EFI
to the same virtual address across kexec, EFI can work with kexec under
virtual mode just like that of IA-64.

So, I think the EFI runtime service is useful and it does not break
anything. But the code duplication between efi_32.c and efi_64.c should
be eliminated and I will work on this.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-25 16:09 ` Thomas Gleixner
                     ` (2 preceding siblings ...)
  2007-10-26  1:03   ` Huang, Ying
@ 2007-10-26  3:36   ` Huang, Ying
  2007-10-26  4:11     ` H. Peter Anvin
  3 siblings, 1 reply; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  3:36 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Thu, 2007-10-25 at 18:09 +0200, Thomas Gleixner wrote:
> On Thu, 25 Oct 2007, Huang, Ying wrote:
> 
> > This patch adds basic runtime services support for EFI x86_64
> > system. The main file of the patch is the addition of efi.c for
> > x86_64. This file is modeled after the EFI IA32 avatar.
> 
> modeled means copied and modified, right?
> 
> This is wrong. I compared efi_32.c and efi_64.c and a large amount of
> the code is simply the same. The small details can be sorted out by
> two sets of macros/inline functions easily.
> 
> Please fix this up.

Yes. There are many duplicated code between efi_32.c and efi_64.c, and
they should be merged. But there are some code that is different between
efi_32.c and efi_64.c. For example, there is different implementations
of efi_call_phys_prelog in both files, and there is an implementation of
efi_memmap_walk only in efi_32.c not in efi_64.c.

3 possible schemes are as follow:

- One efi.c, with EFI 32/64 specific code inside corresponding
#ifdef/#endif.

- 3 files: efi.c, efi_32.c, efi_64.c, common code goes in efi.c, EFI
32/64 specific code goes in efi_32/64.c. This will make some variable,
function external instead of static.

- 3 files: efi.c, efi_32.c, efi_64.c, common code goes in efi.c, EFI
32/64 specific code goes in efi_32/64.c. efi.c include efi_32/64.c
according to architecture.

Which one is preferred? Or I should take another scheme?

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  3:36   ` Huang, Ying
@ 2007-10-26  4:11     ` H. Peter Anvin
  0 siblings, 0 replies; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-26  4:11 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Thomas Gleixner, Andrew Morton, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

Huang, Ying wrote:
> 
> - 3 files: efi.c, efi_32.c, efi_64.c, common code goes in efi.c, EFI
> 32/64 specific code goes in efi_32/64.c. This will make some variable,
> function external instead of static.
> 

This is preferred.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  1:03   ` Huang, Ying
@ 2007-10-26  8:48     ` Thomas Gleixner
  2007-10-26  9:30       ` Huang, Ying
  2007-10-26 11:37       ` Andi Kleen
  2007-10-26 11:31     ` Alan Cox
  1 sibling, 2 replies; 32+ messages in thread
From: Thomas Gleixner @ 2007-10-26  8:48 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Fri, 26 Oct 2007, Huang, Ying wrote:

> On Thu, 2007-10-25 at 18:09 +0200, Thomas Gleixner wrote:
> > > EFI runtime
> > > services initialization are implemented in efi.c. Some x86_64
> > > specifics are worth noting here. On x86_64, parameters passed to UEFI
> > > firmware services need to follow the UEFI calling convention. For this
> > > purpose, a set of functions named lin2win<x> (<x> is the number of
> > > parameters) are implemented. EFI function calls are wrapped before
> > > calling the firmware service.
> > 
> > Why needs this to be called lin2win? We do not call Windows, we call
> > EFI services, so please use a naming convention which is related to
> > the functionality of the code.
> > 
> > > + *
> > > + *  Function calling ABI conversion from SYSV to Windows for x86_64
> > 
> > Again, these are wrappers to access EFI and not Windows.
> 
> EFI uses the Windows x86_64 calling convention. The lin2win may be a
> more general naming convention that can be used for some other code (the
> NDISwrapper?) in the future. Do you agree?

I agree not at all. I do not care whether the EFI creators smoked the
Windows-crackpipe or some other hallucinogen when they decided to use
this calling convention. We definitely do not want to think about
NDISwrapper or any other Windows related hackery in the kernel.

I still do not understand why we need all this EFI hackery at all
aside of the possible usage for saving a crash dump on FLASH, which we
could do directly from the kernel as well.

    tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  8:48     ` Thomas Gleixner
@ 2007-10-26  9:30       ` Huang, Ying
  2007-10-26 10:20         ` Thomas Gleixner
  2007-10-26 11:37       ` Andi Kleen
  1 sibling, 1 reply; 32+ messages in thread
From: Huang, Ying @ 2007-10-26  9:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Fri, 2007-10-26 at 10:48 +0200, Thomas Gleixner wrote:
> > EFI uses the Windows x86_64 calling convention. The lin2win may be a
> > more general naming convention that can be used for some other code (the
> > NDISwrapper?) in the future. Do you agree?
> 
> I agree not at all. I do not care whether the EFI creators smoked the
> Windows-crackpipe or some other hallucinogen when they decided to use
> this calling convention. We definitely do not want to think about
> NDISwrapper or any other Windows related hackery in the kernel.

OK, I will change the name to something like lin2efi.

> I still do not understand why we need all this EFI hackery at all
> aside of the possible usage for saving a crash dump on FLASH, which we
> could do directly from the kernel as well.

Ask every user to setup a crash dump environment is a bit difficult
because some configuration like reserving memory, loading crash dump
kernel must be done. But saving OOPS information in FLASH via EFI
variable runtime service is quite simple, without configuration
requirement. That is, there could be more bug report with OOPS
information. I think this is useful.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  9:30       ` Huang, Ying
@ 2007-10-26 10:20         ` Thomas Gleixner
  0 siblings, 0 replies; 32+ messages in thread
From: Thomas Gleixner @ 2007-10-26 10:20 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Andrew Morton, H. Peter Anvin, Ingo Molnar, Andi Kleen,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven

On Fri, 26 Oct 2007, Huang, Ying wrote:

> On Fri, 2007-10-26 at 10:48 +0200, Thomas Gleixner wrote:
> > > EFI uses the Windows x86_64 calling convention. The lin2win may be a
> > > more general naming convention that can be used for some other code (the
> > > NDISwrapper?) in the future. Do you agree?
> > 
> > I agree not at all. I do not care whether the EFI creators smoked the
> > Windows-crackpipe or some other hallucinogen when they decided to use
> > this calling convention. We definitely do not want to think about
> > NDISwrapper or any other Windows related hackery in the kernel.
> 
> OK, I will change the name to something like lin2efi.

Can we simply use call_efi_XXX(). lin2efi() sounds like a conversion
routine, but it is not.

	 tglx

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  1:03   ` Huang, Ying
  2007-10-26  8:48     ` Thomas Gleixner
@ 2007-10-26 11:31     ` Alan Cox
  2007-10-26 17:00       ` H. Peter Anvin
  2007-10-29  1:05       ` Huang, Ying
  1 sibling, 2 replies; 32+ messages in thread
From: Alan Cox @ 2007-10-26 11:31 UTC (permalink / raw)
  To: Huang, Ying
  Cc: Thomas Gleixner, Andrew Morton, H. Peter Anvin, Ingo Molnar,
	Andi Kleen, Eric W. Biederman, Chandramouli Narayanan, LKML,
	Arjan van de Ven

On Fri, 26 Oct 2007 09:03:11 +0800
"Huang, Ying" <ying.huang@intel.com> wrote:

> On Thu, 2007-10-25 at 18:09 +0200, Thomas Gleixner wrote:
> > > EFI runtime
> > > services initialization are implemented in efi.c. Some x86_64
> > > specifics are worth noting here. On x86_64, parameters passed to UEFI
> > > firmware services need to follow the UEFI calling convention. For this
> > > purpose, a set of functions named lin2win<x> (<x> is the number of
> > > parameters) are implemented. EFI function calls are wrapped before
> > > calling the firmware service.
> > 
> > Why needs this to be called lin2win? We do not call Windows, we call
> > EFI services, so please use a naming convention which is related to
> > the functionality of the code.
> > 
> > > + *
> > > + *  Function calling ABI conversion from SYSV to Windows for x86_64
> > 
> > Again, these are wrappers to access EFI and not Windows.
> 
> EFI uses the Windows x86_64 calling convention. The lin2win may be a
> more general naming convention that can be used for some other code (the
> NDISwrapper?) in the future. Do you agree?

The SYSV description is wrong as well. SYSV has no calling convention. I
think you mean iABI or iBCS2 ?

Whats wrong with following the pattern of other calls like syscall(...)
and just having eficall() ?

Alan.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26  8:48     ` Thomas Gleixner
  2007-10-26  9:30       ` Huang, Ying
@ 2007-10-26 11:37       ` Andi Kleen
  1 sibling, 0 replies; 32+ messages in thread
From: Andi Kleen @ 2007-10-26 11:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Huang, Ying, Andrew Morton, H. Peter Anvin, Ingo Molnar,
	Eric W. Biederman, Chandramouli Narayanan, LKML, Arjan van de Ven


> I still do not understand why we need all this EFI hackery at all
> aside of the possible usage for saving a crash dump on FLASH, which we
> could do directly from the kernel as well.

Battery backed up RAM not Flash. The layout of that RAM is BIOS dependent so 
we need some BIOS support for change it. Essentially it's a way to put 
something into the BIOS event log.

-Andi

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26 11:31     ` Alan Cox
@ 2007-10-26 17:00       ` H. Peter Anvin
  2007-10-29  1:05       ` Huang, Ying
  1 sibling, 0 replies; 32+ messages in thread
From: H. Peter Anvin @ 2007-10-26 17:00 UTC (permalink / raw)
  To: Alan Cox
  Cc: Huang, Ying, Thomas Gleixner, Andrew Morton, Ingo Molnar,
	Andi Kleen, Eric W. Biederman, Chandramouli Narayanan, LKML,
	Arjan van de Ven

Alan Cox wrote:
>>> Again, these are wrappers to access EFI and not Windows.
>> EFI uses the Windows x86_64 calling convention. The lin2win may be a
>> more general naming convention that can be used for some other code (the
>> NDISwrapper?) in the future. Do you agree?
> 
> The SYSV description is wrong as well. SYSV has no calling convention. I
> think you mean iABI or iBCS2 ?
> 
> Whats wrong with following the pattern of other calls like syscall(...)
> and just having eficall() ?

Either that (which is basically his lin2win6), or use wrapper-generators 
like we used to do for system calls -- the latter will produce more 
efficient code.  I don't think it matters.

	-hpa

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support
  2007-10-26 11:31     ` Alan Cox
  2007-10-26 17:00       ` H. Peter Anvin
@ 2007-10-29  1:05       ` Huang, Ying
  1 sibling, 0 replies; 32+ messages in thread
From: Huang, Ying @ 2007-10-29  1:05 UTC (permalink / raw)
  To: Alan Cox
  Cc: Thomas Gleixner, Andrew Morton, H. Peter Anvin, Ingo Molnar,
	Andi Kleen, Eric W. Biederman, Chandramouli Narayanan, LKML,
	Arjan van de Ven

On Fri, 2007-10-26 at 12:31 +0100, Alan Cox wrote:
> On Fri, 26 Oct 2007 09:03:11 +0800
> "Huang, Ying" <ying.huang@intel.com> wrote:
> 
> > On Thu, 2007-10-25 at 18:09 +0200, Thomas Gleixner wrote:
> > > > EFI runtime
> > > > services initialization are implemented in efi.c. Some x86_64
> > > > specifics are worth noting here. On x86_64, parameters passed to UEFI
> > > > firmware services need to follow the UEFI calling convention. For this
> > > > purpose, a set of functions named lin2win<x> (<x> is the number of
> > > > parameters) are implemented. EFI function calls are wrapped before
> > > > calling the firmware service.
> > > 
> > > Why needs this to be called lin2win? We do not call Windows, we call
> > > EFI services, so please use a naming convention which is related to
> > > the functionality of the code.
> > > 
> > > > + *
> > > > + *  Function calling ABI conversion from SYSV to Windows for x86_64
> > > 
> > > Again, these are wrappers to access EFI and not Windows.
> > 
> > EFI uses the Windows x86_64 calling convention. The lin2win may be a
> > more general naming convention that can be used for some other code (the
> > NDISwrapper?) in the future. Do you agree?
> 
> The SYSV description is wrong as well. SYSV has no calling convention. I
> think you mean iABI or iBCS2 ?

The SYSV description comes from the following document:
http://www.x86-64.org/documentation/abi-0.98.pdf


> Whats wrong with following the pattern of other calls like syscall(...)
> and just having eficall() ?

Yes. This is better.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2007-10-29  1:05 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-25  6:57 [PATCH 1/3 -v4] x86_64 EFI runtime service support: EFI basic runtime service support Huang, Ying
2007-10-25 16:09 ` Thomas Gleixner
2007-10-25 16:28   ` H. Peter Anvin
2007-10-25 16:55   ` Eric W. Biederman
2007-10-25 16:56     ` Arjan van de Ven
2007-10-25 17:05       ` H. Peter Anvin
2007-10-25 17:39         ` Eric W. Biederman
2007-10-25 17:51           ` H. Peter Anvin
2007-10-25 18:04             ` Eric W. Biederman
2007-10-25 20:36               ` H. Peter Anvin
2007-10-25 22:20                 ` Eric W. Biederman
2007-10-25 22:29                   ` H. Peter Anvin
2007-10-26  2:31                     ` Huang, Ying
2007-10-26  2:14                 ` Huang, Ying
2007-10-25 17:06       ` Eric W. Biederman
2007-10-26  1:28         ` Huang, Ying
2007-10-25 17:06     ` Andi Kleen
2007-10-25 17:08       ` H. Peter Anvin
2007-10-25 17:30         ` Eric W. Biederman
2007-10-26  2:12           ` Huang, Ying
2007-10-26  1:03   ` Huang, Ying
2007-10-26  8:48     ` Thomas Gleixner
2007-10-26  9:30       ` Huang, Ying
2007-10-26 10:20         ` Thomas Gleixner
2007-10-26 11:37       ` Andi Kleen
2007-10-26 11:31     ` Alan Cox
2007-10-26 17:00       ` H. Peter Anvin
2007-10-29  1:05       ` Huang, Ying
2007-10-26  3:36   ` Huang, Ying
2007-10-26  4:11     ` H. Peter Anvin
2007-10-25 17:01 ` Eric W. Biederman
2007-10-26  1:17   ` Huang, Ying

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox