Message ID | 20221024143417.11463-1-johan+linaro@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp526911wru; Mon, 24 Oct 2022 09:00:02 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7ctoqtcd78ECT5aDMLt4AdqUDRfvzz5Rr80Ic7MXYoTK1CHiSbVYmev5sBXY6i3jimoSYE X-Received: by 2002:a17:907:2e01:b0:78d:e768:e845 with SMTP id ig1-20020a1709072e0100b0078de768e845mr28219220ejc.484.1666627202774; Mon, 24 Oct 2022 09:00:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666627202; cv=none; d=google.com; s=arc-20160816; b=sz+/Nrfc6hnGXgt5a/JIc9mCaXKh1UFcsCcy02Af1ot+4fZAqiN+Kcd2VNCbJM3ihX EHL6wmU3hVh1mUcLleDlzneEIROg4X6KzdFitKT4EeepVNFhgWRQhOad28UIUWcMP6+4 QD8em5UpZQ4XlZyoRJ+tRa0iSm4GT0YY/TmHxavyT+OtwHG4ht5yhCpGmHY0F2BYnzB0 V7WSW6u+wbB3HOwSTe3HYsaDjIz5z7c+O345bYlQJh+/GM6EiRtb/o18dUDh4g7tL/qE dazBW3U7t/SrmSJi5xnjOZkFvrS9nihjFur0R+bWaDLiMJFiywkQxXPBbaN1fiNEaRRT E6eQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=3t5x8whF290JU5HcZdu5kuZ+mvCBejatMNVm/rMoGu8=; b=HmVIIkAcBqYTPgtguVO4GN42NL5th7+fDIXPIY93iKpOvFU4AFlPmSRQh27XJwau+3 YDnHJ1zok0Vlq+LWtna7+1iUZH6zOfvU941wly0vle/OE2GE67g4OegXYG76oG3EjCJR Dx/WLyjD+ANZtWLzHYyYHRIXKyQs3LNhj8mKXy+QhuQP4IPO1Gro85t0SdaBU1EaZ6Rv DntKhwLAuhidr7eo/cPG2FX9S3qV+Y3TDwjQCsu1ivS8OG6SyQfFZceJ/L3MNEKt+de1 9M9l9DIsmvZDdqGuwTkg0aMMzJL29lxYCLICIVWtY8i8DE+ipKMIYj32IH63QDrZjkNv MVYQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=jlYzoYpq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y5-20020a056402440500b004587cd5a5bdsi194164eda.81.2022.10.24.08.59.39; Mon, 24 Oct 2022 09:00:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=jlYzoYpq; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230291AbiJXPuq (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Mon, 24 Oct 2022 11:50:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231817AbiJXPuV (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 24 Oct 2022 11:50:21 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 658601DF16 for <linux-kernel@vger.kernel.org>; Mon, 24 Oct 2022 07:44:30 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2DCA861411 for <linux-kernel@vger.kernel.org>; Mon, 24 Oct 2022 14:36:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8B210C433C1; Mon, 24 Oct 2022 14:36:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666622183; bh=3kVC08NNnjFfypW5XVgXQUIfx+GRlGJ5ATemF0ZLdmw=; h=From:To:Cc:Subject:Date:From; b=jlYzoYpqGGQYY02ihbgVoc81e1agDYRoPao2MzOMdUiTfSm00R8EQj1B0+NtYNZ22 3t3gE0TeKcerQKjRrMvIVjbe3+DVRkQpUKMhrXRQiNvcoS+2WBdyTou6ou4Y2rsZQx eFGFxTtN+QyzIA9ijbAW0JaBNA2Gv1VKXtlX/P9ofeyKcxfbUXF13l0gE+ehTNu4/H VTDJsT2znJ7Y35vnFo68kaubli2YNgpSYAzyzQ7dcu2oAyg8eFdA29W1P5XFioaOuq 1v83QMhC44vsh0vQJez6dgChusgYSM9ykg4XQ29Yx5+w8ee7H0qDZJc6Ze6DU1YJKl D99mm77T8l6wQ== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from <johan+linaro@kernel.org>) id 1omyYh-00030Y-7j; Mon, 24 Oct 2022 16:36:07 +0200 From: Johan Hovold <johan+linaro@kernel.org> To: Mark Rutland <mark.rutland@arm.com>, Lorenzo Pieralisi <lpieralisi@kernel.org> Cc: Ulf Hansson <ulf.hansson@linaro.org>, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Sudeep Holla <sudeep.holla@arm.com>, Daniel Lezcano <daniel.lezcano@linaro.org>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Johan Hovold <johan+linaro@kernel.org> Subject: [PATCH] firmware/psci: demote suspend-mode warning to debug level Date: Mon, 24 Oct 2022 16:34:17 +0200 Message-Id: <20221024143417.11463-1-johan+linaro@kernel.org> X-Mailer: git-send-email 2.37.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747585285996910815?= X-GMAIL-MSGID: =?utf-8?q?1747585285996910815?= |
Series |
firmware/psci: demote suspend-mode warning to debug level
|
|
Commit Message
Johan Hovold
Oct. 24, 2022, 2:34 p.m. UTC
On some Qualcomm platform, like SC8280XP, the attempt to set PC mode
during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb
("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this
is now logged at warning level:
psci: failed to set PC mode: -3
As there is nothing users can do about the firmware behaving this way,
demote the warning to debug level.
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
Note that a separate change to the cpuidle driver will start logging the
mode actually used:
https://lore.kernel.org/all/20221020115513.93809-1-ulf.hansson@linaro.org/
Johan
drivers/firmware/psci/psci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 24/10/2022 17:34, Johan Hovold wrote: > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > is now logged at warning level: > > psci: failed to set PC mode: -3 > > As there is nothing users can do about the firmware behaving this way, > demote the warning to debug level. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > > Note that a separate change to the cpuidle driver will start logging the > mode actually used: > > https://lore.kernel.org/all/20221020115513.93809-1-ulf.hansson@linaro.org/ > > Johan > > > drivers/firmware/psci/psci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c > index e7bcfca4159f..462f37fa6a86 100644 > --- a/drivers/firmware/psci/psci.c > +++ b/drivers/firmware/psci/psci.c > @@ -165,7 +165,7 @@ int psci_set_osi_mode(bool enable) > > err = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, suspend_mode, 0, 0); > if (err < 0) > - pr_warn("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); > + pr_debug("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); > return psci_to_linux_errno(err); > } >
On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote: > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > is now logged at warning level: > > psci: failed to set PC mode: -3 > > As there is nothing users can do about the firmware behaving this way, > demote the warning to debug level. > As mentioned in the other thread I prefer to keep this as error as we shouldn't mask this error and enable more/newer platforms to ignore it when they can go and fix it. So I don't agree with this. -- Regards, Sudeep
On Tue, Oct 25, 2022 at 12:53:55PM +0100, Sudeep Holla wrote: > On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote: > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > > is now logged at warning level: > > > > psci: failed to set PC mode: -3 > > > > As there is nothing users can do about the firmware behaving this way, > > demote the warning to debug level. > > > > As mentioned in the other thread I prefer to keep this as error as we > shouldn't mask this error and enable more/newer platforms to ignore it > when they can go and fix it. So I don't agree with this. But now every owner of an X13s laptop will see this not very informative error at every boot and wonder what it means. Has something gone broken? Should they be worried? Can something be done about it? Remember that this is firmware used by Windows machines so by the time we see this in Linux it's probably way too late to fix in firmware anyway. Johan
On Mon, 24 Oct 2022 at 16:36, Johan Hovold <johan+linaro@kernel.org> wrote: > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > is now logged at warning level: > > psci: failed to set PC mode: -3 > > As there is nothing users can do about the firmware behaving this way, > demote the warning to debug level. > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> FYI, I would be fine with a pr_info() too. Kind regards Uffe > --- > > Note that a separate change to the cpuidle driver will start logging the > mode actually used: > > https://lore.kernel.org/all/20221020115513.93809-1-ulf.hansson@linaro.org/ > > Johan > > > drivers/firmware/psci/psci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c > index e7bcfca4159f..462f37fa6a86 100644 > --- a/drivers/firmware/psci/psci.c > +++ b/drivers/firmware/psci/psci.c > @@ -165,7 +165,7 @@ int psci_set_osi_mode(bool enable) > > err = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, suspend_mode, 0, 0); > if (err < 0) > - pr_warn("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); > + pr_debug("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); > return psci_to_linux_errno(err); > } > > -- > 2.37.3 >
On Tue, Oct 25, 2022 at 02:56:00PM +0200, Johan Hovold wrote: > On Tue, Oct 25, 2022 at 12:53:55PM +0100, Sudeep Holla wrote: > > On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote: > > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > > > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > > > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > > > is now logged at warning level: > > > > > > psci: failed to set PC mode: -3 > > > > > > As there is nothing users can do about the firmware behaving this way, > > > demote the warning to debug level. > > > > > > > As mentioned in the other thread I prefer to keep this as error as we > > shouldn't mask this error and enable more/newer platforms to ignore it > > when they can go and fix it. So I don't agree with this. > > But now every owner of an X13s laptop will see this not very informative > error at every boot and wonder what it means. Has something gone broken? > Should they be worried? Can something be done about it? > I understand that but I have expressed why I am concerned on generalising it. As long as we inform the concerned owners running Linux(which is quite small at the moment), keeping it will help to get these fixed on platforms that are running Linux today for validation and get it fixed if their platform firmware suffers from the same. > Remember that this is firmware used by Windows machines so by the time > we see this in Linux it's probably way too late to fix in firmware > anyway. > I am well aware of that fact, but I am targeting platforms that are using Linux for validation today. Honestly, I am not sure if we need to target for zero errors or warnings on the platforms instead of repeatedly annoy them with warnings until it is fixed. Otherwise I see it won't be fixed ever. That said, this is just my opinion. -- Regards, Sudeep
On Tue, Oct 25, 2022 at 02:26:55PM +0100, Sudeep Holla wrote: > On Tue, Oct 25, 2022 at 02:56:00PM +0200, Johan Hovold wrote: > > On Tue, Oct 25, 2022 at 12:53:55PM +0100, Sudeep Holla wrote: > > > On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote: > > > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > > > > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > > > > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > > > > is now logged at warning level: > > > > > > > > psci: failed to set PC mode: -3 > > > > > > > > As there is nothing users can do about the firmware behaving this way, > > > > demote the warning to debug level. > > > > > > > > > > As mentioned in the other thread I prefer to keep this as error as we > > > shouldn't mask this error and enable more/newer platforms to ignore it > > > when they can go and fix it. So I don't agree with this. > > > > But now every owner of an X13s laptop will see this not very informative > > error at every boot and wonder what it means. Has something gone broken? > > Should they be worried? Can something be done about it? > > > > I understand that but I have expressed why I am concerned on generalising > it. As long as we inform the concerned owners running Linux(which is quite > small at the moment), keeping it will help to get these fixed on platforms > that are running Linux today for validation and get it fixed if their > platform firmware suffers from the same. Trying to inform every user that a warning during boot is actually benign and nothing to worry about generally seems backwards to me and is not something that is likely to scale. > > Remember that this is firmware used by Windows machines so by the time > > we see this in Linux it's probably way too late to fix in firmware > > anyway. > > > > I am well aware of that fact, but I am targeting platforms that are using > Linux for validation today. > > Honestly, I am not sure if we need to target for zero errors or warnings > on the platforms instead of repeatedly annoy them with warnings until it > is fixed. Otherwise I see it won't be fixed ever. I understand the sentiment, but will having this warning there actually lead to any firmware changes? Or will it just lead to having developers and users debug and report issues which cannot be fixed? And surely there must be better ways to check firmware for compliance than scanning Linux boot logs for warnings? Johan
On Tue, Oct 25, 2022 at 04:32:52PM +0200, Johan Hovold wrote: > On Tue, Oct 25, 2022 at 02:26:55PM +0100, Sudeep Holla wrote: > > On Tue, Oct 25, 2022 at 02:56:00PM +0200, Johan Hovold wrote: > > > On Tue, Oct 25, 2022 at 12:53:55PM +0100, Sudeep Holla wrote: > > > > On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote: > > > > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode > > > > > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb > > > > > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this > > > > > is now logged at warning level: > > > > > > > > > > psci: failed to set PC mode: -3 > > > > > > > > > > As there is nothing users can do about the firmware behaving this way, > > > > > demote the warning to debug level. > > > > > > > > > > > > > As mentioned in the other thread I prefer to keep this as error as we > > > > shouldn't mask this error and enable more/newer platforms to ignore it > > > > when they can go and fix it. So I don't agree with this. > > > > > > But now every owner of an X13s laptop will see this not very informative > > > error at every boot and wonder what it means. Has something gone broken? > > > Should they be worried? Can something be done about it? > > > > > > > I understand that but I have expressed why I am concerned on generalising > > it. As long as we inform the concerned owners running Linux(which is quite > > small at the moment), keeping it will help to get these fixed on platforms > > that are running Linux today for validation and get it fixed if their > > platform firmware suffers from the same. > > Trying to inform every user that a warning during boot is actually > benign and nothing to worry about generally seems backwards to me and is > not something that is likely to scale. It's not *entirely* beningn; we still have to bodge around this not being to spec, and there are things that won't work (e.g. if we kexec to a kernel that expects the FW to actually follow the spec it claims to). I agree with Sudeep that we should log something here, but I do appreciate that argumetn that for 90% of users this is not interesting. I would suggest we make this: pr_info(FW_BUG "failed to set PC mode: %d\n", ...); ... so that it's always logged for those who care, but as it's INFO rather than WARNING, it'll easily be filtered out of logs (e.g. for users booting with "quiet"). Adding FW_BUG will also make this clear we're complaining about a FW bug rather than this being a kernel-internal error. Thanks, Mark.
On Wed, Oct 26, 2022 at 02:24:58PM +0100, Mark Rutland wrote: > On Tue, Oct 25, 2022 at 04:32:52PM +0200, Johan Hovold wrote: > > Trying to inform every user that a warning during boot is actually > > benign and nothing to worry about generally seems backwards to me and is > > not something that is likely to scale. > > It's not *entirely* beningn; we still have to bodge around this not being to > spec, and there are things that won't work (e.g. if we kexec to a kernel that > expects the FW to actually follow the spec it claims to). > > I agree with Sudeep that we should log something here, but I do appreciate that > argumetn that for 90% of users this is not interesting. > > I would suggest we make this: > > pr_info(FW_BUG "failed to set PC mode: %d\n", ...); > > ... so that it's always logged for those who care, but as it's INFO rather than > WARNING, it'll easily be filtered out of logs (e.g. for users booting with > "quiet"). Adding FW_BUG will also make this clear we're complaining about a FW > bug rather than this being a kernel-internal error. Sounds good. I'll respin. Johan
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index e7bcfca4159f..462f37fa6a86 100644 --- a/drivers/firmware/psci/psci.c +++ b/drivers/firmware/psci/psci.c @@ -165,7 +165,7 @@ int psci_set_osi_mode(bool enable) err = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, suspend_mode, 0, 0); if (err < 0) - pr_warn("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); + pr_debug("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err); return psci_to_linux_errno(err); }