Re: [PATCH] Mutilated form of Andi Kleen's AMD prefetch errata patch

[Dave Jones <davej@redhat.com>]

On Tue, Sep 30, 2003 at 03:45:26PM +0100, Jamie Lokier wrote:

 > > And those people are wrong. If they want to save bloat, instead of
 > > 'fixing' things by removing <1 page of .text, how about working on
 > > some of the real problems like shrinking some of the growth of various
 > > data structures that actually *matter*.
 > How about both?

Sounds like wasted effort, in the same sense that rewriting a crap
algorithm in assembly won't be better than using a more efficient
algorithm in C.

 > I'm talking about people with embedded 486s or old 486s donated.  P4s
 > are abundant in RAM

Mine has 256MB. Sure its a huge amount in comparison to how we kitted
out 486s a few years back, but still hardly an abundance in todays bloatware..

 > but 2MB is still not unheard of in the small
 > boxes, and in 2MB, 512 bytes of code (which is about the size of the
 > prefetch workaround) is more significant.

I'l be *amazed* if you manage to get a 2.6 kernel to boot and function
efficiently in 2MB without config'ing away a lot of the 'growth' like
sysfs. (Sidenote: Before some loon actually tries this, by function
efficiently, I mean is actually usable for some purpose, not "it booted
to a bash prompt")
 
 > > F00F workaround was enabled on every kernel that is possible
 > > to boot on affected hardware last time I looked.
 > > This is what you seem to be missing, it's not optional.
 > > If its possible to boot that kernel on affected hardware, 
 > > it needs errata workarounds.
 > 
 > We have a few confusing issues here.
 > 
 > 1. First, your point about affected hardware.
 > 
 >    - I don't see anything that prevents a PPro-compiled kernel from booting
 >      on a P5MMX with the F00F erratum.

Compiled with -m686 - Uses CMOV, won't boot.

 >    - Nor do I see anything that prevents a PII-compiled kernel from booting
 >      on a PPro with the store ordering erratum (X86_PPRO_FENCE).

Correct. As noted in another mail, it arguably should contain the
workaround.

 >    Perhaps it's this apparent hypocrisy which needs healing.

Agreed.

 > 2. I'm not sure if you're criticising the other chap who wants
 >    rid of the AMD errata workaround, or my X86_PREFETCH_FIXUP code.

My criticism was twofold.

1. The splitting of X86_FEATURE_XMM into X86_FEATURE_XMM_PREFETCH and
   X86_FEATURE_3DNOW_PREFETCH doesn't seem to really buy us anything
   other than complication.
2. THis chunk...

+       /* Prefetch works ok? */
+#ifndef CONFIG_X86_PREFETCH_FIXUP
+       if (c->x86_vendor != X86_VENDOR_AMD || c->x86 < 6)
+#endif
+       {
+               if (cpu_has(c, X86_FEATURE_XMM))
+                       set_bit(X86_FEATURE_XMM_PREFETCH, c->x86_capability);
+               if (cpu_has(c, X86_FEATURE_3DNOW))
+                       set_bit(X86_FEATURE_3DNOW_PREFETCH, c->x86_capability);
+       }

- If we haven't set CONFIG_X86_PREFETCH_FIXUP (say a P4 kernel), this
  code path isn't taken, and we end up not doing prefetches on P4's too
  as you're not setting X86_FEATURE_XMM_PREFETCH anywhere else, and apply_alternatives
  leaves them as NOPs.
- Newer C3s are 686's with prefetch, this nobbles them too.


 >    In case you hadn't fully grokked it, my code doesn't disable the
 >    workaround!  It simply substitutes it for a smaller, slightly
 >    slower one, on kernels which are not optimised for AMD.

See above. Or have I missed something ?

 >    Given that, I'm not sure what the thrust of your argument is.

It's possible I'm missing something silly..

		Dave

-- 
 Dave Jones     http://www.codemonkey.org.uk