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

From: Len Brown
Date: Thu Mar 31 2011 - 14:08:11 EST


> > > > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/