[PATCH 0/2] Google firmware driver updates

[PATCH 0/2] Google firmware driver updates

[Michel Lespinasse]

Two small fixes for google firmware drivers.

The patches are against v3.13; I am proposing this for inclusion to v3.14.

Michel Lespinasse (2):
  firmware: fix google/gsmi duplicate efivars_sysfs_init()
  drivers-firmware: google memconsole driver fixes

 drivers/firmware/google/gsmi.c       |  7 ------
 drivers/firmware/google/memconsole.c | 47 ++++++++++++++++++++----------------
 2 files changed, 26 insertions(+), 28 deletions(-)

-- 
1.8.5.3

[PATCH 1/2] firmware: fix google/gsmi duplicate efivars_sysfs_init()

[Michel Lespinasse]

Starting in commit e14ab23dde12b80db4c94b684a2e485b72b16af3,
efivars_sysfs_init() is called both by itself as an init function,
and by drivers/firmware/google/gsmi.c gsmi_init().

This results in runtime warnings such as the following:
[    5.651330] WARNING: at fs/sysfs/dir.c:530 sysfs_add_one+0xbd/0xe0()
[    5.657699] sysfs: cannot create duplicate filename '/firmware/gsmi/vars'

Fixing this by removing the redundant efivars_sysfs_init() call in
gsmi_init().

Tested: booted, checked that /firmware/gsmi/vars was still present and
showed the expected contents.

Signed-off-by: Michel Lespinasse <walken@google.com>
---
 drivers/firmware/google/gsmi.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
index e5a67b24587a..f1ab05ea56bb 100644
--- a/drivers/firmware/google/gsmi.c
+++ b/drivers/firmware/google/gsmi.c
@@ -892,13 +892,6 @@ static __init int gsmi_init(void)
 		goto out_remove_sysfs_files;
 	}
 
-	ret = efivars_sysfs_init();
-	if (ret) {
-		printk(KERN_INFO "gsmi: Failed to create efivars files\n");
-		efivars_unregister(&efivars);
-		goto out_remove_sysfs_files;
-	}
-
 	register_reboot_notifier(&gsmi_reboot_notifier);
 	register_die_notifier(&gsmi_die_notifier);
 	atomic_notifier_chain_register(&panic_notifier_list,
-- 
1.8.5.3

[PATCH 2/2] drivers-firmware: google memconsole driver fixes

[Michel Lespinasse]

The google memconsole driver is currently broken upstream, as it tries
to read memory that is described as reserved in /proc/iomem, by
dereferencing a pointer obtained through phys_to_virt(). This triggers
a kernel fault as such regions are unmapped after early boot.

The proper workaround is to use ioremap_cache() / iounmap() around such
accesses.

As some unrelated changes, I also converted some printks to use pr_info()
and added some missing __init annotations.

Tested: booted dbg build, verified I could read /sys/firmware/log

Signed-off-by: Michel Lespinasse <walken@google.com>
---
 drivers/firmware/google/memconsole.c | 47 ++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/google/memconsole.c
index 2a90ba613613..2f569aaed4c7 100644
--- a/drivers/firmware/google/memconsole.c
+++ b/drivers/firmware/google/memconsole.c
@@ -15,6 +15,7 @@
 #include <linux/kobject.h>
 #include <linux/module.h>
 #include <linux/dmi.h>
+#include <linux/io.h>
 #include <asm/bios_ebda.h>
 
 #define BIOS_MEMCONSOLE_V1_MAGIC	0xDEADBABE
@@ -41,15 +42,25 @@ struct biosmemcon_ebda {
 	};
 } __packed;
 
-static char *memconsole_baseaddr;
+static u32 memconsole_baseaddr;
 static size_t memconsole_length;
 
 static ssize_t memconsole_read(struct file *filp, struct kobject *kobp,
 			       struct bin_attribute *bin_attr, char *buf,
 			       loff_t pos, size_t count)
 {
-	return memory_read_from_buffer(buf, count, &pos, memconsole_baseaddr,
-				       memconsole_length);
+	char *memconsole;
+	ssize_t ret;
+
+	memconsole = ioremap_cache(memconsole_baseaddr, memconsole_length);
+	if (!memconsole) {
+		pr_err("memconsole: ioremap_cache failed\n");
+		return -ENOMEM;
+	}
+	ret = memory_read_from_buffer(buf, count, &pos, memconsole,
+				      memconsole_length);
+	iounmap(memconsole);
+	return ret;
 }
 
 static struct bin_attribute memconsole_bin_attr = {
@@ -58,43 +69,42 @@ static struct bin_attribute memconsole_bin_attr = {
 };
 
 
-static void found_v1_header(struct biosmemcon_ebda *hdr)
+static void __init found_v1_header(struct biosmemcon_ebda *hdr)
 {
-	printk(KERN_INFO "BIOS console v1 EBDA structure found at %p\n", hdr);
-	printk(KERN_INFO "BIOS console buffer at 0x%.8x, "
+	pr_info("BIOS console v1 EBDA structure found at %p\n", hdr);
+	pr_info("BIOS console buffer at 0x%.8x, "
 	       "start = %d, end = %d, num = %d\n",
 	       hdr->v1.buffer_addr, hdr->v1.start,
 	       hdr->v1.end, hdr->v1.num_chars);
 
 	memconsole_length = hdr->v1.num_chars;
-	memconsole_baseaddr = phys_to_virt(hdr->v1.buffer_addr);
+	memconsole_baseaddr = hdr->v1.buffer_addr;
 }
 
-static void found_v2_header(struct biosmemcon_ebda *hdr)
+static void __init found_v2_header(struct biosmemcon_ebda *hdr)
 {
-	printk(KERN_INFO "BIOS console v2 EBDA structure found at %p\n", hdr);
-	printk(KERN_INFO "BIOS console buffer at 0x%.8x, "
+	pr_info("BIOS console v2 EBDA structure found at %p\n", hdr);
+	pr_info("BIOS console buffer at 0x%.8x, "
 	       "start = %d, end = %d, num_bytes = %d\n",
 	       hdr->v2.buffer_addr, hdr->v2.start,
 	       hdr->v2.end, hdr->v2.num_bytes);
 
 	memconsole_length = hdr->v2.end - hdr->v2.start;
-	memconsole_baseaddr = phys_to_virt(hdr->v2.buffer_addr
-					   + hdr->v2.start);
+	memconsole_baseaddr = hdr->v2.buffer_addr + hdr->v2.start;
 }
 
 /*
  * Search through the EBDA for the BIOS Memory Console, and
  * set the global variables to point to it.  Return true if found.
  */
-static bool found_memconsole(void)
+static bool __init found_memconsole(void)
 {
 	unsigned int address;
 	size_t length, cur;
 
 	address = get_bios_ebda();
 	if (!address) {
-		printk(KERN_INFO "BIOS EBDA non-existent.\n");
+		pr_info("BIOS EBDA non-existent.\n");
 		return false;
 	}
 
@@ -122,7 +132,7 @@ static bool found_memconsole(void)
 		}
 	}
 
-	printk(KERN_INFO "BIOS console EBDA structure not found!\n");
+	pr_info("BIOS console EBDA structure not found!\n");
 	return false;
 }
 
@@ -139,8 +149,6 @@ MODULE_DEVICE_TABLE(dmi, memconsole_dmi_table);
 
 static int __init memconsole_init(void)
 {
-	int ret;
-
 	if (!dmi_check_system(memconsole_dmi_table))
 		return -ENODEV;
 
@@ -148,10 +156,7 @@ static int __init memconsole_init(void)
 		return -ENODEV;
 
 	memconsole_bin_attr.size = memconsole_length;
-
-	ret = sysfs_create_bin_file(firmware_kobj, &memconsole_bin_attr);
-
-	return ret;
+	return sysfs_create_bin_file(firmware_kobj, &memconsole_bin_attr);
 }
 
 static void __exit memconsole_exit(void)
-- 
1.8.5.3

Re: [PATCH 0/2] Google firmware driver updates

[Mike Waychison]

Thanks Michel!

Acked-by: Mike Waychison <mikew@google.com>

On Tue, Jan 28, 2014 at 5:06 AM, Michel Lespinasse <walken@google.com> wrote:
> Two small fixes for google firmware drivers.
>
> The patches are against v3.13; I am proposing this for inclusion to v3.14.
>
> Michel Lespinasse (2):
>   firmware: fix google/gsmi duplicate efivars_sysfs_init()
>   drivers-firmware: google memconsole driver fixes
>
>  drivers/firmware/google/gsmi.c       |  7 ------
>  drivers/firmware/google/memconsole.c | 47 ++++++++++++++++++++----------------
>  2 files changed, 26 insertions(+), 28 deletions(-)
>
> --
> 1.8.5.3
>

Re: [PATCH 1/2] firmware: fix google/gsmi duplicate efivars_sysfs_init()

[Matt Fleming]

On Tue, 28 Jan, at 05:06:21AM, Michel Lespinasse wrote:
> Starting in commit e14ab23dde12b80db4c94b684a2e485b72b16af3,
> efivars_sysfs_init() is called both by itself as an init function,
> and by drivers/firmware/google/gsmi.c gsmi_init().
> 
> This results in runtime warnings such as the following:
> [    5.651330] WARNING: at fs/sysfs/dir.c:530 sysfs_add_one+0xbd/0xe0()
> [    5.657699] sysfs: cannot create duplicate filename '/firmware/gsmi/vars'
> 
> Fixing this by removing the redundant efivars_sysfs_init() call in
> gsmi_init().
> 
> Tested: booted, checked that /firmware/gsmi/vars was still present and
> showed the expected contents.
> 
> Signed-off-by: Michel Lespinasse <walken@google.com>
> ---
>  drivers/firmware/google/gsmi.c | 7 -------
>  1 file changed, 7 deletions(-)

Oops, my bad.

(If it matters: Acked-by: Matt Fleming <matt.fleming@intel.com>)

-- 
Matt Fleming, Intel Open Source Technology Center