linux-kernel - Re: [PATCH 3/4] ALSA: hda: Refactor controller PM to use direct-complete optimization

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]

Message-Id: <98CFD5BD-3BA0-4A7A-8C24-D6004F019CDF@canonical.com>
Date:   Fri, 23 Oct 2020 21:02:35 +0800
From:   Kai-Heng Feng <kai.heng.feng@...onical.com>
To:     Takashi Iwai <tiwai@...e.de>
Cc:     tiwai@...e.com, Jaroslav Kysela <perex@...ex.cz>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Alex Deucher <alexander.deucher@....com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "moderated list:SOUND" <alsa-devel@...a-project.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] ALSA: hda: Refactor controller PM to use
 direct-complete optimization



> On Oct 23, 2020, at 19:36, Takashi Iwai <tiwai@...e.de> wrote:
> 
> On Fri, 23 Oct 2020 12:23:37 +0200,
> Kai-Heng Feng wrote:
>> @@ -1103,10 +1096,8 @@ static int azx_runtime_suspend(struct device *dev)
>> 	chip = card->private_data;
>> 
>> 	/* enable controller wake up event */
>> -	if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0) {
>> -		azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
>> -			   STATESTS_INT_MASK);
>> -	}
>> +	azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
>> +		   STATESTS_INT_MASK);
> 
> Hrm, this doesn't look safe.  Applying WAKEEN unconditionally means
> that the machine may get woken up from the system suspend, and we
> don't want that.

Yes, WAKEEN should be enabled for runtime suspend and disabled for system suspend.
In principle we should always do runtime-resume -> suspend flow when runtime and system PM requires different wakeup settings.

That also means HDA controllers can't use direct-complete at all.

However, I did some testing on keeping WAKEEN enabled for graphics card's audio controller, and they didn't wake system up.
But yes, in principle they are not safe, I'll change it in v2.

Kai-Heng

> 
> 
> thanks,
> 
> Takashi