| 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
| ||
|
Message-ID: <dcdce7f3-dc51-445f-855c-6b46110d1c2b@redhat.com>
Date: Wed, 11 Sep 2024 14:17:15 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Xi Pardee <xi.pardee@...ux.intel.com>, irenic.rajneesh@...il.com,
david.e.box@...ux.intel.com, ilpo.jarvinen@...ux.intel.com,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v2] platform/x86:intel/pmc: Ignore all LTRs during suspend
Hi,
On 9/11/24 2:16 PM, Hans de Goede wrote:
> Hi,
>
> On 9/6/24 8:40 PM, Xi Pardee wrote:
>> From: Xi Pardee <xi.pardee@...el.com>
>>
>> Add support to ignore all LTRs before suspend and restore the previous
>> LTR values after suspend. This feature could be turned off with module
>> parameter ltr_ignore_all_suspend.
>>
>> LTR value is a mechanism for a device to indicate tolerance to access
>> the corresponding resource. When system suspends, the resource is not
>> available and therefore the LTR value could be ignored. Ignoring all
>> LTR values prevents problematic device from blocking the system to get
>> to the deepest package state during suspend.
>>
>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>> Signed-off-by: Xi Pardee <xi.pardee@...el.com>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
Oops, wrong auto-reply, what I meant is:
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
>> v2:
>> - Add more details to commit message
>> - Fix format: ltr->LTR, S0IX->S0ix, space between name and email
>>
>> ---
>> drivers/platform/x86/intel/pmc/core.c | 53 +++++++++++++++++++++++++++
>> drivers/platform/x86/intel/pmc/core.h | 2 +
>> 2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index 01ae71c6df59..0ec703af16a4 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -714,6 +714,49 @@ static int pmc_core_s0ix_blocker_show(struct seq_file *s, void *unused)
>> }
>> DEFINE_SHOW_ATTRIBUTE(pmc_core_s0ix_blocker);
>>
>> +static void pmc_core_ltr_ignore_all(struct pmc_dev *pmcdev)
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
>> + struct pmc *pmc;
>> + u32 ltr_ign;
>> +
>> + pmc = pmcdev->pmcs[i];
>> + if (!pmc)
>> + continue;
>> +
>> + guard(mutex)(&pmcdev->lock);
>> + pmc->ltr_ign = pmc_core_reg_read(pmc, pmc->map->ltr_ignore_offset);
>> +
>> + /* ltr_ignore_max is the max index value for LTR ignore register */
>> + ltr_ign = pmc->ltr_ign | GENMASK(pmc->map->ltr_ignore_max, 0);
>> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, ltr_ign);
>> + }
>> +
>> + /*
>> + * Ignoring ME during suspend is blocking platforms with ADL PCH to get to
>> + * deeper S0ix substate.
>> + */
>> + pmc_core_send_ltr_ignore(pmcdev, 6, 0);
>> +}
>> +
>> +static void pmc_core_ltr_restore_all(struct pmc_dev *pmcdev)
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); i++) {
>> + struct pmc *pmc;
>> +
>> + pmc = pmcdev->pmcs[i];
>> + if (!pmc)
>> + continue;
>> +
>> + guard(mutex)(&pmcdev->lock);
>> + pmc_core_reg_write(pmc, pmc->map->ltr_ignore_offset, pmc->ltr_ign);
>> + }
>> +}
>> +
>> static inline u64 adjust_lpm_residency(struct pmc *pmc, u32 offset,
>> const int lpm_adj_x2)
>> {
>> @@ -1479,6 +1522,10 @@ static bool warn_on_s0ix_failures;
>> module_param(warn_on_s0ix_failures, bool, 0644);
>> MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix failures");
>>
>> +static bool ltr_ignore_all_suspend = true;
>> +module_param(ltr_ignore_all_suspend, bool, 0644);
>> +MODULE_PARM_DESC(ltr_ignore_all_suspend, "Ignore all LTRs during suspend");
>> +
>> static __maybe_unused int pmc_core_suspend(struct device *dev)
>> {
>> struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>> @@ -1488,6 +1535,9 @@ static __maybe_unused int pmc_core_suspend(struct device *dev)
>> if (pmcdev->suspend)
>> pmcdev->suspend(pmcdev);
>>
>> + if (ltr_ignore_all_suspend)
>> + pmc_core_ltr_ignore_all(pmcdev);
>> +
>> /* Check if the syspend will actually use S0ix */
>> if (pm_suspend_via_firmware())
>> return 0;
>> @@ -1594,6 +1644,9 @@ static __maybe_unused int pmc_core_resume(struct device *dev)
>> {
>> struct pmc_dev *pmcdev = dev_get_drvdata(dev);
>>
>> + if (ltr_ignore_all_suspend)
>> + pmc_core_ltr_restore_all(pmcdev);
>> +
>> if (pmcdev->resume)
>> return pmcdev->resume(pmcdev);
>>
>> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
>> index ea04de7eb9e8..e862ea88b816 100644
>> --- a/drivers/platform/x86/intel/pmc/core.h
>> +++ b/drivers/platform/x86/intel/pmc/core.h
>> @@ -372,6 +372,7 @@ struct pmc_info {
>> * @map: pointer to pmc_reg_map struct that contains platform
>> * specific attributes
>> * @lpm_req_regs: List of substate requirements
>> + * @ltr_ign: Holds LTR ignore data while suspended
>> *
>> * pmc contains info about one power management controller device.
>> */
>> @@ -380,6 +381,7 @@ struct pmc {
>> void __iomem *regbase;
>> const struct pmc_reg_map *map;
>> u32 *lpm_req_regs;
>> + u32 ltr_ign;
>> };
>>
>> /**
>
Powered by blists - more mailing lists