linux-kernel - Re: [Regression, 2.6.32->33-rc1] Commit 9f15226e755 breaks resume on Toshiba Portege R500

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] [day] [month] [year] [list]

Message-Id: <200912240001.26764.rjw@sisk.pl>
Date:	Thu, 24 Dec 2009 00:01:26 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Andreas Herrmann <andreas.herrmann3@....com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Ingo Molnar <mingo@...e.hu>, LKML <linux-kernel@...r.kernel.org>,
	pm list <linux-pm@...ts.linux-foundation.org>
Subject: Re: [Regression, 2.6.32->33-rc1] Commit 9f15226e755 breaks resume on Toshiba Portege R500

On Wednesday 23 December 2009, Rafael J. Wysocki wrote:
> Hi Andreas,
> 
> Unfortunately your commit 9f15226e75583547aaf542c6be4bdac1060dd425
> (x86, ucode-amd: Ensure ucode update on suspend/resume after CPU off/online
> cycle) breaks resume from suspend to RAM on my Toshiba Portege R500 (an attempt
> to resume causes a hard hang with 100% CPU load, reproducible 100% of the
> time).  This has been found by bisection and confirmed by reverting this commit
> on top of 2.6.33-rc1.
> 
> Strangely enough, the box doesn't even have an AMD CPU, there's an Intel Core2
> Duo in it, so I suspect some missing check somewhere.
> 
> If I have the time, I'll look deeper into this tomorrow.

I think commit 9f15226e755 is wrong, since it causes microcode_init_cpu()
to be called during resume even for CPUs for which there's no microcode to
apply.  That, in turn, results in executing request_firmware() (on Intel CPUs
at least) which doesn't work at this stage of resume (we have device interrupts
disabled, I/O devices are still suspended and so on).

If I'm not mistaken, the "if (uci->valid)" logic means "if that CPU is known
to us" , so before commit 9f15226e755 microcode_resume_cpu() was called for
all CPUs already in the system during suspend, which was the right thing to do.
The commit changed it so that the CPUs without microcode to apply are now
treated as "unknown", which is not quite right.

The problem this commit attempted to solve has to be handled differently.

Linus, please revert commit 9f15226e755.

Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/