linux-kernel - [PATCH] x86: Remove readq()/writeq() on 32-bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Apr 2009 12:05:10 -0700
From:	Roland Dreier <rdreier@...co.com>
To:	hpa@...or.com, mingo@...e.hu, tglx@...utronix.de
Cc:	David Miller <davem@...emloft.net>, h.mitake@...il.com,
	rpjday@...shcourse.ca, linux-kernel@...r.kernel.org
Subject: [PATCH] x86: Remove readq()/writeq() on 32-bit

As discussed in <http://lkml.org/lkml/2009/4/19/164> and follow-ups,
readq()/writeq() for 32-bit x86 are implemented as two readl()/writel()
operations.  This is not atomic (in the sense that another MMIO
operation from another CPU or thread can be done in the middle of the
two read/writes), and may not access the two halves of the register in
the correct order to work with hardware.

Rather than silently providing a 32-bit fallback that leaves a
possibility for strange driver bugs, it's better to provide readq()
and writeq() only for 64-bit architectures, and have a compile failure
on 32-bit architectures that forces driver authors to think about what
the correct solution is.

This essentially reverts 2c5643b1 ("x86: provide readq()/writeq() on
32-bit too") and follow-on commits.  If in the future someone wants to
provide a generic solution for all 32-bit architectures, that's great,
but there's not much point in providing (arguably broken)
implementations for only one architecture, since any portable driver
will have to implement fallbacks for other architectures anyway.

Signed-off-by: Roland Dreier <rolandd@...co.com>
---
We never seemed to reach closure on this.  I would strongly suggest
merging something like this, and then if someone has a grand plan to
unify all fallbacks, we can add that when it shows up.  As it stands,
the x86-32 situation is not progress towards that grand unified plans,
and does nothing that I can tell beyond setting a trap for drivers.

 arch/x86/Kconfig          |    2 --
 arch/x86/include/asm/io.h |   23 ++---------------------
 2 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index df9e885..0be277b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -19,8 +19,6 @@ config X86_64
 config X86
 	def_bool y
 	select HAVE_AOUT if X86_32
-	select HAVE_READQ
-	select HAVE_WRITEQ
 	select HAVE_UNSTABLE_SCHED_CLOCK
 	select HAVE_IDE
 	select HAVE_OPROFILE
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 7373932..199c7b9 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -51,27 +51,6 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
 build_mmio_write(writeq, "q", unsigned long, "r", :"memory")

-#else
-
-static inline __u64 readq(const volatile void __iomem *addr)
-{
-	const volatile u32 __iomem *p = addr;
-	u32 low, high;
-
-	low = readl(p);
-	high = readl(p + 1);
-
-	return low + ((u64)high << 32);
-}
-
-static inline void writeq(__u64 val, volatile void __iomem *addr)
-{
-	writel(val, addr);
-	writel(val >> 32, addr+4);
-}
-
-#endif
-
 #define readq_relaxed(a)	readq(a)

 #define __raw_readq(a)		readq(a)
@@ -81,6 +60,8 @@ static inline void writeq(__u64 val, volatile void __iomem *addr)
 #define readq			readq
 #define writeq			writeq

+#endif
+
 /**
  *	virt_to_phys	-	map virtual addresses to physical
  *	@address: address to remap
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/