Re: [PATCH 2/9] x86 idle: remove NOP cpuinfo_x86.hlt_works_ok flag

[Len Brown <lenb@kernel.org>]

> > > > hlt_works_ok was X86_32 only, initialized to 1, and never cleared.
> > > > 
> > > > On 32-bit kernels, this deletes a line from /proc/cpuinfo: "hlt_bug : no"
> > > 
> > > I think you missed the valid usecase where an old CPU with broken halt is 
> > > booted with the no-hlt boot parameter and does not want to crash in the HLT 
> > > instruction.
> > > 
> > > That "no-hlt" boot parameter does:
> > > 
> > >   arch/x86/kernel/cpu/bugs.c:     boot_cpu_data.hlt_works_ok = 0;
> > > 
> > > We can restrict compatibility, but *please* lets do it *explicitly*, not under 
> > > some 'remove unused code' pretense ...
> > > 
> > > Could you please list all CPU models that are affected?
> > 
> > "no-hlt" existed only for 32-bit, and there were exactly zero
> > automatic invocations of it.
> 
> Do we know whether a distro adds this to the boot line? Do we know about users 
> relying on it.

Impossible to know.

> > "idle=poll" does the same thing -- sans change a line
> > in /proc/cpuinfo.
> 
> It also has an effect on halt/poweroff, right?

Yes, if somebody invokes kernel_halt(), then it
uses stop_this_cpu(), which uses HLT unless this flag is cleared.

The functional change of deleting "no-hlt", which was the only
way to clear this flag, was in the previous patch, and so
I should have mentioned it here.

stop_this_cpu() is not in the poweroff path.

It is a different topic, but I'd be curious to know why
any users of the latest kernel on x86 might be invoking
kernel_halt() instead of kernel_power_off().

> > Do we really need both?
> 
> Probably not, as i said i do not disagree - i just think it should be more 
> explicit. Make it a: "users of CPU models X beware" commit title, not 'remove 
> inactive code' ...

My intent was to make the functional change in 1/9
and the NOP cleanup in 2/9.

> So please list the affected hardware and list the affected boot parameter 
> explicitly, in a well-titled commit that phases out this (very likely unused) 
> compatibility hack and *document* the idle=poll workaround for ancient 
> hardware.

There is no known list of affected hardware.

> There's still i386DX CPUs being manufactured these days - a 20+ years old CPU 
> design is surprisingly resilient to spurious patent claims, for obvious 
> reasons.
> 
> Really, there's no need to do things by stealth. There's few things worse than 
> doing the right thing for the wrong reason - it becomes a bad habit of subtly 
> broken thinking quickly.

I agree completely, which is why I've sent this patch -- twice, so far --
to 5000 of my closest friends, including you:-)

thanks,
-Len