diff options
| author | Seiji Aguchi <seiji.aguchi@hds.com> | 2013-02-12 15:59:07 -0500 |
|---|---|---|
| committer | Tony Luck <tony.luck@intel.com> | 2013-02-12 15:59:07 -0500 |
| commit | 81fa4e581d9283f7992a0d8c534bb141eb840a14 (patch) | |
| tree | 2bc7677534c1cef8dfb5388059777bfb1164e66c | |
| parent | e59310adf5eebce108f78b6c47bb330aae2e1666 (diff) | |
efivars: Disable external interrupt while holding efivars->lock
[Problem]
There is a scenario which efi_pstore fails to log messages in a panic case.
- CPUA holds an efi_var->lock in either efivarfs parts
or efi_pstore with interrupt enabled.
- CPUB panics and sends IPI to CPUA in smp_send_stop().
- CPUA stops with holding the lock.
- CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC)
but it returns without logging messages.
[Patch Description]
This patch disables an external interruption while holding efivars->lock
as follows.
In efi_pstore_write() and get_var_data(), spin_lock/spin_unlock is
replaced by spin_lock_irqsave/spin_unlock_irqrestore because they may
be called in an interrupt context.
In other functions, they are replaced by spin_lock_irq/spin_unlock_irq.
because they are all called from a process context.
By applying this patch, we can avoid the problem above with
a following senario.
- CPUA holds an efi_var->lock with interrupt disabled.
- CPUB panics and sends IPI to CPUA in smp_send_stop().
- CPUA receives the IPI after releasing the lock because it is
disabling interrupt while holding the lock.
- CPUB waits for one sec until CPUA releases the lock.
- CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC)
And it can hold the lock successfully.
Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
Acked-by: Mike Waychison <mikew@google.com>
Acked-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
| -rw-r--r-- | drivers/firmware/efivars.c | 86 |
1 files changed, 44 insertions, 42 deletions
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index ef5070d86f88..a64fb7bda365 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c | |||
| @@ -405,10 +405,11 @@ static efi_status_t | |||
| 405 | get_var_data(struct efivars *efivars, struct efi_variable *var) | 405 | get_var_data(struct efivars *efivars, struct efi_variable *var) |
| 406 | { | 406 | { |
| 407 | efi_status_t status; | 407 | efi_status_t status; |
| 408 | unsigned long flags; | ||
| 408 | 409 | ||
| 409 | spin_lock(&efivars->lock); | 410 | spin_lock_irqsave(&efivars->lock, flags); |
| 410 | status = get_var_data_locked(efivars, var); | 411 | status = get_var_data_locked(efivars, var); |
| 411 | spin_unlock(&efivars->lock); | 412 | spin_unlock_irqrestore(&efivars->lock, flags); |
| 412 | 413 | ||
| 413 | if (status != EFI_SUCCESS) { | 414 | if (status != EFI_SUCCESS) { |
| 414 | printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n", | 415 | printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n", |
| @@ -537,14 +538,14 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count) | |||
| 537 | return -EINVAL; | 538 | return -EINVAL; |
| 538 | } | 539 | } |
| 539 | 540 | ||
| 540 | spin_lock(&efivars->lock); | 541 | spin_lock_irq(&efivars->lock); |
| 541 | status = efivars->ops->set_variable(new_var->VariableName, | 542 | status = efivars->ops->set_variable(new_var->VariableName, |
| 542 | &new_var->VendorGuid, | 543 | &new_var->VendorGuid, |
| 543 | new_var->Attributes, | 544 | new_var->Attributes, |
| 544 | new_var->DataSize, | 545 | new_var->DataSize, |
| 545 | new_var->Data); | 546 | new_var->Data); |
| 546 | 547 | ||
| 547 | spin_unlock(&efivars->lock); | 548 | spin_unlock_irq(&efivars->lock); |
| 548 | 549 | ||
| 549 | if (status != EFI_SUCCESS) { | 550 | if (status != EFI_SUCCESS) { |
| 550 | printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", | 551 | printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", |
| @@ -713,7 +714,7 @@ static ssize_t efivarfs_file_write(struct file *file, | |||
| 713 | * amounts of memory. Pick a default size of 64K if | 714 | * amounts of memory. Pick a default size of 64K if |
| 714 | * QueryVariableInfo() isn't supported by the firmware. | 715 | * QueryVariableInfo() isn't supported by the firmware. |
| 715 | */ | 716 | */ |
| 716 | spin_lock(&efivars->lock); | 717 | spin_lock_irq(&efivars->lock); |
| 717 | 718 | ||
| 718 | if (!efivars->ops->query_variable_info) | 719 | if (!efivars->ops->query_variable_info) |
| 719 | status = EFI_UNSUPPORTED; | 720 | status = EFI_UNSUPPORTED; |
| @@ -723,7 +724,7 @@ static ssize_t efivarfs_file_write(struct file *file, | |||
| 723 | &remaining_size, &max_size); | 724 | &remaining_size, &max_size); |
| 724 | } | 725 | } |
| 725 | 726 | ||
| 726 | spin_unlock(&efivars->lock); | 727 | spin_unlock_irq(&efivars->lock); |
| 727 | 728 | ||
| 728 | if (status != EFI_SUCCESS) { | 729 | if (status != EFI_SUCCESS) { |
| 729 | if (status != EFI_UNSUPPORTED) | 730 | if (status != EFI_UNSUPPORTED) |
| @@ -754,7 +755,7 @@ static ssize_t efivarfs_file_write(struct file *file, | |||
| 754 | * set_variable call, and removal of the variable from the efivars | 755 | * set_variable call, and removal of the variable from the efivars |
| 755 | * list (in the case of an authenticated delete). | 756 | * list (in the case of an authenticated delete). |
| 756 | */ | 757 | */ |
| 757 | spin_lock(&efivars->lock); | 758 | spin_lock_irq(&efivars->lock); |
| 758 | 759 | ||
| 759 | status = efivars->ops->set_variable(var->var.VariableName, | 760 | status = efivars->ops->set_variable(var->var.VariableName, |
| 760 | &var->var.VendorGuid, | 761 | &var->var.VendorGuid, |
| @@ -762,7 +763,7 @@ static ssize_t efivarfs_file_write(struct file *file, | |||
| 762 | data); | 763 | data); |
| 763 | 764 | ||
| 764 | if (status != EFI_SUCCESS) { | 765 | if (status != EFI_SUCCESS) { |
| 765 | spin_unlock(&efivars->lock); | 766 | spin_unlock_irq(&efivars->lock); |
| 766 | kfree(data); | 767 | kfree(data); |
| 767 | 768 | ||
| 768 | return efi_status_to_err(status); | 769 | return efi_status_to_err(status); |
| @@ -783,20 +784,20 @@ static ssize_t efivarfs_file_write(struct file *file, | |||
| 783 | NULL); | 784 | NULL); |
| 784 | 785 | ||
| 785 | if (status == EFI_BUFFER_TOO_SMALL) { | 786 | if (status == EFI_BUFFER_TOO_SMALL) { |
| 786 | spin_unlock(&efivars->lock); | 787 | spin_unlock_irq(&efivars->lock); |
| 787 | mutex_lock(&inode->i_mutex); | 788 | mutex_lock(&inode->i_mutex); |
| 788 | i_size_write(inode, newdatasize + sizeof(attributes)); | 789 | i_size_write(inode, newdatasize + sizeof(attributes)); |
| 789 | mutex_unlock(&inode->i_mutex); | 790 | mutex_unlock(&inode->i_mutex); |
| 790 | 791 | ||
| 791 | } else if (status == EFI_NOT_FOUND) { | 792 | } else if (status == EFI_NOT_FOUND) { |
| 792 | list_del(&var->list); | 793 | list_del(&var->list); |
| 793 | spin_unlock(&efivars->lock); | 794 | spin_unlock_irq(&efivars->lock); |
| 794 | efivar_unregister(var); | 795 | efivar_unregister(var); |
| 795 | drop_nlink(inode); | 796 | drop_nlink(inode); |
| 796 | dput(file->f_dentry); | 797 | dput(file->f_dentry); |
| 797 | 798 | ||
| 798 | } else { | 799 | } else { |
| 799 | spin_unlock(&efivars->lock); | 800 | spin_unlock_irq(&efivars->lock); |
| 800 | pr_warn("efivarfs: inconsistent EFI variable implementation? " | 801 | pr_warn("efivarfs: inconsistent EFI variable implementation? " |
| 801 | "status = %lx\n", status); | 802 | "status = %lx\n", status); |
| 802 | } | 803 | } |
| @@ -818,11 +819,11 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, | |||
| 818 | void *data; | 819 | void *data; |
| 819 | ssize_t size = 0; | 820 | ssize_t size = 0; |
| 820 | 821 | ||
| 821 | spin_lock(&efivars->lock); | 822 | spin_lock_irq(&efivars->lock); |
| 822 | status = efivars->ops->get_variable(var->var.VariableName, | 823 | status = efivars->ops->get_variable(var->var.VariableName, |
| 823 | &var->var.VendorGuid, | 824 | &var->var.VendorGuid, |
| 824 | &attributes, &datasize, NULL); | 825 | &attributes, &datasize, NULL); |
| 825 | spin_unlock(&efivars->lock); | 826 | spin_unlock_irq(&efivars->lock); |
| 826 | 827 | ||
| 827 | if (status != EFI_BUFFER_TOO_SMALL) | 828 | if (status != EFI_BUFFER_TOO_SMALL) |
| 828 | return efi_status_to_err(status); | 829 | return efi_status_to_err(status); |
| @@ -832,12 +833,12 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, | |||
| 832 | if (!data) | 833 | if (!data) |
| 833 | return -ENOMEM; | 834 | return -ENOMEM; |
| 834 | 835 | ||
| 835 | spin_lock(&efivars->lock); | 836 | spin_lock_irq(&efivars->lock); |
| 836 | status = efivars->ops->get_variable(var->var.VariableName, | 837 | status = efivars->ops->get_variable(var->var.VariableName, |
| 837 | &var->var.VendorGuid, | 838 | &var->var.VendorGuid, |
| 838 | &attributes, &datasize, | 839 | &attributes, &datasize, |
| 839 | (data + sizeof(attributes))); | 840 | (data + sizeof(attributes))); |
| 840 | spin_unlock(&efivars->lock); | 841 | spin_unlock_irq(&efivars->lock); |
| 841 | 842 | ||
| 842 | if (status != EFI_SUCCESS) { | 843 | if (status != EFI_SUCCESS) { |
| 843 | size = efi_status_to_err(status); | 844 | size = efi_status_to_err(status); |
| @@ -965,9 +966,9 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, | |||
| 965 | goto out; | 966 | goto out; |
| 966 | 967 | ||
| 967 | kobject_uevent(&var->kobj, KOBJ_ADD); | 968 | kobject_uevent(&var->kobj, KOBJ_ADD); |
| 968 | spin_lock(&efivars->lock); | 969 | spin_lock_irq(&efivars->lock); |
| 969 | list_add(&var->list, &efivars->list); | 970 | list_add(&var->list, &efivars->list); |
| 970 | spin_unlock(&efivars->lock); | 971 | spin_unlock_irq(&efivars->lock); |
| 971 | d_instantiate(dentry, inode); | 972 | d_instantiate(dentry, inode); |
| 972 | dget(dentry); | 973 | dget(dentry); |
| 973 | out: | 974 | out: |
| @@ -984,7 +985,7 @@ static int efivarfs_unlink(struct inode *dir, struct dentry *dentry) | |||
| 984 | struct efivars *efivars = var->efivars; | 985 | struct efivars *efivars = var->efivars; |
| 985 | efi_status_t status; | 986 | efi_status_t status; |
| 986 | 987 | ||
| 987 | spin_lock(&efivars->lock); | 988 | spin_lock_irq(&efivars->lock); |
| 988 | 989 | ||
| 989 | status = efivars->ops->set_variable(var->var.VariableName, | 990 | status = efivars->ops->set_variable(var->var.VariableName, |
| 990 | &var->var.VendorGuid, | 991 | &var->var.VendorGuid, |
| @@ -992,14 +993,14 @@ static int efivarfs_unlink(struct inode *dir, struct dentry *dentry) | |||
| 992 | 993 | ||
| 993 | if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) { | 994 | if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) { |
