From patchwork Wed Nov 8 16:06:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Morse X-Patchwork-Id: 10048739 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E1B10603FA for ; Wed, 8 Nov 2017 16:08:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D2A972A66B for ; Wed, 8 Nov 2017 16:08:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C729B2A6BA; Wed, 8 Nov 2017 16:08:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 461DD2A5B8 for ; Wed, 8 Nov 2017 16:08:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=esSJcBhaX2/gOThu5sHk86u1vo/UYCJ/rlWBPkDqsMU=; b=jrc/wgpF/eFoS6T4HktuFJrfks sXBHT6VlbbilkcbULIrpFQq4LR9DxDY9/yzw7gZTNt9zvuWUxFL3l7nPOW6qn5rwJNiPzeOdLIC91 EJbd1zWmiZJYxk9rdFztpZGmuVOAZsV8x+UvjxJMA46fdTB8xAIFpPMTmNCbzuJ9XpyDfgVHw1mXa uSE12ipIePsBWBRV0F0teKg2nsDqkjVtTBBqPH9zMERcIFYG7mQaFRFSDctFPnTQevQWDfMryuUTj 2V+RyYSkJxYUMeYCX0ARw45g8lj6nNmfpRdoUbD6ej+a9Ef+DIGsxOblBlzHS5TB6okFB6Nh7lZf1 mU7gVHIg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eCSuB-0006Yl-Hz; Wed, 08 Nov 2017 16:08:43 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eCSu6-0006Uj-Sp for linux-arm-kernel@lists.infradead.org; Wed, 08 Nov 2017 16:08:41 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 625EF1529; Wed, 8 Nov 2017 08:08:18 -0800 (PST) Received: from melchizedek.cambridge.arm.com (melchizedek.cambridge.arm.com [10.1.207.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 72A243F40C; Wed, 8 Nov 2017 08:08:16 -0800 (PST) From: James Morse To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v4 15/13] firmware: arm_sdei: be more robust against cpu-hotplug Date: Wed, 8 Nov 2017 16:06:24 +0000 Message-Id: <20171108160624.10355-1-james.morse@arm.com> X-Mailer: git-send-email 2.15.0 In-Reply-To: <5A031E9E.2090809@arm.com> References: <5A031E9E.2090809@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171108_080839_004104_2A3152C1 X-CRM114-Status: GOOD ( 17.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Lorenzo Pieralisi , Marc Zyngier , Catalin Marinas , Will Deacon , Rob Herring , Loc Ho , kvmarm@lists.cs.columbia.edu, Christoffer Dall MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP dpm_suspend() calls the freeze/thaw callbacks for hibernate before disable_non_bootcpus() takes down secondaries. This leads to a fun race where the freeze/thaw callbacks reset the SDEI interface (as we may be restoring a kernel with a different layout due to KASLR), then the cpu-hotplug callbacks come in to save the current state, which has already been reset. I tried to solve this with a 'frozen' flag that stops the hotplug callback from overwriting the saved values. Instead this just moves the race around and makes it even harder to think about. Instead, make it look like the secondaries have gone offline. Call cpuhp_remove_state() in the freeze callback, this will call the teardown hook on all online CPUs, then remove the state. This saves all private events and makes future CPU up/down events invisible. Change sdei_event_unregister_all()/sdei_reregister_events() to only save/restore shared events, which are all that is left. With this we can remove the frozen flag. We can remove the device suspend/resume calls too as cpuhotplug's teardown call has masked the CPUs. All that is left is the reboot notifier, (which was abusing the frozen flag). Call cpuhp_remove_state() to make it look like secondary CPUs have gone offline. Suggested-by: Will Deacon Signed-off-by: James Morse --- drivers/firmware/arm_sdei.c | 60 +++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c index 65a8f122f545..d50634a25954 100644 --- a/drivers/firmware/arm_sdei.c +++ b/drivers/firmware/arm_sdei.c @@ -86,9 +86,6 @@ struct sdei_event { static LIST_HEAD(sdei_events); static DEFINE_SPINLOCK(sdei_events_lock); -/* When frozen, cpu-hotplug notifiers shouldn't unregister/re-register events */ -static bool frozen; - static DEFINE_PER_CPU(u64, sdei_running_event) = -1; /* Private events are registered/enabled via IPI passing one of these */ @@ -625,15 +622,18 @@ EXPORT_SYMBOL(sdei_event_unregister); /* * unregister events, but don't destroy them as they are re-registered by - * sdei_reregister_events(). + * sdei_reregister_shared(). */ -static int sdei_event_unregister_all(void) +static int sdei_unregister_shared(void) { int err = 0; struct sdei_event *event; spin_lock(&sdei_events_lock); list_for_each_entry(event, &sdei_events, list) { + if (event->type != SDEI_EVENT_TYPE_SHARED) + continue; + err = _sdei_event_unregister(event); if (err) break; @@ -841,13 +841,16 @@ static int sdei_reregister_event(struct sdei_event *event) return err; } -static int sdei_reregister_events(void) +static int sdei_reregister_shared(void) { int err = 0; struct sdei_event *event; spin_lock(&sdei_events_lock); list_for_each_entry(event, &sdei_events, list) { + if (event->type != SDEI_EVENT_TYPE_SHARED) + continue; + err = sdei_reregister_event(event); if (err) break; @@ -862,11 +865,6 @@ static int sdei_cpuhp_down(unsigned int cpu) struct sdei_event *event; struct sdei_crosscall_args arg; - if (frozen) { - /* All events unregistered */ - return sdei_mask_local_cpu(); - } - /* un-register private events */ spin_lock(&sdei_events_lock); list_for_each_entry(event, &sdei_events, list) { @@ -890,11 +888,6 @@ static int sdei_cpuhp_up(unsigned int cpu) struct sdei_event *event; struct sdei_crosscall_args arg; - if (frozen) { - /* Events will be re-registered when we thaw. */ - return sdei_unmask_local_cpu(); - } - /* re-register/enable private events */ spin_lock(&sdei_events_lock); list_for_each_entry(event, &sdei_events, list) { @@ -1004,22 +997,33 @@ static int sdei_device_freeze(struct device *dev) { int err; - frozen = true; - err = sdei_event_unregister_all(); + /* save and unregister private events */ + cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING); + + err = sdei_unregister_shared(); if (err) return err; - return sdei_device_suspend(dev); + return 0; } static int sdei_device_thaw(struct device *dev) { int err; - sdei_device_resume(dev); + /* re-register shared events */ + err = sdei_reregister_shared(); + if (err) { + pr_warn("Failed to re-register shared events...\n"); + sdei_mark_interface_broken(); + return err; + } + + err = cpuhp_setup_state(CPUHP_AP_ARM_SDEI_STARTING, "SDEI", + &sdei_cpuhp_up, &sdei_cpuhp_down); + if (err) + pr_warn("Failed to re-register CPU hotplug notifier...\n"); - err = sdei_reregister_events(); - frozen = false; return err; } @@ -1048,15 +1052,13 @@ static const struct dev_pm_ops sdei_pm_ops = { static int sdei_reboot_notifier(struct notifier_block *nb, unsigned long action, void *data) { - on_each_cpu(&_ipi_mask_cpu, NULL, true); - - sdei_platform_reset(); - /* - * There is now no point trying to unregister private events if we go on - * to take CPUs offline. + * We are going to reset the interface, after this there is no point + * doing work when we take CPUs offline. */ - frozen = true; + cpuhp_remove_state(CPUHP_AP_ARM_SDEI_STARTING); + + sdei_platform_reset(); return NOTIFY_OK; }