mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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®