[PATCH] X86: fix typo PAT to X86_PAT

[PATCH] X86: fix typo PAT to X86_PAT

[Yinghai Lu]

[PATCH] X86: fix typo PAT to X86_PAT

Signed-off-by: Yinghai Lu <yinghai.lu@sun.com>

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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> 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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Yinghai Lu]

On Friday 18 January 2008 12:10:40 am Ingo Molnar wrote:
> 
> * Yinghai Lu <Yinghai.Lu@Sun.COM> 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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Ingo Molnar]

* Yinghai Lu <Yinghai.Lu@Sun.COM> 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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Dave Jones]

On Fri, Jan 18, 2008 at 01:31:40PM +0100, Ingo Molnar wrote:
 
 > * Yinghai Lu <Yinghai.Lu@Sun.COM> 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

RE: [PATCH] X86: fix typo PAT to X86_PAT

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: Dave Jones [mailto:davej@redhat.com] 
>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 <Yinghai.Lu@Sun.COM> 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 

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Dave Jones]

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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Ingo Molnar]

* Dave Jones <davej@redhat.com> 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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Ingo Molnar]

* Pallipadi, Venkatesh <venkatesh.pallipadi@intel.com> 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

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Dave Jones]

On Fri, Jan 18, 2008 at 10:02:10PM +0100, Ingo Molnar wrote:
 > 
 > * Dave Jones <davej@redhat.com> 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

[Yinghai Lu]

[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 <yinghai.lu@sun.com>

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 */

Re: [PATCH] X86: fix typo PAT to X86_PAT

[Yinghai Lu]

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 <davej@redhat.com> 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

RE: [PATCH] X86: fix typo PAT to X86_PAT

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: Dave Jones [mailto:davej@redhat.com] 
>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 <davej@redhat.com> 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