[patch 0/9 v3] kexec kernel efi runtime support

[patch 0/9 v3] kexec kernel efi runtime support

[dyoung]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

Hi,

Here is the V3 for supporting kexec kernel efi runtime.
Per pervious discussion I pass the 1st kernel efi runtime mapping
via setup_data to 2nd kernel. Besides of the runtime mapping
info I also pass the fw_vendor, runtime, config table, smbios
physical address in setup_data. EFI spec mentioned fw_vendor,
runtime, config table addresses will be converted to virt address
after entering virtual mode, but we will use it as physical address
in efi_init. For smbios EFI spec did not mention about the address
updating, but during my test on a HP workstation, the bios will
convert it to Virt addr, thus pass it in setup_data as well.

For fw_vendor, runtime, config table, I export them in /sys/firmware/
efi/, smbios is already in /sys/firmware/efi/systab.

For efi runtime mapping I add a new directory /sys/firmware/efi/
runtime-map/ like below
[dave@darkstar ~]$ tree /sys/firmware/efi/runtime-map/
/sys/firmware/efi/runtime-map/
├── 0
│   ├── attribute
│   ├── num_pages
│   ├── phys_addr
│   ├── type
│   └── virt_addr
├── 1
│   ├── attribute
│   ├── num_pages
│   ├── phys_addr
│   ├── type
│   └── virt_addr
[snip]
 
kexec-tools will assemble them as setup_data and pass to 2nd kernel.
I will send userspace patches as well.

Limitation is I only write support for x86_64, test on below machines:
Lenovo thinkpad t420
Dell inspiron 14 - 3421
HP Z420 workstation
Qemu + OVMF

Please help to review the patches.

The patches are based on matt's efi master tree

Changes from v1:
add one flag in xloadflags, so kexec-tools can safely load old kernel
without efi support.
coding style fixes
function name for map phys_addr to fixed virt_addr
Add ABI documentation for sysfs files

Changes from v2:
01/09: a new patch to remove unused variables in __map_region function
       catched by Toshi Kani
09/09: a new patch to export x86 boot_params to sysfs instead of use
       debugfs files
Matt: reuse __map_region instead do same thing in another function.
      add a wrapper function efi_map_region_fixed [02/09]
      check return value of krealloc
      sysfs dir name s/efi-runtime-map/runtime-map [06/09]
      use desc_size in efi_runtime_map
      for the xloadflags defination: +&& defined(CONFIG_KEXEC)
Greg: sysfs : one file one value for fw_vendor, runtime, tables. [05/09]
      Document them in ABI testing
HPA:  Document the new xloadflag

Also there's other function cleanup and improvement for error handling.

--
Thanks
Dave

[patch 1/9 v3] efi: remove unused variables in __map_region

[dyoung]

[-- Attachment #1: 01-remove-unused-var-in-map-region.patch --]
[-- Type: text/plain, Size: 874 bytes --]

variables size and end is useless in this function, thus remove them.

Reported-by: Toshi Kani <toshi.kani@hp.com>
Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/platform/efi/efi_64.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

--- efi.orig/arch/x86/platform/efi/efi_64.c
+++ efi/arch/x86/platform/efi/efi_64.c
@@ -148,15 +148,11 @@ void efi_setup_page_tables(void)
 static void __init __map_region(efi_memory_desc_t *md, u64 va)
 {
 	pgd_t *pgd = (pgd_t *)__va(real_mode_header->trampoline_pgd);
-	unsigned long pf = 0, size;
-	u64 end;
+	unsigned long pf = 0;
 
 	if (!(md->attribute & EFI_MEMORY_WB))
 		pf |= _PAGE_PCD;
 
-	size = md->num_pages << PAGE_SHIFT;
-	end  = va + size;
-
 	if (kernel_map_pages_in_pgd(pgd, md->phys_addr, va, md->num_pages, pf))
 		pr_warn("Error mapping PA 0x%llx -> VA 0x%llx!\n",
 			   md->phys_addr, va);

[patch 2/9 v3] efi: add a wrapper function efi_map_region_fixed

[dyoung]

[-- Attachment #1: 02-efi-map-fixed-addr.patch --]
[-- Type: text/plain, Size: 1859 bytes --]

Kexec kernel will use saved runtime virtual mapping, so add a
new function efi_map_region_fixed for directly mapping a md
to md->virt.

The md is passed in from 1st kernel, the virtual addr is
saved in md->virt_addr.

Matt: coding style
      reuse __map_region

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/include/asm/efi.h     |    1 +
 arch/x86/platform/efi/efi_32.c |    2 ++
 arch/x86/platform/efi/efi_64.c |   10 ++++++++++
 3 files changed, 13 insertions(+)

--- efi.orig/arch/x86/include/asm/efi.h
+++ efi/arch/x86/include/asm/efi.h
@@ -128,6 +128,7 @@ extern void efi_call_phys_epilog(void);
 extern void efi_unmap_memmap(void);
 extern void efi_memory_uc(u64 addr, unsigned long size);
 extern void __init efi_map_region(efi_memory_desc_t *md);
+extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
 extern void efi_sync_low_kernel_mappings(void);
 extern void efi_setup_page_tables(void);
 extern void __init old_map_region(efi_memory_desc_t *md);
--- efi.orig/arch/x86/platform/efi/efi_64.c
+++ efi/arch/x86/platform/efi/efi_64.c
@@ -199,6 +199,16 @@ void __init efi_map_region(efi_memory_de
 	md->virt_addr = efi_va;
 }
 
+/*
+ * kexec kernel will use efi_map_region_fixed to map efi
+ * runtime memory ranges. md->virt_addr is the original virtual
+ * address which had been mapped in kexec 1st kernel.
+ */
+void __init efi_map_region_fixed(efi_memory_desc_t *md)
+{
+	__map_region(md, md->virt_addr);
+}
+
 void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
 				 u32 type, u64 attribute)
 {
--- efi.orig/arch/x86/platform/efi/efi_32.c
+++ efi/arch/x86/platform/efi/efi_32.c
@@ -47,6 +47,8 @@ void __init efi_map_region(efi_memory_de
 	old_map_region(md);
 }
 
+void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
+
 void efi_call_phys_prelog(void)
 {
 	struct desc_ptr gdt_descr;

[patch 3/9 v3] efi: reserve boot service fix

[dyoung]

[-- Attachment #1: 03-boot-service-fix.patch --]
[-- Type: text/plain, Size: 944 bytes --]

Current code check boot service region with kernel text region by: 
start+size >= __pa_symbol(_text)
The end of the above region should be start + size - 1 instead.

I see this problem in ovmf + Fedora 19 grub boot:
text start: 1000000 md start: 800000 md size: 800000

Signed-off-by: Dave Young <dyoung@redhat.com>
Acked-by: Borislav Petkov <bp@suse.de>
Acked-by: Toshi Kani <toshi.kani@hp.com>
---
 arch/x86/platform/efi/efi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.orig/arch/x86/platform/efi/efi.c
+++ linux-2.6/arch/x86/platform/efi/efi.c
@@ -438,7 +438,7 @@ void __init efi_reserve_boot_services(vo
 		 * - Not within any part of the kernel
 		 * - Not the bios reserved area
 		*/
-		if ((start+size >= __pa_symbol(_text)
+		if ((start + size > __pa_symbol(_text)
 				&& start <= __pa_symbol(_end)) ||
 			!e820_all_mapped(start, start+size, E820_RAM) ||
 			memblock_is_region_reserved(start, size)) {

[patch 4/9 v3] efi: cleanup efi_enter_virtual_mode function

[dyoung]

[-- Attachment #1: 04-efi-enter-virtual-mode-cleanup.patch --]
[-- Type: text/plain, Size: 4746 bytes --]

Add two small functions:
efi_merge_regions and efi_map_regions, efi_enter_virtual_mode
calls them instead of embedding two long for loop.

v1->v2:
refresh; coding style fixes.

v2->v3:
Toshi Kani:
remove unused variable
Matt: check return value of krealloc.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/platform/efi/efi.c |  109 ++++++++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 43 deletions(-)

--- efi.orig/arch/x86/platform/efi/efi.c
+++ efi/arch/x86/platform/efi/efi.c
@@ -773,44 +773,12 @@ void __init old_map_region(efi_memory_de
 		       (unsigned long long)md->phys_addr);
 }
 
-/*
- * This function will switch the EFI runtime services to virtual mode.
- * Essentially, we look through the EFI memmap and map every region that
- * has the runtime attribute bit set in its memory descriptor into the
- * ->trampoline_pgd page table using a top-down VA allocation scheme.
- *
- * The old method which used to update that memory descriptor with the
- * virtual address obtained from ioremap() is still supported when the
- * kernel is booted with efi=old_map on its command line. Same old
- * method enabled the runtime services to be called without having to
- * thunk back into physical mode for every invocation.
- *
- * The new method does a pagetable switch in a preemption-safe manner
- * so that we're in a different address space when calling a runtime
- * function. For function arguments passing we do copy the PGDs of the
- * kernel page table into ->trampoline_pgd prior to each call.
- */
-void __init efi_enter_virtual_mode(void)
+/* Merge contiguous regions of the same type and attribute */
+static void __init efi_merge_regions(void)
 {
+	void *p;
 	efi_memory_desc_t *md, *prev_md = NULL;
-	void *p, *new_memmap = NULL;
-	unsigned long size;
-	efi_status_t status;
-	u64 end, systab;
-	int count = 0;
-
-	efi.systab = NULL;
-
-	/*
-	 * We don't do virtual mode, since we don't do runtime services, on
-	 * non-native EFI
-	 */
-	if (!efi_is_native()) {
-		efi_unmap_memmap();
-		return;
-	}
 
-	/* Merge contiguous regions of the same type and attribute */
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		u64 prev_size;
 		md = p;
@@ -837,6 +805,18 @@ void __init efi_enter_virtual_mode(void)
 		prev_md = md;
 
 	}
+}
+
+/*
+ * Map efi memory ranges for runtime serivce and
+ * update new_memmap with virtual addresses.
+ */
+static void * __init efi_map_regions(int *count)
+{
+	efi_memory_desc_t *md;
+	void *p, *new_memmap = NULL;
+	unsigned long size;
+	u64 end, systab;
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
@@ -861,14 +841,60 @@ void __init efi_enter_virtual_mode(void)
 		}
 
 		new_memmap = krealloc(new_memmap,
-				      (count + 1) * memmap.desc_size,
-				      GFP_KERNEL);
+					(*count + 1) * memmap.desc_size,
+					GFP_KERNEL);
 		if (!new_memmap)
-			goto err_out;
+			goto ret;
 
-		memcpy(new_memmap + (count * memmap.desc_size), md,
+		memcpy(new_memmap + (*count * memmap.desc_size), md,
 		       memmap.desc_size);
-		count++;
+		(*count)++;
+	}
+
+ret:
+	return new_memmap;
+}
+
+/*
+ * This function will switch the EFI runtime services to virtual mode.
+ * Essentially, we look through the EFI memmap and map every region that
+ * has the runtime attribute bit set in its memory descriptor into the
+ * ->trampoline_pgd page table using a top-down VA allocation scheme.
+ *
+ * The old method which used to update that memory descriptor with the
+ * virtual address obtained from ioremap() is still supported when the
+ * kernel is booted with efi=old_map on its command line. Same old
+ * method enabled the runtime services to be called without having to
+ * thunk back into physical mode for every invocation.
+ *
+ * The new method does a pagetable switch in a preemption-safe manner
+ * so that we're in a different address space when calling a runtime
+ * function. For function arguments passing we do copy the PGDs of the
+ * kernel page table into ->trampoline_pgd prior to each call.
+ */
+void __init efi_enter_virtual_mode(void)
+{
+	efi_status_t status;
+	void *new_memmap = NULL;
+	int count = 0;
+
+	efi.systab = NULL;
+
+	/*
+	 * We don't do virtual mode, since we don't do runtime services, on
+	 * non-native EFI
+	 */
+	if (!efi_is_native()) {
+		efi_unmap_memmap();
+		return;
+	}
+
+	efi_merge_regions();
+
+	new_memmap = efi_map_regions(&count);
+	if (!new_memmap) {
+		pr_err("Error reallocating memory, EFI runtime non-functional!\n");
+		return;
 	}
 
 	BUG_ON(!efi.systab);
@@ -922,9 +948,6 @@ void __init efi_enter_virtual_mode(void)
 			 0, NULL);
 
 	return;
-
- err_out:
-	pr_err("Error reallocating memory, EFI runtime non-functional!\n");
 }
 
 /*

[patch 5/9 v3] efi: export more efi table variable to sysfs

[dyoung]

[-- Attachment #1: 05-export-more-efi-sysfs-vars.patch --]
[-- Type: text/plain, Size: 5861 bytes --]

Export fw_vendor, runtime and config tables physical
addresses to /sys/firmware/efi/systab becaue kexec
kernel will need them.

>From EFI spec these 3 variables will be updated to
virtual address after entering virtual mode. But
kernel startup code will need the physical address.

changelog:
Greg: add standalone sysfs files instead of add lines to systab
Document them as testing ABI

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/ABI/testing/sysfs-firmware-efi |   26 +++++++++
 arch/x86/platform/efi/efi.c                  |    4 +
 drivers/firmware/efi/efi.c                   |   71 +++++++++++++++++++++++++--
 include/linux/efi.h                          |    3 +
 4 files changed, 101 insertions(+), 3 deletions(-)

--- efi.orig/drivers/firmware/efi/efi.c
+++ efi/drivers/firmware/efi/efi.c
@@ -32,6 +32,9 @@ struct efi __read_mostly efi = {
 	.hcdp       = EFI_INVALID_TABLE_ADDR,
 	.uga        = EFI_INVALID_TABLE_ADDR,
 	.uv_systab  = EFI_INVALID_TABLE_ADDR,
+	.fw_vendor  = EFI_INVALID_TABLE_ADDR,
+	.runtime    = EFI_INVALID_TABLE_ADDR,
+	.config_table  = EFI_INVALID_TABLE_ADDR,
 };
 EXPORT_SYMBOL(efi);
 
@@ -71,6 +74,31 @@ static ssize_t systab_show(struct kobjec
 static struct kobj_attribute efi_attr_systab =
 			__ATTR(systab, 0400, systab_show, NULL);
 
+static ssize_t fw_vendor_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "0x%lx\n", efi.fw_vendor);
+}
+
+static ssize_t runtime_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "0x%lx\n", efi.runtime);
+}
+
+static ssize_t config_table_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "0x%lx\n", efi.config_table);
+}
+
+static struct kobj_attribute efi_attr_fw_vendor =
+				__ATTR(fw_vendor, 0400, fw_vendor_show, NULL);
+static struct kobj_attribute efi_attr_runtime =
+			__ATTR(runtime, 0400, runtime_show, NULL);
+static struct kobj_attribute efi_attr_config_table =
+			__ATTR(config_table, 0400, config_table_show, NULL);
+
 static struct attribute *efi_subsys_attrs[] = {
 	&efi_attr_systab.attr,
 	NULL,	/* maybe more in the future? */
@@ -123,21 +151,58 @@ static int __init efisubsys_init(void)
 
 	error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
 	if (error) {
-		pr_err("efi: Sysfs attribute export failed with error %d.\n",
-		       error);
+		pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
+		       efi_attr_systab.attr.name, error);
 		goto err_unregister;
 	}
 
+	if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR) {
+		error = sysfs_create_file(efi_kobj, &efi_attr_fw_vendor.attr);
+		if (error) {
+			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
+				efi_attr_fw_vendor.attr.name, error);
+			goto err_remove_group;
+		}
+	}
+
+	if (efi.runtime != EFI_INVALID_TABLE_ADDR) {
+		error = sysfs_create_file(efi_kobj, &efi_attr_runtime.attr);
+		if (error) {
+			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
+				efi_attr_runtime.attr.name, error);
+			goto err_remove_fw_vendor;
+		}
+	}
+
+	if (efi.config_table != EFI_INVALID_TABLE_ADDR) {
+		error = sysfs_create_file(efi_kobj,
+					&efi_attr_config_table.attr);
+		if (error) {
+			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
+				efi_attr_config_table.attr.name, error);
+			goto err_remove_runtime;
+		}
+	}
+
 	/* and the standard mountpoint for efivarfs */
 	efivars_kobj = kobject_create_and_add("efivars", efi_kobj);
 	if (!efivars_kobj) {
 		pr_err("efivars: Subsystem registration failed.\n");
 		error = -ENOMEM;
-		goto err_remove_group;
+		goto err_remove_config_table;
 	}
 
 	return 0;
 
+err_remove_config_table:
+	if (efi.config_table != EFI_INVALID_TABLE_ADDR)
+		sysfs_remove_file(efi_kobj, &efi_attr_config_table.attr);
+err_remove_runtime:
+	if (efi.runtime != EFI_INVALID_TABLE_ADDR)
+		sysfs_remove_file(efi_kobj, &efi_attr_runtime.attr);
+err_remove_fw_vendor:
+	if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR)
+		sysfs_remove_file(efi_kobj, &efi_attr_fw_vendor.attr);
 err_remove_group:
 	sysfs_remove_group(efi_kobj, &efi_subsys_attr_group);
 err_unregister:
--- efi.orig/include/linux/efi.h
+++ efi/include/linux/efi.h
@@ -556,6 +556,9 @@ extern struct efi {
 	unsigned long hcdp;		/* HCDP table */
 	unsigned long uga;		/* UGA table */
 	unsigned long uv_systab;	/* UV system table */
+	unsigned long fw_vendor;	/* fw_vendor */
+	unsigned long runtime;		/* runtime table */
+	unsigned long config_table;	/* config tables */
 	efi_get_time_t *get_time;
 	efi_set_time_t *set_time;
 	efi_get_wakeup_time_t *get_wakeup_time;
--- efi.orig/arch/x86/platform/efi/efi.c
+++ efi/arch/x86/platform/efi/efi.c
@@ -653,6 +653,10 @@ void __init efi_init(void)
 
 	set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);
 
+	efi.fw_vendor = (unsigned long)efi.systab->fw_vendor;
+	efi.runtime = (unsigned long)efi.systab->runtime;
+	efi.config_table = (unsigned long)efi.systab->tables;
+
 	/*
 	 * Show what we know for posterity
 	 */
--- /dev/null
+++ efi/Documentation/ABI/testing/sysfs-firmware-efi
@@ -0,0 +1,26 @@
+What:		/sys/firmware/efi/fw_vendor
+Date:		Nov 2013
+Contact:	Dave Young <dyoung@redhat.org>
+Description:
+		Shows the physical address of firmware vendor in the EFI
+		system table.
+Users:
+		Kexec Mailing List <kexec@lists.infradead.org>
+
+What:		/sys/firmware/efi/runtime
+Date:		Nov 2013
+Contact:	Dave Young <dyoung@redhat.org>
+Description:
+		Shows the physical address of runtime service table in the
+		EFI system table.
+Users:
+		Kexec Mailing List <kexec@lists.infradead.org>
+
+What:		/sys/firmware/efi/config_table
+Date:		Nov 2013
+Contact:	Dave Young <dyoung@redhat.org>
+Description:
+		Shows the physical address of config table in the EFI
+		system table.
+Users:
+		Kexec Mailing List <kexec@lists.infradead.org>

[patch 6/9 v3] efi: export efi runtime memory mapping to sysfs

[dyoung]

[-- Attachment #1: 06-export-efi-runtime-mapping.patch --]
[-- Type: text/plain, Size: 11534 bytes --]

kexec kernel will need exactly same mapping for
efi runtime memory ranges. Thus here export the
runtime ranges mapping to sysfs, kexec-tools
will assemble them and pass to 2nd kernel via
setup_data.

Introducing a new directly /sys/firmware/efi/runtime-map
Just like /sys/firmware/memmap. Containing below attribute
in each file of that directory:
attribute  num_pages  phys_addr  type  virt_addr

It will not work for efi 32bit. Only x86_64 currently.

Matt: s/efi-runtime-map.c/runtime-map.c
      change dir name to runtime-map
update to use desc_size in efi_runtime_map
cleaup the code, add function efi_save_runtime_map
improve err handling

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/ABI/testing/sysfs-firmware-efi-runtime-map |   45 +++
 arch/x86/platform/efi/efi.c                              |   26 +
 drivers/firmware/efi/Kconfig                             |   10 
 drivers/firmware/efi/Makefile                            |    1 
 drivers/firmware/efi/efi.c                               |    3 
 drivers/firmware/efi/runtime-map.c                       |  200 +++++++++++++++
 include/linux/efi.h                                      |    6 
 7 files changed, 290 insertions(+), 1 deletion(-)

--- efi.orig/arch/x86/platform/efi/efi.c
+++ efi/arch/x86/platform/efi/efi.c
@@ -76,6 +76,9 @@ static __initdata efi_config_table_type_
 	{NULL_GUID, NULL, NULL},
 };
 
+void *efi_runtime_map;
+int nr_efi_runtime_map;
+
 /*
  * Returns 1 if 'facility' is enabled, 0 otherwise.
  */
@@ -811,6 +814,21 @@ static void __init efi_merge_regions(voi
 	}
 }
 
+static int __init efi_save_runtime_map(efi_memory_desc_t *md)
+{
+	efi_runtime_map = krealloc(efi_runtime_map,
+			(nr_efi_runtime_map + 1) *
+			memmap.desc_size, GFP_KERNEL);
+	if (!efi_runtime_map)
+		return -ENOMEM;
+
+	memcpy(efi_runtime_map + nr_efi_runtime_map * memmap.desc_size,
+		md, memmap.desc_size);
+	nr_efi_runtime_map++;
+
+	return 0;
+}
+
 /*
  * Map efi memory ranges for runtime serivce and
  * update new_memmap with virtual addresses.
@@ -821,6 +839,7 @@ static void * __init efi_map_regions(int
 	void *p, *new_memmap = NULL;
 	unsigned long size;
 	u64 end, systab;
+	int error = 0;
 
 	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
 		md = p;
@@ -852,9 +871,16 @@ static void * __init efi_map_regions(int
 
 		memcpy(new_memmap + (*count * memmap.desc_size), md,
 		       memmap.desc_size);
+		if (!error && md->type != EFI_BOOT_SERVICES_CODE &&
+				md->type != EFI_BOOT_SERVICES_DATA)
+			error = efi_save_runtime_map(md);
 		(*count)++;
 	}
 
+	if (error) {
+		nr_efi_runtime_map = 0;
+		kfree(efi_runtime_map);
+	}
 ret:
 	return new_memmap;
 }
--- efi.orig/drivers/firmware/efi/Kconfig
+++ efi/drivers/firmware/efi/Kconfig
@@ -36,4 +36,14 @@ config EFI_VARS_PSTORE_DEFAULT_DISABLE
 	  backend for pstore by default. This setting can be overridden
 	  using the efivars module's pstore_disable parameter.
 
+config EFI_RUNTIME_MAP
+	bool "Export efi runtime maps to sysfs" if EXPERT
+	default X86 && EFI
+	help
+	  Export efi runtime memory maps to /sys/firmware/efi/runtime-map.
+	  That memory map is used for example by kexec to set up efi virtual
+	  mapping the 2nd kernel, but can also be used for debugging purposes.
+
+	  See also Documentation/ABI/testing/sysfs-firmware-efi-runtime-map.
+
 endmenu
--- efi.orig/drivers/firmware/efi/Makefile
+++ efi/drivers/firmware/efi/Makefile
@@ -4,3 +4,4 @@
 obj-y					+= efi.o vars.o
 obj-$(CONFIG_EFI_VARS)			+= efivars.o
 obj-$(CONFIG_EFI_VARS_PSTORE)		+= efi-pstore.o
+obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
--- /dev/null
+++ efi/drivers/firmware/efi/runtime-map.c
@@ -0,0 +1,200 @@
+/*
+ * linux/drivers/efi/runtime-map.c
+ * Copyright (C) 2013 Red Hat, Inc., Dave Young <dyoung@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation
+ *
+ * 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.
+ *
+ */
+
+#include <linux/string.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/efi.h>
+#include <linux/slab.h>
+
+#include <asm/setup.h>
+
+struct efi_runtime_map_entry {
+	efi_memory_desc_t md;
+	struct kobject kobj;   /* kobject for each entry */
+};
+
+static struct efi_runtime_map_entry **map_entries;
+
+static ssize_t map_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf);
+static ssize_t type_show(struct efi_runtime_map_entry *entry, char *buf);
+static ssize_t phys_addr_show(struct efi_runtime_map_entry *entry, char *buf);
+static ssize_t virt_addr_show(struct efi_runtime_map_entry *entry, char *buf);
+static ssize_t num_pages_show(struct efi_runtime_map_entry *entry, char *buf);
+static ssize_t attribute_show(struct efi_runtime_map_entry *entry, char *buf);
+
+struct map_attribute {
+	struct attribute attr;
+	ssize_t (*show)(struct efi_runtime_map_entry *entry, char *buf);
+};
+
+static struct map_attribute map_type_attr = __ATTR_RO(type);
+static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
+static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
+static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
+static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+
+/*
+ * These are default attributes that are added for every memmap entry.
+ */
+static struct attribute *def_attrs[] = {
+	&map_type_attr.attr,
+	&map_phys_addr_attr.attr,
+	&map_virt_addr_attr.attr,
+	&map_num_pages_attr.attr,
+	&map_attribute_attr.attr,
+	NULL
+};
+
+static const struct sysfs_ops map_attr_ops = {
+	.show = map_attr_show,
+};
+
+static inline struct efi_runtime_map_entry *
+to_map_entry(struct kobject *kobj)
+{
+	return container_of(kobj, struct efi_runtime_map_entry, kobj);
+}
+
+static void map_release(struct kobject *kobj)
+{
+	struct efi_runtime_map_entry *entry;
+
+	entry = to_map_entry(kobj);
+	kfree(entry);
+}
+
+static struct kobj_type __refdata map_ktype = {
+	.sysfs_ops	= &map_attr_ops,
+	.default_attrs	= def_attrs,
+	.release	= map_release,
+};
+
+static ssize_t type_show(struct efi_runtime_map_entry *entry, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%x\n", entry->md.type);
+}
+
+static ssize_t phys_addr_show(struct efi_runtime_map_entry *entry, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n", entry->md.phys_addr);
+}
+
+static ssize_t virt_addr_show(struct efi_runtime_map_entry *entry, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n", entry->md.virt_addr);
+}
+
+static ssize_t num_pages_show(struct efi_runtime_map_entry *entry, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n", entry->md.num_pages);
+}
+
+static ssize_t attribute_show(struct efi_runtime_map_entry *entry, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n", entry->md.attribute);
+}
+
+static inline struct map_attribute *to_map_attr(struct attribute *attr)
+{
+	return container_of(attr, struct map_attribute, attr);
+}
+
+static ssize_t map_attr_show(struct kobject *kobj,
+				struct attribute *attr, char *buf)
+{
+	struct efi_runtime_map_entry *entry = to_map_entry(kobj);
+	struct map_attribute *map_attr = to_map_attr(attr);
+
+	return map_attr->show(entry, buf);
+}
+
+static struct kset *map_kset;
+
+/*
+ * Add map entry on sysfs
+ */
+static struct efi_runtime_map_entry *add_sysfs_runtime_map_entry(int nr)
+{
+	int ret;
+	unsigned long desc_size;
+	struct efi_runtime_map_entry *entry;
+	struct efi_info *e = &boot_params.efi_info;
+
+	if (!map_kset) {
+		map_kset = kset_create_and_add("runtime-map", NULL,
+				efi_kobj);
+		if (!map_kset)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry) {
+		kset_unregister(map_kset);
+		return entry;
+	}
+
+	desc_size = e->efi_memdesc_size;
+	memcpy(&entry->md, efi_runtime_map + nr * desc_size,
+		sizeof(efi_memory_desc_t));
+
+	kobject_init(&entry->kobj, &map_ktype);
+	entry->kobj.kset = map_kset;
+	ret = kobject_add(&entry->kobj, NULL, "%d", nr);
+	if (ret) {
+		kobject_put(&entry->kobj);
+		kset_unregister(map_kset);
+		return ERR_PTR(ret);
+	}
+
+	return entry;
+}
+
+static int __init efi_runtime_map_init(void)
+{
+	int i, j, ret = 0;
+	struct efi_runtime_map_entry *entry;
+
+	if (!efi_runtime_map) {
+		pr_warn("no efi_runtime_map found\n");
+		return -EINVAL;
+	}
+
+	map_entries = kzalloc(nr_efi_runtime_map *
+		sizeof(struct efi_runtime_map_entry *), GFP_KERNEL);
+	if (!map_entries)
+		return -ENOMEM;
+
+	for (i = 0; i < nr_efi_runtime_map; i++) {
+		entry = add_sysfs_runtime_map_entry(i);
+		if (IS_ERR(entry)) {
+			for (j = i - 1; j > 0; j--) {
+				entry = *(map_entries + j);
+				kobject_put(&entry->kobj);
+			}
+			if (map_kset)
+				kset_unregister(map_kset);
+			ret = PTR_ERR(entry);
+			goto out;
+		}
+		*(map_entries + i) = entry;
+	}
+
+out:
+	return ret;
+}
+late_initcall(efi_runtime_map_init);
--- efi.orig/drivers/firmware/efi/efi.c
+++ efi/drivers/firmware/efi/efi.c
@@ -38,7 +38,8 @@ struct efi __read_mostly efi = {
 };
 EXPORT_SYMBOL(efi);
 
-static struct kobject *efi_kobj;
+struct kobject *efi_kobj;
+EXPORT_SYMBOL_GPL(efi_kobj);
 static struct kobject *efivars_kobj;
 
 /*
--- /dev/null
+++ efi/Documentation/ABI/testing/sysfs-firmware-efi-runtime-map
@@ -0,0 +1,45 @@
+What:		/sys/firmware/efi/runtime-map/
+Date:		Oct 2013
+Contact:	Dave Young <dyoung@redhat.com>
+Description:
+		Switching efi runtime services to virtual mode requires
+		that all efi memory ranges which has the runtime attribute
+		bit set to be mapped to virtual addresses.
+
+		In kexec kernel kernel can not entering virtual mode again
+		because there's a limitation that SetVirtualAddressMap can
+		only be called once for entering virtual mode. But kexec
+		kernel still need maintain same physical address to virtual
+		address mapping as the 1st kernel. The mappings are exported
+		to sysfs so userspace tools can reassemble them and pass them
+		into kexec kernel.
+
+		/sys/firmware/efi/runtim-map/ is what kernel export for
+		this purpose. The structure is as follows:
+
+		subdirectories are named with the number of the memory range:
+
+			/sys/firmware/efi/runtime-map/0
+			/sys/firmware/efi/runtime-map/1
+			/sys/firmware/efi/runtime-map/2
+			/sys/firmware/efi/runtime-map/3
+			...
+
+		Each subdirectory contains five files:
+
+		attribute : The attribute of the memory range.
+		num_pages : The size of the memory range in page number.
+		phys_addr : The start physical address of the memory range.
+		type      : The type of the memory range.
+		virt_addr : The start virtual address of the memory range.
+
+		Above values are all hexadecimal number with the '0x' prefix.
+
+		So, for example:
+
+			/sys/firmware/efi/runtime-map/0/attribute
+			/sys/firmware/efi/runtime-map/0/num_pages
+			/sys/firmware/efi/runtime-map/0/phys_addr
+			/sys/firmware/efi/runtime-map/0/type
+			/sys/firmware/efi/runtime-map/0/virt_addr
+                        ...
--- efi.orig/include/linux/efi.h
+++ efi/include/linux/efi.h
@@ -872,4 +872,10 @@ int efivars_sysfs_init(void);
 
 #endif /* CONFIG_EFI_VARS */
 
+#ifdef CONFIG_EFI_RUNTIME_MAP
+extern void *efi_runtime_map;
+extern int nr_efi_runtime_map;
+extern struct kobject *efi_kobj;
+#endif
+
 #endif /* _LINUX_EFI_H */

[patch 7/9 v3] efi: passing kexec necessary efi data via setup_data

[dyoung]

[-- Attachment #1: 07-use-efi-setup-data.patch --]
[-- Type: text/plain, Size: 8655 bytes --]

Add a new setup_data type SETUP_EFI for kexec use.
Passing the saved fw_vendor, runtime, config tables and
efi runtime mappings.

When entering virtual mode, directly mapping the efi
runtime ragions which we passed in previously. And skip
the step to call SetVirtualAddressMap.

Specially for HP z420 workstation it need another variable
saving, it's the smbios physical address, the HP bios
also update the SMBIOS address after entering virtual mode
besides of the standard fw_vendor,runtime and config table.

Tested on ovmf+qemu, lenovo thinkpad, a dell laptop and an
HP z420 workstation.

v2: refresh based on previous patch changes, code cleanup.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 arch/x86/include/asm/efi.h            |   12 ++
 arch/x86/include/uapi/asm/bootparam.h |    1 
 arch/x86/kernel/setup.c               |    3 
 arch/x86/platform/efi/efi.c           |  158 ++++++++++++++++++++++++++++++----
 4 files changed, 157 insertions(+), 17 deletions(-)

--- efi.orig/arch/x86/kernel/setup.c
+++ efi/arch/x86/kernel/setup.c
@@ -447,6 +447,9 @@ static void __init parse_setup_data(void
 		case SETUP_DTB:
 			add_dtb(pa_data);
 			break;
+		case SETUP_EFI:
+			parse_efi_setup(pa_data, data_len);
+			break;
 		default:
 			break;
 		}
--- efi.orig/arch/x86/platform/efi/efi.c
+++ efi/arch/x86/platform/efi/efi.c
@@ -78,6 +78,7 @@ static __initdata efi_config_table_type_
 
 void *efi_runtime_map;
 int nr_efi_runtime_map;
+struct efi_setup_data *esdata;
 
 /*
  * Returns 1 if 'facility' is enabled, 0 otherwise.
@@ -115,6 +116,32 @@ static int __init setup_storage_paranoia
 }
 early_param("efi_no_storage_paranoia", setup_storage_paranoia);
 
+void __init parse_efi_setup(u64 phys_addr, u32 data_len)
+{
+	int size;
+	struct setup_data *sdata;
+	u64 esdata_phys;
+
+	if (!efi_enabled(EFI_64BIT)) {
+		pr_warn("skipping setup_data on EFI 32BIT!");
+		return;
+	}
+
+	sdata = early_memremap(phys_addr, data_len);
+	if (!sdata)
+		return;
+
+	size = data_len - sizeof(struct setup_data);
+
+	esdata_phys = phys_addr + sizeof(struct setup_data);
+
+	nr_efi_runtime_map = (size - sizeof(struct efi_setup_data)) /
+			sizeof(efi_memory_desc_t);
+	early_iounmap(sdata, data_len);
+
+	/* setup data regions have been reserved by default */
+	esdata = phys_to_virt(esdata_phys);
+}
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {
@@ -504,8 +531,12 @@ static int __init efi_systab_init(void *
 		}
 
 		efi_systab.hdr = systab64->hdr;
-		efi_systab.fw_vendor = systab64->fw_vendor;
-		tmp |= systab64->fw_vendor;
+
+		if (esdata)
+			efi_systab.fw_vendor = (unsigned long)esdata->fw_vendor;
+		else
+			efi_systab.fw_vendor = systab64->fw_vendor;
+		tmp |= efi_systab.fw_vendor;
 		efi_systab.fw_revision = systab64->fw_revision;
 		efi_systab.con_in_handle = systab64->con_in_handle;
 		tmp |= systab64->con_in_handle;
@@ -519,13 +550,21 @@ static int __init efi_systab_init(void *
 		tmp |= systab64->stderr_handle;
 		efi_systab.stderr = systab64->stderr;
 		tmp |= systab64->stderr;
-		efi_systab.runtime = (void *)(unsigned long)systab64->runtime;
-		tmp |= systab64->runtime;
+		if (esdata)
+			efi_systab.runtime =
+				(void *)(unsigned long)esdata->runtime;
+		else
+			efi_systab.runtime =
+				(void *)(unsigned long)systab64->runtime;
+		tmp |= (unsigned long)efi_systab.runtime;
 		efi_systab.boottime = (void *)(unsigned long)systab64->boottime;
 		tmp |= systab64->boottime;
 		efi_systab.nr_tables = systab64->nr_tables;
-		efi_systab.tables = systab64->tables;
-		tmp |= systab64->tables;
+		if (esdata)
+			efi_systab.tables = (unsigned long)esdata->tables;
+		else
+			efi_systab.tables = systab64->tables;
+		tmp |= efi_systab.tables;
 
 		early_iounmap(systab64, sizeof(*systab64));
 #ifdef CONFIG_X86_32
@@ -631,6 +670,41 @@ static int __init efi_memmap_init(void)
 	return 0;
 }
 
+static int __init efi_reuse_config(u64 tables, int nr_tables)
+{
+	void *p, *tablep;
+	int i, sz;
+
+	if (!efi_enabled(EFI_64BIT))
+		return 0;
+
+	sz = sizeof(efi_config_table_64_t);
+
+	p = tablep = early_memremap(tables, nr_tables * sz);
+	if (!p) {
+		pr_err("Could not map Configuration table!\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < efi.systab->nr_tables; i++) {
+		efi_guid_t guid;
+
+		guid = ((efi_config_table_64_t *)p)->guid;
+
+		/*
+		HP z420 workstation smbios will be convert to
+		virtual address after enter virtual mode.
+		Thus in case kexec/kdump the physical address
+		will be passed in setup_data.
+		*/
+		if (!efi_guidcmp(guid, SMBIOS_TABLE_GUID))
+			((efi_config_table_64_t *)p)->table = esdata->smbios;
+		p += sz;
+	}
+	early_iounmap(tablep, nr_tables * sz);
+	return 0;
+}
+
 void __init efi_init(void)
 {
 	efi_char16_t *c16;
@@ -676,6 +750,9 @@ void __init efi_init(void)
 		efi.systab->hdr.revision >> 16,
 		efi.systab->hdr.revision & 0xffff, vendor);
 
+	if (esdata && esdata->smbios)
+		efi_reuse_config(efi.systab->tables, efi.systab->nr_tables);
+
 	if (efi_config_init(arch_tables))
 		return;
 
@@ -886,6 +963,43 @@ ret:
 }
 
 /*
+ * map efi regions which was passed via setup_data
+ * the virt_addr is a fixed addr which was used in
+ * 1st kernel of kexec boot.
+ */
+static void __init efi_map_regions_fixed(void)
+{
+	int i;
+	unsigned long size;
+	efi_memory_desc_t *md;
+	u64 end, systab;
+	void *p;
+
+	efi_runtime_map = kzalloc(nr_efi_runtime_map * memmap.desc_size,
+				GFP_KERNEL);
+	if (!efi_runtime_map)
+		pr_err("Out of memory, EFI runtime on nested kexec non-functional!\n");
+
+	for (i = 0, p = efi_runtime_map; i < nr_efi_runtime_map; i++) {
+		md = esdata->map + i;
+		efi_map_region_fixed(md);
+		size = md->num_pages << PAGE_SHIFT;
+		end = md->phys_addr + size;
+
+		systab = (u64) (unsigned long) efi_phys.systab;
+		if (md->phys_addr <= systab && systab < end) {
+			systab += md->virt_addr - md->phys_addr;
+			efi.systab =
+				(efi_system_table_t *) (unsigned long) systab;
+		}
+		if (efi_runtime_map) {
+			memcpy(p, md, memmap.desc_size);
+			p += memmap.desc_size;
+		}
+	}
+}
+
+/*
  * This function will switch the EFI runtime services to virtual mode.
  * Essentially, we look through the EFI memmap and map every region that
  * has the runtime attribute bit set in its memory descriptor into the
@@ -901,6 +1015,10 @@ ret:
  * so that we're in a different address space when calling a runtime
  * function. For function arguments passing we do copy the PGDs of the
  * kernel page table into ->trampoline_pgd prior to each call.
+ *
+ * Specially for kexec boot efi runtime maps in previous kernel should
+ * be passed in via setup_data. In that case runtime ranges will be mapped
+ * to fixed virtual addresses exactly same as the ones in previous kernel.
  */
 void __init efi_enter_virtual_mode(void)
 {
@@ -919,12 +1037,15 @@ void __init efi_enter_virtual_mode(void)
 		return;
 	}
 
-	efi_merge_regions();
-
-	new_memmap = efi_map_regions(&count);
-	if (!new_memmap) {
-		pr_err("Error reallocating memory, EFI runtime non-functional!\n");
-		return;
+	if (esdata)
+		efi_map_regions_fixed();
+	else {
+		efi_merge_regions();
+		new_memmap = efi_map_regions(&count);
+		if (!new_memmap) {
+			pr_err("Error reallocating memory, EFI runtime non-functional!\n");
+			return;
+		}
 	}
 
 	BUG_ON(!efi.systab);
@@ -932,11 +1053,14 @@ void __init efi_enter_virtual_mode(void)
 	efi_setup_page_tables();
 	efi_sync_low_kernel_mappings();
 
-	status = phys_efi_set_virtual_address_map(
-		memmap.desc_size * count,
-		memmap.desc_size,
-		memmap.desc_version,
-		(efi_memory_desc_t *)__pa(new_memmap));
+	if (esdata)
+		status = EFI_SUCCESS;
+	else
+		status = phys_efi_set_virtual_address_map(
+			memmap.desc_size * count,
+			memmap.desc_size,
+			memmap.desc_version,
+			(efi_memory_desc_t *)__pa(new_memmap));
 
 	if (status != EFI_SUCCESS) {
 		pr_alert("Unable to switch EFI into virtual mode "
--- efi.orig/arch/x86/include/uapi/asm/bootparam.h
+++ efi/arch/x86/include/uapi/asm/bootparam.h
@@ -6,6 +6,7 @@
 #define SETUP_E820_EXT			1
 #define SETUP_DTB			2
 #define SETUP_PCI			3
+#define SETUP_EFI			4
 
 /* ram_size flags */
 #define RAMDISK_IMAGE_START_MASK	0x07FF
--- efi.orig/arch/x86/include/asm/efi.h
+++ efi/arch/x86/include/asm/efi.h
@@ -133,6 +133,18 @@ extern void efi_sync_low_kernel_mappings
 extern void efi_setup_page_tables(void);
 extern void __init old_map_region(efi_memory_desc_t *md);
 
+struct efi_setup_data {
+	u64 fw_vendor;
+	u64 runtime;
+	u64 tables;
+	u64 smbios;
+	u64 reserved[8];
+	efi_memory_desc_t map[0];
+};
+
+extern void parse_efi_setup(u64 phys_addr, u32 data_len);
+extern struct efi_setup_data *esdata;
+
 #ifdef CONFIG_EFI
 
 static inline bool efi_is_native(void)

[patch 8/9 v3] x86: add xloadflags bit for efi runtime support on kexec

[dyoung]

[-- Attachment #1: 08-add-xloadflags-for-kexec-efi-runtime-feature.patch --]
[-- Type: text/plain, Size: 1953 bytes --]

Old kexec-tools can not load new kernel. The reason is previously kexec-tools
do not fill efi_info in x86 setup header thus efi init fail and switch
to noefi boot. In new kexec-tools it will by default fill efi_info and
pass other efi required infomation to 2nd kernel so kexec kernel efi
initialization will success finally.

To prevent from breaking userspace, add a new xloadflags bit so kexec-tools
will check the flag and switch to old logic.

changelog:
Matt: +&& defined(CONFIG_KEXEC)
HPA: document the flag.

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/x86/boot.txt            |    3 +++
 arch/x86/boot/header.S                |    9 ++++++++-
 arch/x86/include/uapi/asm/bootparam.h |    1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

--- efi.orig/arch/x86/boot/header.S
+++ efi/arch/x86/boot/header.S
@@ -391,7 +391,14 @@ xloadflags:
 #else
 # define XLF23 0
 #endif
-			.word XLF0 | XLF1 | XLF23
+
+#if defined(CONFIG_X86_64) && defined(CONFIG_EFI) && defined(CONFIG_KEXEC)
+# define XLF4 XLF_EFI_KEXEC
+#else
+# define XLF4 0
+#endif
+
+			.word XLF0 | XLF1 | XLF23 | XLF4
 
 cmdline_size:   .long   COMMAND_LINE_SIZE-1     #length of the command line,
                                                 #added with boot protocol
--- efi.orig/arch/x86/include/uapi/asm/bootparam.h
+++ efi/arch/x86/include/uapi/asm/bootparam.h
@@ -24,6 +24,7 @@
 #define XLF_CAN_BE_LOADED_ABOVE_4G	(1<<1)
 #define XLF_EFI_HANDOVER_32		(1<<2)
 #define XLF_EFI_HANDOVER_64		(1<<3)
+#define XLF_EFI_KEXEC			(1<<4)
 
 #ifndef __ASSEMBLY__
 
--- efi.orig/Documentation/x86/boot.txt
+++ efi/Documentation/x86/boot.txt
@@ -608,6 +608,9 @@ Protocol:       2.12+
 	- If 1, the kernel supports the 64-bit EFI handoff entry point
           given at handover_offset + 0x200.
 
+  Bit 4 (read): XLF_EFI_KEXEC
+	- If 1, the kernel supports kexec EFI boot with EFI runtime support.
+
 Field name:	cmdline_size
 Type:		read
 Offset/size:	0x238/4

[patch 9/9 v3] x86: export x86 boot_params to sysfs

[dyoung]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 09-add-sysfs-boot-param.patch --]
[-- Type: text/plain, Size: 9999 bytes --]

kexec-tools use boot_params for getting the 1st kernel hardware_subarch,
the kexec kernel efi runtime support also need read the old efi_info from
boot_params. Currently it exists in debugfs which is not a good place for
such infomation. Per HPA, we should avoid of "sploit debugfs".

In this patch /sys/kernel/boot_params are exported, also the setup_data
is exported as a subdirectory. For original debugfs since it's already
there for long time and kexec-tools is using it for hardware_subarch so
let's do not remove them for now.

Structure are like below:

/sys/kernel/boot_params
├── data		/* binary data for boot_params */
├── setup_data  	/* subdirectory for setup_data if there's any */
│   ├── 0		/* the first setup_data node */
│   │   ├── data	/* binary data for setup_data node 0 */
│   │   └── type	/* setup_data type of setup_data node 0, hex string */
|   [snip]		/* other setup_data nodes ... */
└── version		/* hex string for boot protocal version */

Signed-off-by: Dave Young <dyoung@redhat.com>
---
 Documentation/ABI/testing/sysfs-kernel-boot_params |   40 ++
 arch/x86/kernel/Makefile                           |    2 
 arch/x86/kernel/ksysfs.c                           |  326 +++++++++++++++++++++
 3 files changed, 367 insertions(+), 1 deletion(-)

--- /dev/null
+++ efi/Documentation/ABI/testing/sysfs-kernel-boot_params
@@ -0,0 +1,40 @@
+What:		/sys/kernel/boot_params
+Date:		November 2013
+Contact:	Dave Young <dyoung@redhat.com>
+Description:
+		The /sys/kernel/boot_params directory contains two
+		files: "data" and "version" and one subdirectory "setup_data".
+		It is used to export the kernel boot parameters of x86
+		platform to user space for kexec and debugging purpose.
+
+		If there's no setup_data in boot_params the subdirectory will
+		not be created.
+
+		"data" file is the binary representation of struct boot_params.
+
+		"version" file is the string representation of boot
+		protocol version.
+
+		"setup_data" subdirectory contains the setup_data data
+		structure in boot_params. setup_data is maintained in kernel
+		as a link list. In "setup_data" subdirectory there's one
+		subdirectory for each link list node named with the number
+		of the list nodes. The list node subdirectory contains two
+		files "type" and "data". "type" file is the string
+		representation of setup_data type. "data" file is the binary
+		representation of setup_data payload.
+
+		The whole boot_params directory structure is like below:
+		/sys/kernel/boot_params
+		├── data
+		├── setup_data
+		│   ├── 0
+		│   │   ├── data
+		│   │   └── type
+		│   └── 1
+		│       ├── data
+		│       └── type
+		└── version
+
+Users:
+		Kexec Mailing List <kexec@lists.infradead.org>
--- efi.orig/arch/x86/kernel/Makefile
+++ efi/arch/x86/kernel/Makefile
@@ -35,7 +35,7 @@ obj-y			+= alternative.o i8253.o pci-nom
 obj-y			+= tsc.o io_delay.o rtc.o
 obj-y			+= pci-iommu_table.o
 obj-y			+= resource.o
-
+obj-$(CONFIG_SYSFS)	+= ksysfs.o
 obj-y				+= process.o
 obj-y				+= i387.o xsave.o
 obj-y				+= ptrace.o
--- /dev/null
+++ efi/arch/x86/kernel/ksysfs.c
@@ -0,0 +1,326 @@
+/*
+ * Architecture specific sysfs attributes in /sys/kernel
+ *
+ * Copyright (C) 2007, Intel Corp.
+ *      Huang Ying <ying.huang@intel.com>
+ * Copyright (C) 2013, 2013 Red Hat, Inc.
+ *      Dave Young <dyoung@redhat.com>
+ *
+ * This file is released under the GPLv2
+ */
+
+#include <linux/kobject.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/init.h>
+#include <linux/stat.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+
+#include <asm/setup.h>
+
+static ssize_t boot_params_version_show(struct kobject *kobj,
+					struct kobj_attribute *attr, char *buf)
+{
+	return sprintf(buf, "0x%04x\n", boot_params.hdr.version);
+}
+
+static struct kobj_attribute boot_params_version_attr =
+	__ATTR(version, S_IRUGO, boot_params_version_show, NULL);
+
+static ssize_t boot_params_data_read(struct file *fp, struct kobject *kobj,
+				     struct bin_attribute *bin_attr,
+				     char *buf, loff_t off, size_t count)
+{
+	memcpy(buf, (void *)&boot_params + off, count);
+	return count;
+}
+
+static struct bin_attribute boot_params_data_attr = {
+	.attr = {
+		.name = "data",
+		.mode = S_IRUGO,
+	},
+	.read = boot_params_data_read,
+	.size = sizeof(boot_params),
+};
+
+static int kobj_to_setup_data_nr(struct kobject *kobj, int *nr)
+{
+	const char *name;
+
+	name = kobject_name(kobj);
+	return kstrtoint(name, 10, nr);
+}
+
+static int get_setup_data_paddr(int nr, u64 *paddr)
+{
+	int i = 0;
+	struct setup_data *data;
+	u64 pa_data = boot_params.hdr.setup_data;
+
+	while (pa_data) {
+		if (nr == i) {
+			*paddr = pa_data;
+			return 0;
+		}
+		data = ioremap_cache(pa_data, sizeof(*data));
+		if (!data)
+			return -ENOMEM;
+
+		pa_data = data->next;
+		iounmap(data);
+		i++;
+	}
+	return -EINVAL;
+}
+
+static int __init get_setup_data_size(int nr, size_t *size)
+{
+	int i = 0;
+	struct setup_data *data;
+	u64 pa_data = boot_params.hdr.setup_data;
+
+	while (pa_data) {
+		data = ioremap_cache(pa_data, sizeof(*data));
+		if (!data)
+			return -ENOMEM;
+		if (nr == i) {
+			*size = data->len;
+			iounmap(data);
+			return 0;
+		}
+
+		pa_data = data->next;
+		iounmap(data);
+		i++;
+	}
+	return -EINVAL;
+}
+
+static ssize_t setup_data_type_show(struct kobject *kobj,
+					struct kobj_attribute *attr, char *buf)
+{
+	int nr, ret;
+	u64 paddr;
+	struct setup_data *data;
+
+	ret = kobj_to_setup_data_nr(kobj, &nr);
+	if (ret)
+		return ret;
+
+	ret = get_setup_data_paddr(nr, &paddr);
+	if (ret)
+		return ret;
+	data = ioremap_cache(paddr, sizeof(*data));
+	if (!data)
+		return -ENOMEM;
+
+	ret = sprintf(buf, "0x%x\n", data->type);
+	iounmap(data);
+	return ret;
+}
+
+static ssize_t setup_data_data_read(struct file *fp,
+				    struct kobject *kobj,
+				    struct bin_attribute *bin_attr,
+				    char *buf,
+				    loff_t off, size_t count)
+{
+	int nr, ret = 0;
+	u64 paddr;
+	struct setup_data *data;
+	void *p;
+
+	ret = kobj_to_setup_data_nr(kobj, &nr);
+	if (ret)
+		return ret;
+
+	ret = get_setup_data_paddr(nr, &paddr);
+	if (ret)
+		return ret;
+	data = ioremap_cache(paddr, sizeof(*data));
+	if (!data)
+		return -ENOMEM;
+
+	if (off > data->len) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (count > data->len - off)
+		count = data->len - off;
+
+	if (!count)
+		goto out;
+
+	ret = count;
+	p = ioremap_cache(paddr + sizeof(*data), data->len);
+	if (!p) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	memcpy(buf, p + off, count);
+	iounmap(p);
+out:
+	iounmap(data);
+	return ret;
+}
+
+static struct kobj_attribute type_attr =
+		__ATTR(type, S_IRUGO, setup_data_type_show, NULL);
+static struct bin_attribute data_attr = {
+	.attr = {
+		.name = "data",
+		.mode = S_IRUGO,
+	},
+	.read = setup_data_data_read,
+};
+
+static int __init create_setup_data_node(struct kobject *parent,
+					struct kobject **kobjp, int nr)
+{
+	int ret = 0;
+	size_t size;
+	struct kobject *kobj;
+	char name[16]; /* should be enough for setup_data nodes numbers */
+	snprintf(name, 16, "%d", nr);
+
+	kobj = kobject_create_and_add(name, parent);
+	if (!kobj)
+		return -ENOMEM;
+
+	ret = sysfs_create_file(kobj, &type_attr.attr);
+	if (ret)
+		goto out_kobj;
+
+	ret = get_setup_data_size(nr, &size);
+	if (ret)
+		goto out_kobj;
+
+	data_attr.size = size;
+	ret = sysfs_create_bin_file(kobj, &data_attr);
+	if (ret)
+		goto out_file;
+	*kobjp = kobj;
+
+	return 0;
+
+out_file:
+	sysfs_remove_file(kobj, &type_attr.attr);
+out_kobj:
+	kobject_put(kobj);
+	return ret;
+}
+
+static void __init cleanup_setup_data_node(struct kobject *kobj)
+{
+	sysfs_remove_file(kobj, &type_attr.attr);
+	sysfs_remove_bin_file(kobj, &data_attr);
+	kobject_put(kobj);
+}
+
+static int __init get_setup_data_total_num(u64 pa_data, int *nr)
+{
+	int ret = 0;
+	struct setup_data *data;
+
+	*nr = 0;
+	while (pa_data) {
+		*nr += 1;
+		data = ioremap_cache(pa_data, sizeof(*data));
+		if (!data) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		pa_data = data->next;
+		iounmap(data);
+	}
+
+out:
+	return ret;
+}
+
+static int __init create_setup_data_nodes(struct kobject *parent)
+{
+	struct kobject *setup_data_kobj, **kobjp;
+	u64 pa_data;
+	int i, j, nr, ret = 0;
+
+	pa_data = boot_params.hdr.setup_data;
+	if (!pa_data)
+		return 0;
+
+	setup_data_kobj = kobject_create_and_add("setup_data", parent);
+	if (!setup_data_kobj) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = get_setup_data_total_num(pa_data, &nr);
+	if (ret)
+		goto out_setup_data_kobj;
+
+	kobjp = kmalloc(sizeof(*kobjp) * nr, GFP_KERNEL);
+	if (!kobjp) {
+		ret = -ENOMEM;
+		goto out_setup_data_kobj;
+	}
+
+	for (i = 0; i < nr; i++) {
+		ret = create_setup_data_node(setup_data_kobj, kobjp + i, i);
+		if (ret) {
+			for (j = i - 1; j > 0; j--)
+				cleanup_setup_data_node(*(kobjp + j));
+			goto out_kmalloc;
+		}
+	}
+
+	kfree(kobjp);
+	return 0;
+
+out_kmalloc:
+	kfree(kobjp);
+out_setup_data_kobj:
+	kobject_put(setup_data_kobj);
+out:
+	return ret;
+}
+
+static int __init boot_params_ksysfs_init(void)
+{
+	int ret;
+	struct kobject *boot_params_kobj;
+
+	boot_params_kobj = kobject_create_and_add("boot_params",
+						       kernel_kobj);
+	if (!boot_params_kobj) {
+		ret = -ENOMEM;
+		goto out;
+	}
+	ret = sysfs_create_file(boot_params_kobj,
+				&boot_params_version_attr.attr);
+	if (ret)
+		goto out_boot_params_kobj;
+	ret = sysfs_create_bin_file(boot_params_kobj,
+				      &boot_params_data_attr);
+	if (ret)
+		goto out_create_file;
+
+	ret = create_setup_data_nodes(boot_params_kobj);
+	if (ret)
+		goto out_create_bin;
+
+	return 0;
+
+out_create_bin:
+	sysfs_remove_bin_file(boot_params_kobj, &boot_params_data_attr);
+out_create_file:
+	sysfs_remove_file(boot_params_kobj, &boot_params_version_attr.attr);
+out_boot_params_kobj:
+	kobject_put(boot_params_kobj);
+out:
+	return ret;
+}
+
+arch_initcall(boot_params_ksysfs_init);

Re: [patch 0/9 v3] kexec kernel efi runtime support

[Dave Young]

kexec-tools patches see below:
http://lists.infradead.org/pipermail/kexec/2013-November/010229.html

Re: [patch 1/9 v3] efi: remove unused variables in __map_region

[Borislav Petkov]

On Thu, Nov 21, 2013 at 02:17:05PM +0800, dyoung@redhat.com wrote:
> variables size and end is useless in this function, thus remove them.
> 
> Reported-by: Toshi Kani <toshi.kani@hp.com>
> Signed-off-by: Dave Young <dyoung@redhat.com>

Good catch.

Acked-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [patch 2/9 v3] efi: add a wrapper function efi_map_region_fixed

[Borislav Petkov]

On Thu, Nov 21, 2013 at 02:17:06PM +0800, dyoung@redhat.com wrote:
> Kexec kernel will use saved runtime virtual mapping, so add a
> new function efi_map_region_fixed for directly mapping a md
> to md->virt.
> 
> The md is passed in from 1st kernel, the virtual addr is
> saved in md->virt_addr.
> 
> Matt: coding style
>       reuse __map_region
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>

Except minor nitpick below:

Acked-by: Borislav Petkov <bp@suse.de>

> ---
>  arch/x86/include/asm/efi.h     |    1 +
>  arch/x86/platform/efi/efi_32.c |    2 ++
>  arch/x86/platform/efi/efi_64.c |   10 ++++++++++
>  3 files changed, 13 insertions(+)
> 
> --- efi.orig/arch/x86/include/asm/efi.h
> +++ efi/arch/x86/include/asm/efi.h
> @@ -128,6 +128,7 @@ extern void efi_call_phys_epilog(void);
>  extern void efi_unmap_memmap(void);
>  extern void efi_memory_uc(u64 addr, unsigned long size);
>  extern void __init efi_map_region(efi_memory_desc_t *md);
> +extern void __init efi_map_region_fixed(efi_memory_desc_t *md);
>  extern void efi_sync_low_kernel_mappings(void);
>  extern void efi_setup_page_tables(void);
>  extern void __init old_map_region(efi_memory_desc_t *md);
> --- efi.orig/arch/x86/platform/efi/efi_64.c
> +++ efi/arch/x86/platform/efi/efi_64.c
> @@ -199,6 +199,16 @@ void __init efi_map_region(efi_memory_de
>  	md->virt_addr = efi_va;
>  }
>  
> +/*
> + * kexec kernel will use efi_map_region_fixed to map efi
> + * runtime memory ranges. md->virt_addr is the original virtual
> + * address which had been mapped in kexec 1st kernel.
> + */

Why not stretch this comment to the full 80 cols?

/*
 * kexec kernel will use efi_map_region_fixed to map efi runtime memory ranges.
 * md->virt_addr is the original virtual address which had been mapped in kexec
 * 1st kernel.
 */

> +void __init efi_map_region_fixed(efi_memory_desc_t *md)
> +{
> +	__map_region(md, md->virt_addr);
> +}
> +
>  void __iomem *__init efi_ioremap(unsigned long phys_addr, unsigned long size,
>  				 u32 type, u64 attribute)
>  {
> --- efi.orig/arch/x86/platform/efi/efi_32.c
> +++ efi/arch/x86/platform/efi/efi_32.c
> @@ -47,6 +47,8 @@ void __init efi_map_region(efi_memory_de
>  	old_map_region(md);
>  }
>  
> +void __init efi_map_region_fixed(efi_memory_desc_t *md) {}
> +
>  void efi_call_phys_prelog(void)
>  {
>  	struct desc_ptr gdt_descr;
> 
> 

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Greg KH]

On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyoung@redhat.com wrote:
> Export fw_vendor, runtime and config tables physical
> addresses to /sys/firmware/efi/systab becaue kexec
> kernel will need them.
> 
> >From EFI spec these 3 variables will be updated to
> virtual address after entering virtual mode. But
> kernel startup code will need the physical address.
> 
> changelog:
> Greg: add standalone sysfs files instead of add lines to systab
> Document them as testing ABI
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  Documentation/ABI/testing/sysfs-firmware-efi |   26 +++++++++
>  arch/x86/platform/efi/efi.c                  |    4 +
>  drivers/firmware/efi/efi.c                   |   71 +++++++++++++++++++++++++--
>  include/linux/efi.h                          |    3 +
>  4 files changed, 101 insertions(+), 3 deletions(-)
> 
> --- efi.orig/drivers/firmware/efi/efi.c
> +++ efi/drivers/firmware/efi/efi.c
> @@ -32,6 +32,9 @@ struct efi __read_mostly efi = {
>  	.hcdp       = EFI_INVALID_TABLE_ADDR,
>  	.uga        = EFI_INVALID_TABLE_ADDR,
>  	.uv_systab  = EFI_INVALID_TABLE_ADDR,
> +	.fw_vendor  = EFI_INVALID_TABLE_ADDR,
> +	.runtime    = EFI_INVALID_TABLE_ADDR,
> +	.config_table  = EFI_INVALID_TABLE_ADDR,
>  };
>  EXPORT_SYMBOL(efi);
>  
> @@ -71,6 +74,31 @@ static ssize_t systab_show(struct kobjec
>  static struct kobj_attribute efi_attr_systab =
>  			__ATTR(systab, 0400, systab_show, NULL);
>  
> +static ssize_t fw_vendor_show(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "0x%lx\n", efi.fw_vendor);
> +}
> +
> +static ssize_t runtime_show(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "0x%lx\n", efi.runtime);
> +}
> +
> +static ssize_t config_table_show(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "0x%lx\n", efi.config_table);
> +}
> +
> +static struct kobj_attribute efi_attr_fw_vendor =
> +				__ATTR(fw_vendor, 0400, fw_vendor_show, NULL);
> +static struct kobj_attribute efi_attr_runtime =
> +			__ATTR(runtime, 0400, runtime_show, NULL);
> +static struct kobj_attribute efi_attr_config_table =
> +			__ATTR(config_table, 0400, config_table_show, NULL);
> +
>  static struct attribute *efi_subsys_attrs[] = {
>  	&efi_attr_systab.attr,
>  	NULL,	/* maybe more in the future? */
> @@ -123,21 +151,58 @@ static int __init efisubsys_init(void)
>  
>  	error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
>  	if (error) {
> -		pr_err("efi: Sysfs attribute export failed with error %d.\n",
> -		       error);
> +		pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
> +		       efi_attr_systab.attr.name, error);
>  		goto err_unregister;
>  	}
>  
> +	if (efi.fw_vendor != EFI_INVALID_TABLE_ADDR) {
> +		error = sysfs_create_file(efi_kobj, &efi_attr_fw_vendor.attr);
> +		if (error) {
> +			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
> +				efi_attr_fw_vendor.attr.name, error);
> +			goto err_remove_group;
> +		}
> +	}
> +
> +	if (efi.runtime != EFI_INVALID_TABLE_ADDR) {
> +		error = sysfs_create_file(efi_kobj, &efi_attr_runtime.attr);
> +		if (error) {
> +			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
> +				efi_attr_runtime.attr.name, error);
> +			goto err_remove_fw_vendor;
> +		}
> +	}
> +
> +	if (efi.config_table != EFI_INVALID_TABLE_ADDR) {
> +		error = sysfs_create_file(efi_kobj,
> +					&efi_attr_config_table.attr);
> +		if (error) {
> +			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
> +				efi_attr_config_table.attr.name, error);
> +			goto err_remove_runtime;
> +		}
> +	}

You don't need to do this "if SOMETHING then create the file", just use
the "is_visible" attribute in the group to do this as a callback to
determine this when the group is registered.

That should clean up this logic a bunch.

thanks,

greg k-h

Re: [patch 9/9 v3] x86: export x86 boot_params to sysfs

[Greg KH]

On Thu, Nov 21, 2013 at 02:17:13PM +0800, dyoung@redhat.com wrote:
> kexec-tools use boot_params for getting the 1st kernel hardware_subarch,
> the kexec kernel efi runtime support also need read the old efi_info from
> boot_params. Currently it exists in debugfs which is not a good place for
> such infomation. Per HPA, we should avoid of "sploit debugfs".
> 
> In this patch /sys/kernel/boot_params are exported, also the setup_data
> is exported as a subdirectory. For original debugfs since it's already
> there for long time and kexec-tools is using it for hardware_subarch so
> let's do not remove them for now.
> 
> Structure are like below:
> 
> /sys/kernel/boot_params
> ├── data		/* binary data for boot_params */
> ├── setup_data  	/* subdirectory for setup_data if there's any */
> │   ├── 0		/* the first setup_data node */
> │   │   ├── data	/* binary data for setup_data node 0 */
> │   │   └── type	/* setup_data type of setup_data node 0, hex string */
> |   [snip]		/* other setup_data nodes ... */
> └── version		/* hex string for boot protocal version */
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>
> ---
>  Documentation/ABI/testing/sysfs-kernel-boot_params |   40 ++
>  arch/x86/kernel/Makefile                           |    2 
>  arch/x86/kernel/ksysfs.c                           |  326 +++++++++++++++++++++
>  3 files changed, 367 insertions(+), 1 deletion(-)
> 
> --- /dev/null
> +++ efi/Documentation/ABI/testing/sysfs-kernel-boot_params
> @@ -0,0 +1,40 @@
> +What:		/sys/kernel/boot_params
> +Date:		November 2013
> +Contact:	Dave Young <dyoung@redhat.com>
> +Description:
> +		The /sys/kernel/boot_params directory contains two
> +		files: "data" and "version" and one subdirectory "setup_data".
> +		It is used to export the kernel boot parameters of x86
> +		platform to user space for kexec and debugging purpose.
> +
> +		If there's no setup_data in boot_params the subdirectory will
> +		not be created.
> +
> +		"data" file is the binary representation of struct boot_params.
> +
> +		"version" file is the string representation of boot
> +		protocol version.
> +
> +		"setup_data" subdirectory contains the setup_data data
> +		structure in boot_params. setup_data is maintained in kernel
> +		as a link list. In "setup_data" subdirectory there's one
> +		subdirectory for each link list node named with the number
> +		of the list nodes. The list node subdirectory contains two
> +		files "type" and "data". "type" file is the string
> +		representation of setup_data type. "data" file is the binary
> +		representation of setup_data payload.
> +
> +		The whole boot_params directory structure is like below:
> +		/sys/kernel/boot_params
> +		├── data
> +		├── setup_data
> +		│   ├── 0
> +		│   │   ├── data
> +		│   │   └── type
> +		│   └── 1
> +		│       ├── data
> +		│       └── type
> +		└── version
> +
> +Users:
> +		Kexec Mailing List <kexec@lists.infradead.org>
> --- efi.orig/arch/x86/kernel/Makefile
> +++ efi/arch/x86/kernel/Makefile
> @@ -35,7 +35,7 @@ obj-y			+= alternative.o i8253.o pci-nom
>  obj-y			+= tsc.o io_delay.o rtc.o
>  obj-y			+= pci-iommu_table.o
>  obj-y			+= resource.o
> -
> +obj-$(CONFIG_SYSFS)	+= ksysfs.o
>  obj-y				+= process.o
>  obj-y				+= i387.o xsave.o
>  obj-y				+= ptrace.o
> --- /dev/null
> +++ efi/arch/x86/kernel/ksysfs.c
> @@ -0,0 +1,326 @@
> +/*
> + * Architecture specific sysfs attributes in /sys/kernel
> + *
> + * Copyright (C) 2007, Intel Corp.
> + *      Huang Ying <ying.huang@intel.com>
> + * Copyright (C) 2013, 2013 Red Hat, Inc.
> + *      Dave Young <dyoung@redhat.com>
> + *
> + * This file is released under the GPLv2
> + */
> +
> +#include <linux/kobject.h>
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
> +#include <linux/init.h>
> +#include <linux/stat.h>
> +#include <linux/slab.h>
> +#include <linux/mm.h>
> +
> +#include <asm/setup.h>
> +
> +static ssize_t boot_params_version_show(struct kobject *kobj,
> +					struct kobj_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "0x%04x\n", boot_params.hdr.version);
> +}
> +
> +static struct kobj_attribute boot_params_version_attr =
> +	__ATTR(version, S_IRUGO, boot_params_version_show, NULL);

__ATTR_RO() please.

> +static struct kobj_attribute type_attr =
> +		__ATTR(type, S_IRUGO, setup_data_type_show, NULL);

__ATTR_RO() here too.


> +static struct bin_attribute data_attr = {
> +	.attr = {
> +		.name = "data",
> +		.mode = S_IRUGO,
> +	},
> +	.read = setup_data_data_read,
> +};
> +
> +static int __init create_setup_data_node(struct kobject *parent,
> +					struct kobject **kobjp, int nr)
> +{
> +	int ret = 0;
> +	size_t size;
> +	struct kobject *kobj;
> +	char name[16]; /* should be enough for setup_data nodes numbers */
> +	snprintf(name, 16, "%d", nr);
> +
> +	kobj = kobject_create_and_add(name, parent);
> +	if (!kobj)
> +		return -ENOMEM;
> +
> +	ret = sysfs_create_file(kobj, &type_attr.attr);
> +	if (ret)
> +		goto out_kobj;
> +
> +	ret = get_setup_data_size(nr, &size);
> +	if (ret)
> +		goto out_kobj;
> +
> +	data_attr.size = size;
> +	ret = sysfs_create_bin_file(kobj, &data_attr);
> +	if (ret)
> +		goto out_file;
> +	*kobjp = kobj;

You just raced with userspace (creating and announcing the kobject and
then, afterward, at some later point in time, created the sysfs files.
Please use the groups option to create the files properly before
announcing the kobject to userspace.

> +static int __init boot_params_ksysfs_init(void)
> +{
> +	int ret;
> +	struct kobject *boot_params_kobj;
> +
> +	boot_params_kobj = kobject_create_and_add("boot_params",
> +						       kernel_kobj);
> +	if (!boot_params_kobj) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	ret = sysfs_create_file(boot_params_kobj,
> +				&boot_params_version_attr.attr);
> +	if (ret)
> +		goto out_boot_params_kobj;
> +	ret = sysfs_create_bin_file(boot_params_kobj,
> +				      &boot_params_data_attr);
> +	if (ret)
> +		goto out_create_file;
> +
> +	ret = create_setup_data_nodes(boot_params_kobj);
> +	if (ret)
> +		goto out_create_bin;
> +
> +	return 0;

Same race condition here as well.  Use a groups structure please.

thanks,

greg k-h

Re: [patch 4/9 v3] efi: cleanup efi_enter_virtual_mode function

[Borislav Petkov]

On Thu, Nov 21, 2013 at 02:17:08PM +0800, dyoung@redhat.com wrote:
> Add two small functions:
> efi_merge_regions and efi_map_regions, efi_enter_virtual_mode
> calls them instead of embedding two long for loop.
> 
> v1->v2:
> refresh; coding style fixes.
> 
> v2->v3:
> Toshi Kani:
> remove unused variable
> Matt: check return value of krealloc.
> 
> Signed-off-by: Dave Young <dyoung@redhat.com>

Same short comment nitpick as earlier :)

Otherwise:

Acked-by: Borislav Petkov <bp@suse.de>

> ---
>  arch/x86/platform/efi/efi.c |  109 ++++++++++++++++++++++++++------------------
>  1 file changed, 66 insertions(+), 43 deletions(-)
> 
> --- efi.orig/arch/x86/platform/efi/efi.c
> +++ efi/arch/x86/platform/efi/efi.c

[ … ]

> @@ -837,6 +805,18 @@ void __init efi_enter_virtual_mode(void)
>  		prev_md = md;
>  
>  	}
> +}
> +
> +/*
> + * Map efi memory ranges for runtime serivce and
> + * update new_memmap with virtual addresses.
> + */

Stretch comment to 80 cols:

/*
 * Map efi memory ranges for runtime serivce and update new_memmap with virtual
 * addresses.
 */

like it is customary for the rest of the file.

> +static void * __init efi_map_regions(int *count)
> +{
> +	efi_memory_desc_t *md;
> +	void *p, *new_memmap = NULL;
> +	unsigned long size;
> +	u64 end, systab;
>  
>  	for (p = memmap.map; p < memmap.map_end; p += memmap.desc_size) {
>  		md = p;

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Borislav Petkov]

On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyoung@redhat.com wrote:
> --- efi.orig/arch/x86/platform/efi/efi.c
> +++ efi/arch/x86/platform/efi/efi.c
> @@ -653,6 +653,10 @@ void __init efi_init(void)
>  
>  	set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);
>  
> +	efi.fw_vendor = (unsigned long)efi.systab->fw_vendor;
> +	efi.runtime = (unsigned long)efi.systab->runtime;
> +	efi.config_table = (unsigned long)efi.systab->tables;

A bit more readable:

	efi.config_table = (unsigned long)efi.systab->tables;
	efi.fw_vendor	 = (unsigned long)efi.systab->fw_vendor;
	efi.runtime	 = (unsigned long)efi.systab->runtime;

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [patch 9/9 v3] x86: export x86 boot_params to sysfs

[Dave Young]

On 11/21/13 at 08:45am, Greg KH wrote:
> > +static struct kobj_attribute boot_params_version_attr =
> > +	__ATTR(version, S_IRUGO, boot_params_version_show, NULL);
> 
> __ATTR_RO() please.

Will do

> 
> > +static struct kobj_attribute type_attr =
> > +		__ATTR(type, S_IRUGO, setup_data_type_show, NULL);
> 
> __ATTR_RO() here too.

Will do

[snip]
> > +
> > +	ret = get_setup_data_size(nr, &size);
> > +	if (ret)
> > +		goto out_kobj;
> > +
> > +	data_attr.size = size;
> > +	ret = sysfs_create_bin_file(kobj, &data_attr);
> > +	if (ret)
> > +		goto out_file;
> > +	*kobjp = kobj;
> 
> You just raced with userspace (creating and announcing the kobject and
> then, afterward, at some later point in time, created the sysfs files.
> Please use the groups option to create the files properly before
> announcing the kobject to userspace.

Will do

[snip]
> > +		goto out_boot_params_kobj;
> > +	ret = sysfs_create_bin_file(boot_params_kobj,
> > +				      &boot_params_data_attr);
> > +	if (ret)
> > +		goto out_create_file;
> > +
> > +	ret = create_setup_data_nodes(boot_params_kobj);
> > +	if (ret)
> > +		goto out_create_bin;
> > +
> > +	return 0;
> 
> Same race condition here as well.  Use a groups structure please.
> 

Will use group attributes

Thanks for review
Dave

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Dave Young]

On 11/21/13 at 05:57pm, Borislav Petkov wrote:
> On Thu, Nov 21, 2013 at 02:17:09PM +0800, dyoung@redhat.com wrote:
> > --- efi.orig/arch/x86/platform/efi/efi.c
> > +++ efi/arch/x86/platform/efi/efi.c
> > @@ -653,6 +653,10 @@ void __init efi_init(void)
> >  
> >  	set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);
> >  
> > +	efi.fw_vendor = (unsigned long)efi.systab->fw_vendor;
> > +	efi.runtime = (unsigned long)efi.systab->runtime;
> > +	efi.config_table = (unsigned long)efi.systab->tables;
> 
> A bit more readable:
> 
> 	efi.config_table = (unsigned long)efi.systab->tables;
> 	efi.fw_vendor	 = (unsigned long)efi.systab->fw_vendor;
> 	efi.runtime	 = (unsigned long)efi.systab->runtime;

Hmm, UEFI spec mentions the them like below so I use the order:

Several fields of the EFI System Table must be converted from
physical pointers to virtual pointers using the ConvertPointer()
service. These fields include FirmwareVendor, RuntimeServices,
and ConfigurationTable.

But since you like the reverse I can change it in next version.

--
Thanks for review
Dave

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Dave Young]

> > +
> > +	if (efi.config_table != EFI_INVALID_TABLE_ADDR) {
> > +		error = sysfs_create_file(efi_kobj,
> > +					&efi_attr_config_table.attr);
> > +		if (error) {
> > +			pr_err("efi: Sysfs attribute %s export failed with error %d.\n",
> > +				efi_attr_config_table.attr.name, error);
> > +			goto err_remove_runtime;
> > +		}
> > +	}
> 
> You don't need to do this "if SOMETHING then create the file", just use
> the "is_visible" attribute in the group to do this as a callback to
> determine this when the group is registered.

I did not know the is_visible before, thanks for the hint, will use it

--
Thanks for review
Dave

Re: [patch 4/9 v3] efi: cleanup efi_enter_virtual_mode function

[Dave Young]

> > +
> > +/*
> > + * Map efi memory ranges for runtime serivce and
> > + * update new_memmap with virtual addresses.
> > + */
> 
> Stretch comment to 80 cols:
> 
> /*
>  * Map efi memory ranges for runtime serivce and update new_memmap with virtual
>  * addresses.
>  */
> 
> like it is customary for the rest of the file.

Ok, will do.

--
Thanks for review
Dave

Re: [patch 2/9 v3] efi: add a wrapper function efi_map_region_fixed

[Dave Young]

> >  }
> >  
> > +/*
> > + * kexec kernel will use efi_map_region_fixed to map efi
> > + * runtime memory ranges. md->virt_addr is the original virtual
> > + * address which had been mapped in kexec 1st kernel.
> > + */
> 
> Why not stretch this comment to the full 80 cols?

Since stretch them to 80 cols I still need 3 lines so I want the
length to be more close to the length of context code lines, so
It looks better to my eys :)

I can change to 80 cols as you said below, thanks.

> 
> /*
>  * kexec kernel will use efi_map_region_fixed to map efi runtime memory ranges.
>  * md->virt_addr is the original virtual address which had been mapped in kexec
>  * 1st kernel.
>  */
> 

--
Thanks for review
Dave

Re: [patch 0/9 v3] kexec kernel efi runtime support

[Toshi Kani]

On Thu, 2013-11-21 at 14:17 +0800, dyoung@redhat.com wrote:
> Hi,
> 
> Here is the V3 for supporting kexec kernel efi runtime.
> Per pervious discussion I pass the 1st kernel efi runtime mapping
> via setup_data to 2nd kernel. Besides of the runtime mapping
> info I also pass the fw_vendor, runtime, config table, smbios
> physical address in setup_data. EFI spec mentioned fw_vendor,
> runtime, config table addresses will be converted to virt address
> after entering virtual mode, but we will use it as physical address
> in efi_init. For smbios EFI spec did not mention about the address
> updating, but during my test on a HP workstation, the bios will
> convert it to Virt addr, thus pass it in setup_data as well.
> 
> For fw_vendor, runtime, config table, I export them in /sys/firmware/
> efi/, smbios is already in /sys/firmware/efi/systab.
> 
> For efi runtime mapping I add a new directory /sys/firmware/efi/
> runtime-map/ like below
> [dave@darkstar ~]$ tree /sys/firmware/efi/runtime-map/
> /sys/firmware/efi/runtime-map/
> ├── 0
> │   ├── attribute
> │   ├── num_pages
> │   ├── phys_addr
> │   ├── type
> │   └── virt_addr
> ├── 1
> │   ├── attribute
> │   ├── num_pages
> │   ├── phys_addr
> │   ├── type
> │   └── virt_addr
> [snip]
>  
> kexec-tools will assemble them as setup_data and pass to 2nd kernel.
> I will send userspace patches as well.
> 
> Limitation is I only write support for x86_64, test on below machines:
> Lenovo thinkpad t420
> Dell inspiron 14 - 3421
> HP Z420 workstation
> Qemu + OVMF

Tested on an HP EFI-based 60-way server (prototype).  For the series:

Tested-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Borislav Petkov]

On Fri, Nov 22, 2013 at 10:48:50AM +0800, Dave Young wrote:
> > 	efi.config_table = (unsigned long)efi.systab->tables;
> > 	efi.fw_vendor	 = (unsigned long)efi.systab->fw_vendor;
> > 	efi.runtime	 = (unsigned long)efi.systab->runtime;
> 
> Hmm, UEFI spec mentions the them like below so I use the order:

I'm sure by now you know you should not really trust the UEFI spec, or
any other spec for that matter :)

> Several fields of the EFI System Table must be converted from
> physical pointers to virtual pointers using the ConvertPointer()
> service. These fields include FirmwareVendor, RuntimeServices,
> and ConfigurationTable.
> 
> But since you like the reverse I can change it in next version.

The reverse was simply a suggestion. The vertical alignment was more
what I aimed at because it makes this chunk much more readable IMO.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [patch 5/9 v3] efi: export more efi table variable to sysfs

[Dave Young]

On 11/23/13 at 02:15pm, Borislav Petkov wrote:
> On Fri, Nov 22, 2013 at 10:48:50AM +0800, Dave Young wrote:
> > > 	efi.config_table = (unsigned long)efi.systab->tables;
> > > 	efi.fw_vendor	 = (unsigned long)efi.systab->fw_vendor;
> > > 	efi.runtime	 = (unsigned long)efi.systab->runtime;
> > 
> > Hmm, UEFI spec mentions the them like below so I use the order:
> 
> I'm sure by now you know you should not really trust the UEFI spec, or
> any other spec for that matter :)
> 
> > Several fields of the EFI System Table must be converted from
> > physical pointers to virtual pointers using the ConvertPointer()
> > service. These fields include FirmwareVendor, RuntimeServices,
> > and ConfigurationTable.
> > 
> > But since you like the reverse I can change it in next version.
> 
> The reverse was simply a suggestion. The vertical alignment was more
> what I aimed at because it makes this chunk much more readable IMO.
> 

Got your point about alignment, will update.

--
Thanks
Dave