All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [patch] x86, voyager: fix ioremap_nocache()
Date: Sun, 27 Apr 2008 23:48:37 +0200	[thread overview]
Message-ID: <20080427214837.GA11631@elte.hu> (raw)
In-Reply-To: <1209329485.3801.46.camel@localhost.localdomain>


* James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> This patch:
> 
> commit 6371b495991debfd1417b17c2bc4f7d7bae05739
> Author: Ingo Molnar <mingo@elte.hu>
> Date:   Wed Jan 30 13:33:40 2008 +0100
> 
>     x86: change ioremap() to default to uncached
> 
> As far as I can tell went blindly into the x86 tree without being 
> shared on any mailing list at all.  How can something that completely 
> alters the semantics of ioremap on x86 platforms go in without any 
> review.

what happened before was that on x86 ioremap() was "lax" about the PTE 
cachability and just said "writeback cached". That was utterly false for 
most of the real ioremap()s done by drivers, but it happened to work out 
fine due to the courtesy of BIOSes setting up UC MTRRs that forced those 
areas into uncachable.

with the PAT changes, what used to be this default and careless 
ioremap() behavior by x86 turned into a hard cache aliasing conflict. So 
we pretty much _have to_ default to uncached - like all other sane 
architectures already did forever. This is a direct consequence of the 
PAT changes which were discussed on lkml.

But with PAT we take over from the MTRRs on x86 and using a cacheable 
ioremap() would give us aliasing conflicts and trouble all around the 
place.

In the Voyager case we should use ioremap_cache(), and thanks for 
pointing out that dependency of the QIC. Does the patch below fix it for 
you?

	Ingo

--------------->
Subject: x86, voyager: fix ioremap_nocache()
From: Ingo Molnar <mingo@elte.hu>
Date: Sun Apr 27 23:21:03 CEST 2008

James Bottomley reported that the following PAT related commit:

| commit 6371b495991debfd1417b17c2bc4f7d7bae05739
| Author: Ingo Molnar <mingo@elte.hu>
| Date:   Wed Jan 30 13:33:40 2008 +0100
|
|     x86: change ioremap() to default to uncached

broke Voyager.

James says:

" it broke a class of voyager machines: those which
  rely on the quad interrupt controller (QIC).  The precis of why they
  broke is because the QIC does IPIs (or CPIs in its terminology) via
  cache line interference: you interrupt a processor by moving a
  designated memory area to write exclusive in the cache (by simply
  writing to the line) and the CPU acks the interrupt by moving it back to
  read shared (by reading from it).  That area, is, of course, mapped by
  ioremap, so reversing the ioremap semantics and adding the uncached bit
  completely breaks the QIC. "

Sorry about that!

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/mach-voyager/voyager_cat.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-x86.q/arch/x86/mach-voyager/voyager_cat.c
===================================================================
--- linux-x86.q.orig/arch/x86/mach-voyager/voyager_cat.c
+++ linux-x86.q/arch/x86/mach-voyager/voyager_cat.c
@@ -602,7 +602,7 @@ void __init voyager_cat_init(void)
 
 		request_resource(&iomem_resource, &res);
 		voyager_SUS = (struct voyager_SUS *)
-		    ioremap(addr, 0x400);
+		    ioremap_cache(addr, 0x400);
 		printk(KERN_NOTICE "Voyager SUS mailbox version 0x%x\n",
 		       voyager_SUS->SUS_version);
 		voyager_SUS->kernel_version = VOYAGER_MAILBOX_VERSION;
@@ -877,7 +877,7 @@ void __init voyager_cat_init(void)
 			request_resource(&iomem_resource, res);
 		}
 
-		qic_addr = (unsigned long)ioremap(qic_addr, 0x400);
+		qic_addr = (unsigned long)ioremap_cache(qic_addr, 0x400);
 
 		for (j = 0; j < 4; j++) {
 			__u8 cpu;

  parent reply	other threads:[~2008-04-27 21:49 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-27 20:51 Breakage caused by unreviewed patch in x86 tree James Bottomley
2008-04-27 20:53 ` David Miller
2008-04-27 21:48 ` Ingo Molnar [this message]
2008-04-27 22:05   ` [patch] x86, voyager: fix ioremap_nocache() James Bottomley
2008-04-27 22:36     ` Willy Tarreau
2008-04-27 22:41     ` Ingo Molnar
2008-04-27 23:18     ` Ingo Molnar
2008-04-27 23:31       ` David Miller
2008-04-28  0:31         ` Rik van Riel
2008-04-28  0:45           ` Al Viro
2008-04-28  0:52             ` H. Peter Anvin
2008-04-28  9:01         ` Alan Cox
2008-04-28  9:17           ` David Miller
2008-04-28  9:48             ` Adrian Bunk
2008-04-28 11:50             ` Ingo Molnar
2008-04-28  6:10       ` Christoph Hellwig
2008-04-28 16:55         ` H. Peter Anvin
2008-04-27 22:34   ` James Bottomley
2008-04-27 22:39     ` Jeff Garzik
2008-04-27 22:44       ` H. Peter Anvin
2008-04-27 22:46       ` David Miller
2008-04-27 22:52         ` H. Peter Anvin
2008-04-27 22:58           ` David Miller
2008-04-27 23:04             ` H. Peter Anvin
2008-04-30 20:35               ` Eric W. Biederman
2008-04-27 23:34           ` Jeff Garzik
2008-04-27 23:39             ` H. Peter Anvin
2008-04-27 22:53         ` Jeff Garzik
2008-04-27 22:56           ` H. Peter Anvin
2008-04-27 22:59             ` David Miller
2008-04-27 23:02             ` Jeff Garzik
2008-04-27 23:14               ` Arjan van de Ven
2008-04-27 23:01         ` Arjan van de Ven
2008-04-30 21:44           ` James Bottomley
2008-04-30 22:39             ` H. Peter Anvin
2008-04-27 23:01       ` Thomas Gleixner
2008-04-28 14:10       ` Arjan van de Ven
2008-04-28 14:29         ` James Bottomley
2008-04-28 15:07           ` Arjan van de Ven
2008-04-28 19:59             ` H. Peter Anvin
2008-04-27 22:00 ` Breakage caused by unreviewed patch in x86 tree H. Peter Anvin
2008-04-27 22:10   ` James Bottomley
2008-04-27 22:13     ` H. Peter Anvin
2008-04-27 22:18       ` James Bottomley
2008-04-27 22:31         ` H. Peter Anvin
2008-04-27 22:58 ` Arjan van de Ven
2008-04-27 23:00   ` David Miller
2008-04-27 23:07     ` Arjan van de Ven
2008-04-27 23:03   ` James Bottomley
2008-04-27 23:11     ` Arjan van de Ven
2008-04-27 23:17     ` 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=20080427214837.GA11631@elte.hu \
    --to=mingo@elte.hu \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=davem@davemloft.net \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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.