* Andi Kleen <[EMAIL PROTECTED]> wrote:
> Ingo Molnar <[EMAIL PROTECTED]> writes:
> >
> > Thomas Gleixner (1):
> > x86: EFI: fix use of unitialized variable and the cache logic
>
> Your honor, I would like to register a differing opinion...
As i mentioned it (in the portion of my email that you clipped), there
are other pending CPA patches in x86.git:
> > [there are more CPA cleanups still brewing but none of that is
> > urgent.]
We didnt plan to push these secondary alias fixes out yet with
yesterday's push, and i disagree with you about their urgency.
> I submitted that fix originally in a different form, but it got
> finally stripped down to this. However in my opinion the full version
> of the patch is still needed, otherwise EFI still does the mapping
> wrong.
well, not really. Firtly, and most importantly, can you see any failures
on any real boxes?
I'd be surprised if you could see any failures, because secondary
aliases have little practical relevance today. Getting the _primary_
alias right is the most important task of CPA and -git does that all
correctly. (we'd be seeing crashes left and right in -rc1 if it didnt)
Secondary virtual alias discovery indeed has an ugly-looking but
fortunately harmless bug in -git (fixed in x86.git#mm).
The fix is rather straightforward, but we want to test it some more.
We'll push the fix probably before -rc2 though, in the next few days.
> On 64bit cpa will still change cache attributes on random low pages
> and cause illegal caching attribute aliases on both 32bit and 64bit.
Well, this is moot with latest x86.git, but you are still wrong.
Firstly, what we do currently incorrectly (and which is fixed in
x86.git) is that we do a __pa() on a vmalloc/ioremap-range address on
64-bit in secondary alias discovery. But as i showed it to you days ago
in my reply, while it will yield a "random" looking address, but that
address is a guaranteed-high physical address in the higher than
0x410000000000 (i.e. 65TB+) physical range, so we'll just return
harmlessly from the filter function. So yes, it's very much ugly and
unintended and we've been working on fixing it (and have fixed it).
Secondly, your suggestion that inconsistent caching attribute aliases
are not permitted is incorrect. They are very much not allowed under
PAT, and they are "nice to avoid" even on non-PAT (early CPU iterations
sometimes have erratas in this area), but we always had them in one way
or another - permanently and temporarily as well. What Linux still
relies on fundamentally for cache attributes are the MTRR attributes.
Anyway, please take a look at today's x86.git#mm:
git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git mm
it should handle secondary aliases correctly - but we are still working
on some other cleanups to make it complete.
If anyone proves us wrong (with a specific example - what is mapped
where, and what bad effect does it have, or if it's available a
crashlog, etc.) we'll certainly reconsider.
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
