[patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ravikiran G Thirumalai]

Fix mmap to /dev/mem when CONFIG_X86_PAT is off and CONFIG_STRICT_DEVMEM is
off

mmap to /dev/mem on kernel memory has been failing since the
introduction of PAT (CONFIG_STRICT_DEVMEM=n case).   Seems like
the check to avoid cache aliasing with PAT is kicking in even
when PAT is disabled. The bug seems to have crept in 2.6.26.

This patch makes sure that mmap to regular kernel memory
succeeds if CONFIG_STRICT_DEVMEM=n and
PAT is disabled.  The checks to avoid cache aliasing
still happens if PAT is enabled.

Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
Tested-by: Tim Sirianni <tim@scalemp.com>

Index: git.tip/arch/x86/mm/pat.c
===================================================================
--- git.tip.orig/arch/x86/mm/pat.c	2008-10-21 14:02:57.000000000 -0700
+++ git.tip/arch/x86/mm/pat.c	2008-10-29 13:19:47.000000000 -0800
@@ -481,6 +481,7 @@
 	return 1;
 }
 #else
+/* This check is needed to avoid cache aliasing when PAT is enabled */
 static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 {
 	u64 from = ((u64)pfn) << PAGE_SHIFT;
@@ -508,7 +509,8 @@
 	unsigned long flags = -1;
 	int retval;
 
-	if (!range_is_allowed(pfn, size))
+	/* Ensure cache aliasing does not occur if and only if PAT is on */
+	if (pat_enabled && !range_is_allowed(pfn, size))
 		return 0;
 
 	if (file->f_flags & O_SYNC) {

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> Fix mmap to /dev/mem when CONFIG_X86_PAT is off and CONFIG_STRICT_DEVMEM is
> off
> 
> mmap to /dev/mem on kernel memory has been failing since the
> introduction of PAT (CONFIG_STRICT_DEVMEM=n case).   Seems like
> the check to avoid cache aliasing with PAT is kicking in even
> when PAT is disabled. The bug seems to have crept in 2.6.26.
> 
> This patch makes sure that mmap to regular kernel memory
> succeeds if CONFIG_STRICT_DEVMEM=n and
> PAT is disabled.  The checks to avoid cache aliasing
> still happens if PAT is enabled.
> 
> Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
> Tested-by: Tim Sirianni <tim@scalemp.com>
> 
> Index: git.tip/arch/x86/mm/pat.c
> ===================================================================
> --- git.tip.orig/arch/x86/mm/pat.c	2008-10-21 14:02:57.000000000 -0700
> +++ git.tip/arch/x86/mm/pat.c	2008-10-29 13:19:47.000000000 -0800
> @@ -481,6 +481,7 @@
>  	return 1;
>  }
>  #else
> +/* This check is needed to avoid cache aliasing when PAT is enabled */
>  static inline int range_is_allowed(unsigned long pfn, unsigned long size)
>  {
>  	u64 from = ((u64)pfn) << PAGE_SHIFT;
> @@ -508,7 +509,8 @@
>  	unsigned long flags = -1;
>  	int retval;
>  
> -	if (!range_is_allowed(pfn, size))
> +	/* Ensure cache aliasing does not occur if and only if PAT is on */
> +	if (pat_enabled && !range_is_allowed(pfn, size))
>  		return 0;

I guess it might be cleaner to push this check into the 
range_is_allowed() function?

	Ingo

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ravikiran G Thirumalai]

On Thu, Oct 30, 2008 at 07:29:02PM +0100, Ingo Molnar wrote:
>
>I guess it might be cleaner to push this check into the 
>range_is_allowed() function?
>

OK.  Here it is.


Fix mmap to /dev/mem when CONFIG_X86_PAT is off and CONFIG_STRICT_DEVMEM is
off

mmap to /dev/mem on kernel memory has been failing since the
introduction of PAT (CONFIG_STRICT_DEVMEM=n case).   Seems like
the check to avoid cache aliasing with PAT is kicking in even
when PAT is disabled. The bug seems to have crept in 2.6.26.

This patch makes sure that the mmap to regular
kernel memory succeeds if CONFIG_STRICT_DEVMEM=n and
PAT is disabled, and the checks to avoid cache aliasing
still happens if PAT is enabled.

Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
Tested-by: Tim Sirianni <tim@scalemp.com>

Index: git.tip/arch/x86/mm/pat.c
===================================================================
--- git.tip.orig/arch/x86/mm/pat.c	2008-10-29 13:19:51.000000000 -0800
+++ git.tip/arch/x86/mm/pat.c	2008-10-30 10:44:46.000000000 -0800
@@ -481,12 +481,16 @@
 	return 1;
 }
 #else
+/* This check is needed to avoid cache aliasing when PAT is enabled */
 static inline int range_is_allowed(unsigned long pfn, unsigned long size)
 {
 	u64 from = ((u64)pfn) << PAGE_SHIFT;
 	u64 to = from + size;
 	u64 cursor = from;
 
+	if (!pat_enabled)
+		return 1;
+
 	while (cursor < to) {
 		if (!devmem_is_allowed(pfn)) {
 			printk(KERN_INFO

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> On Thu, Oct 30, 2008 at 07:29:02PM +0100, Ingo Molnar wrote:
> >
> >I guess it might be cleaner to push this check into the 
> >range_is_allowed() function?
> >
> 
> OK.  Here it is.

applied to tip/x86/urgent, thanks!

	Ingo

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Arjan van de Ven]

On Wed, 29 Oct 2008 19:02:03 -0700
Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> Fix mmap to /dev/mem when CONFIG_X86_PAT is off and
> CONFIG_STRICT_DEVMEM is off
> 
> mmap to /dev/mem on kernel memory has been failing since the
> introduction of PAT (CONFIG_STRICT_DEVMEM=n case).   Seems like
> the check to avoid cache aliasing with PAT is kicking in even
> when PAT is disabled. The bug seems to have crept in 2.6.26.
> 
> This patch makes sure that mmap to regular kernel memory
> succeeds if CONFIG_STRICT_DEVMEM=n and
> PAT is disabled.  The checks to avoid cache aliasing
> still happens if PAT is enabled.

well... technically the aliases are bad without PAT as well...


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ravikiran G Thirumalai]

On Thu, Oct 30, 2008 at 01:21:03PM -0700, Arjan van de Ven wrote:
>On Wed, 29 Oct 2008 19:02:03 -0700
>Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
>
>well... technically the aliases are bad without PAT as well...
>

Hmm! Well, you mean if mtrr is used to change attributes?

I guess there is always a risk associated with luser space mmaping /dev/mem,
and we have CONFIG_STRICT_DEVMEM to avoid this in production.
However, for debugging tools, it is worth having the capability in _debug_
kernels and living with the risk IMHO.

Thanks,
Kiran

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Arjan van de Ven]

On Thu, 30 Oct 2008 14:21:21 -0700
Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> On Thu, Oct 30, 2008 at 01:21:03PM -0700, Arjan van de Ven wrote:
> >On Wed, 29 Oct 2008 19:02:03 -0700
> >Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
> >
> >well... technically the aliases are bad without PAT as well...
> >
> 
> Hmm! Well, you mean if mtrr is used to change attributes?

you can also set uncached in the pagetables, like via ioremap_uncached
and other ways.

and /dev/mem may or may not imply cached/uncached, depending on the
open flags.

On memory... what would that mean?

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled

[Ravikiran G Thirumalai]

On Thu, Oct 30, 2008 at 02:29:49PM -0700, Arjan van de Ven wrote:
>On Thu, 30 Oct 2008 14:21:21 -0700
>Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
>
>> On Thu, Oct 30, 2008 at 01:21:03PM -0700, Arjan van de Ven wrote:
>> >On Wed, 29 Oct 2008 19:02:03 -0700
>> >Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
>> >
>> >well... technically the aliases are bad without PAT as well...
>> >
>> 
>> Hmm! Well, you mean if mtrr is used to change attributes?
>
>you can also set uncached in the pagetables, like via ioremap_uncached
>and other ways.
>
>and /dev/mem may or may not imply cached/uncached, depending on the
>open flags.
>
>On memory... what would that mean?
>

Well, I am not denying that an incorrectly written userspace program could cause
cache aliasing/crash the system  I am not arguing for or against this
argument at all.

However, there is what seems like an unintended change in behavior from
2.6.25 to 2.6.26 when CONFIG_STRICT_DEVMEM is disabled and PAT is disabled,
which is a bug, and this patch fixes it that's all. If this was intentional,
then I don't see the reason for having CONFIG_STRICT_DEVMEM!!