* [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
@ 2015-07-28 20:29 U. Artie Eoff
2015-07-29 7:03 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: U. Artie Eoff @ 2015-07-28 20:29 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, zhuo-hao.lee, dgreid, U. Artie Eoff
PM ops could be triggered before HDA is done initializing
and cause PM to set HDA controller to D3Hot. This can result
in "CORB reset timeout#2, CORBRP = 65535" and "no codecs
initialized". Additionally, PM ops can be triggered before
azx_probe_continue finishes (async probe). This can result
in a NULL deref kernel crash.
To fix this, avoid PM ops if !chip->running.
Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
---
sound/pci/hda/hda_intel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 735bdcb04ce8..c38c68f57938 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -867,7 +867,7 @@ static int azx_suspend(struct device *dev)
chip = card->private_data;
hda = container_of(chip, struct hda_intel, chip);
- if (chip->disabled || hda->init_failed)
+ if (chip->disabled || hda->init_failed || !chip->running)
return 0;
bus = azx_bus(chip);
@@ -902,7 +902,7 @@ static int azx_resume(struct device *dev)
chip = card->private_data;
hda = container_of(chip, struct hda_intel, chip);
- if (chip->disabled || hda->init_failed)
+ if (chip->disabled || hda->init_failed || !chip->running)
return 0;
if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
@@ -1027,7 +1027,7 @@ static int azx_runtime_idle(struct device *dev)
return 0;
if (!power_save_controller || !azx_has_pm_runtime(chip) ||
- azx_bus(chip)->codec_powered)
+ azx_bus(chip)->codec_powered || !chip->running)
return -EBUSY;
return 0;
--
2.1.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
2015-07-28 20:29 [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe U. Artie Eoff
@ 2015-07-29 7:03 ` Takashi Iwai
2015-07-29 14:45 ` Eoff, Ullysses A
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2015-07-29 7:03 UTC (permalink / raw)
To: U. Artie Eoff; +Cc: libin.yang, alsa-devel, dgreid, zhuo-hao.lee
On Tue, 28 Jul 2015 22:29:56 +0200,
U. Artie Eoff wrote:
>
> PM ops could be triggered before HDA is done initializing
> and cause PM to set HDA controller to D3Hot. This can result
> in "CORB reset timeout#2, CORBRP = 65535" and "no codecs
> initialized". Additionally, PM ops can be triggered before
> azx_probe_continue finishes (async probe). This can result
> in a NULL deref kernel crash.
>
> To fix this, avoid PM ops if !chip->running.
>
> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> ---
> sound/pci/hda/hda_intel.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index 735bdcb04ce8..c38c68f57938 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -867,7 +867,7 @@ static int azx_suspend(struct device *dev)
>
> chip = card->private_data;
> hda = container_of(chip, struct hda_intel, chip);
> - if (chip->disabled || hda->init_failed)
> + if (chip->disabled || hda->init_failed || !chip->running)
This is superfluous, as azx_runtime_idle() returns -EBUSY.
> return 0;
>
> bus = azx_bus(chip);
> @@ -902,7 +902,7 @@ static int azx_resume(struct device *dev)
>
> chip = card->private_data;
> hda = container_of(chip, struct hda_intel, chip);
> - if (chip->disabled || hda->init_failed)
> + if (chip->disabled || hda->init_failed || !chip->running)
Ditto.
Takashi
> return 0;
>
> if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
> @@ -1027,7 +1027,7 @@ static int azx_runtime_idle(struct device *dev)
> return 0;
>
> if (!power_save_controller || !azx_has_pm_runtime(chip) ||
> - azx_bus(chip)->codec_powered)
> + azx_bus(chip)->codec_powered || !chip->running)
> return -EBUSY;
>
> return 0;
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
2015-07-29 7:03 ` Takashi Iwai
@ 2015-07-29 14:45 ` Eoff, Ullysses A
2015-07-29 17:38 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Eoff, Ullysses A @ 2015-07-29 14:45 UTC (permalink / raw)
To: Takashi Iwai
Cc: Yang, Libin, alsa-devel@alsa-project.org, dgreid@chromium.org,
Lee, Zhuo-hao
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, July 29, 2015 12:03 AM
> To: Eoff, Ullysses A
> Cc: alsa-devel@alsa-project.org; Lee, Zhuo-hao; Yang, Libin; dgreid@chromium.org
> Subject: Re: [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
>
> On Tue, 28 Jul 2015 22:29:56 +0200,
> U. Artie Eoff wrote:
> >
> > PM ops could be triggered before HDA is done initializing
> > and cause PM to set HDA controller to D3Hot. This can result
> > in "CORB reset timeout#2, CORBRP = 65535" and "no codecs
> > initialized". Additionally, PM ops can be triggered before
> > azx_probe_continue finishes (async probe). This can result
> > in a NULL deref kernel crash.
> >
> > To fix this, avoid PM ops if !chip->running.
> >
> > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> > ---
> > sound/pci/hda/hda_intel.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 735bdcb04ce8..c38c68f57938 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -867,7 +867,7 @@ static int azx_suspend(struct device *dev)
> >
> > chip = card->private_data;
> > hda = container_of(chip, struct hda_intel, chip);
> > - if (chip->disabled || hda->init_failed)
> > + if (chip->disabled || hda->init_failed || !chip->running)
>
> This is superfluous, as azx_runtime_idle() returns -EBUSY.
>
This is the sleep suspend, not runtime. Runtime idle result does not
influence sleep pm ops.
> > return 0;
> >
> > bus = azx_bus(chip);
> > @@ -902,7 +902,7 @@ static int azx_resume(struct device *dev)
> >
> > chip = card->private_data;
> > hda = container_of(chip, struct hda_intel, chip);
> > - if (chip->disabled || hda->init_failed)
> > + if (chip->disabled || hda->init_failed || !chip->running)
>
> Ditto.
>
Similar here... not runtime.
>
> Takashi
>
> > return 0;
> >
> > if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
> > @@ -1027,7 +1027,7 @@ static int azx_runtime_idle(struct device *dev)
> > return 0;
> >
> > if (!power_save_controller || !azx_has_pm_runtime(chip) ||
> > - azx_bus(chip)->codec_powered)
> > + azx_bus(chip)->codec_powered || !chip->running)
> > return -EBUSY;
> >
> > return 0;
> > --
> > 2.1.0
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
2015-07-29 14:45 ` Eoff, Ullysses A
@ 2015-07-29 17:38 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2015-07-29 17:38 UTC (permalink / raw)
To: Eoff, Ullysses A
Cc: Yang, Libin, alsa-devel@alsa-project.org, dgreid@chromium.org,
Lee, Zhuo-hao
On Wed, 29 Jul 2015 16:45:52 +0200,
Eoff, Ullysses A wrote:
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, July 29, 2015 12:03 AM
> > To: Eoff, Ullysses A
> > Cc: alsa-devel@alsa-project.org; Lee, Zhuo-hao; Yang, Libin; dgreid@chromium.org
> > Subject: Re: [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe
> >
> > On Tue, 28 Jul 2015 22:29:56 +0200,
> > U. Artie Eoff wrote:
> > >
> > > PM ops could be triggered before HDA is done initializing
> > > and cause PM to set HDA controller to D3Hot. This can result
> > > in "CORB reset timeout#2, CORBRP = 65535" and "no codecs
> > > initialized". Additionally, PM ops can be triggered before
> > > azx_probe_continue finishes (async probe). This can result
> > > in a NULL deref kernel crash.
> > >
> > > To fix this, avoid PM ops if !chip->running.
> > >
> > > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> > > ---
> > > sound/pci/hda/hda_intel.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > > index 735bdcb04ce8..c38c68f57938 100644
> > > --- a/sound/pci/hda/hda_intel.c
> > > +++ b/sound/pci/hda/hda_intel.c
> > > @@ -867,7 +867,7 @@ static int azx_suspend(struct device *dev)
> > >
> > > chip = card->private_data;
> > > hda = container_of(chip, struct hda_intel, chip);
> > > - if (chip->disabled || hda->init_failed)
> > > + if (chip->disabled || hda->init_failed || !chip->running)
> >
> > This is superfluous, as azx_runtime_idle() returns -EBUSY.
> >
>
> This is the sleep suspend, not runtime. Runtime idle result does not
> influence sleep pm ops.
Fair enough, I applied the patch now.
thanks,
Takashi
>
> > > return 0;
> > >
> > > bus = azx_bus(chip);
> > > @@ -902,7 +902,7 @@ static int azx_resume(struct device *dev)
> > >
> > > chip = card->private_data;
> > > hda = container_of(chip, struct hda_intel, chip);
> > > - if (chip->disabled || hda->init_failed)
> > > + if (chip->disabled || hda->init_failed || !chip->running)
> >
> > Ditto.
> >
>
> Similar here... not runtime.
>
> >
> > Takashi
> >
> > > return 0;
> > >
> > > if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL
> > > @@ -1027,7 +1027,7 @@ static int azx_runtime_idle(struct device *dev)
> > > return 0;
> > >
> > > if (!power_save_controller || !azx_has_pm_runtime(chip) ||
> > > - azx_bus(chip)->codec_powered)
> > > + azx_bus(chip)->codec_powered || !chip->running)
> > > return -EBUSY;
> > >
> > > return 0;
> > > --
> > > 2.1.0
> > >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-29 17:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-28 20:29 [PATCH] ALSA: hda - Fix race between PM ops and HDA init/probe U. Artie Eoff
2015-07-29 7:03 ` Takashi Iwai
2015-07-29 14:45 ` Eoff, Ullysses A
2015-07-29 17:38 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox