Message ID | 20221116025417.2590275-1-srinivas.pandruvada@linux.intel.com |
---|---|
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 l7csp3064520wru; Tue, 15 Nov 2022 19:02:41 -0800 (PST) X-Google-Smtp-Source: AA0mqf7k/6Et6SROdHwXaP8PXxs+3FqrrdbIsJ21QHGNNAyzMbzauMfv3kWK7T80ICMRL+kkE3+e X-Received: by 2002:a17:906:2b01:b0:7ad:9028:4b16 with SMTP id a1-20020a1709062b0100b007ad90284b16mr16409732ejg.604.1668567761096; Tue, 15 Nov 2022 19:02:41 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668567761; cv=none; d=google.com; s=arc-20160816; b=WOvg8KJ9kXXdKGOooBL5iuvQ0CHJRunbCD1PLsyd5HTHxSpu+4hwSyIQzJWisVf3qW bC5laiu1QrEQ/rAlQX29sdBoQUp6zwgCQ1ugYCAhDXyfCCwUAL1hgJrLkn90FHAxjjcj pHZKNfqXQ22485ux3ISVzUeydZwW8Xv9JCrxwop06LxP9y5+mmjthql9CFOmrPyYXAo6 CrjYKjzqct+rbEbGgTrCbd4fzbuOJW19DpoYemiXpoBES3MmdvZIQzjzmhcChKSqH4cl ndcivMWomtuiqHGD/4UERCEvioJZlEKQ6ir3ykeOzJS8bXFWbW2ssbrBEvEulYKU2zzr U1ww== 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=q7MuN902CgSwiocj9E42WnCUzgMHlAb7x5SCPvNTJ+E=; b=GzM+XA+i6Ro/9CdDdgky32VjH+lz5zipN+iPJalIV/MafqEnrr/fEoiKPSMA2uy0+P oaZWZUQYKhQ2YmdXE/2U7aaWYoWXYRz2/xQQ5hfw9PbIg6xLn/E37RoWHfQjU/wmL2UQ /GNkRnW1ID02u+0Rb0ATPE2K6oooXhdu433NjdQfV6e1v4bNXpmQOYfnRV/YpFZc/0AW zseEZKk+7tRuWD1kGO4Nm+oUWnDaY5yuagX0mef8qmfKupKifibpHm8MlpDsKQdOu8KC hau7HXXpBdZiEyYxfVDxJalX91OeVHb+l7P3CSjoDNMOjOCz9gs0VYJhpCSB4MsMn3Qh lOVQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=TDDkV1By; 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=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cq3-20020a056402220300b0045c9dbe290csi12867210edb.406.2022.11.15.19.01.50; Tue, 15 Nov 2022 19:02:41 -0800 (PST) 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=@intel.com header.s=Intel header.b=TDDkV1By; 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=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232348AbiKPCzh (ORCPT <rfc822;maxim.cournoyer@gmail.com> + 99 others); Tue, 15 Nov 2022 21:55:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231939AbiKPCzT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 15 Nov 2022 21:55:19 -0500 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C5A3B1261B; Tue, 15 Nov 2022 18:54:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1668567265; x=1700103265; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=hiUBpViMiMZ1GDXtqT+dOQIXeDtwXLX9aMXKnoRYsNA=; b=TDDkV1ByrD0fE6AiNHgu0k7x3TPn/J2cMK5iW4CfxsNc5q8cJLFRnCBA mWgI5nN9//m+3crM3bx1YunrgvsnBP9IMwTxif6mPF1PUHG/Gz/+3MMwU Iew49x5tQwFc5GtgEMK6gJCS+sxoZ0l2HtXJq5CVJA64Td+hEBIgzHUUA bfV71reNHXPbeHxmBFXxqJf0Wg2pLVEYK46tKcNHtANphh12FwYHr/J3X zN/tToUioXrV3gqTDBV5NoK8wjQbmaSGG+iq7L8Z/jPM+YBqvkmF//stJ Oksk1rKHxZjavOxhJ2fI+iQ2QOG+GjgqK7E8Nz5zwFajKm/UV6XCGCc91 g==; X-IronPort-AV: E=McAfee;i="6500,9779,10532"; a="292138217" X-IronPort-AV: E=Sophos;i="5.96,167,1665471600"; d="scan'208";a="292138217" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Nov 2022 18:54:24 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10532"; a="633466006" X-IronPort-AV: E=Sophos;i="5.96,167,1665471600"; d="scan'208";a="633466006" Received: from spandruv-desk.jf.intel.com ([10.54.75.8]) by orsmga007.jf.intel.com with ESMTP; 15 Nov 2022 18:54:24 -0800 From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> To: rafael@kernel.org, daniel.lezcano@linaro.org, amitk@kernel.org, rui.zhang@intel.com Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>, Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Subject: [PATCH RESEND 1/2] thermal: intel: Prevent accidental clearing of HFI status Date: Tue, 15 Nov 2022 18:54:16 -0800 Message-Id: <20221116025417.2590275-1-srinivas.pandruvada@linux.intel.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_PASS, SPF_NONE 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?1749619562923500333?= X-GMAIL-MSGID: =?utf-8?q?1749620108767953760?= |
Series |
[RESEND,1/2] thermal: intel: Prevent accidental clearing of HFI status
|
|
Commit Message
srinivas pandruvada
Nov. 16, 2022, 2:54 a.m. UTC
When there is a package thermal interrupt with PROCHOT log, it will be processed and cleared. It is possible that there is an active HFI event status, which is about to get processed or getting processed. While clearing PROCHOT log bit, it will also clear HFI status bit. This means that hardware is free to update HFI memory. When clearing a package thermal interrupt, some processors will generate a "general protection fault" when any of the read only bit is set to 1. The driver maintains a mask of all read-write bits which can be set. This mask doesn't include HFI status bit. This bit will also be cleared, as it will be assumed read-only bit. So, add HFI status bit 26 to the mask. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> --- Email address was wrong, so sending again. drivers/thermal/intel/therm_throt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Wed, Nov 16, 2022 at 3:54 AM Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > When there is a package thermal interrupt with PROCHOT log, it will be > processed and cleared. It is possible that there is an active HFI event > status, which is about to get processed or getting processed. While > clearing PROCHOT log bit, it will also clear HFI status bit. This means > that hardware is free to update HFI memory. > > When clearing a package thermal interrupt, some processors will generate > a "general protection fault" when any of the read only bit is set to 1. > The driver maintains a mask of all read-write bits which can be set. > This mask doesn't include HFI status bit. This bit will also be cleared, > as it will be assumed read-only bit. So, add HFI status bit 26 to the > mask. > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Is a Fixes tag missing here? Also, do you want it in 6.1-rc7 or would 6.2 suffice? > --- > Email address was wrong, so sending again. > > drivers/thermal/intel/therm_throt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c > index 8352083b87c7..9e8ab31d756e 100644 > --- a/drivers/thermal/intel/therm_throt.c > +++ b/drivers/thermal/intel/therm_throt.c > @@ -197,7 +197,7 @@ static const struct attribute_group thermal_attr_group = { > #define THERM_STATUS_PROCHOT_LOG BIT(1) > > #define THERM_STATUS_CLEAR_CORE_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(13) | BIT(15)) > -#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11)) > +#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(26)) > > static void clear_therm_status_log(int level) > { > -- > 2.31.1 >
On Fri, 2022-11-18 at 18:54 +0100, Rafael J. Wysocki wrote: > On Wed, Nov 16, 2022 at 3:54 AM Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > When there is a package thermal interrupt with PROCHOT log, it will > > be > > processed and cleared. It is possible that there is an active HFI > > event > > status, which is about to get processed or getting processed. While > > clearing PROCHOT log bit, it will also clear HFI status bit. This > > means > > that hardware is free to update HFI memory. > > > > When clearing a package thermal interrupt, some processors will > > generate > > a "general protection fault" when any of the read only bit is set > > to 1. > > The driver maintains a mask of all read-write bits which can be > > set. > > This mask doesn't include HFI status bit. This bit will also be > > cleared, > > as it will be assumed read-only bit. So, add HFI status bit 26 to > > the > > mask. > > > > Signed-off-by: Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com> > > Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > Is a Fixes tag missing here? While adding the following change, this should have been take care of: ab09b0744a99 ("thermal: intel: hfi: Enable notification interrupt") But the above change didn't add this line, which this patch is changing. We can add: Fixes: ab09b0744a99 ("thermal: intel: hfi: Enable notification interrupt") Do you want me to send another PATCH with fixes. > > Also, do you want it in 6.1-rc7 or would 6.2 suffice? Not urgent. 6.2 should be fine. Thanks, Srinivas > > > --- > > Email address was wrong, so sending again. > > > > drivers/thermal/intel/therm_throt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/thermal/intel/therm_throt.c > > b/drivers/thermal/intel/therm_throt.c > > index 8352083b87c7..9e8ab31d756e 100644 > > --- a/drivers/thermal/intel/therm_throt.c > > +++ b/drivers/thermal/intel/therm_throt.c > > @@ -197,7 +197,7 @@ static const struct attribute_group > > thermal_attr_group = { > > #define THERM_STATUS_PROCHOT_LOG BIT(1) > > > > #define THERM_STATUS_CLEAR_CORE_MASK (BIT(1) | BIT(3) | BIT(5) | > > BIT(7) | BIT(9) | BIT(11) | BIT(13) | BIT(15)) > > -#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | > > BIT(7) | BIT(9) | BIT(11)) > > +#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | > > BIT(7) | BIT(9) | BIT(11) | BIT(26)) > > > > static void clear_therm_status_log(int level) > > { > > -- > > 2.31.1 > >
On Fri, Nov 18, 2022 at 8:38 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > On Fri, 2022-11-18 at 18:54 +0100, Rafael J. Wysocki wrote: > > On Wed, Nov 16, 2022 at 3:54 AM Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com> wrote: > > > > > > When there is a package thermal interrupt with PROCHOT log, it will > > > be > > > processed and cleared. It is possible that there is an active HFI > > > event > > > status, which is about to get processed or getting processed. While > > > clearing PROCHOT log bit, it will also clear HFI status bit. This > > > means > > > that hardware is free to update HFI memory. > > > > > > When clearing a package thermal interrupt, some processors will > > > generate > > > a "general protection fault" when any of the read only bit is set > > > to 1. > > > The driver maintains a mask of all read-write bits which can be > > > set. > > > This mask doesn't include HFI status bit. This bit will also be > > > cleared, > > > as it will be assumed read-only bit. So, add HFI status bit 26 to > > > the > > > mask. > > > > > > Signed-off-by: Srinivas Pandruvada > > > <srinivas.pandruvada@linux.intel.com> > > > Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > > > Is a Fixes tag missing here? > While adding the following change, this should have been take care of: > ab09b0744a99 ("thermal: intel: hfi: Enable notification interrupt") > > But the above change didn't add this line, which this patch is > changing. We can add: > > Fixes: ab09b0744a99 ("thermal: intel: hfi: Enable notification > interrupt") OK > Do you want me to send another PATCH with fixes. No, I can take care of this. > > > > Also, do you want it in 6.1-rc7 or would 6.2 suffice? > Not urgent. 6.2 should be fine. OK, thanks!
On Fri, Nov 18, 2022 at 8:49 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Nov 18, 2022 at 8:38 PM srinivas pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > On Fri, 2022-11-18 at 18:54 +0100, Rafael J. Wysocki wrote: > > > On Wed, Nov 16, 2022 at 3:54 AM Srinivas Pandruvada > > > <srinivas.pandruvada@linux.intel.com> wrote: > > > > > > > > When there is a package thermal interrupt with PROCHOT log, it will > > > > be > > > > processed and cleared. It is possible that there is an active HFI > > > > event > > > > status, which is about to get processed or getting processed. While > > > > clearing PROCHOT log bit, it will also clear HFI status bit. This > > > > means > > > > that hardware is free to update HFI memory. > > > > > > > > When clearing a package thermal interrupt, some processors will > > > > generate > > > > a "general protection fault" when any of the read only bit is set > > > > to 1. > > > > The driver maintains a mask of all read-write bits which can be > > > > set. > > > > This mask doesn't include HFI status bit. This bit will also be > > > > cleared, > > > > as it will be assumed read-only bit. So, add HFI status bit 26 to > > > > the > > > > mask. > > > > > > > > Signed-off-by: Srinivas Pandruvada > > > > <srinivas.pandruvada@linux.intel.com> > > > > Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > > > > > > Is a Fixes tag missing here? > > While adding the following change, this should have been take care of: > > ab09b0744a99 ("thermal: intel: hfi: Enable notification interrupt") > > > > But the above change didn't add this line, which this patch is > > changing. We can add: > > > > Fixes: ab09b0744a99 ("thermal: intel: hfi: Enable notification > > interrupt") > > OK > > > Do you want me to send another PATCH with fixes. > > No, I can take care of this. > > > > > > > Also, do you want it in 6.1-rc7 or would 6.2 suffice? > > Not urgent. 6.2 should be fine. > > OK, thanks! Both applied as 6.2 material now, thanks!
diff --git a/drivers/thermal/intel/therm_throt.c b/drivers/thermal/intel/therm_throt.c index 8352083b87c7..9e8ab31d756e 100644 --- a/drivers/thermal/intel/therm_throt.c +++ b/drivers/thermal/intel/therm_throt.c @@ -197,7 +197,7 @@ static const struct attribute_group thermal_attr_group = { #define THERM_STATUS_PROCHOT_LOG BIT(1) #define THERM_STATUS_CLEAR_CORE_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(13) | BIT(15)) -#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11)) +#define THERM_STATUS_CLEAR_PKG_MASK (BIT(1) | BIT(3) | BIT(5) | BIT(7) | BIT(9) | BIT(11) | BIT(26)) static void clear_therm_status_log(int level) {