All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	Ingo Molnar <mingo@elte.hu>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Xen-devel <xen-devel@lists.xensource.com>
Subject: Re: [GIT PULL] xen /proc/mtrr implementation
Date: Fri, 15 May 2009 21:26:47 -0700	[thread overview]
Message-ID: <m1iqk1k708.fsf@fess.ebiederm.org> (raw)
In-Reply-To: <20090515202250.0f1218ef@jbarnes-g45> (Jesse Barnes's message of "Fri\, 15 May 2009 20\:22\:50 -0700")

Jesse Barnes <jbarnes@virtuousgeek.org> writes:

> On Fri, 15 May 2009 16:49:12 -0700
> Jeremy Fitzhardinge <jeremy@goop.org> wrote:
>
>> Eric W. Biederman wrote:
>> >> /proc/mtrr is in wide use today.  It may be planned for
>> >> obsolescence, but there's no way you can claim its obsolete today
>> >> (my completely up-to-date F10 X server is using it, for example).
>> >> We don't break oldish usermode ABIs in new kernels.
>> >>     
>> >
>> > Sure it is.  There is a better newer replacement.  It is taking a
>> > while to get userspace transitioned but that is different.
>> > Honestly I am puzzled why that it but whatever.
>> >   
>> 
>> There's no mention in feature-removal-schedule.txt.

I don't know that it makes sense to remove mtrrs but it certainly
doesn't make sense to use them if you can avoid it.

>> >> Besides, the MTRR code is also a kernel-internal API, used by DRM
>> >> and other drivers to configure the system MTRR state.  Those
>> >> drivers will either perform badly or outright fail if they can't
>> >> set the appropriate cachability properties. That is not obsolete
>> >> in any way. 
>> >
>> > There are about 5 of them so let's fix them.
>> >   
>> 
>> Well, I count at least 30+, but anyway.

Wow.  We had a lot of those slip in.  Definitely time to fix the
drivers.

>> > With PAT we are in a much better position both for portability and
>> > for flexibility.
>> >   
>> 
>> PAT is relatively recent, and even more recently bug-free.  There are 
>> many people with processors which can't or won't do PAT; what's the
>> plan to support them?  Just hit them with a performance regression?
>> Or wrap MTRR in some other API?

PPro is roughly when PAT came out.  I remember discussing this a while
ago and the conclusion was that there are very few systems with MTRRs
that don't have a usable PAT implementation.  I expect many of those
systems are on their last legs today.

>> Sure, when available.  We're sorting out the details for Xen, but
>> even then it may not be available, either because we're running on an
>> old version of Xen, or because some other guest is using PAT
>> differently.

There are only 3 states that are interesting.  WB UC and WC.  Since
Xen controls the page tables anyway.  I expect it can even remap
it feels like it.

>> But I honestly don't understand the hostility towards 120 lines of
>> code to make an interface (albeit legacy/deprecated/whatever) behave
>> in an expected way.

> FWIW I think supporting the MTRR API in Xen makes sense.  There's a lot
> of old code out there that wants it; would be nice if it mostly worked,
> especially at such a minimal cost.  It's taken awhile to get PAT going
> (and there are still issues here and there) so having the MTRR stuffa
> available is awfully nice.

I won't argue that having MTRRs when you can makes sense.  It is a bit
weird in a vitalized system.  At a practical level there are an
increasing number of systems for which MTRRs are unusable because the
BIOS sets up overlapping mtrrs.  With cheap entry level systems
shipping with 4G I expect it is becoming a majority of systems.

Eric

  reply	other threads:[~2009-05-16  4:27 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-12 23:27 [GIT PULL] xen /proc/mtrr implementation Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 1/6] xen: set cpu_callout_mask to make mtrr work Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 2/6] xen mtrr: Use specific cpu_has_foo macros instead of generic cpu_has() Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 3/6] xen mtrr: Use generic_validate_add_page() Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 4/6] xen mtrr: Implement xen_get_free_region() Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 5/6] xen mtrr: Add xen_{get,set}_mtrr() implementations Jeremy Fitzhardinge
2009-05-12 23:27 ` [PATCH 6/6] xen mtrr: Kill some unnecessary includes Jeremy Fitzhardinge
2009-05-13 13:30 ` [GIT PULL] xen /proc/mtrr implementation Ingo Molnar
2009-05-13 14:39   ` Jeremy Fitzhardinge
2009-05-15 18:27     ` Ingo Molnar
2009-05-15 20:09       ` Jeremy Fitzhardinge
2009-05-15 23:26         ` Eric W. Biederman
2009-05-15 23:49           ` Jeremy Fitzhardinge
2009-05-16  3:22             ` Jesse Barnes
2009-05-16  4:26               ` Eric W. Biederman [this message]
2009-05-16 18:22                 ` Jesse Barnes
2009-05-18  5:02                   ` Jeremy Fitzhardinge
2009-05-18  4:57                 ` Jeremy Fitzhardinge
2009-05-18  8:59                   ` Ingo Molnar
2009-05-18 13:17                     ` [Xen-devel] " Jan Beulich
2009-05-18 17:51                     ` Chris Wright
2009-05-18 18:07                     ` Jeremy Fitzhardinge
2009-05-19  9:59                       ` Ingo Molnar
2009-05-19 10:22                         ` [Xen-devel] " Jan Beulich
2009-05-19 11:08                           ` Ingo Molnar
2009-05-19 12:04                             ` Gerd Hoffmann
2009-05-19 12:26                               ` Ingo Molnar
2009-05-19 12:32                                 ` Alan Cox
2009-05-19 12:37                                   ` Ingo Molnar
2009-05-19 13:21                                 ` Gerd Hoffmann
2009-05-19 13:31                                   ` Ingo Molnar
2009-05-19 13:51                                     ` Gerd Hoffmann
2009-05-19 14:17                                       ` Ingo Molnar
2009-05-19 14:55                                         ` Gerd Hoffmann
2009-05-19 15:24                                           ` Ingo Molnar
2009-05-20  8:01                                             ` Gerd Hoffmann
2009-05-20 16:35                             ` Jeremy Fitzhardinge
2009-05-20 16:12                         ` Jeremy Fitzhardinge
2009-05-20  8:16                       ` Andi Kleen
2009-05-20 16:39                         ` Jeremy Fitzhardinge
2009-05-20 22:52                           ` Andi Kleen
2009-05-20 22:49                             ` Jeremy Fitzhardinge
2009-05-20 23:03                             ` H. Peter Anvin

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=m1iqk1k708.fsf@fess.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.