linux-kernel - Re: [PATCH v5 3/4] platform/x86/intel/pmc: core: Always capture counters on suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 3 Apr 2023 16:58:21 +0000
From:   "Box, David E" <david.e.box@...el.com>
To:     "jstultz@...gle.com" <jstultz@...gle.com>,
        "mario.limonciello@....com" <mario.limonciello@....com>,
        "svenva@...omium.org" <svenva@...omium.org>,
        "irenic.rajneesh@...il.com" <irenic.rajneesh@...il.com>
CC:     "Shyam-sundar.S-k@....com" <Shyam-sundar.S-k@....com>,
        "markgross@...nel.org" <markgross@...nel.org>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "rrangel@...omium.org" <rrangel@...omium.org>,
        "Jain, Rajat" <rajatja@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "hdegoede@...hat.com" <hdegoede@...hat.com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>
Subject: Re: [PATCH v5 3/4] platform/x86/intel/pmc: core: Always capture
 counters on suspend

On Thu, 2023-03-30 at 14:44 -0500, Mario Limonciello wrote:
> Currently counters are only captured during suspend when the
> warn_on_s0ix_failures module parameter is set.
> 
> In order to relay this counter information to the kernel reporting
> infrastructure adjust it so that the counters are always captured.
> 
> warn_on_s0ix_failures will be utilized solely for messaging by
> the driver instead.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>

Reviewed-by: David E. Box <david.e.box@...ux.intel.com>

> ---
> v4->v5:
>  * Squash patches together
>  * Add extra pm_suspend_via_firmware() check for resume routine too
> ---
>  drivers/platform/x86/intel/pmc/core.c | 13 +++++--------
>  drivers/platform/x86/intel/pmc/core.h |  2 --
>  2 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c
> b/drivers/platform/x86/intel/pmc/core.c
> index 3a15d32d7644..e2f171fac094 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1168,12 +1168,6 @@ static __maybe_unused int pmc_core_suspend(struct
> device *dev)
>  {
>         struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>  
> -       pmcdev->check_counters = false;
> -
> -       /* No warnings on S0ix failures */
> -       if (!warn_on_s0ix_failures)
> -               return 0;
> -
>         /* Check if the syspend will actually use S0ix */
>         if (pm_suspend_via_firmware())
>                 return 0;
> @@ -1186,7 +1180,6 @@ static __maybe_unused int pmc_core_suspend(struct device
> *dev)
>         if (pmc_core_dev_state_get(pmcdev, &pmcdev->s0ix_counter))
>                 return -EIO;
>  
> -       pmcdev->check_counters = true;
>         return 0;
>  }
>  
> @@ -1222,12 +1215,16 @@ static __maybe_unused int pmc_core_resume(struct
> device *dev)
>         const struct pmc_bit_map **maps = pmcdev->map->lpm_sts;
>         int offset = pmcdev->map->lpm_status_offset;
>  
> -       if (!pmcdev->check_counters)
> +       /* Check if the syspend used S0ix */
> +       if (pm_suspend_via_firmware())
>                 return 0;
>  
>         if (!pmc_core_is_s0ix_failed(pmcdev))
>                 return 0;
>  
> +       if (!warn_on_s0ix_failures)
> +               return 0;
> +
>         if (pmc_core_is_pc10_failed(pmcdev)) {
>                 /* S0ix failed because of PC10 entry failure */
>                 dev_info(dev, "CPU did not enter PC10!!! (PC10 cnt=0x%llx)\n",
> diff --git a/drivers/platform/x86/intel/pmc/core.h
> b/drivers/platform/x86/intel/pmc/core.h
> index 810204d758ab..51d73efceaf3 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -319,7 +319,6 @@ struct pmc_reg_map {
>   * @pmc_xram_read_bit: flag to indicate whether PMC XRAM shadow registers
>   *                     used to read MPHY PG and PLL status are available
>   * @mutex_lock:                mutex to complete one transcation
> - * @check_counters:    On resume, check if counters are getting incremented
>   * @pc10_counter:      PC10 residency counter
>   * @s0ix_counter:      S0ix residency (step adjusted)
>   * @num_lpm_modes:     Count of enabled modes
> @@ -338,7 +337,6 @@ struct pmc_dev {
>         int pmc_xram_read_bit;
>         struct mutex lock; /* generic mutex lock for PMC Core */
>  
> -       bool check_counters; /* Check for counter increments on resume */
>         u64 pc10_counter;
>         u64 s0ix_counter;
>         int num_lpm_modes;