LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
@ 2008-10-30  2:02 Ravikiran G Thirumalai
  2008-10-30 18:29 ` Ingo Molnar
  2008-10-30 20:21 ` Arjan van de Ven
  0 siblings, 2 replies; 8+ messages in thread
From: Ravikiran G Thirumalai @ 2008-10-30  2:02 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: venkatesh.pallipadi, linux-kernel, tim, shai

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) {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30  2:02 [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled Ravikiran G Thirumalai
@ 2008-10-30 18:29 ` Ingo Molnar
  2008-10-30 20:59   ` Ravikiran G Thirumalai
  2008-10-30 20:21 ` Arjan van de Ven
  1 sibling, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2008-10-30 18:29 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: venkatesh.pallipadi, linux-kernel, tim, shai, Suresh Siddha,
	Arjan van de Ven, H. Peter Anvin, Thomas Gleixner


* 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30  2:02 [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled Ravikiran G Thirumalai
  2008-10-30 18:29 ` Ingo Molnar
@ 2008-10-30 20:21 ` Arjan van de Ven
  2008-10-30 21:21   ` Ravikiran G Thirumalai
  1 sibling, 1 reply; 8+ messages in thread
From: Arjan van de Ven @ 2008-10-30 20:21 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: Ingo Molnar, venkatesh.pallipadi, linux-kernel, tim, shai

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30 18:29 ` Ingo Molnar
@ 2008-10-30 20:59   ` Ravikiran G Thirumalai
  2008-10-30 22:55     ` Ingo Molnar
  0 siblings, 1 reply; 8+ messages in thread
From: Ravikiran G Thirumalai @ 2008-10-30 20:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: venkatesh.pallipadi, linux-kernel, tim, shai, Suresh Siddha,
	Arjan van de Ven, H. Peter Anvin, Thomas Gleixner

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30 20:21 ` Arjan van de Ven
@ 2008-10-30 21:21   ` Ravikiran G Thirumalai
  2008-10-30 21:29     ` Arjan van de Ven
  0 siblings, 1 reply; 8+ messages in thread
From: Ravikiran G Thirumalai @ 2008-10-30 21:21 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Ingo Molnar, venkatesh.pallipadi, linux-kernel, tim, shai

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30 21:21   ` Ravikiran G Thirumalai
@ 2008-10-30 21:29     ` Arjan van de Ven
  2008-10-30 21:54       ` Ravikiran G Thirumalai
  0 siblings, 1 reply; 8+ messages in thread
From: Arjan van de Ven @ 2008-10-30 21:29 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: Ingo Molnar, venkatesh.pallipadi, linux-kernel, tim, shai

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30 21:29     ` Arjan van de Ven
@ 2008-10-30 21:54       ` Ravikiran G Thirumalai
  0 siblings, 0 replies; 8+ messages in thread
From: Ravikiran G Thirumalai @ 2008-10-30 21:54 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Ingo Molnar, venkatesh.pallipadi, linux-kernel, tim, shai

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!!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled
  2008-10-30 20:59   ` Ravikiran G Thirumalai
@ 2008-10-30 22:55     ` Ingo Molnar
  0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2008-10-30 22:55 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: venkatesh.pallipadi, linux-kernel, tim, shai, Suresh Siddha,
	Arjan van de Ven, H. Peter Anvin, Thomas Gleixner


* 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-10-30 22:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-30  2:02 [patch] x86: Fix /dev/mem mmap breakage when PAT is disabled Ravikiran G Thirumalai
2008-10-30 18:29 ` Ingo Molnar
2008-10-30 20:59   ` Ravikiran G Thirumalai
2008-10-30 22:55     ` Ingo Molnar
2008-10-30 20:21 ` Arjan van de Ven
2008-10-30 21:21   ` Ravikiran G Thirumalai
2008-10-30 21:29     ` Arjan van de Ven
2008-10-30 21:54       ` Ravikiran G Thirumalai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).