| From 66c64a9ae041089f8f6e86868e9551858a82db4b Mon Sep 17 00:00:00 2001 |
| From: James Morse <james.morse@arm.com> |
| Date: Fri, 21 Feb 2020 16:35:06 +0000 |
| Subject: [PATCH] firmware: arm_sdei: fix double-lock on hibernate with shared |
| events |
| |
| commit 6ded0b61cf638bf9f8efe60ab8ba23db60ea9763 upstream. |
| |
| SDEI has private events that must be registered on each CPU. When |
| CPUs come and go they must re-register and re-enable their private |
| events. Each event has flags to indicate whether this should happen |
| to protect against an event being registered on a CPU coming online, |
| while all the others are unregistering the event. |
| |
| These flags are protected by the sdei_list_lock spinlock, because |
| the cpuhp callbacks can't take the mutex. |
| |
| Hibernate needs to unregister all events, but keep the in-memory |
| re-register and re-enable as they are. sdei_unregister_shared() |
| takes the spinlock to walk the list, then calls _sdei_event_unregister() |
| on each shared event. _sdei_event_unregister() tries to take the |
| same spinlock to update re-register and re-enable. This doesn't go |
| so well. |
| |
| Push the re-register and re-enable updates out to their callers. |
| sdei_unregister_shared() doesn't want these values updated, so |
| doesn't need to do anything. |
| |
| This also fixes shared events getting lost over hibernate as this |
| path made them look unregistered. |
| |
| Fixes: da351827240e ("firmware: arm_sdei: Add support for CPU and system power states") |
| Reported-by: Liguang Zhang <zhangliguang@linux.alibaba.com> |
| Signed-off-by: James Morse <james.morse@arm.com> |
| Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c |
| index 9cd70d1a5622..eb2df89d4924 100644 |
| --- a/drivers/firmware/arm_sdei.c |
| +++ b/drivers/firmware/arm_sdei.c |
| @@ -491,11 +491,6 @@ static int _sdei_event_unregister(struct sdei_event *event) |
| { |
| lockdep_assert_held(&sdei_events_lock); |
| |
| - spin_lock(&sdei_list_lock); |
| - event->reregister = false; |
| - event->reenable = false; |
| - spin_unlock(&sdei_list_lock); |
| - |
| if (event->type == SDEI_EVENT_TYPE_SHARED) |
| return sdei_api_event_unregister(event->event_num); |
| |
| @@ -518,6 +513,11 @@ int sdei_event_unregister(u32 event_num) |
| break; |
| } |
| |
| + spin_lock(&sdei_list_lock); |
| + event->reregister = false; |
| + event->reenable = false; |
| + spin_unlock(&sdei_list_lock); |
| + |
| err = _sdei_event_unregister(event); |
| if (err) |
| break; |
| @@ -585,26 +585,15 @@ static int _sdei_event_register(struct sdei_event *event) |
| |
| lockdep_assert_held(&sdei_events_lock); |
| |
| - spin_lock(&sdei_list_lock); |
| - event->reregister = true; |
| - spin_unlock(&sdei_list_lock); |
| - |
| if (event->type == SDEI_EVENT_TYPE_SHARED) |
| return sdei_api_event_register(event->event_num, |
| sdei_entry_point, |
| event->registered, |
| SDEI_EVENT_REGISTER_RM_ANY, 0); |
| |
| - |
| err = sdei_do_cross_call(_local_event_register, event); |
| - if (err) { |
| - spin_lock(&sdei_list_lock); |
| - event->reregister = false; |
| - event->reenable = false; |
| - spin_unlock(&sdei_list_lock); |
| - |
| + if (err) |
| sdei_do_cross_call(_local_event_unregister, event); |
| - } |
| |
| return err; |
| } |
| @@ -632,8 +621,17 @@ int sdei_event_register(u32 event_num, sdei_event_callback *cb, void *arg) |
| break; |
| } |
| |
| + spin_lock(&sdei_list_lock); |
| + event->reregister = true; |
| + spin_unlock(&sdei_list_lock); |
| + |
| err = _sdei_event_register(event); |
| if (err) { |
| + spin_lock(&sdei_list_lock); |
| + event->reregister = false; |
| + event->reenable = false; |
| + spin_unlock(&sdei_list_lock); |
| + |
| sdei_event_destroy(event); |
| pr_warn("Failed to register event %u: %d\n", event_num, |
| err); |
| -- |
| 2.7.4 |
| |