2011-03-12 01:43:13

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 00/12] google firmware support

This patchset applies to v2.6.38-rc8.

The following series implements some support for interfaces exposed by
google's servers' firmware. Unlike the previous send-out, it only
includes two of the three drivers as I'm not yet finished fixing the
"bootlog" driver from the last send-out (and I've discovered more
"undocumented" firmware functionality that I'd rather unravel before
sending out again).

In order to address various concerns had with the previous patchset,
I've changed the user ABI significantly:

Instead of messing with the dmesg log_buf ring-buffer, we now expose
firmware logs via /sys/firmware/log.

As well, instead of using a ioctl interface the gsmi driver, I've
changed it completely to use sysfs instead (rooted at
/sys/firmware/gsmi). gsmi provides access to EFI variables through a
means other than through the EFI runtime services page, so I've reused
the bulk of the code exposed by the efivars driver. To make this work,
I had to refactor a bit of the efivars driver. The only user-visible
change here is that the module will successfully load now even if you
aren't on an EFI system (which is much more consistent with other
drivers anyway).

I'd like to have these small drivers included as they are required for
proper use of the kernel in our infrastructure. They may not seem like
much, but a lot of our health automation as well as our human debugging
efforts are dependent on the functionality herein. Getting these in the
public Linux tree would bring us closer to being able to easily test
kernels as they are released.

Thanks,

Patchset summary
================

Patches [1 through 6] refactor the 'efivars' module to let the "gsmi"
driver in patch [11] re-use the variable functionality.

Patch [7] documents existing 'efivars' functionality that has been
present in the tree since 2004.

Patches [8] and [9] add sanity checking for accessing the EBDA.

Patch [10] introduces CONFIG_GOOGLE_FIRMWARE that is disabled by
default and which gates Google-specific drivers.

Patch [11] adds the "gsmi" driver that we use to make calls into our
firmware.

Patch [12] adds the "memconsole" driver that finds the firmware's logs
and exposes them on /sys/firmware/log.

Diffstat
========
Documentation/ABI/stable/sysfs-firmware-efi-vars | 75 +
Documentation/ABI/testing/sysfs-firmware-gsmi | 58 +
Documentation/ABI/testing/sysfs-firmware-log | 7
arch/x86/include/asm/bios_ebda.h | 28
drivers/firmware/Kconfig | 2
drivers/firmware/Makefile | 2
drivers/firmware/efivars.c | 343 +++++---
drivers/firmware/google/Kconfig | 33
drivers/firmware/google/Makefile | 3
drivers/firmware/google/gsmi.c | 927 +++++++++++++++++++++++
drivers/firmware/google/memconsole.c | 151 +++
include/linux/efi.h | 37
12 files changed, 1526 insertions(+), 140 deletions(-)

ChangeLog:
==========
- v2
- Efivars can now be used by other drivers.
- Documentation added for /sys/firmware/efi/vars
- Memory console no longer touches log_buf ring buffer.
- The firmware log is exported to userland as /sys/firmware/log
- Ioctls for accessing nvram variables in gsmi driver replaced with
efivars at /sys/firmware/gsmi/vars.
- die_notifier is used instead of adding new notifer_lists.
- EBDA scrubbing for memconsole now checks that we aren't walking
into 0xA0000.
- Documentation added for /sys/firmware/log
- Documentation added for /sys/firmware/gsmi
- Use kernel fixed width types instead of C99 types.
- v1
- Initial public send-out.


2011-03-12 01:43:20

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 01/12] efivars: move efivars globals into struct efivars

In preparation for abstracting out efivars to be usable by other similar
variable services, move the global lock, list and kset into a structure.
Later patches will change the scope of 'efivars' and have it be passed
by function argument.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 86 ++++++++++++++++++++++++--------------------
1 files changed, 46 insertions(+), 40 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 2a62ec6..f10ecb6 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -89,16 +89,21 @@ MODULE_DESCRIPTION("sysfs interface to EFI Variables");
MODULE_LICENSE("GPL");
MODULE_VERSION(EFIVARS_VERSION);

-/*
- * efivars_lock protects two things:
- * 1) efivar_list - adds, removals, reads, writes
- * 2) efi.[gs]et_variable() calls.
- * It must not be held when creating sysfs entries or calling kmalloc.
- * efi.get_next_variable() is only called from efivars_init(),
- * which is protected by the BKL, so that path is safe.
- */
-static DEFINE_SPINLOCK(efivars_lock);
-static LIST_HEAD(efivar_list);
+struct efivars {
+ /*
+ * ->lock protects two things:
+ * 1) ->list - adds, removals, reads, writes
+ * 2) efi.[gs]et_variable() calls.
+ * It must not be held when creating sysfs entries or calling kmalloc.
+ * efi.get_next_variable() is only called from efivars_init(),
+ * which is protected by the BKL, so that path is safe.
+ */
+ spinlock_t lock;
+ struct list_head list;
+ struct kset *kset;
+};
+static struct efivars __efivars;
+static struct efivars *efivars = &__efivars;

/*
* The maximum size of VariableName + Data = 1024
@@ -174,14 +179,14 @@ get_var_data(struct efi_variable *var)
{
efi_status_t status;

- spin_lock(&efivars_lock);
+ spin_lock(&efivars->lock);
var->DataSize = 1024;
status = efi.get_variable(var->VariableName,
&var->VendorGuid,
&var->Attributes,
&var->DataSize,
var->Data);
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n",
status);
@@ -291,14 +296,14 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count)
return -EINVAL;
}

- spin_lock(&efivars_lock);
+ spin_lock(&efivars->lock);
status = efi.set_variable(new_var->VariableName,
&new_var->VendorGuid,
new_var->Attributes,
new_var->DataSize,
new_var->Data);

- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);

if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
@@ -415,12 +420,12 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

- spin_lock(&efivars_lock);
+ spin_lock(&efivars->lock);

/*
* Does this variable already exist?
*/
- list_for_each_entry_safe(search_efivar, n, &efivar_list, list) {
+ list_for_each_entry_safe(search_efivar, n, &efivars->list, list) {
strsize1 = utf8_strsize(search_efivar->var.VariableName, 1024);
strsize2 = utf8_strsize(new_var->VariableName, 1024);
if (strsize1 == strsize2 &&
@@ -433,7 +438,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
}
}
if (found) {
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
return -EINVAL;
}

@@ -447,10 +452,10 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
status);
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
return -EIO;
}
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);

/* Create the entry in sysfs. Locking is not required here */
status = efivar_create_sysfs_entry(utf8_strsize(new_var->VariableName,
@@ -474,12 +479,12 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
if (!capable(CAP_SYS_ADMIN))
return -EACCES;

- spin_lock(&efivars_lock);
+ spin_lock(&efivars->lock);

/*
* Does this variable already exist?
*/
- list_for_each_entry_safe(search_efivar, n, &efivar_list, list) {
+ list_for_each_entry_safe(search_efivar, n, &efivars->list, list) {
strsize1 = utf8_strsize(search_efivar->var.VariableName, 1024);
strsize2 = utf8_strsize(del_var->VariableName, 1024);
if (strsize1 == strsize2 &&
@@ -492,7 +497,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
}
}
if (!found) {
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
return -EINVAL;
}
/* force the Attributes/DataSize to 0 to ensure deletion */
@@ -508,12 +513,12 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
status);
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
return -EIO;
}
list_del(&search_efivar->list);
/* We need to release this lock before unregistering. */
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
efivar_unregister(search_efivar);

/* It's dead Jim.... */
@@ -572,8 +577,6 @@ static struct attribute_group efi_subsys_attr_group = {
.attrs = efi_subsys_attrs,
};

-
-static struct kset *vars_kset;
static struct kobject *efi_kobj;

/*
@@ -582,7 +585,7 @@ static struct kobject *efi_kobj;
* variable_name_size = number of bytes required to hold
* variable_name (not counting the NULL
* character at the end.
- * efivars_lock is not held on entry or exit.
+ * efivars->lock is not held on entry or exit.
* Returns 1 on failure, 0 on success
*/
static int
@@ -618,7 +621,7 @@ efivar_create_sysfs_entry(unsigned long variable_name_size,
*(short_name + strlen(short_name)) = '-';
efi_guid_unparse(vendor_guid, short_name + strlen(short_name));

- new_efivar->kobj.kset = vars_kset;
+ new_efivar->kobj.kset = efivars->kset;
i = kobject_init_and_add(&new_efivar->kobj, &efivar_ktype, NULL,
"%s", short_name);
if (i) {
@@ -631,9 +634,9 @@ efivar_create_sysfs_entry(unsigned long variable_name_size,
kfree(short_name);
short_name = NULL;

- spin_lock(&efivars_lock);
- list_add(&new_efivar->list, &efivar_list);
- spin_unlock(&efivars_lock);
+ spin_lock(&efivars->lock);
+ list_add(&new_efivar->list, &efivars->list);
+ spin_unlock(&efivars->lock);

return 0;
}
@@ -674,8 +677,11 @@ efivars_init(void)
goto out_free;
}

- vars_kset = kset_create_and_add("vars", NULL, efi_kobj);
- if (!vars_kset) {
+ spin_lock_init(&efivars->lock);
+ INIT_LIST_HEAD(&efivars->list);
+
+ efivars->kset = kset_create_and_add("vars", NULL, efi_kobj);
+ if (!efivars->kset) {
printk(KERN_ERR "efivars: Subsystem registration failed.\n");
error = -ENOMEM;
goto out_firmware_unregister;
@@ -712,12 +718,12 @@ efivars_init(void)
* Now add attributes to allow creation of new vars
* and deletion of existing ones...
*/
- error = sysfs_create_bin_file(&vars_kset->kobj,
+ error = sysfs_create_bin_file(&efivars->kset->kobj,
&var_subsys_attr_new_var);
if (error)
printk(KERN_ERR "efivars: unable to create new_var sysfs file"
" due to error %d\n", error);
- error = sysfs_create_bin_file(&vars_kset->kobj,
+ error = sysfs_create_bin_file(&efivars->kset->kobj,
&var_subsys_attr_del_var);
if (error)
printk(KERN_ERR "efivars: unable to create del_var sysfs file"
@@ -730,7 +736,7 @@ efivars_init(void)
else
goto out_free;

- kset_unregister(vars_kset);
+ kset_unregister(efivars->kset);

out_firmware_unregister:
kobject_put(efi_kobj);
@@ -746,14 +752,14 @@ efivars_exit(void)
{
struct efivar_entry *entry, *n;

- list_for_each_entry_safe(entry, n, &efivar_list, list) {
- spin_lock(&efivars_lock);
+ list_for_each_entry_safe(entry, n, &efivars->list, list) {
+ spin_lock(&efivars->lock);
list_del(&entry->list);
- spin_unlock(&efivars_lock);
+ spin_unlock(&efivars->lock);
efivar_unregister(entry);
}

- kset_unregister(vars_kset);
+ kset_unregister(efivars->kset);
kobject_put(efi_kobj);
}

2011-03-12 01:43:43

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 05/12] efivars: Parameterize operations.

Instead of letting efivars access struct efi directly when dealing with
variables, use an operations structure. This allows a later change to
reuse the efivars logic without having to pretend to support everything
in struct efi.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 61 +++++++++++++++++++++++++++-----------------
1 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index f22bfcf..b10b7d7 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -89,19 +89,26 @@ MODULE_DESCRIPTION("sysfs interface to EFI Variables");
MODULE_LICENSE("GPL");
MODULE_VERSION(EFIVARS_VERSION);

+struct efivar_operations {
+ efi_get_variable_t *get_variable;
+ efi_get_next_variable_t *get_next_variable;
+ efi_set_variable_t *set_variable;
+};
+
struct efivars {
/*
* ->lock protects two things:
* 1) ->list - adds, removals, reads, writes
- * 2) efi.[gs]et_variable() calls.
+ * 2) ops.[gs]et_variable() calls.
* It must not be held when creating sysfs entries or calling kmalloc.
- * efi.get_next_variable() is only called from register_efivars(),
+ * ops.get_next_variable() is only called from register_efivars(),
* which is protected by the BKL, so that path is safe.
*/
spinlock_t lock;
struct list_head list;
struct kset *kset;
struct bin_attribute *new_var, *del_var;
+ const struct efivar_operations *ops;
};

/*
@@ -182,11 +189,11 @@ get_var_data(struct efivars *efivars, struct efi_variable *var)

spin_lock(&efivars->lock);
var->DataSize = 1024;
- status = efi.get_variable(var->VariableName,
- &var->VendorGuid,
- &var->Attributes,
- &var->DataSize,
- var->Data);
+ status = efivars->ops->get_variable(var->VariableName,
+ &var->VendorGuid,
+ &var->Attributes,
+ &var->DataSize,
+ var->Data);
spin_unlock(&efivars->lock);
if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n",
@@ -299,11 +306,11 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count)
}

spin_lock(&efivars->lock);
- status = efi.set_variable(new_var->VariableName,
- &new_var->VendorGuid,
- new_var->Attributes,
- new_var->DataSize,
- new_var->Data);
+ status = efivars->ops->set_variable(new_var->VariableName,
+ &new_var->VendorGuid,
+ new_var->Attributes,
+ new_var->DataSize,
+ new_var->Data);

spin_unlock(&efivars->lock);

@@ -446,11 +453,11 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
}

/* now *really* create the variable via EFI */
- status = efi.set_variable(new_var->VariableName,
- &new_var->VendorGuid,
- new_var->Attributes,
- new_var->DataSize,
- new_var->Data);
+ status = efivars->ops->set_variable(new_var->VariableName,
+ &new_var->VendorGuid,
+ new_var->Attributes,
+ new_var->DataSize,
+ new_var->Data);

if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
@@ -511,11 +518,11 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
del_var->Attributes = 0;
del_var->DataSize = 0;

- status = efi.set_variable(del_var->VariableName,
- &del_var->VendorGuid,
- del_var->Attributes,
- del_var->DataSize,
- del_var->Data);
+ status = efivars->ops->set_variable(del_var->VariableName,
+ &del_var->VendorGuid,
+ del_var->Attributes,
+ del_var->DataSize,
+ del_var->Data);

if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
@@ -719,6 +726,7 @@ static void unregister_efivars(struct efivars *efivars)
}

static int register_efivars(struct efivars *efivars,
+ const struct efivar_operations *ops,
struct kobject *parent_kobj)
{
efi_status_t status = EFI_NOT_FOUND;
@@ -735,6 +743,7 @@ static int register_efivars(struct efivars *efivars,

spin_lock_init(&efivars->lock);
INIT_LIST_HEAD(&efivars->list);
+ efivars->ops = ops;

efivars->kset = kset_create_and_add("vars", NULL, parent_kobj);
if (!efivars->kset) {
@@ -751,7 +760,7 @@ static int register_efivars(struct efivars *efivars,
do {
variable_name_size = 1024;

- status = efi.get_next_variable(&variable_name_size,
+ status = ops->get_next_variable(&variable_name_size,
variable_name,
&vendor_guid);
switch (status) {
@@ -782,6 +791,7 @@ out:
}

static struct efivars __efivars;
+static struct efivar_operations ops;

/*
* For now we register the efi subsystem with the firmware subsystem
@@ -809,7 +819,10 @@ efivars_init(void)
return -ENOMEM;
}

- error = register_efivars(&__efivars, efi_kobj);
+ ops.get_variable = efi.get_variable;
+ ops.set_variable = efi.set_variable;
+ ops.get_next_variable = efi.get_next_variable;
+ error = register_efivars(&__efivars, &ops, efi_kobj);

/* Don't forget the systab entry */
error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);

2011-03-12 01:43:29

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 03/12] efivars: parameterize efivars

Now that we all global variable state is encapsulated by struct efivars,
parameterize all functions to the efivars local to the control flow rather
than at file scope. We do this by removing the variable "efivars" at file
scope and move its storage down to the end of the file.

Variables get at efivars by storing the efivars pointer within each
efivar_entry. The "new_var" and "del_var" binary attribute files get at
the efivars through the private pointer.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 49 ++++++++++++++++++++++++++++----------------
1 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 528ce47..5633018 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -103,8 +103,6 @@ struct efivars {
struct kset *kset;
struct bin_attribute *new_var, *del_var;
};
-static struct efivars __efivars;
-static struct efivars *efivars = &__efivars;

/*
* The maximum size of VariableName + Data = 1024
@@ -124,6 +122,7 @@ struct efi_variable {


struct efivar_entry {
+ struct efivars *efivars;
struct efi_variable var;
struct list_head list;
struct kobject kobj;
@@ -150,9 +149,10 @@ struct efivar_attribute efivar_attr_##_name = { \
* Prototype for sysfs creation function
*/
static int
-efivar_create_sysfs_entry(unsigned long variable_name_size,
- efi_char16_t *variable_name,
- efi_guid_t *vendor_guid);
+efivar_create_sysfs_entry(struct efivars *efivars,
+ unsigned long variable_name_size,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor_guid);

/* Return the number of unicode characters in data */
static unsigned long
@@ -176,7 +176,7 @@ utf8_strsize(efi_char16_t *data, unsigned long maxlength)
}

static efi_status_t
-get_var_data(struct efi_variable *var)
+get_var_data(struct efivars *efivars, struct efi_variable *var)
{
efi_status_t status;

@@ -221,7 +221,7 @@ efivar_attr_read(struct efivar_entry *entry, char *buf)
if (!entry || !buf)
return -EINVAL;

- status = get_var_data(var);
+ status = get_var_data(entry->efivars, var);
if (status != EFI_SUCCESS)
return -EIO;

@@ -244,7 +244,7 @@ efivar_size_read(struct efivar_entry *entry, char *buf)
if (!entry || !buf)
return -EINVAL;

- status = get_var_data(var);
+ status = get_var_data(entry->efivars, var);
if (status != EFI_SUCCESS)
return -EIO;

@@ -261,7 +261,7 @@ efivar_data_read(struct efivar_entry *entry, char *buf)
if (!entry || !buf)
return -EINVAL;

- status = get_var_data(var);
+ status = get_var_data(entry->efivars, var);
if (status != EFI_SUCCESS)
return -EIO;

@@ -276,6 +276,7 @@ static ssize_t
efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count)
{
struct efi_variable *new_var, *var = &entry->var;
+ struct efivars *efivars = entry->efivars;
efi_status_t status = EFI_NOT_FOUND;

if (count != sizeof(struct efi_variable))
@@ -325,7 +326,7 @@ efivar_show_raw(struct efivar_entry *entry, char *buf)
if (!entry || !buf)
return 0;

- status = get_var_data(var);
+ status = get_var_data(entry->efivars, var);
if (status != EFI_SUCCESS)
return -EIO;

@@ -413,6 +414,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
char *buf, loff_t pos, size_t count)
{
struct efi_variable *new_var = (struct efi_variable *)buf;
+ struct efivars *efivars = bin_attr->private;
struct efivar_entry *search_efivar, *n;
unsigned long strsize1, strsize2;
efi_status_t status = EFI_NOT_FOUND;
@@ -459,8 +461,11 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
spin_unlock(&efivars->lock);

/* Create the entry in sysfs. Locking is not required here */
- status = efivar_create_sysfs_entry(utf8_strsize(new_var->VariableName,
- 1024), new_var->VariableName, &new_var->VendorGuid);
+ status = efivar_create_sysfs_entry(efivars,
+ utf8_strsize(new_var->VariableName,
+ 1024),
+ new_var->VariableName,
+ &new_var->VendorGuid);
if (status) {
printk(KERN_WARNING "efivars: variable created, but sysfs entry wasn't.\n");
}
@@ -472,6 +477,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
char *buf, loff_t pos, size_t count)
{
struct efi_variable *del_var = (struct efi_variable *)buf;
+ struct efivars *efivars = bin_attr->private;
struct efivar_entry *search_efivar, *n;
unsigned long strsize1, strsize2;
efi_status_t status = EFI_NOT_FOUND;
@@ -580,9 +586,10 @@ static struct kobject *efi_kobj;
* Returns 1 on failure, 0 on success
*/
static int
-efivar_create_sysfs_entry(unsigned long variable_name_size,
- efi_char16_t *variable_name,
- efi_guid_t *vendor_guid)
+efivar_create_sysfs_entry(struct efivars *efivars,
+ unsigned long variable_name_size,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor_guid)
{
int i, short_name_size = variable_name_size / sizeof(efi_char16_t) + 38;
char *short_name;
@@ -597,6 +604,7 @@ efivar_create_sysfs_entry(unsigned long variable_name_size,
return 1;
}

+ new_efivar->efivars = efivars;
memcpy(new_efivar->var.VariableName, variable_name,
variable_name_size);
memcpy(&(new_efivar->var.VendorGuid), vendor_guid, sizeof(efi_guid_t));
@@ -691,6 +699,8 @@ out_free:
return error;
}

+static struct efivars __efivars;
+
/*
* For now we register the efi subsystem with the firmware subsystem
* and the vars subsystem with the efi subsystem. In the future, it
@@ -706,6 +716,7 @@ efivars_init(void)
efi_guid_t vendor_guid;
efi_char16_t *variable_name;
unsigned long variable_name_size = 1024;
+ struct efivars *efivars = &__efivars;
int error = 0;

if (!efi_enabled)
@@ -751,9 +762,10 @@ efivars_init(void)
&vendor_guid);
switch (status) {
case EFI_SUCCESS:
- efivar_create_sysfs_entry(variable_name_size,
- variable_name,
- &vendor_guid);
+ efivar_create_sysfs_entry(efivars,
+ variable_name_size,
+ variable_name,
+ &vendor_guid);
break;
case EFI_NOT_FOUND:
break;
@@ -788,6 +800,7 @@ out_free:
static void __exit
efivars_exit(void)
{
+ struct efivars *efivars = &__efivars;
struct efivar_entry *entry, *n;

list_for_each_entry_safe(entry, n, &efivars->list, list) {

2011-03-12 01:43:39

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 04/12] efivars: Split out variable registration

In anticipation of re-using the variable facilities in efivars from
elsewhere, split out the registration and unregistration of struct
efivars from the rest of the EFI specific sysfs code.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 122 ++++++++++++++++++++++++--------------------
1 files changed, 67 insertions(+), 55 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 5633018..f22bfcf 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -95,7 +95,7 @@ struct efivars {
* 1) ->list - adds, removals, reads, writes
* 2) efi.[gs]et_variable() calls.
* It must not be held when creating sysfs entries or calling kmalloc.
- * efi.get_next_variable() is only called from efivars_init(),
+ * efi.get_next_variable() is only called from register_efivars(),
* which is protected by the BKL, so that path is safe.
*/
spinlock_t lock;
@@ -699,54 +699,48 @@ out_free:
return error;
}

-static struct efivars __efivars;
+static void unregister_efivars(struct efivars *efivars)
+{
+ struct efivar_entry *entry, *n;

-/*
- * For now we register the efi subsystem with the firmware subsystem
- * and the vars subsystem with the efi subsystem. In the future, it
- * might make sense to split off the efi subsystem into its own
- * driver, but for now only efivars will register with it, so just
- * include it here.
- */
+ list_for_each_entry_safe(entry, n, &efivars->list, list) {
+ spin_lock(&efivars->lock);
+ list_del(&entry->list);
+ spin_unlock(&efivars->lock);
+ efivar_unregister(entry);
+ }
+ if (efivars->new_var)
+ sysfs_remove_bin_file(&efivars->kset->kobj, efivars->new_var);
+ if (efivars->del_var)
+ sysfs_remove_bin_file(&efivars->kset->kobj, efivars->del_var);
+ kfree(efivars->new_var);
+ kfree(efivars->del_var);
+ kset_unregister(efivars->kset);
+}

-static int __init
-efivars_init(void)
+static int register_efivars(struct efivars *efivars,
+ struct kobject *parent_kobj)
{
efi_status_t status = EFI_NOT_FOUND;
efi_guid_t vendor_guid;
efi_char16_t *variable_name;
unsigned long variable_name_size = 1024;
- struct efivars *efivars = &__efivars;
int error = 0;

- if (!efi_enabled)
- return -ENODEV;
-
variable_name = kzalloc(variable_name_size, GFP_KERNEL);
if (!variable_name) {
printk(KERN_ERR "efivars: Memory allocation failed.\n");
return -ENOMEM;
}

- printk(KERN_INFO "EFI Variables Facility v%s %s\n", EFIVARS_VERSION,
- EFIVARS_DATE);
-
- /* For now we'll register the efi directory at /sys/firmware/efi */
- efi_kobj = kobject_create_and_add("efi", firmware_kobj);
- if (!efi_kobj) {
- printk(KERN_ERR "efivars: Firmware registration failed.\n");
- error = -ENOMEM;
- goto out_free;
- }
-
spin_lock_init(&efivars->lock);
INIT_LIST_HEAD(&efivars->list);

- efivars->kset = kset_create_and_add("vars", NULL, efi_kobj);
+ efivars->kset = kset_create_and_add("vars", NULL, parent_kobj);
if (!efivars->kset) {
printk(KERN_ERR "efivars: Subsystem registration failed.\n");
error = -ENOMEM;
- goto out_firmware_unregister;
+ goto out;
}

/*
@@ -778,21 +772,54 @@ efivars_init(void)
} while (status != EFI_NOT_FOUND);

error = create_efivars_bin_attributes(efivars);
-
- /* Don't forget the systab entry */
- error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
if (error)
- printk(KERN_ERR "efivars: Sysfs attribute export failed with error %d.\n", error);
- else
- goto out_free;
+ unregister_efivars(efivars);

- kset_unregister(efivars->kset);
+out:
+ kfree(variable_name);

-out_firmware_unregister:
- kobject_put(efi_kobj);
+ return error;
+}

-out_free:
- kfree(variable_name);
+static struct efivars __efivars;
+
+/*
+ * For now we register the efi subsystem with the firmware subsystem
+ * and the vars subsystem with the efi subsystem. In the future, it
+ * might make sense to split off the efi subsystem into its own
+ * driver, but for now only efivars will register with it, so just
+ * include it here.
+ */
+
+static int __init
+efivars_init(void)
+{
+ int error = 0;
+
+ printk(KERN_INFO "EFI Variables Facility v%s %s\n", EFIVARS_VERSION,
+ EFIVARS_DATE);
+
+ if (!efi_enabled)
+ return -ENODEV;
+
+ /* For now we'll register the efi directory at /sys/firmware/efi */
+ efi_kobj = kobject_create_and_add("efi", firmware_kobj);
+ if (!efi_kobj) {
+ printk(KERN_ERR "efivars: Firmware registration failed.\n");
+ return -ENOMEM;
+ }
+
+ error = register_efivars(&__efivars, efi_kobj);
+
+ /* Don't forget the systab entry */
+ error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
+ if (error) {
+ printk(KERN_ERR
+ "efivars: Sysfs attribute export failed with error %d.\n",
+ error);
+ unregister_efivars(&__efivars);
+ kobject_put(efi_kobj);
+ }

return error;
}
@@ -800,22 +827,7 @@ out_free:
static void __exit
efivars_exit(void)
{
- struct efivars *efivars = &__efivars;
- struct efivar_entry *entry, *n;
-
- list_for_each_entry_safe(entry, n, &efivars->list, list) {
- spin_lock(&efivars->lock);
- list_del(&entry->list);
- spin_unlock(&efivars->lock);
- efivar_unregister(entry);
- }
- if (efivars->new_var)
- sysfs_remove_bin_file(&efivars->kset->kobj, efivars->new_var);
- if (efivars->del_var)
- sysfs_remove_bin_file(&efivars->kset->kobj, efivars->del_var);
- kfree(efivars->new_var);
- kfree(efivars->del_var);
- kset_unregister(efivars->kset);
+ unregister_efivars(&__efivars);
kobject_put(efi_kobj);
}

2011-03-12 01:43:50

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 07/12] efivars: Add Documentation

This patch documents the interface exposed by the 'efivars' module.

Signed-off-by: Mike Waychison <[email protected]>
---
Documentation/ABI/stable/sysfs-firmware-efi-vars | 75 ++++++++++++++++++++++
1 files changed, 75 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-firmware-efi-vars

diff --git a/Documentation/ABI/stable/sysfs-firmware-efi-vars b/Documentation/ABI/stable/sysfs-firmware-efi-vars
new file mode 100644
index 0000000..5def20b
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-firmware-efi-vars
@@ -0,0 +1,75 @@
+What: /sys/firmware/efi/vars
+Date: April 2004
+Contact: Matt Domsch <[email protected]>
+Description:
+ This directory exposes interfaces for interactive with
+ EFI variables. For more information on EFI variables,
+ see 'Variable Services' in the UEFI specification
+ (section 7.2 in specification version 2.3 Errata D).
+
+ In summary, EFI variables are named, and are classified
+ into separate namespaces through the use of a vendor
+ GUID. They also have an arbitrary binary value
+ associated with them.
+
+ The efivars module enumerates these variables and
+ creates a separate directory for each one found. Each
+ directory has a name of the form "<key>-<vendor guid>"
+ and contains the following files:
+
+ attributes: A read-only text file enumerating the
+ EFI variable flags. Potential values
+ include:
+
+ EFI_VARIABLE_NON_VOLATILE
+ EFI_VARIABLE_BOOTSERVICE_ACCESS
+ EFI_VARIABLE_RUNTIME_ACCESS
+ EFI_VARIABLE_HARDWARE_ERROR_RECORD
+ EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS
+
+ See the EFI documentation for an
+ explanation of each of these variables.
+
+ data: A read-only binary file that can be read
+ to attain the value of the EFI variable
+
+ guid: The vendor GUID of the variable. This
+ should always match the GUID in the
+ variable's name.
+
+ raw_var: A binary file that can be read to obtain
+ a structure that contains everything
+ there is to know about the variable.
+ For structure definition see "struct
+ efi_variable" in the kernel sources.
+
+ This file can also be written to in
+ order to update the value of a variable.
+ For this to work however, all fields of
+ the "struct efi_variable" passed must
+ match byte for byte with the structure
+ read out of the file, save for the value
+ portion.
+
+ **Note** the efi_variable structure
+ read/written with this file contains a
+ 'long' type that may change widths
+ depending on your underlying
+ architecture.
+
+ size: As ASCII representation of the size of
+ the variable's value.
+
+
+ In addition, two other magic binary files are provided
+ in the top-level directory and are used for adding and
+ removing variables:
+
+ new_var: Takes a "struct efi_variable" and
+ instructs the EFI firmware to create a
+ new variable.
+
+ del_var: Takes a "struct efi_variable" and
+ instructs the EFI firmware to remove any
+ variable that has a matching vendor GUID
+ and variable key name.

2011-03-12 01:43:57

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 08/12] x86: get_bios_ebda_length()

Add a wrapper routine that tells us the length of the EBDA if it is
present. This guy also ensures that the returned length doesn't let the
EBDA run past the 640KiB mark.

Signed-off-by: Mike Waychison <[email protected]>
---
arch/x86/include/asm/bios_ebda.h | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/bios_ebda.h b/arch/x86/include/asm/bios_ebda.h
index 3c75210..5174cf0 100644
--- a/arch/x86/include/asm/bios_ebda.h
+++ b/arch/x86/include/asm/bios_ebda.h
@@ -14,6 +14,27 @@ static inline unsigned int get_bios_ebda(void)
return address; /* 0 means none */
}

+/*
+ * Return the sanitized length of the EBDA in bytes, if it exists.
+ */
+static inline unsigned int get_bios_ebda_length(void)
+{
+ unsigned int address;
+ unsigned int length;
+
+ address = get_bios_ebda();
+ if (!address)
+ return 0;
+
+ /* EBDA length is byte 0 of the EBDA (stored in KiB) */
+ length = *(unsigned char *)phys_to_virt(address);
+ length <<= 10;
+
+ /* Trim the length if it extends beyond 640KiB */
+ length = min_t(unsigned int, (640 * 1024) - address, length);
+ return length;
+}
+
void reserve_ebda_region(void);

#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION

2011-03-12 01:44:04

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 09/12] x86: Better comments for get_bios_ebda()

Make the comments a bit clearer for get_bios_ebda so that it actually
tells us what it is returning.

Signed-off-by: Mike Waychison <[email protected]>
---
arch/x86/include/asm/bios_ebda.h | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/bios_ebda.h b/arch/x86/include/asm/bios_ebda.h
index 5174cf0..aa6a317 100644
--- a/arch/x86/include/asm/bios_ebda.h
+++ b/arch/x86/include/asm/bios_ebda.h
@@ -4,11 +4,14 @@
#include <asm/io.h>

/*
- * there is a real-mode segmented pointer pointing to the
- * 4K EBDA area at 0x40E.
+ * Returns physical address of EBDA. Returns 0 if there is no EBDA.
*/
static inline unsigned int get_bios_ebda(void)
{
+ /*
+ * There is a real-mode segmented pointer pointing to the
+ * 4K EBDA area at 0x40E.
+ */
unsigned int address = *(unsigned short *)phys_to_virt(0x40E);
address <<= 4;
return address; /* 0 means none */

2011-03-12 01:44:10

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 10/12] Introduce CONFIG_GOOGLE_FIRMWARE

In order to keep Google's firmware drivers organized amongst themselves,
create a new directory for them to live in. As well, all Google
firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
to 'n' in the kernel build.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/Kconfig | 2 ++
drivers/firmware/Makefile | 2 ++
drivers/firmware/google/Kconfig | 14 ++++++++++++++
drivers/firmware/google/Makefile | 1 +
4 files changed, 19 insertions(+), 0 deletions(-)
create mode 100644 drivers/firmware/google/Kconfig
create mode 100644 drivers/firmware/google/Makefile

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index e710424..d848b26 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -134,4 +134,6 @@ config ISCSI_IBFT
detect iSCSI boot parameters dynamically during system boot, say Y.
Otherwise, say N.

+source "drivers/firmware/google/Kconfig"
+
endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 1c3c173..6f68007 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -11,3 +11,5 @@ obj-$(CONFIG_DMIID) += dmi-id.o
obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o
obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
+
+obj-$(CONFIG_GOOGLE_FIRMWARE) += google/
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
new file mode 100644
index 0000000..7834729
--- /dev/null
+++ b/drivers/firmware/google/Kconfig
@@ -0,0 +1,14 @@
+config GOOGLE_FIRMWARE
+ bool "Google Firmware Drivers"
+ depends on X86
+ default n
+ help
+ These firmware drivers are used by Google's servers. They are
+ only useful if you are working directly on one of their
+ proprietary servers or implementing similar firmware
+ interfaces. If in doubt, say "N".
+
+menu "Google Firmware Drivers"
+ depends on GOOGLE_FIRMWARE
+
+endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/drivers/firmware/google/Makefile
@@ -0,0 +1 @@
+

2011-03-12 01:44:17

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 12/12] driver: Google Memory Console

This patch introduces the 'memconsole' driver.

Our firmware gives us access to an in-memory log of the firmware's
output. This gives us visibility in a data-center of headless machines
as to what the firmware is doing.

The memory console is found by the driver by finding a header block in
the EBDA. The buffer is then copied out, and is exported to userland in
the file /sys/firmware/log.

Signed-off-by: San Mehat <[email protected]>
Signed-off-by: Mike Waychison <[email protected]>
---
Changelog:
v2:
- Removed bits that had this driver muck with the kernel's ring
dmesg buffer (log_buf).
- Introduced the log as /sys/firmware/log, a binary file that isn't
interpreted by the kernel.
- Don't use C99 fixed width types; use kernel fixed width types.
- Added file to Documentation/ABI for /sys/firmware/log
---
Documentation/ABI/testing/sysfs-firmware-log | 7 +
drivers/firmware/google/Kconfig | 8 +
drivers/firmware/google/Makefile | 1
drivers/firmware/google/memconsole.c | 151 ++++++++++++++++++++++++++
4 files changed, 167 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-log
create mode 100644 drivers/firmware/google/memconsole.c

diff --git a/Documentation/ABI/testing/sysfs-firmware-log b/Documentation/ABI/testing/sysfs-firmware-log
new file mode 100644
index 0000000..9b58e7c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-log
@@ -0,0 +1,7 @@
+What: /sys/firmware/log
+Date: February 2011
+Contact: Mike Waychison <[email protected]>
+Description:
+ The /sys/firmware/log is a binary file that represents a
+ read-only copy of the firmware's log if one is
+ available.
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index dfc3edc..5c3fc2e 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -22,4 +22,12 @@ config GOOGLE_SMI
clearing the EFI event log and reading and writing NVRAM
variables.

+config GOOGLE_MEMCONSOLE
+ tristate "Firmware Memory Console"
+ default y
+ help
+ This option enables the kernel to search for a firmware log in
+ the EBDA. If found, this log is prepended to the kernel's
+ dmesg logs so they are visible to the user.
+
endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index fb127d7..54a294e 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1,2 +1,3 @@

obj-$(CONFIG_GOOGLE_SMI) += gsmi.o
+obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o
diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/google/memconsole.c
new file mode 100644
index 0000000..bef93dc
--- /dev/null
+++ b/drivers/firmware/google/memconsole.c
@@ -0,0 +1,151 @@
+/*
+ * memconsole.c
+ *
+ * Infrastructure for importing the BIOS memory based console
+ * into the kernel log ringbuffer.
+ *
+ * Copyright 2010 Google Inc. All rights reserved.
+ */
+
+#include <linux/ctype.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+#include <linux/module.h>
+#include <asm/bios_ebda.h>
+
+#define BIOS_MEMCONSOLE_V1_MAGIC 0xDEADBABE
+#define BIOS_MEMCONSOLE_V2_MAGIC (('M')|('C'<<8)|('O'<<16)|('N'<<24))
+
+struct biosmemcon_ebda {
+ u32 signature;
+ union {
+ struct {
+ u8 enabled;
+ u32 buffer_addr;
+ u16 start;
+ u16 end;
+ u16 num_chars;
+ u8 wrapped;
+ } __packed v1;
+ struct {
+ u32 buffer_addr;
+ /* Misdocumented as number of pages! */
+ u16 num_bytes;
+ u16 start;
+ u16 end;
+ } __packed v2;
+ };
+} __packed;
+
+static char *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);
+}
+
+static struct bin_attribute memconsole_bin_attr = {
+ .attr = {.name = "log", .mode = 0444},
+ .read = memconsole_read,
+};
+
+
+static void 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, "
+ "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);
+}
+
+static void 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, "
+ "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);
+}
+
+/*
+ * 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)
+{
+ unsigned int address;
+ size_t length, cur;
+
+ address = get_bios_ebda();
+ if (!address) {
+ printk(KERN_INFO "BIOS EBDA non-existent.\n");
+ return false;
+ }
+
+ /* EBDA length is byte 0 of EBDA (in KB) */
+ length = *(u8 *)phys_to_virt(address);
+ length <<= 10; /* convert to bytes */
+
+ /*
+ * Search through EBDA for BIOS memory console structure
+ * note: signature is not necessarily dword-aligned
+ */
+ for (cur = 0; cur < length; cur++) {
+ struct biosmemcon_ebda *hdr = phys_to_virt(address + cur);
+
+ /* memconsole v1 */
+ if (hdr->signature == BIOS_MEMCONSOLE_V1_MAGIC) {
+ found_v1_header(hdr);
+ return true;
+ }
+
+ /* memconsole v2 */
+ if (hdr->signature == BIOS_MEMCONSOLE_V2_MAGIC) {
+ found_v2_header(hdr);
+ return true;
+ }
+ }
+
+ printk(KERN_INFO "BIOS console EBDA structure not found!\n");
+ return false;
+}
+
+static int __init memconsole_init(void)
+{
+ int ret;
+
+ if (!found_memconsole())
+ return -ENODEV;
+
+ memconsole_bin_attr.size = memconsole_length;
+
+ ret = sysfs_create_bin_file(firmware_kobj, &memconsole_bin_attr);
+
+ return ret;
+}
+
+static void __exit memconsole_exit(void)
+{
+ sysfs_remove_bin_file(firmware_kobj, &memconsole_bin_attr);
+}
+
+module_init(memconsole_init);
+module_exit(memconsole_exit);
+
+MODULE_AUTHOR("Google, Inc.");
+MODULE_LICENSE("GPL");

2011-03-12 01:44:35

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 11/12] driver: Google EFI SMI

The "gsmi" driver bridges userland with firmware specific routines for
accessing hardware.

Currently, this driver only supports NVRAM and eventlog information.
Deprecated functions have been removed from the driver, though their
op-codes are left in place so that they are not re-used.

This driver works by trampolining into the firmware via the smi_command
outlined in the FADT table. Three protocols are used due to various
limitations over time, but all are included herein.

Signed-off-by: Duncan Laurie <[email protected]>
Signed-off-by: Aaron Durbin <[email protected]>
Signed-off-by: Mike Waychison <[email protected]>
---
Changelog:
v2:
- Use ARRAY_SIZE() rather than open coded construct.
- Fix off-by-one in array lookup.
- Removed superfluous memset.
- Removed superfluous variable clear.
- switch from posix integer types to linux kernel integer types.
- Replaced use of dropped oops_notifier_list with die_notifier_list.
- Added dependency on CONFIG_DMI.
- Fixed a bug which crashed the kernel if no DMI tables were found.
- Replace nvram ioctls with efivars interface at
/sys/firmware/gsmi/vars.
- Removed NOOP command that is unused.
- Change gsmi_dev to not be statically allocated.
- Removed ioctl interface
- Removed chardev interface
- Removed header as user ABI is all done via sysfs.
- Fix output clobber for firmware call so that we don't clobber the
compiler-cached value for 'cmd'.
- Added docs to Documentation/ABI for exposed system call interface.
---
Documentation/ABI/testing/sysfs-firmware-gsmi | 58 ++
drivers/firmware/google/Kconfig | 11
drivers/firmware/google/Makefile | 1
drivers/firmware/google/gsmi.c | 927 +++++++++++++++++++++++++
4 files changed, 997 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-firmware-gsmi
create mode 100644 drivers/firmware/google/gsmi.c

diff --git a/Documentation/ABI/testing/sysfs-firmware-gsmi b/Documentation/ABI/testing/sysfs-firmware-gsmi
new file mode 100644
index 0000000..0faa0aa
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-gsmi
@@ -0,0 +1,58 @@
+What: /sys/firmware/gsmi
+Date: March 2011
+Contact: Mike Waychison <[email protected]>
+Description:
+ Some servers used internally at Google have firmware
+ that provides callback functionality via explicit SMI
+ triggers. Some of the callbacks are similar to those
+ provided by the EFI runtime services page, but due to
+ historical reasons this different entry-point has been
+ used.
+
+ The gsmi driver implements the kernel's abstraction for
+ these firmware callbacks. Currently, this functionality
+ is limited to handling the system event log and getting
+ access to EFI-style variables stored in nvram.
+
+ Layout:
+
+ /sys/firmware/gsmi/vars:
+
+ This directory has the same layout (and
+ underlying implementation as /sys/firmware/efi/vars.
+ See Documentation/ABI/*/sysfs-firmware-efi-vars
+ for more information on how to interact with
+ this structure.
+
+ /sys/firmware/gsmi/append_to_eventlog - write-only:
+
+ This file takes a binary blob and passes it onto
+ the firmware to be timestamped and appended to
+ the system eventlog. The binary format is
+ interpreted by the firmware and may change from
+ platform to platform. The only kernel-enforced
+ requirement is that the blob be prefixed with a
+ 32bit host-endian type used as part of the
+ firmware call.
+
+ /sys/firmware/gsmi/clear_config - write-only:
+
+ Writing any value to this file will cause the
+ entire firmware configuration to be reset to
+ "factory defaults". Callers should assume that
+ a reboot is required for the configuration to be
+ cleared.
+
+ /sys/firmware/gsmi/clear_eventlog - write-only:
+
+ This file is used to clear out a portion/the
+ whole of the system event log. Values written
+ should be values between 1 and 100 inclusive (in
+ ASCII) representing the fraction of the log to
+ clear. Not all platforms support fractional
+ clearing though, and this writes to this file
+ will error out if the firmware doesn't like your
+ submitted fraction.
+
+ Callers should assume that a reboot is needed
+ for this operation to complete.
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index 7834729..dfc3edc 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -11,4 +11,15 @@ config GOOGLE_FIRMWARE
menu "Google Firmware Drivers"
depends on GOOGLE_FIRMWARE

+config GOOGLE_SMI
+ bool "SMI interface for Google platforms"
+ depends on ACPI && DMI
+ select EFI_VARS
+ default y
+ help
+ Say Y here if you want to enable SMI callbacks for Google
+ platforms. This provides an interface for writing to and
+ clearing the EFI event log and reading and writing NVRAM
+ variables.
+
endmenu
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index 8b13789..fb127d7 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -1 +1,2 @@

+obj-$(CONFIG_GOOGLE_SMI) += gsmi.o
diff --git a/drivers/firmware/google/gsmi.c b/drivers/firmware/google/gsmi.c
new file mode 100644
index 0000000..d1a2807
--- /dev/null
+++ b/drivers/firmware/google/gsmi.c
@@ -0,0 +1,927 @@
+/*
+ * Copyright 2010 Google Inc. All Rights Reserved.
+ * Author: [email protected] (Duncan Laurie)
+ *
+ * Re-worked to expose sysfs APIs by [email protected] (Mike Waychison)
+ *
+ * EFI SMI interface for Google platforms
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/spinlock.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/ioctl.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+#include <linux/dmi.h>
+#include <linux/kdebug.h>
+#include <linux/reboot.h>
+#include <linux/efi.h>
+
+#define GSMI_SHUTDOWN_CLEAN 0 /* Clean Shutdown */
+#define GSMI_SHUTDOWN_NMIWDT 1 /* NMI Watchdog */
+#define GSMI_SHUTDOWN_PANIC 2 /* Panic */
+#define GSMI_SHUTDOWN_OOPS 3 /* Oops */
+#define GSMI_SHUTDOWN_DIE 4 /* Die -- No longer meaningful */
+#define GSMI_SHUTDOWN_MCE 5 /* Machine Check */
+#define GSMI_SHUTDOWN_SOFTWDT 6 /* Software Watchdog */
+#define GSMI_SHUTDOWN_MBE 7 /* Uncorrected ECC */
+#define GSMI_SHUTDOWN_TRIPLE 8 /* Triple Fault */
+
+#define DRIVER_VERSION "1.0"
+#define GSMI_GUID_SIZE 16
+#define GSMI_BUF_SIZE 1024
+#define GSMI_BUF_ALIGN sizeof(u64)
+#define GSMI_CALLBACK 0xef
+
+/* SMI return codes */
+#define GSMI_SUCCESS 0x00
+#define GSMI_UNSUPPORTED2 0x03
+#define GSMI_LOG_FULL 0x0b
+#define GSMI_VAR_NOT_FOUND 0x0e
+#define GSMI_HANDSHAKE_SPIN 0x7d
+#define GSMI_HANDSHAKE_CF 0x7e
+#define GSMI_HANDSHAKE_NONE 0x7f
+#define GSMI_INVALID_PARAMETER 0x82
+#define GSMI_UNSUPPORTED 0x83
+#define GSMI_BUFFER_TOO_SMALL 0x85
+#define GSMI_NOT_READY 0x86
+#define GSMI_DEVICE_ERROR 0x87
+#define GSMI_NOT_FOUND 0x8e
+
+#define QUIRKY_BOARD_HASH 0x78a30a50
+
+/* Internally used commands passed to the firmware */
+#define GSMI_CMD_GET_NVRAM_VAR 0x01
+#define GSMI_CMD_GET_NEXT_VAR 0x02
+#define GSMI_CMD_SET_NVRAM_VAR 0x03
+#define GSMI_CMD_SET_EVENT_LOG 0x08
+#define GSMI_CMD_CLEAR_EVENT_LOG 0x09
+#define GSMI_CMD_CLEAR_CONFIG 0x20
+#define GSMI_CMD_HANDSHAKE_TYPE 0xC1
+
+/* Magic entry type for kernel events */
+#define GSMI_LOG_ENTRY_TYPE_KERNEL 0xDEAD
+
+/* SMI buffers must be in 32bit physical address space */
+struct gsmi_buf {
+ u8 *start; /* start of buffer */
+ size_t length; /* length of buffer */
+ dma_addr_t handle; /* dma allocation handle */
+ u32 address; /* physical address of buffer */
+};
+
+struct gsmi_device {
+ struct platform_device *pdev; /* platform device */
+ struct gsmi_buf *name_buf; /* variable name buffer */
+ struct gsmi_buf *data_buf; /* generic data buffer */
+ struct gsmi_buf *param_buf; /* parameter buffer */
+ spinlock_t lock; /* serialize access to SMIs */
+ u16 smi_cmd; /* SMI command port */
+ int handshake_type; /* firmware handler interlock type */
+ struct dma_pool *dma_pool; /* DMA buffer pool */
+} gsmi_dev;
+
+/* Packed structures for communicating with the firmware */
+struct gsmi_nvram_var_param {
+ efi_guid_t guid;
+ u32 name_ptr;
+ u32 attributes;
+ u32 data_len;
+ u32 data_ptr;
+} __packed;
+
+struct gsmi_get_next_var_param {
+ u8 guid[GSMI_GUID_SIZE];
+ u32 name_ptr;
+ u32 name_len;
+} __packed;
+
+struct gsmi_set_eventlog_param {
+ u32 data_ptr;
+ u32 data_len;
+ u32 type;
+} __packed;
+
+/* Event log formats */
+struct gsmi_log_entry_type_1 {
+ u16 type;
+ u32 instance;
+} __packed;
+
+
+/*
+ * Some platforms don't have explicit SMI handshake
+ * and need to wait for SMI to complete.
+ */
+#define GSMI_DEFAULT_SPINCOUNT 0x10000
+static unsigned int gsmi_spincount = GSMI_DEFAULT_SPINCOUNT;
+
+static int __init gsmi_setup_spincount(char *str)
+{
+ unsigned long res;
+ if (!strict_strtoul(str, 0, &res))
+ gsmi_spincount = (unsigned int)res;
+ return 1;
+}
+__setup("gsmi_spincount=", gsmi_setup_spincount);
+
+static struct gsmi_buf *gsmi_buf_alloc(void)
+{
+ struct gsmi_buf *smibuf;
+
+ smibuf = kzalloc(sizeof(*smibuf), GFP_KERNEL);
+ if (!smibuf) {
+ printk(KERN_ERR "gsmi: out of memory\n");
+ return NULL;
+ }
+
+ /* allocate buffer in 32bit address space */
+ smibuf->start = dma_pool_alloc(gsmi_dev.dma_pool, GFP_KERNEL,
+ &smibuf->handle);
+ if (!smibuf->start) {
+ printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
+ kfree(smibuf);
+ return NULL;
+ }
+
+ /* fill in the buffer handle */
+ smibuf->length = GSMI_BUF_SIZE;
+ smibuf->address = (u32)virt_to_phys(smibuf->start);
+
+ return smibuf;
+}
+
+static void gsmi_buf_free(struct gsmi_buf *smibuf)
+{
+ if (smibuf) {
+ if (smibuf->start)
+ dma_pool_free(gsmi_dev.dma_pool, smibuf->start,
+ smibuf->handle);
+ kfree(smibuf);
+ }
+}
+
+/*
+ * Make a call to gsmi func(sub). GSMI error codes are translated to
+ * in-kernel errnos (0 on success, -ERRNO on error).
+ */
+static int gsmi_exec(u8 func, u8 sub)
+{
+ u16 cmd = (sub << 8) | func;
+ u16 result = 0;
+ int rc = 0;
+
+ /*
+ * AH : Subfunction number
+ * AL : Function number
+ * EBX : Parameter block address
+ * DX : SMI command port
+ *
+ * Three protocols here. See also the comment in gsmi_init().
+ */
+ if (gsmi_dev.handshake_type == GSMI_HANDSHAKE_CF) {
+ /*
+ * If handshake_type == HANDSHAKE_CF then set CF on the
+ * way in and wait for the handler to clear it; this avoids
+ * corrupting register state on those chipsets which have
+ * a delay between writing the SMI trigger register and
+ * entering SMM.
+ */
+ asm volatile (
+ "stc\n"
+ "outb %%al, %%dx\n"
+ "1: jc 1b\n"
+ : "=a" (result)
+ : "0" (cmd),
+ "d" (gsmi_dev.smi_cmd),
+ "b" (gsmi_dev.param_buf->address)
+ : "memory", "cc"
+ );
+ } else if (gsmi_dev.handshake_type == GSMI_HANDSHAKE_SPIN) {
+ /*
+ * If handshake_type == HANDSHAKE_SPIN we spin a
+ * hundred-ish usecs to ensure the SMI has triggered.
+ */
+ asm volatile (
+ "outb %%al, %%dx\n"
+ "1: loop 1b\n"
+ : "=a" (result)
+ : "0" (cmd),
+ "d" (gsmi_dev.smi_cmd),
+ "b" (gsmi_dev.param_buf->address),
+ "c" (gsmi_spincount)
+ : "memory", "cc"
+ );
+ } else {
+ /*
+ * If handshake_type == HANDSHAKE_NONE we do nothing;
+ * either we don't need to or it's legacy firmware that
+ * doesn't understand the CF protocol.
+ */
+ asm volatile (
+ "outb %%al, %%dx\n\t"
+ : "=a" (result)
+ : "0" (cmd),
+ "d" (gsmi_dev.smi_cmd),
+ "b" (gsmi_dev.param_buf->address)
+ : "memory", "cc"
+ );
+ }
+
+ /* check return code from SMI handler */
+ switch (result) {
+ case GSMI_SUCCESS:
+ break;
+ case GSMI_VAR_NOT_FOUND:
+ /* not really an error, but let the caller know */
+ rc = 1;
+ break;
+ case GSMI_INVALID_PARAMETER:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Invalid parameter\n", cmd);
+ rc = -EINVAL;
+ break;
+ case GSMI_BUFFER_TOO_SMALL:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Buffer too small\n", cmd);
+ rc = -ENOMEM;
+ break;
+ case GSMI_UNSUPPORTED:
+ case GSMI_UNSUPPORTED2:
+ if (sub != GSMI_CMD_HANDSHAKE_TYPE)
+ printk(KERN_ERR "gsmi: exec 0x%04x: Not supported\n",
+ cmd);
+ rc = -ENOSYS;
+ break;
+ case GSMI_NOT_READY:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Not ready\n", cmd);
+ rc = -EBUSY;
+ break;
+ case GSMI_DEVICE_ERROR:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Device error\n", cmd);
+ rc = -EFAULT;
+ break;
+ case GSMI_NOT_FOUND:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Data not found\n", cmd);
+ rc = -ENOENT;
+ break;
+ case GSMI_LOG_FULL:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Log full\n", cmd);
+ rc = -ENOSPC;
+ break;
+ case GSMI_HANDSHAKE_CF:
+ case GSMI_HANDSHAKE_SPIN:
+ case GSMI_HANDSHAKE_NONE:
+ rc = result;
+ break;
+ default:
+ printk(KERN_ERR "gsmi: exec 0x%04x: Unknown error 0x%04x\n",
+ cmd, result);
+ rc = -ENXIO;
+ }
+
+ return rc;
+}
+
+/* Return the number of unicode characters in data */
+static size_t
+utf16_strlen(efi_char16_t *data, unsigned long maxlength)
+{
+ unsigned long length = 0;
+
+ while (*data++ != 0 && length < maxlength)
+ length++;
+ return length;
+}
+
+static efi_status_t gsmi_get_variable(efi_char16_t *name,
+ efi_guid_t *vendor, u32 *attr,
+ unsigned long *data_size,
+ void *data)
+{
+ struct gsmi_nvram_var_param param = {
+ .name_ptr = gsmi_dev.name_buf->address,
+ .data_ptr = gsmi_dev.data_buf->address,
+ .data_len = (u32)*data_size,
+ };
+ efi_status_t ret = EFI_SUCCESS;
+ unsigned long flags;
+ size_t name_len = utf16_strlen(name, GSMI_BUF_SIZE / 2);
+ int rc;
+
+ if (name_len >= GSMI_BUF_SIZE / 2)
+ return EFI_BAD_BUFFER_SIZE;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* Vendor guid */
+ memcpy(&param.guid, vendor, sizeof(param.guid));
+
+ /* variable name, already in UTF-16 */
+ memset(gsmi_dev.name_buf->start, 0, gsmi_dev.name_buf->length);
+ memcpy(gsmi_dev.name_buf->start, name, name_len * 2);
+
+ /* data pointer */
+ memset(gsmi_dev.data_buf->start, 0, gsmi_dev.data_buf->length);
+
+ /* parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_GET_NVRAM_VAR);
+ if (rc < 0) {
+ printk(KERN_ERR "gsmi: Get Variable failed\n");
+ ret = EFI_LOAD_ERROR;
+ } else if (rc == 1) {
+ /* variable was not found */
+ ret = EFI_NOT_FOUND;
+ } else {
+ /* Get the arguments back */
+ memcpy(&param, gsmi_dev.param_buf->start, sizeof(param));
+
+ /* The size reported is the min of all of our buffers */
+ *data_size = min(*data_size, gsmi_dev.data_buf->length);
+ *data_size = min_t(unsigned long, *data_size, param.data_len);
+
+ /* Copy data back to return buffer. */
+ memcpy(data, gsmi_dev.data_buf->start, *data_size);
+
+ /* All variables are have the following attributes */
+ *attr = EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS;
+ }
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ return ret;
+}
+
+static efi_status_t gsmi_get_next_variable(unsigned long *name_size,
+ efi_char16_t *name,
+ efi_guid_t *vendor)
+{
+ struct gsmi_get_next_var_param param = {
+ .name_ptr = gsmi_dev.name_buf->address,
+ .name_len = gsmi_dev.name_buf->length,
+ };
+ efi_status_t ret = EFI_SUCCESS;
+ int rc;
+ unsigned long flags;
+
+ /* For the moment, only support buffers that exactly match in size */
+ if (*name_size != GSMI_BUF_SIZE)
+ return EFI_BAD_BUFFER_SIZE;
+
+ /* Let's make sure the thing is at least null-terminated */
+ if (utf16_strlen(name, GSMI_BUF_SIZE / 2) == GSMI_BUF_SIZE / 2)
+ return EFI_INVALID_PARAMETER;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* guid */
+ memcpy(&param.guid, vendor, sizeof(param.guid));
+
+ /* variable name, already in UTF-16 */
+ memcpy(gsmi_dev.name_buf->start, name, *name_size);
+
+ /* parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_GET_NEXT_VAR);
+ if (rc < 0) {
+ printk(KERN_ERR "gsmi: Get Next Variable Name failed\n");
+ ret = EFI_LOAD_ERROR;
+ } else if (rc == 1) {
+ /* variable not found -- end of list */
+ ret = EFI_NOT_FOUND;
+ } else {
+ /* copy variable data back to return buffer */
+ memcpy(&param, gsmi_dev.param_buf->start, sizeof(param));
+
+ /* Copy the name back */
+ memcpy(name, gsmi_dev.name_buf->start, GSMI_BUF_SIZE);
+ *name_size = utf16_strlen(name, GSMI_BUF_SIZE / 2) * 2;
+
+ /* copy guid to return buffer */
+ memcpy(vendor, &param.guid, sizeof(param.guid));
+ ret = EFI_SUCCESS;
+ }
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ return ret;
+}
+
+static efi_status_t gsmi_set_variable(efi_char16_t *name,
+ efi_guid_t *vendor,
+ unsigned long attr,
+ unsigned long data_size,
+ void *data)
+{
+ struct gsmi_nvram_var_param param = {
+ .name_ptr = gsmi_dev.name_buf->address,
+ .data_ptr = gsmi_dev.data_buf->address,
+ .data_len = (u32)data_size,
+ .attributes = EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ };
+ size_t name_len = utf16_strlen(name, GSMI_BUF_SIZE / 2);
+ efi_status_t ret = EFI_SUCCESS;
+ int rc;
+ unsigned long flags;
+
+ if (name_len >= GSMI_BUF_SIZE / 2)
+ return EFI_BAD_BUFFER_SIZE;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* guid */
+ memcpy(&param.guid, vendor, sizeof(param.guid));
+
+ /* variable name, already in UTF-16 */
+ memset(gsmi_dev.name_buf->start, 0, gsmi_dev.name_buf->length);
+ memcpy(gsmi_dev.name_buf->start, name, name_len * 2);
+
+ /* data pointer */
+ memset(gsmi_dev.data_buf->start, 0, gsmi_dev.data_buf->length);
+ memcpy(gsmi_dev.data_buf->start, data, data_size);
+
+ /* parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_SET_NVRAM_VAR);
+ if (rc < 0) {
+ printk(KERN_ERR "gsmi: Set Variable failed\n");
+ ret = EFI_INVALID_PARAMETER;
+ }
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ return ret;
+}
+
+static const struct efivar_operations efivar_ops = {
+ .get_variable = gsmi_get_variable,
+ .set_variable = gsmi_set_variable,
+ .get_next_variable = gsmi_get_next_variable,
+};
+
+static ssize_t eventlog_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct gsmi_set_eventlog_param param = {
+ .data_ptr = gsmi_dev.data_buf->address,
+ };
+ int rc = 0;
+ unsigned long flags;
+
+ /* Pull the type out */
+ if (count < sizeof(u32))
+ return -EINVAL;
+ param.type = *(u32 *)buf;
+ count -= sizeof(u32);
+ buf += sizeof(u32);
+
+ /* The remaining buffer is the data payload */
+ if (count > gsmi_dev.data_buf->length)
+ return -EINVAL;
+ param.data_len = count - sizeof(u32);
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* data pointer */
+ memset(gsmi_dev.data_buf->start, 0, gsmi_dev.data_buf->length);
+ memcpy(gsmi_dev.data_buf->start, buf, param.data_len);
+
+ /* parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_SET_EVENT_LOG);
+ if (rc < 0)
+ printk(KERN_ERR "gsmi: Set Event Log failed\n");
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ return rc;
+
+}
+
+static struct bin_attribute eventlog_bin_attr = {
+ .attr = {.name = "append_to_eventlog", .mode = 0200},
+ .write = eventlog_write,
+};
+
+static ssize_t gsmi_clear_eventlog_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc;
+ unsigned long flags;
+ unsigned long val;
+ struct {
+ u32 percentage;
+ u32 data_type;
+ } param;
+
+ rc = strict_strtoul(buf, 0, &val);
+ if (rc)
+ return rc;
+
+ /*
+ * Value entered is a percentage, 0 through 100, anything else
+ * is invalid.
+ */
+ if (val > 100)
+ return -EINVAL;
+
+ /* data_type here selects the smbios event log. */
+ param.percentage = val;
+ param.data_type = 0;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_CLEAR_EVENT_LOG);
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ if (rc)
+ return rc;
+ return count;
+}
+
+static struct kobj_attribute gsmi_clear_eventlog_attr = {
+ .attr = {.name = "clear_eventlog", .mode = 0200},
+ .store = gsmi_clear_eventlog_store,
+};
+
+static ssize_t gsmi_clear_config_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ /* clear parameter buffer */
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_CLEAR_CONFIG);
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ if (rc)
+ return rc;
+ return count;
+}
+
+static struct kobj_attribute gsmi_clear_config_attr = {
+ .attr = {.name = "clear_config", .mode = 0200},
+ .store = gsmi_clear_config_store,
+};
+
+static const struct attribute *gsmi_attrs[] = {
+ &gsmi_clear_config_attr.attr,
+ &gsmi_clear_eventlog_attr.attr,
+ NULL,
+};
+
+static int gsmi_shutdown_reason(int reason)
+{
+ struct gsmi_log_entry_type_1 entry = {
+ .type = GSMI_LOG_ENTRY_TYPE_KERNEL,
+ .instance = reason,
+ };
+ struct gsmi_set_eventlog_param param = {
+ .data_len = sizeof(entry),
+ .type = 1,
+ };
+ static int saved_reason;
+ int rc = 0;
+ unsigned long flags;
+
+ /* avoid duplicate entries in the log */
+ if (saved_reason & (1 << reason))
+ return 0;
+
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+
+ saved_reason |= (1 << reason);
+
+ /* data pointer */
+ memset(gsmi_dev.data_buf->start, 0, gsmi_dev.data_buf->length);
+ memcpy(gsmi_dev.data_buf->start, &entry, sizeof(entry));
+
+ /* parameter buffer */
+ param.data_ptr = gsmi_dev.data_buf->address;
+ memset(gsmi_dev.param_buf->start, 0, gsmi_dev.param_buf->length);
+ memcpy(gsmi_dev.param_buf->start, &param, sizeof(param));
+
+ rc = gsmi_exec(GSMI_CALLBACK, GSMI_CMD_SET_EVENT_LOG);
+
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ if (rc < 0)
+ printk(KERN_ERR "gsmi: Log Shutdown Reason failed\n");
+ else
+ printk(KERN_EMERG "gsmi: Log Shutdown Reason 0x%02x\n",
+ reason);
+
+ return rc;
+}
+
+static int gsmi_reboot_callback(struct notifier_block *nb,
+ unsigned long reason, void *arg)
+{
+ gsmi_shutdown_reason(GSMI_SHUTDOWN_CLEAN);
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_reboot_notifier = {
+ .notifier_call = gsmi_reboot_callback
+};
+
+static int gsmi_die_callback(struct notifier_block *nb,
+ unsigned long reason, void *arg)
+{
+ if (reason == DIE_NMIWATCHDOG)
+ gsmi_shutdown_reason(GSMI_SHUTDOWN_NMIWDT);
+ else if (reason == DIE_OOPS)
+ gsmi_shutdown_reason(GSMI_SHUTDOWN_OOPS);
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_die_notifier = {
+ .notifier_call = gsmi_die_callback
+};
+
+static int gsmi_panic_callback(struct notifier_block *nb,
+ unsigned long reason, void *arg)
+{
+ gsmi_shutdown_reason(GSMI_SHUTDOWN_PANIC);
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block gsmi_panic_notifier = {
+ .notifier_call = gsmi_panic_callback,
+};
+
+/*
+ * This hash function was blatantly copied from include/linux/hash.h.
+ * It is used by this driver to obfuscate a board name that requires a
+ * quirk within this driver.
+ *
+ * Please do not remove this copy of the function as any changes to the
+ * global utility hash_64() function would break this driver's ability
+ * to identify a board and provide the appropriate quirk -- [email protected]
+ */
+static u64 __init local_hash_64(u64 val, unsigned bits)
+{
+ u64 hash = val;
+
+ /* Sigh, gcc can't optimise this alone like it does for 32 bits. */
+ u64 n = hash;
+ n <<= 18;
+ hash -= n;
+ n <<= 33;
+ hash -= n;
+ n <<= 3;
+ hash += n;
+ n <<= 3;
+ hash -= n;
+ n <<= 4;
+ hash += n;
+ n <<= 2;
+ hash += n;
+
+ /* High bits are more random, so use them. */
+ return hash >> (64 - bits);
+}
+
+static u32 __init hash_oem_table_id(char s[8])
+{
+ u64 input;
+ memcpy(&input, s, 8);
+ return local_hash_64(input, 32);
+}
+
+static int gsmi_system_valid(void)
+{
+ const char *board_vendor;
+ u32 hash;
+
+ /*
+ * Check platform compatibility. Only ever load on Google's
+ * machines.
+ */
+ board_vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
+ if (strncmp(acpi_gbl_FADT.header.oem_id, "GOOGLE",
+ ACPI_OEM_ID_SIZE)) {
+ if (!board_vendor || strcmp("Google, Inc.", board_vendor)) {
+ printk(KERN_INFO "gsmi: Not a google board\n");
+ return -ENODEV;
+ }
+ }
+
+ /*
+ * Only newer firmware supports the gsmi interface. All older
+ * firmware that didn't support this interface used to plug the
+ * table name in the first four bytes of the oem_table_id field.
+ * Newer firmware doesn't do that though, so use that as the
+ * discriminant factor. We have to do this in order to
+ * whitewash our board names out of the public driver.
+ */
+ if (!strncmp(acpi_gbl_FADT.header.oem_table_id, "FACP", 4)) {
+ printk(KERN_INFO "gsmi: Board is too old\n");
+ return -ENODEV;
+ }
+
+ /* Disable on board with 1.0 BIOS due to Google bug 2602657 */
+ hash = hash_oem_table_id(acpi_gbl_FADT.header.oem_table_id);
+ if (hash == QUIRKY_BOARD_HASH) {
+ const char *bios_ver = dmi_get_system_info(DMI_BIOS_VERSION);
+ if (strncmp(bios_ver, "1.0", 3) == 0) {
+ pr_info("gsmi: disabled on this board's BIOS %s\n",
+ bios_ver);
+ return -ENODEV;
+ }
+ }
+
+ /* check for valid SMI command port in ACPI FADT */
+ if (acpi_gbl_FADT.smi_command == 0) {
+ pr_info("gsmi: missing smi_command\n");
+ return -ENODEV;
+ }
+
+ /* Found */
+ return 0;
+}
+
+static struct kobject *gsmi_kobj;
+static struct efivars efivars;
+
+static __init int gsmi_init(void)
+{
+ unsigned long flags;
+ int ret;
+
+ ret = gsmi_system_valid();
+ if (ret)
+ return ret;
+
+ gsmi_dev.smi_cmd = acpi_gbl_FADT.smi_command;
+
+ /* register device */
+ gsmi_dev.pdev = platform_device_register_simple("gsmi", -1, NULL, 0);
+ if (IS_ERR(gsmi_dev.pdev)) {
+ printk(KERN_ERR "gsmi: unable to register platform device\n");
+ return PTR_ERR(gsmi_dev.pdev);
+ }
+
+ /* SMI access needs to be serialized */
+ spin_lock_init(&gsmi_dev.lock);
+
+ /* SMI callbacks require 32bit addresses */
+ gsmi_dev.pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+ gsmi_dev.pdev->dev.dma_mask =
+ &gsmi_dev.pdev->dev.coherent_dma_mask;
+ ret = -ENOMEM;
+ gsmi_dev.dma_pool = dma_pool_create("gsmi", &gsmi_dev.pdev->dev,
+ GSMI_BUF_SIZE, GSMI_BUF_ALIGN, 0);
+ if (!gsmi_dev.dma_pool)
+ goto out_err;
+
+ /*
+ * pre-allocate buffers because sometimes we are called when
+ * this is not feasible: oops, panic, die, mce, etc
+ */
+ gsmi_dev.name_buf = gsmi_buf_alloc();
+ if (!gsmi_dev.name_buf) {
+ printk(KERN_ERR "gsmi: failed to allocate name buffer\n");
+ goto out_err;
+ }
+
+ gsmi_dev.data_buf = gsmi_buf_alloc();
+ if (!gsmi_dev.data_buf) {
+ printk(KERN_ERR "gsmi: failed to allocate data buffer\n");
+ goto out_err;
+ }
+
+ gsmi_dev.param_buf = gsmi_buf_alloc();
+ if (!gsmi_dev.param_buf) {
+ printk(KERN_ERR "gsmi: failed to allocate param buffer\n");
+ goto out_err;
+ }
+
+ /*
+ * Determine type of handshake used to serialize the SMI
+ * entry. See also gsmi_exec().
+ *
+ * There's a "behavior" present on some chipsets where writing the
+ * SMI trigger register in the southbridge doesn't result in an
+ * immediate SMI. Rather, the processor can execute "a few" more
+ * instructions before the SMI takes effect. To ensure synchronous
+ * behavior, implement a handshake between the kernel driver and the
+ * firmware handler to spin until released. This ioctl determines
+ * the type of handshake.
+ *
+ * NONE: The firmware handler does not implement any
+ * handshake. Either it doesn't need to, or it's legacy firmware
+ * that doesn't know it needs to and never will.
+ *
+ * CF: The firmware handler will clear the CF in the saved
+ * state before returning. The driver may set the CF and test for
+ * it to clear before proceeding.
+ *
+ * SPIN: The firmware handler does not implement any handshake
+ * but the driver should spin for a hundred or so microseconds
+ * to ensure the SMI has triggered.
+ *
+ * Finally, the handler will return -ENOSYS if
+ * GSMI_CMD_HANDSHAKE_TYPE is unimplemented, which implies
+ * HANDSHAKE_NONE.
+ */
+ spin_lock_irqsave(&gsmi_dev.lock, flags);
+ gsmi_dev.handshake_type = GSMI_HANDSHAKE_SPIN;
+ gsmi_dev.handshake_type =
+ gsmi_exec(GSMI_CALLBACK, GSMI_CMD_HANDSHAKE_TYPE);
+ if (gsmi_dev.handshake_type == -ENOSYS)
+ gsmi_dev.handshake_type = GSMI_HANDSHAKE_NONE;
+ spin_unlock_irqrestore(&gsmi_dev.lock, flags);
+
+ /* Remove and clean up gsmi if the handshake could not complete. */
+ if (gsmi_dev.handshake_type == -ENXIO) {
+ printk(KERN_INFO "gsmi version " DRIVER_VERSION
+ " failed to load\n");
+ ret = -ENODEV;
+ goto out_err;
+ }
+
+ printk(KERN_INFO "gsmi version " DRIVER_VERSION " loaded\n");
+
+ /* Register in the firmware directory */
+ ret = -ENOMEM;
+ gsmi_kobj = kobject_create_and_add("gsmi", firmware_kobj);
+ if (!gsmi_kobj) {
+ printk(KERN_INFO "gsmi: Failed to create firmware kobj\n");
+ goto out_err;
+ }
+
+ /* Setup eventlog access */
+ ret = sysfs_create_bin_file(gsmi_kobj, &eventlog_bin_attr);
+ if (ret) {
+ printk(KERN_INFO "gsmi: Failed to setup eventlog");
+ goto out_err;
+ }
+
+ /* Other attributes */
+ ret = sysfs_create_files(gsmi_kobj, gsmi_attrs);
+ if (ret) {
+ printk(KERN_INFO "gsmi: Failed to add attrs");
+ goto out_err;
+ }
+
+ if (register_efivars(&efivars, &efivar_ops, gsmi_kobj)) {
+ printk(KERN_INFO "gsmi: Failed to register efivars\n");
+ goto out_err;
+ }
+
+ register_reboot_notifier(&gsmi_reboot_notifier);
+ register_die_notifier(&gsmi_die_notifier);
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &gsmi_panic_notifier);
+
+ return 0;
+
+ out_err:
+ kobject_put(gsmi_kobj);
+ gsmi_buf_free(gsmi_dev.param_buf);
+ gsmi_buf_free(gsmi_dev.data_buf);
+ gsmi_buf_free(gsmi_dev.name_buf);
+ if (gsmi_dev.dma_pool)
+ dma_pool_destroy(gsmi_dev.dma_pool);
+ platform_device_unregister(gsmi_dev.pdev);
+ pr_info("gsmi: failed to load: %d\n", ret);
+ return ret;
+}
+
+late_initcall(gsmi_init);

2011-03-12 01:43:47

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 06/12] efivars: Expose efivars functionality to external drivers.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 34 +++++++---------------------------
include/linux/efi.h | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index b10b7d7..ff0c373 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -89,28 +89,6 @@ MODULE_DESCRIPTION("sysfs interface to EFI Variables");
MODULE_LICENSE("GPL");
MODULE_VERSION(EFIVARS_VERSION);

-struct efivar_operations {
- efi_get_variable_t *get_variable;
- efi_get_next_variable_t *get_next_variable;
- efi_set_variable_t *set_variable;
-};
-
-struct efivars {
- /*
- * ->lock protects two things:
- * 1) ->list - adds, removals, reads, writes
- * 2) ops.[gs]et_variable() calls.
- * It must not be held when creating sysfs entries or calling kmalloc.
- * ops.get_next_variable() is only called from register_efivars(),
- * which is protected by the BKL, so that path is safe.
- */
- spinlock_t lock;
- struct list_head list;
- struct kset *kset;
- struct bin_attribute *new_var, *del_var;
- const struct efivar_operations *ops;
-};
-
/*
* The maximum size of VariableName + Data = 1024
* Therefore, it's reasonable to save that much
@@ -706,7 +684,7 @@ out_free:
return error;
}

-static void unregister_efivars(struct efivars *efivars)
+void unregister_efivars(struct efivars *efivars)
{
struct efivar_entry *entry, *n;

@@ -724,10 +702,11 @@ static void unregister_efivars(struct efivars *efivars)
kfree(efivars->del_var);
kset_unregister(efivars->kset);
}
+EXPORT_SYMBOL_GPL(unregister_efivars);

-static int register_efivars(struct efivars *efivars,
- const struct efivar_operations *ops,
- struct kobject *parent_kobj)
+int register_efivars(struct efivars *efivars,
+ const struct efivar_operations *ops,
+ struct kobject *parent_kobj)
{
efi_status_t status = EFI_NOT_FOUND;
efi_guid_t vendor_guid;
@@ -789,6 +768,7 @@ out:

return error;
}
+EXPORT_SYMBOL_GPL(register_efivars);

static struct efivars __efivars;
static struct efivar_operations ops;
@@ -810,7 +790,7 @@ efivars_init(void)
EFIVARS_DATE);

if (!efi_enabled)
- return -ENODEV;
+ return 0;

/* For now we'll register the efi directory at /sys/firmware/efi */
efi_kobj = kobject_create_and_add("efi", firmware_kobj);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index fb737bc..33fa120 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -397,4 +397,41 @@ static inline void memrange_efi_to_native(u64 *addr, u64 *npages)
*addr &= PAGE_MASK;
}

+#if defined(CONFIG_EFI_VARS) || defined(CONFIG_EFI_VARS_MODULE)
+/*
+ * EFI Variable support.
+ *
+ * Different firmware drivers can expose their EFI-like variables using
+ * the following.
+ */
+
+struct efivar_operations {
+ efi_get_variable_t *get_variable;
+ efi_get_next_variable_t *get_next_variable;
+ efi_set_variable_t *set_variable;
+};
+
+struct efivars {
+ /*
+ * ->lock protects two things:
+ * 1) ->list - adds, removals, reads, writes
+ * 2) ops.[gs]et_variable() calls.
+ * It must not be held when creating sysfs entries or calling kmalloc.
+ * ops.get_next_variable() is only called from register_efivars(),
+ * which is protected by the BKL, so that path is safe.
+ */
+ spinlock_t lock;
+ struct list_head list;
+ struct kset *kset;
+ struct bin_attribute *new_var, *del_var;
+ const struct efivar_operations *ops;
+};
+
+int register_efivars(struct efivars *efivars,
+ const struct efivar_operations *ops,
+ struct kobject *parent_kobj);
+void unregister_efivars(struct efivars *efivars);
+
+#endif /* CONFIG_EFI_VARS */
+
#endif /* _LINUX_EFI_H */

2011-03-12 01:45:28

by Mike Waychison

[permalink] [raw]
Subject: [PATCH v2 02/12] efivars: Make efivars bin_attributes dynamic

In preparation for encapsulating efivars, we need to have the
bin_attributes be dynamically allocated so that we can use their
->private fields to get back to the struct efivars structure.

Signed-off-by: Mike Waychison <[email protected]>
---
drivers/firmware/efivars.c | 93 ++++++++++++++++++++++++++++++++------------
1 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index f10ecb6..528ce47 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -101,6 +101,7 @@ struct efivars {
spinlock_t lock;
struct list_head list;
struct kset *kset;
+ struct bin_attribute *new_var, *del_var;
};
static struct efivars __efivars;
static struct efivars *efivars = &__efivars;
@@ -525,16 +526,6 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
return count;
}

-static struct bin_attribute var_subsys_attr_new_var = {
- .attr = {.name = "new_var", .mode = 0200},
- .write = efivar_create,
-};
-
-static struct bin_attribute var_subsys_attr_del_var = {
- .attr = {.name = "del_var", .mode = 0200},
- .write = efivar_delete,
-};
-
/*
* Let's not leave out systab information that snuck into
* the efivars driver
@@ -640,6 +631,66 @@ efivar_create_sysfs_entry(unsigned long variable_name_size,

return 0;
}
+
+static int
+create_efivars_bin_attributes(struct efivars *efivars)
+{
+ struct bin_attribute *attr;
+ int error;
+
+ /* new_var */
+ attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+ if (!attr)
+ return -ENOMEM;
+
+ attr->attr.name = "new_var";
+ attr->attr.mode = 0200;
+ attr->write = efivar_create;
+ attr->private = efivars;
+ efivars->new_var = attr;
+
+ /* del_var */
+ attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+ if (!attr) {
+ error = -ENOMEM;
+ goto out_free;
+ }
+ attr->attr.name = "del_var";
+ attr->attr.mode = 0200;
+ attr->write = efivar_delete;
+ attr->private = efivars;
+ efivars->del_var = attr;
+
+ sysfs_bin_attr_init(efivars->new_var);
+ sysfs_bin_attr_init(efivars->del_var);
+
+ /* Register */
+ error = sysfs_create_bin_file(&efivars->kset->kobj,
+ efivars->new_var);
+ if (error) {
+ printk(KERN_ERR "efivars: unable to create new_var sysfs file"
+ " due to error %d\n", error);
+ goto out_free;
+ }
+ error = sysfs_create_bin_file(&efivars->kset->kobj,
+ efivars->del_var);
+ if (error) {
+ printk(KERN_ERR "efivars: unable to create del_var sysfs file"
+ " due to error %d\n", error);
+ sysfs_remove_bin_file(&efivars->kset->kobj,
+ efivars->new_var);
+ goto out_free;
+ }
+
+ return 0;
+out_free:
+ kfree(efivars->new_var);
+ efivars->new_var = NULL;
+ kfree(efivars->new_var);
+ efivars->new_var = NULL;
+ return error;
+}
+
/*
* For now we register the efi subsystem with the firmware subsystem
* and the vars subsystem with the efi subsystem. In the future, it
@@ -714,20 +765,7 @@ efivars_init(void)
}
} while (status != EFI_NOT_FOUND);

- /*
- * Now add attributes to allow creation of new vars
- * and deletion of existing ones...
- */
- error = sysfs_create_bin_file(&efivars->kset->kobj,
- &var_subsys_attr_new_var);
- if (error)
- printk(KERN_ERR "efivars: unable to create new_var sysfs file"
- " due to error %d\n", error);
- error = sysfs_create_bin_file(&efivars->kset->kobj,
- &var_subsys_attr_del_var);
- if (error)
- printk(KERN_ERR "efivars: unable to create del_var sysfs file"
- " due to error %d\n", error);
+ error = create_efivars_bin_attributes(efivars);

/* Don't forget the systab entry */
error = sysfs_create_group(efi_kobj, &efi_subsys_attr_group);
@@ -758,7 +796,12 @@ efivars_exit(void)
spin_unlock(&efivars->lock);
efivar_unregister(entry);
}
-
+ if (efivars->new_var)
+ sysfs_remove_bin_file(&efivars->kset->kobj, efivars->new_var);
+ if (efivars->del_var)
+ sysfs_remove_bin_file(&efivars->kset->kobj, efivars->del_var);
+ kfree(efivars->new_var);
+ kfree(efivars->del_var);
kset_unregister(efivars->kset);
kobject_put(efi_kobj);
}

2011-03-12 03:54:59

by Matt Domsch

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] google firmware support

On Fri, Mar 11, 2011 at 05:42:55PM -0800, Mike Waychison wrote:
> Patchset summary
> ================
>
> Patches [1 through 6] refactor the 'efivars' module to let the "gsmi"
> driver in patch [11] re-use the variable functionality.
>
> Patch [7] documents existing 'efivars' functionality that has been
> present in the tree since 2004.

Very nice work Mike. I can't test the efivars changes for another
week or more, but reading it all, looks like a nice cleanup while
allowing you to reuse the underlying code.


Thanks,
Matt

--
Matt Domsch
Technology Strategist
Dell | Office of the CTO

2011-03-14 04:56:29

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

On 03/11/2011 05:43 PM, Mike Waychison wrote:
> This patch introduces the 'memconsole' driver.
>
> Our firmware gives us access to an in-memory log of the firmware's
> output. This gives us visibility in a data-center of headless machines
> as to what the firmware is doing.
>
> The memory console is found by the driver by finding a header block in
> the EBDA. The buffer is then copied out, and is exported to userland in
> the file /sys/firmware/log.
>

OK, I really don't like this.

All it has is a 32-bit nonrandom signature, no DMI keying or anything
else to protect it. This really isn't sufficient.

-hpa

2011-03-14 04:59:32

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

"Sufficient" or not, that's how the BIOSes we've got work. We can't
retrofit that.

On Sun, Mar 13, 2011 at 9:54 PM, H. Peter Anvin <[email protected]> wrote:
> On 03/11/2011 05:43 PM, Mike Waychison wrote:
>> This patch introduces the 'memconsole' driver.
>>
>> Our firmware gives us access to an in-memory log of the firmware's
>> output. ? This gives us visibility in a data-center of headless machines
>> as to what the firmware is doing.
>>
>> The memory console is found by the driver by finding a header block in
>> the EBDA. ?The buffer is then copied out, and is exported to userland in
>> the file /sys/firmware/log.
>>
>
> OK, I really don't like this.
>
> All it has is a 32-bit nonrandom signature, no DMI keying or anything
> else to protect it. ?This really isn't sufficient.
>
> ? ? ? ?-hpa
>

2011-03-14 05:04:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

On 03/13/2011 09:58 PM, Tim Hockin wrote:
> "Sufficient" or not, that's how the BIOSes we've got work. We can't
> retrofit that.

Uhm... what about, say, a DMI signature?

-hpa

2011-03-14 09:22:49

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console


* H. Peter Anvin <[email protected]> wrote:

> On 03/13/2011 09:58 PM, Tim Hockin wrote:
> > "Sufficient" or not, that's how the BIOSes we've got work. We can't
> > retrofit that.
>
> Uhm... what about, say, a DMI signature?

Yes - how does the dmidecode output look like on such a box?

Thanks,

Ingo

2011-03-14 14:01:32

by Tim Hockin

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

This same BIOS logic is used across a number of Platforms. We could
use something like the vendor strings to trigger this code off, I
suppose, but ick. It's already in a google/ subdir and wrapped with
CONFIG_GOOGLE_MEMCONSOLE.



On Mon, Mar 14, 2011 at 2:22 AM, Ingo Molnar <[email protected]> wrote:
>
> * H. Peter Anvin <[email protected]> wrote:
>
>> On 03/13/2011 09:58 PM, Tim Hockin wrote:
>> > "Sufficient" or not, that's how the BIOSes we've got work. ?We can't
>> > retrofit that.
>>
>> Uhm... what about, say, a DMI signature?
>
> Yes - how does the dmidecode output look like on such a box?
>
> Thanks,
>
> ? ? ? ?Ingo
>

2011-03-14 14:23:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console


* Tim Hockin <[email protected]> wrote:

> This same BIOS logic is used across a number of Platforms. We could
> use something like the vendor strings to trigger this code off, [...]

Correct.

> [...] I suppose, but ick. It's already in a google/ subdir and wrapped with
> CONFIG_GOOGLE_MEMCONSOLE.

That's not the point though - in this day and age we simply do not add non-generic
hacks like that - we've come a long way since the ISA days. The point is to make it
safe enough to limit its effects not just to CONFIG_GOOGLE_MEMCONSOLE but also to
affected hardware only.

Thanks,

Ingo

2011-03-14 15:49:57

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 09/12] x86: Better comments for get_bios_ebda()

On Fri, Mar 11, 2011 at 05:43:42PM -0800, Mike Waychison wrote:
> Make the comments a bit clearer for get_bios_ebda so that it actually
> tells us what it is returning.
>
> Signed-off-by: Mike Waychison <[email protected]>
> ---
> arch/x86/include/asm/bios_ebda.h | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)

Any objection/acks from the x86 maintainers about me including this in
the driver-core-next tree along with the other efi patches?

thanks,

greg k-h

2011-03-14 15:50:01

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

On Mon, Mar 14, 2011 at 03:23:15PM +0100, Ingo Molnar wrote:
>
> * Tim Hockin <[email protected]> wrote:
>
> > This same BIOS logic is used across a number of Platforms. We could
> > use something like the vendor strings to trigger this code off, [...]
>
> Correct.
>
> > [...] I suppose, but ick. It's already in a google/ subdir and wrapped with
> > CONFIG_GOOGLE_MEMCONSOLE.
>
> That's not the point though - in this day and age we simply do not add non-generic
> hacks like that - we've come a long way since the ISA days. The point is to make it
> safe enough to limit its effects not just to CONFIG_GOOGLE_MEMCONSOLE but also to
> affected hardware only.

I strongly agree, we can't take this as-is without this type of code
added to the driver, sorry.

Thanks,

greg k-h

2011-03-14 15:49:55

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] google firmware support

On Fri, Mar 11, 2011 at 09:54:56PM -0600, Matt Domsch wrote:
> On Fri, Mar 11, 2011 at 05:42:55PM -0800, Mike Waychison wrote:
> > Patchset summary
> > ================
> >
> > Patches [1 through 6] refactor the 'efivars' module to let the "gsmi"
> > driver in patch [11] re-use the variable functionality.
> >
> > Patch [7] documents existing 'efivars' functionality that has been
> > present in the tree since 2004.
>
> Very nice work Mike. I can't test the efivars changes for another
> week or more, but reading it all, looks like a nice cleanup while
> allowing you to reuse the underlying code.

I agree, I've applied these 7 patches to my tree for merging with Linus
in .39.

I'll wait on the other 5 as I need acks from others to apply them.

thanks,

greg k-h

2011-03-14 15:50:04

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] driver: Google EFI SMI

On Fri, Mar 11, 2011 at 05:43:53PM -0800, Mike Waychison wrote:
> The "gsmi" driver bridges userland with firmware specific routines for
> accessing hardware.

As with the other driver in this series, what keeps this driver from
being loaded on hardware that does not support this functionality? If
it is loaded, will it cause bad things to happen?

Also, what causes it to be loaded on hardware that needs it? There
should be some MODULE_DEVICE_TABLE somewhere in this thing to cause it
to be autoloaded?

thanks,

greg k-h

2011-03-14 15:50:40

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] Introduce CONFIG_GOOGLE_FIRMWARE

On Fri, Mar 11, 2011 at 05:43:47PM -0800, Mike Waychison wrote:
> In order to keep Google's firmware drivers organized amongst themselves,
> create a new directory for them to live in. As well, all Google
> firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
> to 'n' in the kernel build.

Everything defaults to 'n' in the kernel build system, so this really
isn't a big deal :)

>
> Signed-off-by: Mike Waychison <[email protected]>
> ---
> drivers/firmware/Kconfig | 2 ++
> drivers/firmware/Makefile | 2 ++
> drivers/firmware/google/Kconfig | 14 ++++++++++++++
> drivers/firmware/google/Makefile | 1 +
> 4 files changed, 19 insertions(+), 0 deletions(-)
> create mode 100644 drivers/firmware/google/Kconfig
> create mode 100644 drivers/firmware/google/Makefile
>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index e710424..d848b26 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -134,4 +134,6 @@ config ISCSI_IBFT
> detect iSCSI boot parameters dynamically during system boot, say Y.
> Otherwise, say N.
>
> +source "drivers/firmware/google/Kconfig"
> +
> endmenu
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 1c3c173..6f68007 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -11,3 +11,5 @@ obj-$(CONFIG_DMIID) += dmi-id.o
> obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o
> obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
> obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
> +
> +obj-$(CONFIG_GOOGLE_FIRMWARE) += google/
> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
> new file mode 100644
> index 0000000..7834729
> --- /dev/null
> +++ b/drivers/firmware/google/Kconfig
> @@ -0,0 +1,14 @@
> +config GOOGLE_FIRMWARE
> + bool "Google Firmware Drivers"
> + depends on X86
> + default n
> + help
> + These firmware drivers are used by Google's servers. They are
> + only useful if you are working directly on one of their
> + proprietary servers or implementing similar firmware
> + interfaces. If in doubt, say "N".

How about dropping the "or implementing..." part of this sentance, as
that leaves a lot open for some people to justify enabling this option
for their machines, when it really isn't true at all at this point in
time.

thanks,

greg k-h

2011-03-14 15:51:13

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 08/12] x86: get_bios_ebda_length()

On Fri, Mar 11, 2011 at 05:43:37PM -0800, Mike Waychison wrote:
> Add a wrapper routine that tells us the length of the EBDA if it is
> present. This guy also ensures that the returned length doesn't let the
> EBDA run past the 640KiB mark.
>
> Signed-off-by: Mike Waychison <[email protected]>
> ---
> arch/x86/include/asm/bios_ebda.h | 21 +++++++++++++++++++++
> 1 files changed, 21 insertions(+), 0 deletions(-)

Any objections from the x86 developers about me including this with the
other efi patches in my driver-core-next tree?

thanks,

greg k-h

2011-03-14 19:49:41

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] Introduce CONFIG_GOOGLE_FIRMWARE

On Mon, Mar 14, 2011 at 8:45 AM, Greg KH <[email protected]> wrote:
> On Fri, Mar 11, 2011 at 05:43:47PM -0800, Mike Waychison wrote:
>> In order to keep Google's firmware drivers organized amongst themselves,
>> create a new directory for them to live in. ?As well, all Google
>> firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
>> to 'n' in the kernel build.
>
> Everything defaults to 'n' in the kernel build system, so this really
> isn't a big deal :)

Well, I was hoping that we could have options gating on
CONFIG_GOOGLE_FIRMWARE, but default to 'y'. This makes our lives a
bit easier as it means we have a single switch we need to enable on
our side and all the options we care about (presumably all
google-specific drivers) get enabled.

If you have strong objections to this, I can flip the default values
of the specific config options to 'n' as well though...

>
>>
>> Signed-off-by: Mike Waychison <[email protected]>
>> ---
>> ?drivers/firmware/Kconfig ? ? ? ? | ? ?2 ++
>> ?drivers/firmware/Makefile ? ? ? ?| ? ?2 ++
>> ?drivers/firmware/google/Kconfig ?| ? 14 ++++++++++++++
>> ?drivers/firmware/google/Makefile | ? ?1 +
>> ?4 files changed, 19 insertions(+), 0 deletions(-)
>> ?create mode 100644 drivers/firmware/google/Kconfig
>> ?create mode 100644 drivers/firmware/google/Makefile
>>
>> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
>> index e710424..d848b26 100644
>> --- a/drivers/firmware/Kconfig
>> +++ b/drivers/firmware/Kconfig
>> @@ -134,4 +134,6 @@ config ISCSI_IBFT
>> ? ? ? ? detect iSCSI boot parameters dynamically during system boot, say Y.
>> ? ? ? ? Otherwise, say N.
>>
>> +source "drivers/firmware/google/Kconfig"
>> +
>> ?endmenu
>> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
>> index 1c3c173..6f68007 100644
>> --- a/drivers/firmware/Makefile
>> +++ b/drivers/firmware/Makefile
>> @@ -11,3 +11,5 @@ obj-$(CONFIG_DMIID) ? ? ? ? += dmi-id.o
>> ?obj-$(CONFIG_ISCSI_IBFT_FIND) ? ? ? ?+= iscsi_ibft_find.o
>> ?obj-$(CONFIG_ISCSI_IBFT) ? ? += iscsi_ibft.o
>> ?obj-$(CONFIG_FIRMWARE_MEMMAP) ? ? ? ?+= memmap.o
>> +
>> +obj-$(CONFIG_GOOGLE_FIRMWARE) ? ? ? ?+= google/
>> diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
>> new file mode 100644
>> index 0000000..7834729
>> --- /dev/null
>> +++ b/drivers/firmware/google/Kconfig
>> @@ -0,0 +1,14 @@
>> +config GOOGLE_FIRMWARE
>> + ? ? bool "Google Firmware Drivers"
>> + ? ? depends on X86
>> + ? ? default n
>> + ? ? help
>> + ? ? ? These firmware drivers are used by Google's servers. ?They are
>> + ? ? ? only useful if you are working directly on one of their
>> + ? ? ? proprietary servers or implementing similar firmware
>> + ? ? ? interfaces. ?If in doubt, say "N".
>
> How about dropping the "or implementing..." part of this sentance, as
> that leaves a lot open for some people to justify enabling this option
> for their machines, when it really isn't true at all at this point in
> time.
>

OK.

2011-03-14 20:02:11

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] Introduce CONFIG_GOOGLE_FIRMWARE

On Mon, Mar 14, 2011 at 12:49:17PM -0700, Mike Waychison wrote:
> On Mon, Mar 14, 2011 at 8:45 AM, Greg KH <[email protected]> wrote:
> > On Fri, Mar 11, 2011 at 05:43:47PM -0800, Mike Waychison wrote:
> >> In order to keep Google's firmware drivers organized amongst themselves,
> >> create a new directory for them to live in. ?As well, all Google
> >> firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
> >> to 'n' in the kernel build.
> >
> > Everything defaults to 'n' in the kernel build system, so this really
> > isn't a big deal :)
>
> Well, I was hoping that we could have options gating on
> CONFIG_GOOGLE_FIRMWARE, but default to 'y'. This makes our lives a
> bit easier as it means we have a single switch we need to enable on
> our side and all the options we care about (presumably all
> google-specific drivers) get enabled.
>
> If you have strong objections to this, I can flip the default values
> of the specific config options to 'n' as well though...

As decreed by Linus and others, all new config options should default to
'n', unless your box is going to blow up into tiny pieces if you don't
select it.

thanks,

greg k-h

2011-03-14 20:02:17

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] driver: Google EFI SMI

On Mon, Mar 14, 2011 at 8:47 AM, Greg KH <[email protected]> wrote:
> On Fri, Mar 11, 2011 at 05:43:53PM -0800, Mike Waychison wrote:
>> The "gsmi" driver bridges userland with firmware specific routines for
>> accessing hardware.
>
> As with the other driver in this series, what keeps this driver from
> being loaded on hardware that does not support this functionality? ?If
> it is loaded, will it cause bad things to happen?

gsmi itself is better guarded than the memconsole driver that the x86
maintainers objected to.

It relies on keying off of a couple different strings, looking for
"GOOGLE" as either the OEM ID in the FADT table, or "Google, Inc." as
the board vendor. We further discriminate whether the driver should
load (it doesn't apply to all of our boards) with a couple other
checks. See gsmi_system_valid(). I've added a comment to the patch
description indicating this for the upcoming v3 send-out.

>
> Also, what causes it to be loaded on hardware that needs it? ?There
> should be some MODULE_DEVICE_TABLE somewhere in this thing to cause it
> to be autoloaded?

I don't know if there is a good way to have this guy autoloaded, but
that's probably fine. We will likely compile it in as a built-in or
adjust our userland to have the module loaded. We use it on all
machines we've been building for the last 4-5 years, and a table of
device IDs would just contain a list of a bunch of parts that aren't
really google-specific.

2011-03-14 20:03:55

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

On Sun, Mar 13, 2011 at 9:54 PM, H. Peter Anvin <[email protected]> wrote:
> On 03/11/2011 05:43 PM, Mike Waychison wrote:
>> This patch introduces the 'memconsole' driver.
>>
>> Our firmware gives us access to an in-memory log of the firmware's
>> output. ? This gives us visibility in a data-center of headless machines
>> as to what the firmware is doing.
>>
>> The memory console is found by the driver by finding a header block in
>> the EBDA. ?The buffer is then copied out, and is exported to userland in
>> the file /sys/firmware/log.
>>
>
> OK, I really don't like this.
>
> All it has is a 32-bit nonrandom signature, no DMI keying or anything
> else to protect it. ?This really isn't sufficient.

Good point. I'll add some DMI strings checks. It *should* be
sufficient to just ensure that "Google, Inc." is the board vendor
string (though I'll need to run some tests to verify that this covers
the boards we care about).

2011-03-14 20:07:06

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 10/12] Introduce CONFIG_GOOGLE_FIRMWARE

On Mon, Mar 14, 2011 at 12:59 PM, Greg KH <[email protected]> wrote:
> On Mon, Mar 14, 2011 at 12:49:17PM -0700, Mike Waychison wrote:
>> On Mon, Mar 14, 2011 at 8:45 AM, Greg KH <[email protected]> wrote:
>> > On Fri, Mar 11, 2011 at 05:43:47PM -0800, Mike Waychison wrote:
>> >> In order to keep Google's firmware drivers organized amongst themselves,
>> >> create a new directory for them to live in. ?As well, all Google
>> >> firmware drivers are gated on CONFIG_GOOGLE_FIRMWARE=y, which defaults
>> >> to 'n' in the kernel build.
>> >
>> > Everything defaults to 'n' in the kernel build system, so this really
>> > isn't a big deal :)
>>
>> Well, I was hoping that we could have options gating on
>> CONFIG_GOOGLE_FIRMWARE, but default to 'y'. ?This makes our lives a
>> bit easier as it means we have a single switch we need to enable on
>> our side and all the options we care about (presumably all
>> google-specific drivers) get enabled.
>>
>> If you have strong objections to this, I can flip the default values
>> of the specific config options to 'n' as well though...
>
> As decreed by Linus and others, all new config options should default to
> 'n', unless your box is going to blow up into tiny pieces if you don't
> select it.

Fair enough.

>
> thanks,
>
> greg k-h
>

2011-03-14 20:54:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] driver: Google EFI SMI

On Mon, Mar 14, 2011 at 01:01:50PM -0700, Mike Waychison wrote:
> On Mon, Mar 14, 2011 at 8:47 AM, Greg KH <[email protected]> wrote:
> > On Fri, Mar 11, 2011 at 05:43:53PM -0800, Mike Waychison wrote:
> >> The "gsmi" driver bridges userland with firmware specific routines for
> >> accessing hardware.
> >
> > As with the other driver in this series, what keeps this driver from
> > being loaded on hardware that does not support this functionality? ?If
> > it is loaded, will it cause bad things to happen?
>
> gsmi itself is better guarded than the memconsole driver that the x86
> maintainers objected to.
>
> It relies on keying off of a couple different strings, looking for
> "GOOGLE" as either the OEM ID in the FADT table, or "Google, Inc." as
> the board vendor. We further discriminate whether the driver should
> load (it doesn't apply to all of our boards) with a couple other
> checks. See gsmi_system_valid(). I've added a comment to the patch
> description indicating this for the upcoming v3 send-out.

Ok, fair enough. I'm guessing that you have tried testing the module by
loading it in a machine where this doesn't work?

> > Also, what causes it to be loaded on hardware that needs it? ?There
> > should be some MODULE_DEVICE_TABLE somewhere in this thing to cause it
> > to be autoloaded?
>
> I don't know if there is a good way to have this guy autoloaded, but
> that's probably fine. We will likely compile it in as a built-in or
> adjust our userland to have the module loaded. We use it on all
> machines we've been building for the last 4-5 years, and a table of
> device IDs would just contain a list of a bunch of parts that aren't
> really google-specific.

What's wrong with using DMI strings? Are there going to be that many
different ones of them here?

thanks,

greg k-h

2011-03-14 21:09:59

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] driver: Google EFI SMI

On Mon, Mar 14, 2011 at 1:13 PM, Greg KH <[email protected]> wrote:
> On Mon, Mar 14, 2011 at 01:01:50PM -0700, Mike Waychison wrote:
>> On Mon, Mar 14, 2011 at 8:47 AM, Greg KH <[email protected]> wrote:
>> > On Fri, Mar 11, 2011 at 05:43:53PM -0800, Mike Waychison wrote:
>> >> The "gsmi" driver bridges userland with firmware specific routines for
>> >> accessing hardware.
>> >
>> > As with the other driver in this series, what keeps this driver from
>> > being loaded on hardware that does not support this functionality? ?If
>> > it is loaded, will it cause bad things to happen?
>>
>> gsmi itself is better guarded than the memconsole driver that the x86
>> maintainers objected to.
>>
>> It relies on keying off of a couple different strings, looking for
>> "GOOGLE" as either the OEM ID in the ?FADT table, or "Google, Inc." as
>> the board vendor. ?We further discriminate whether the driver should
>> load (it doesn't apply to all of our boards) with a couple other
>> checks. ?See gsmi_system_valid(). ?I've added a comment to the patch
>> description indicating this for the upcoming v3 send-out.
>
> Ok, fair enough. ?I'm guessing that you have tried testing the module by
> loading it in a machine where this doesn't work?
>
>> > Also, what causes it to be loaded on hardware that needs it? ?There
>> > should be some MODULE_DEVICE_TABLE somewhere in this thing to cause it
>> > to be autoloaded?
>>
>> I don't know if there is a good way to have this guy autoloaded, but
>> that's probably fine. We will likely compile it in as a built-in or
>> adjust our userland to have the module loaded. ?We use it on all
>> machines we've been building for the last 4-5 years, and a table of
>> device IDs would just contain a list of a bunch of parts that aren't
>> really google-specific.
>
> What's wrong with using DMI strings? ?Are there going to be that many
> different ones of them here?

DMI strings would probably work. I totally missed the fact that I
could use them for MODULE_DEVICE_TABLE. Let me re-work patch 11 and
12 to use this matching stuff.

2011-03-14 22:48:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v2 12/12] driver: Google Memory Console

On 03/14/2011 01:03 PM, Mike Waychison wrote:
> On Sun, Mar 13, 2011 at 9:54 PM, H. Peter Anvin <[email protected]> wrote:
>> On 03/11/2011 05:43 PM, Mike Waychison wrote:
>>> This patch introduces the 'memconsole' driver.
>>>
>>> Our firmware gives us access to an in-memory log of the firmware's
>>> output. This gives us visibility in a data-center of headless machines
>>> as to what the firmware is doing.
>>>
>>> The memory console is found by the driver by finding a header block in
>>> the EBDA. The buffer is then copied out, and is exported to userland in
>>> the file /sys/firmware/log.
>>>
>>
>> OK, I really don't like this.
>>
>> All it has is a 32-bit nonrandom signature, no DMI keying or anything
>> else to protect it. This really isn't sufficient.
>
> Good point. I'll add some DMI strings checks. It *should* be
> sufficient to just ensure that "Google, Inc." is the board vendor
> string (though I'll need to run some tests to verify that this covers
> the boards we care about).
>

Sounds good.

-hpa

2011-03-14 23:08:29

by Alan

[permalink] [raw]
Subject: Re: [PATCH v2 11/12] driver: Google EFI SMI


> > I don't know if there is a good way to have this guy autoloaded, but
> > that's probably fine. We will likely compile it in as a built-in or
> > adjust our userland to have the module loaded. We use it on all
> > machines we've been building for the last 4-5 years, and a table of
> > device IDs would just contain a list of a bunch of parts that aren't
> > really google-specific.
>
> What's wrong with using DMI strings? Are there going to be that many
> different ones of them here?

If you've got them in the ACPI tables that's as good as DMI if not better
IMHO