Re: [PATCH 1/10] Cr4 is valid on some 486s

[Gerd Knorr <kraxel@suse.de>]

Linus Torvalds wrote:
> 
> On Mon, 14 Nov 2005, Gerd Knorr wrote:
>> Throwing another patch into the discussion ;)
> 
> Ouch, this one is really ugly.

I somehow expected that answer, it took me quite some time to figure 
what the patch does.  It certainly needs at least a number of cleanups 
before I'd consider it mergable.  The alternative() macro is much easier 
to read.

> If you want to go this way, then you should instead add an X86_FEATURE_SMP 
> that gets cleared on UP and on SMP with just one core (and detect when CPU 
> hotplug ain't gonna happen ;), and then do

Well, the "no hotplug" probably is exactly the reason why the patch 
doesn't use the existing alternatives mechanism, it's a boot-time 
one-way ticket.  The xenified linux kernel actually switches both ways 
at runtime if you plug in/out a second virtual CPU.

> 	#ifdef CONFIG_SMP
> 	#define smp_alternative(x,y) alternative(x,y,X86_FEATURE_SMP)
> 	#else
> 	#define smp_alternative(x,y) asm(x)
> 	#endif

I don't like the idea very much.  That covers only 50% of what the patch 
does, you can patch SMP => UP but not the other way around.  Doesn't 
matter much on real hardware, but for virtual it is quite useful.

> or something similar, instead of creating a totally new infrastructure to 
> do the thing that "alternative()" already does.

Yep, extending alternatives is probably better than duplicating the 
code.  Maybe having some alternative_smp() macro which places both code 
versions into the .altinstr_replacement table?  If that sounds ok I'll 
try to come up with a experimental patch.  If not: other ideas are welcome.

cheers,

   Gerd