This driver is intended to be used exclusively for suspend to idle so callbacks to send OS_HINT during hibernate and S5 will set OS_HINT at the wrong time leading to an undefined behavior.
Cc: stable@vger.kernel.org Signed-off-by: Mario Limonciello mario.limonciello@amd.com --- drivers/platform/x86/amd-pmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c index 841c44cd64c2..230593ae5d6d 100644 --- a/drivers/platform/x86/amd-pmc.c +++ b/drivers/platform/x86/amd-pmc.c @@ -508,7 +508,8 @@ static int __maybe_unused amd_pmc_resume(struct device *dev) }
static const struct dev_pm_ops amd_pmc_pm_ops = { - SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend, amd_pmc_resume) + .suspend_noirq = amd_pmc_suspend, + .resume_noirq = amd_pmc_resume, };
static const struct pci_device_id pmc_pci_ids[] = {
Hi Mario,
On 12/10/21 15:35, Mario Limonciello wrote:
This driver is intended to be used exclusively for suspend to idle so callbacks to send OS_HINT during hibernate and S5 will set OS_HINT at the wrong time leading to an undefined behavior.
Cc: stable@vger.kernel.org Signed-off-by: Mario Limonciello mario.limonciello@amd.com
I notice that there are no [Bug]Link tags here ? It would be helpful to have some links to tickets / forum-posts from people who are actually hitting issues because of this. Both so that people with similar issues can then compare the symptoms as described in the links, as well as for me to get an idea of how urgent of a fix this is.
Regards,
Hans
drivers/platform/x86/amd-pmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c index 841c44cd64c2..230593ae5d6d 100644 --- a/drivers/platform/x86/amd-pmc.c +++ b/drivers/platform/x86/amd-pmc.c @@ -508,7 +508,8 @@ static int __maybe_unused amd_pmc_resume(struct device *dev) } static const struct dev_pm_ops amd_pmc_pm_ops = {
- SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend, amd_pmc_resume)
- .suspend_noirq = amd_pmc_suspend,
- .resume_noirq = amd_pmc_resume,
}; static const struct pci_device_id pmc_pci_ids[] = {
[Public]
-----Original Message----- From: Hans de Goede hdegoede@redhat.com Sent: Friday, December 10, 2021 10:57 To: Limonciello, Mario Mario.Limonciello@amd.com; Mark Gross mgross@linux.intel.com; open list:X86 PLATFORM DRIVERS <platform-driver- x86@vger.kernel.org>; S-k, Shyam-sundar Shyam-sundar.S-k@amd.com Cc: stable@vger.kernel.org Subject: Re: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
Hi Mario,
On 12/10/21 15:35, Mario Limonciello wrote:
This driver is intended to be used exclusively for suspend to idle so callbacks to send OS_HINT during hibernate and S5 will set OS_HINT at the wrong time leading to an undefined behavior.
Cc: stable@vger.kernel.org Signed-off-by: Mario Limonciello mario.limonciello@amd.com
I notice that there are no [Bug]Link tags here ? It would be helpful to have some links to tickets / forum-posts from people who are actually hitting issues because of this. Both so that people with similar issues can then compare the symptoms as described in the links, as well as for me to get an idea of how urgent of a fix this is.
It was the result of an internal testing that this issue was raised. It hasn't yet been hit in the wild that I'm aware of. However in the circumstances that lead to this failure it was approximately a 83% failure rate when it occurred from this problem.
Regards,
Hans
drivers/platform/x86/amd-pmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-
pmc.c
index 841c44cd64c2..230593ae5d6d 100644 --- a/drivers/platform/x86/amd-pmc.c +++ b/drivers/platform/x86/amd-pmc.c @@ -508,7 +508,8 @@ static int __maybe_unused amd_pmc_resume(struct
device *dev)
}
static const struct dev_pm_ops amd_pmc_pm_ops = {
- SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend,
amd_pmc_resume)
- .suspend_noirq = amd_pmc_suspend,
- .resume_noirq = amd_pmc_resume,
};
static const struct pci_device_id pmc_pci_ids[] = {
On Fri, Dec 10, 2021 at 9:06 PM Mario Limonciello mario.limonciello@amd.com wrote:
This driver is intended to be used exclusively for suspend to idle so callbacks to send OS_HINT during hibernate and S5 will set OS_HINT at the wrong time leading to an undefined behavior.
...
static const struct dev_pm_ops amd_pmc_pm_ops = {
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend, amd_pmc_resume)
.suspend_noirq = amd_pmc_suspend,.resume_noirq = amd_pmc_resume,
Can you simply switch to SET_NOIRQ_SYSTEM_SLEEP_PM_OPS()?
};
On Mon, Dec 13, 2021 at 1:07 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Dec 10, 2021 at 9:06 PM Mario Limonciello mario.limonciello@amd.com wrote:
...
static const struct dev_pm_ops amd_pmc_pm_ops = {
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend, amd_pmc_resume)
.suspend_noirq = amd_pmc_suspend,.resume_noirq = amd_pmc_resume,Can you simply switch to SET_NOIRQ_SYSTEM_SLEEP_PM_OPS()?
My gosh, I see that it was like this... Sorry for the noise.
};
Hi,
On 12/10/21 15:35, Mario Limonciello wrote:
This driver is intended to be used exclusively for suspend to idle so callbacks to send OS_HINT during hibernate and S5 will set OS_HINT at the wrong time leading to an undefined behavior.
Cc: stable@vger.kernel.org Signed-off-by: Mario Limonciello mario.limonciello@amd.com
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.g...
I will also add this to the fixes branch and include it in my next fixes pull-req for 5.17.
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
drivers/platform/x86/amd-pmc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c index 841c44cd64c2..230593ae5d6d 100644 --- a/drivers/platform/x86/amd-pmc.c +++ b/drivers/platform/x86/amd-pmc.c @@ -508,7 +508,8 @@ static int __maybe_unused amd_pmc_resume(struct device *dev) } static const struct dev_pm_ops amd_pmc_pm_ops = {
- SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(amd_pmc_suspend, amd_pmc_resume)
- .suspend_noirq = amd_pmc_suspend,
- .resume_noirq = amd_pmc_resume,
}; static const struct pci_device_id pmc_pci_ids[] = {
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Hans de Goede -
Limonciello, Mario -
Mario Limonciello