[PATCH] X86: fix typo PAT to X86_PAT
Signed-off-by: Yinghai Lu <[email protected]>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1b38c21..980f054 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -953,7 +953,7 @@ config MATH_EMULATION
config MTRR
bool "MTRR (Memory Type Range Register) support"
- depends on !PAT
+ depends on !X86_PAT
---help---
On Intel P6 family processors (Pentium Pro, Pentium II and later)
the Memory Type Range Registers (MTRRs) may be used to control
* Yinghai Lu <[email protected]> wrote:
> config MTRR
> bool "MTRR (Memory Type Range Register) support"
> - depends on !PAT
> + depends on !X86_PAT
> ---help---
> On Intel P6 family processors (Pentium Pro, Pentium II and later)
> the Memory Type Range Registers (MTRRs) may be used to control
thanks. But, i think we should rather do the following: if X86_PAT is
eanbled then /proc/mtrr should be read-only. There's no problem
_looking_ at MTRR contents, as long as we do not try to modify them. Hm?
Ingo
On Friday 18 January 2008 12:10:40 am Ingo Molnar wrote:
>
> * Yinghai Lu <[email protected]> wrote:
>
> > config MTRR
> > bool "MTRR (Memory Type Range Register) support"
> > - depends on !PAT
> > + depends on !X86_PAT
> > ---help---
> > On Intel P6 family processors (Pentium Pro, Pentium II and later)
> > the Memory Type Range Registers (MTRRs) may be used to control
>
> thanks. But, i think we should rather do the following: if X86_PAT is
> eanbled then /proc/mtrr should be read-only. There's no problem
> _looking_ at MTRR contents, as long as we do not try to modify them. Hm?
anyway
depends on !PAT
need to be removed.
it seems when PAT is used, some code still touch MTRR.
YH
* Yinghai Lu <[email protected]> wrote:
> > thanks. But, i think we should rather do the following: if X86_PAT
> > is eanbled then /proc/mtrr should be read-only. There's no problem
> > _looking_ at MTRR contents, as long as we do not try to modify them.
> > Hm?
>
> anyway
>
> depends on !PAT
>
> need to be removed.
>
> it seems when PAT is used, some code still touch MTRR.
you mean modifies MTRRs? Which code is that? (besides the /proc/mtrr
userspace API)
Ingo
On Fri, Jan 18, 2008 at 01:31:40PM +0100, Ingo Molnar wrote:
> * Yinghai Lu <[email protected]> wrote:
>
> > > thanks. But, i think we should rather do the following: if X86_PAT
> > > is eanbled then /proc/mtrr should be read-only. There's no problem
> > > _looking_ at MTRR contents, as long as we do not try to modify them.
> > > Hm?
> >
> > anyway
> >
> > depends on !PAT
> >
> > need to be removed.
> >
> > it seems when PAT is used, some code still touch MTRR.
>
> you mean modifies MTRRs? Which code is that? (besides the /proc/mtrr
> userspace API)
This exclusion is going to be a real pain in the ass for distro kernels.
It's impossible for example to build a kernel that will now support
the MTRR-alike registers on the AMD K6/early Cyrix etc and also
support PAT.
Additionally, given people tend to update their kernels a lot more often
than they update to a whole new version of X, it means until userspace
has caught up, we can't ship a kernel with PAT supported, or else
X gets a lot slower due to the missing mtrr support.
Dave
--
http://www.codemonkey.org.uk
>-----Original Message-----
>From: Dave Jones [mailto:[email protected]]
>Sent: Friday, January 18, 2008 10:25 AM
>To: Ingo Molnar
>Cc: Yinghai Lu; Pallipadi, Venkatesh; LKML
>Subject: Re: [PATCH] X86: fix typo PAT to X86_PAT
>
>On Fri, Jan 18, 2008 at 01:31:40PM +0100, Ingo Molnar wrote:
>
> > * Yinghai Lu <[email protected]> wrote:
> >
> > > > thanks. But, i think we should rather do the following:
>if X86_PAT
> > > > is eanbled then /proc/mtrr should be read-only. There's
>no problem
> > > > _looking_ at MTRR contents, as long as we do not try to
>modify them.
> > > > Hm?
> > >
> > > anyway
> > >
> > > depends on !PAT
> > >
> > > need to be removed.
> > >
> > > it seems when PAT is used, some code still touch MTRR.
> >
> > you mean modifies MTRRs? Which code is that? (besides the /proc/mtrr
> > userspace API)
>
>This exclusion is going to be a real pain in the ass for
>distro kernels.
>It's impossible for example to build a kernel that will now support
>the MTRR-alike registers on the AMD K6/early Cyrix etc and also
>support PAT.
>
Actually, this exclusion will not work at all with the current code.
Infact it should be PAT selects MTRR, for the current code. As
pat_init() is called during mtrr init as the rules for how to change PAT
and how to change MTRR are same. Further, MTRR is always required on
SMP, as we read the MTRR setting from boot CPU and set it on Aps at boot
time. We should only remove the /proc/mtrr write permissions with
CONFIG_PAT. We need to deprecate it for a while before that...
Ingo, can you remove this PAT MTRR exclusion.
Thanks,
Venki
On Fri, Jan 18, 2008 at 10:47:05AM -0800, Venki Pallipadi wrote:
> >This exclusion is going to be a real pain in the ass for
> >distro kernels.
> >It's impossible for example to build a kernel that will now support
> >the MTRR-alike registers on the AMD K6/early Cyrix etc and also
> >support PAT.
>
> Actually, this exclusion will not work at all with the current code.
> Infact it should be PAT selects MTRR, for the current code. As
> pat_init() is called during mtrr init as the rules for how to change PAT
> and how to change MTRR are same. Further, MTRR is always required on
> SMP, as we read the MTRR setting from boot CPU and set it on Aps at boot
> time. We should only remove the /proc/mtrr write permissions with
> CONFIG_PAT. We need to deprecate it for a while before that...
> Ingo, can you remove this PAT MTRR exclusion.
The removal of write-permission also needs to be decided at runtime rather than
compile time, or we screw over the "doesn't support PAT" CPUs in distro kernels.
Dave
--
http://www.codemonkey.org.uk
* Dave Jones <[email protected]> wrote:
> > you mean modifies MTRRs? Which code is that? (besides the
> > /proc/mtrr userspace API)
>
> This exclusion is going to be a real pain in the ass for distro
> kernels. It's impossible for example to build a kernel that will now
> support the MTRR-alike registers on the AMD K6/early Cyrix etc and
> also support PAT.
>
> Additionally, given people tend to update their kernels a lot more
> often than they update to a whole new version of X, it means until
> userspace has caught up, we can't ship a kernel with PAT supported, or
> else X gets a lot slower due to the missing mtrr support.
there's no exclusion enforced right now, and if a CPU is PAT-incapable
(or if the kernel is booted nopat) then the MTRR bits should be usable.
But if we boot with PAT enabled, and Xorg gets /proc/mtrr wrong, we'll
see nasty crashes. If it gets them right, it should all still work just
fine. Is this ok? Then, in a year or two, distros can disable write
support to /proc/mtrr. Hm?
Ingo
* Pallipadi, Venkatesh <[email protected]> wrote:
> Ingo, can you remove this PAT MTRR exclusion.
yeah, already did that.
> Actually, this exclusion will not work at all with the current code.
> Infact it should be PAT selects MTRR, for the current code. As
> pat_init() is called during mtrr init as the rules for how to change
> PAT and how to change MTRR are same. Further, MTRR is always required
> on SMP, as we read the MTRR setting from boot CPU and set it on Aps at
> boot time. We should only remove the /proc/mtrr write permissions with
> CONFIG_PAT. We need to deprecate it for a while before that...
ok.
Ingo
On Fri, Jan 18, 2008 at 10:02:10PM +0100, Ingo Molnar wrote:
>
> * Dave Jones <[email protected]> wrote:
>
> > > you mean modifies MTRRs? Which code is that? (besides the
> > > /proc/mtrr userspace API)
> >
> > This exclusion is going to be a real pain in the ass for distro
> > kernels. It's impossible for example to build a kernel that will now
> > support the MTRR-alike registers on the AMD K6/early Cyrix etc and
> > also support PAT.
> >
> > Additionally, given people tend to update their kernels a lot more
> > often than they update to a whole new version of X, it means until
> > userspace has caught up, we can't ship a kernel with PAT supported, or
> > else X gets a lot slower due to the missing mtrr support.
>
> there's no exclusion enforced right now, and if a CPU is PAT-incapable
> (or if the kernel is booted nopat) then the MTRR bits should be usable.
> But if we boot with PAT enabled, and Xorg gets /proc/mtrr wrong, we'll
> see nasty crashes. If it gets them right, it should all still work just
> fine. Is this ok? Then, in a year or two, distros can disable write
> support to /proc/mtrr. Hm?
A crazy idea just occured to me.. We could make /proc/mtrr an interface
to set PAT on a range of memory. This would make it transparently work
without any changes in X or anything else that sets them in userspace.
Dave
--
http://www.codemonkey.org.uk
[PATCH] X86: disable X86_PAT really
when X86_PAT is not selected, we don't need to do anything in reserve_mattr and free_mattr
also need to bail out if cpu not support PAT.
Signed-off-by: Yinghai Lu <[email protected]>
diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
index 1036134..b3cdee1 100644
--- a/arch/x86/mm/pat.c
+++ b/arch/x86/mm/pat.c
@@ -57,12 +57,9 @@ static int pat_known_cpu(void)
void pat_init(void)
{
+#ifdef CONFIG_X86_PAT
u64 pat;
-#ifndef CONFIG_X86_PAT
- nopat(NULL);
-#endif
-
if (!smp_processor_id() && !pat_known_cpu())
return;
@@ -90,6 +87,7 @@ void pat_init(void)
wrmsrl(MSR_IA32_CR_PAT, pat);
printk(KERN_INFO "x86 PAT enabled: cpu %d, old 0x%Lx, new 0x%Lx\n",
smp_processor_id(), boot_pat_state, pat);
+#endif
}
#undef PAT
@@ -135,9 +133,13 @@ static DEFINE_SPINLOCK(mattr_lock); /* protects memattr list */
int reserve_mattr(u64 start, u64 end, unsigned long attr, unsigned long *fattr)
{
+#ifdef CONFIG_X86_PAT
struct memattr *ma = NULL, *ml;
int err = 0;
+ if (!pat_wc_enabled)
+ return 0;
+
if (fattr)
*fattr = attr;
@@ -191,13 +193,20 @@ int reserve_mattr(u64 start, u64 end, unsigned long attr, unsigned long *fattr)
spin_unlock(&mattr_lock);
return err;
+#else
+ return 0;
+#endif
}
int free_mattr(u64 start, u64 end, unsigned long attr)
{
+#ifdef CONFIG_X86_PAT
struct memattr *ml;
int err = attr ? -EBUSY : 0;
+ if (!pat_wc_enabled)
+ return 0;
+
if (is_memory_any_valid(start, end))
return 0;
@@ -221,6 +230,9 @@ int free_mattr(u64 start, u64 end, unsigned long attr)
current->comm, current->pid,
start, end, cattr_name(attr));
return err;
+#else
+ return 0;
+#endif
}
/* /dev/mem interface. Use the previous mapping */
On Friday 18 January 2008 07:28:49 pm Dave Jones wrote:
> On Fri, Jan 18, 2008 at 10:02:10PM +0100, Ingo Molnar wrote:
> >
> > * Dave Jones <[email protected]> wrote:
> >
> > > > you mean modifies MTRRs? Which code is that? (besides the
> > > > /proc/mtrr userspace API)
> > >
> > > This exclusion is going to be a real pain in the ass for distro
> > > kernels. It's impossible for example to build a kernel that will now
> > > support the MTRR-alike registers on the AMD K6/early Cyrix etc and
> > > also support PAT.
> > >
> > > Additionally, given people tend to update their kernels a lot more
> > > often than they update to a whole new version of X, it means until
> > > userspace has caught up, we can't ship a kernel with PAT supported, or
> > > else X gets a lot slower due to the missing mtrr support.
> >
> > there's no exclusion enforced right now, and if a CPU is PAT-incapable
> > (or if the kernel is booted nopat) then the MTRR bits should be usable.
> > But if we boot with PAT enabled, and Xorg gets /proc/mtrr wrong, we'll
> > see nasty crashes. If it gets them right, it should all still work just
> > fine. Is this ok? Then, in a year or two, distros can disable write
> > support to /proc/mtrr. Hm?
>
> A crazy idea just occured to me.. We could make /proc/mtrr an interface
> to set PAT on a range of memory. This would make it transparently work
> without any changes in X or anything else that sets them in userspace.
goog idea...
we need to make X86_PAT depend on MTRR in arch/x86/Kconfig
YH
>-----Original Message-----
>From: Dave Jones [mailto:[email protected]]
>Sent: Friday, January 18, 2008 7:29 PM
>To: Ingo Molnar
>Cc: Yinghai Lu; Pallipadi, Venkatesh; LKML
>Subject: Re: [PATCH] X86: fix typo PAT to X86_PAT
>
>On Fri, Jan 18, 2008 at 10:02:10PM +0100, Ingo Molnar wrote:
> >
> > * Dave Jones <[email protected]> wrote:
> >
> > > > you mean modifies MTRRs? Which code is that? (besides the
> > > > /proc/mtrr userspace API)
> > >
> > > This exclusion is going to be a real pain in the ass for distro
> > > kernels. It's impossible for example to build a kernel
>that will now
> > > support the MTRR-alike registers on the AMD K6/early
>Cyrix etc and
> > > also support PAT.
> > >
> > > Additionally, given people tend to update their kernels a
>lot more
> > > often than they update to a whole new version of X, it
>means until
> > > userspace has caught up, we can't ship a kernel with PAT
>supported, or
> > > else X gets a lot slower due to the missing mtrr support.
> >
> > there's no exclusion enforced right now, and if a CPU is
>PAT-incapable
> > (or if the kernel is booted nopat) then the MTRR bits
>should be usable.
> > But if we boot with PAT enabled, and Xorg gets /proc/mtrr
>wrong, we'll
> > see nasty crashes. If it gets them right, it should all
>still work just
> > fine. Is this ok? Then, in a year or two, distros can disable write
> > support to /proc/mtrr. Hm?
>
>A crazy idea just occured to me.. We could make /proc/mtrr an
>interface
>to set PAT on a range of memory. This would make it transparently work
>without any changes in X or anything else that sets them in userspace.
>
Yes. We actually used this earlier while we were testing PAT
functionality internally :).
There are some issues though.
1) Current X does /dev/mem mapping of the region followed by MTRR
setting for this region. For this to work with PAT based MTRR, either
the order has to change (so that there wont be any conflict due to WB
devmem mapping when we try to simulate mtrr) or we need a mechanism to
go and change devmem mapping to reflect the later PAT attribute changes.
2) We will have to fail mtrr setting when there are hard conflicts with
PAT requests.
We will look at this as a possible optimization for next round of PAT
patches. But, to work with existing X, we will have to have mechanism to
go and change existing mappings which is slightly more complicated than
what we already have with current PAT changes.
Thanks,
Venki