From: Ingo Molnar <mingo@elte.hu>
To: Andi Kleen <andi@firstfloor.org>
Cc: davej@codemonkey.org.uk, tglx@linutronix.de,
linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH] Use global TLB flushes in MTRR code
Date: Sat, 9 Feb 2008 12:47:25 +0100 [thread overview]
Message-ID: <20080209114725.GB5273@elte.hu> (raw)
In-Reply-To: <20080209114826.GA21172@one.firstfloor.org>
* Andi Kleen <andi@firstfloor.org> wrote:
> > > It changes behaviour in some minor ways but I don't think it makes
> > > any difference. PGE only influences TLB flushes (according to its
> > > specification) and all the TLB flushes still run with PGE
> > > disabled.
> >
> > now that i pointed out the difference, your position changed to
> > "changes behavior in minor ways" ;-)
>
> The instruction stream changes (more cr* accesses), but the actual
> flushes do not. [...]
You are still dead wrong. You refuse to realize the _obvious_ and
_fundamental_ thing that the PGE bit does: it enables global TLBs while
we change MTRRs!
That would be a completely new and totally untested modus operandi for a
large array of x86 CPUs. You should read the Intel manual about the
recommended way to change MTRRs (Vol. 3A 10-41) - it describes disabling
the PGE during MTRR changes if it's enabled. The primary purpose of that
is of course to flush the TLB entries - but still it's documented that
way and the current code does it that way. It would be rather stupid for
us to do otherwise: we simply cannot guarantee that x86 CPUs that
implement MTRRs and PGE have no undocumented or _unknown_ erratas in
this area. It _might_ be fine, while what you say is that it _is_ fine -
which we simply cannot know. (And yes, there are documented CPU erratas
related to global TLBs and MTRR's, so this area is far from being an
unwritten page.)
So i refuse to apply such a pointless and risky "cleanup" patch from you
to arch/x86 [which increases code size as well], _especially_ given how
many times i've explained this very issue to you already and especially
because you continue to mislead about the true effects of the patch.
Ingo
next prev parent reply other threads:[~2008-02-09 11:47 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-07 19:02 Andi Kleen
2008-02-07 19:08 ` Oliver Pinter
2008-02-07 19:13 ` Ingo Molnar
2008-02-07 20:03 ` Andi Kleen
2008-02-07 20:37 ` Ingo Molnar
2008-02-08 11:44 ` Andi Kleen
2008-02-09 9:40 ` Ingo Molnar
2008-02-09 11:48 ` Andi Kleen
2008-02-09 11:47 ` Ingo Molnar [this message]
2008-02-09 14:12 ` Andi Kleen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080209114725.GB5273@elte.hu \
--to=mingo@elte.hu \
--cc=andi@firstfloor.org \
--cc=davej@codemonkey.org.uk \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®