* [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
@ 2021-12-10 14:35 Mario Limonciello
2021-12-10 16:57 ` Hans de Goede
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mario Limonciello @ 2021-12-10 14:35 UTC (permalink / raw)
To: Hans de Goede, Mark Gross, open list:X86 PLATFORM DRIVERS,
Shyam Sundar S K
Cc: Mario Limonciello, stable
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[] = {
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
2021-12-10 14:35 [PATCH] platform/x86: amd-pmc: only use callbacks for suspend Mario Limonciello
@ 2021-12-10 16:57 ` Hans de Goede
2021-12-11 1:59 ` Limonciello, Mario
2021-12-13 11:07 ` Andy Shevchenko
2021-12-21 17:55 ` Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2021-12-10 16:57 UTC (permalink / raw)
To: Mario Limonciello, Mark Gross, open list:X86 PLATFORM DRIVERS,
Shyam Sundar S K
Cc: stable
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[] = {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
2021-12-10 16:57 ` Hans de Goede
@ 2021-12-11 1:59 ` Limonciello, Mario
0 siblings, 0 replies; 6+ messages in thread
From: Limonciello, Mario @ 2021-12-11 1:59 UTC (permalink / raw)
To: Hans de Goede, Mark Gross, open list:X86 PLATFORM DRIVERS, S-k,
Shyam-sundar
Cc: stable
[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[] = {
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
2021-12-10 14:35 [PATCH] platform/x86: amd-pmc: only use callbacks for suspend Mario Limonciello
2021-12-10 16:57 ` Hans de Goede
@ 2021-12-13 11:07 ` Andy Shevchenko
2021-12-13 11:07 ` Andy Shevchenko
2021-12-21 17:55 ` Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2021-12-13 11:07 UTC (permalink / raw)
To: Mario Limonciello
Cc: Hans de Goede, Mark Gross, open list:X86 PLATFORM DRIVERS,
Shyam Sundar S K, Stable
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()?
> };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
2021-12-13 11:07 ` Andy Shevchenko
@ 2021-12-13 11:07 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-12-13 11:07 UTC (permalink / raw)
To: Mario Limonciello
Cc: Hans de Goede, Mark Gross, open list:X86 PLATFORM DRIVERS,
Shyam Sundar S K, Stable
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.
> > };
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] platform/x86: amd-pmc: only use callbacks for suspend
2021-12-10 14:35 [PATCH] platform/x86: amd-pmc: only use callbacks for suspend Mario Limonciello
2021-12-10 16:57 ` Hans de Goede
2021-12-13 11:07 ` Andy Shevchenko
@ 2021-12-21 17:55 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2021-12-21 17:55 UTC (permalink / raw)
To: Mario Limonciello, Mark Gross, open list:X86 PLATFORM DRIVERS,
Shyam Sundar S K
Cc: stable
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.git/log/?h=review-hans
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[] = {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-21 17:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 14:35 [PATCH] platform/x86: amd-pmc: only use callbacks for suspend Mario Limonciello
2021-12-10 16:57 ` Hans de Goede
2021-12-11 1:59 ` Limonciello, Mario
2021-12-13 11:07 ` Andy Shevchenko
2021-12-13 11:07 ` Andy Shevchenko
2021-12-21 17:55 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).