All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Xen-devel <xen-devel@lists.xensource.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [GIT PULL] xen /proc/mtrr implementation
Date: Mon, 18 May 2009 11:07:52 -0700	[thread overview]
Message-ID: <4A11A3F8.1010202@goop.org> (raw)
In-Reply-To: <20090518085902.GE10687@elte.hu>

Ingo Molnar wrote:
> Here Xen invades an already fragile piece of upstream code 
> (/proc/mtrr) that is obsolete and on the way out. If you want a 
> solution you should add PAT support to Xen and you should use recent 
> upstream kernels. Or you should emulate /proc/mtrr in _Xen the 
> hypervisor_, if you really care that much - without increasing the 
> amount of crap in Linux.
>   

That's a gross mis-characterisation of what we're talking about here.

arch/x86 already defines an mtrr_ops, which defines how to manipulate 
the MTRR registers.  There are currently several implementations of that 
interface.  In Xen the MTRR registers belong to the hypervisor, but it 
allows a privileged kernel to modify them via hypercalls.  I simply 
added a new, straightforward mtrr_ops implementation to do that.  It 
adds about 120 lines of new code, in a single mtrr/xen.c file.

That's it.  I could add any number of bizarre convolutions to achieve 
the same effect, but given that there's an existing interface that is 
exactly designed for what we want to achieve, I have to admit it didn't 
occur to me to do anything else.

MTRR may well be on its way out, and PAT is the proper way to achieve 
the same effect from now on.  But MTRR is still a widely used 
kernel-internal API as well as usermode ABI, and it seems just 
gratuitous to not support it when doing so is such a low-impact change.  
And if/when it comes to be time to completely deprecate/remove mtrr 
support, we can delete it along with everything else.

I'm honestly at a loss to explain the vehemence of your objection to 
these changes given their nature.

We're also working on making PAT support work where possible.  But that 
doesn't mean we want to do without anything in the meantime.

But more generally, we need to work out how to move things forward.

That said, we can live without.  If these MTRR patches are your biggest 
concern, drop them, pull the rest so we can get them seen, tested, in 
linux-next, etc, ready for the next merge window.  You know, like you 
said you wanted to do the last time you blocked the Xen patches.

I would prefer to move them through tip.git: I appreciate your 
(constructive) comments and reviews, the testing infrastructure has 
proved very valuable in the past, and they'll generally get wider 
exposure than my pool of testers.  And its just the right way to do it.

But if you're so concerned that they'll simply give Linus more 
ammunition to beat you up with, to the extent that you're 
second-guessing yourself, then I'm perfectly happy to submit them 
myself.  I don't think it would be an overall better outcome, but it 
keeps the heat off you, and we'd be making some progress.

    J

  parent reply	other threads:[~2009-05-18 18:08 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
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 [this message]
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=4A11A3F8.1010202@goop.org \
    --to=jeremy@goop.org \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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.