linux-kernel - Re: [PATCH] arch/x86/kernel/cpu/umwait.c

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: <alpine.DEB.2.21.1908091147290.21433@nanos.tec.linutronix.de>
Date:   Fri, 9 Aug 2019 11:49:49 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Fenghua Yu <fenghua.yu@...el.com>
cc:     Valdis Klētnieks <valdis.kletnieks@...edu>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arch/x86/kernel/cpu/umwait.c - remove unused variable

On Thu, 8 Aug 2019, Fenghua Yu wrote:
> On Thu, Aug 08, 2019 at 10:32:38PM +0200, Thomas Gleixner wrote:
> > Valdis,
> > 
> > On Thu, 8 Aug 2019, Valdis Klētnieks wrote:
> > > On Thu, 08 Aug 2019 22:04:03 +0200, Thomas Gleixner said: It isn't 
> > > clear that whatever is doing the device_initcall()s will be able to 
> > > do any reasonable recovery if we return an error, so any error 
> > > recovery is going to have to happen before the function returns. It 
> > > might make sense to do an 'if (ret) return;' before going further in 
> > > the function, but given the comment a few lines further down about 
> > > ignoring errors, it was apparently considered more important to 
> > > struggle through and register stuff in sysfs even if umwait was 
> > > broken....
> > 
> > I missed that when going through it.
> > 
> > The right thing to do is to have a cpu_offline callback which clears 
> > the umwait MSR. That covers also the failure in the cpu hotplug setup. 
> > Then handling an error return makes sense and keeps everything in a 
> > workable shape.
> 
> When cpu is offline, the MSR won't be used. We don't need to clear it, right?

Groan. If soemthing goes wrong when registering the hotplug callback, what
undoes the MSR setup which might have happened and what takes care of it on
cpus coming online later? Exactly nothing. Then you have a non-consistent
behaviour.

Make stuff symmmetric and correct and not optimized for the sunshine case.

Thanks,

	tglx