Provide a stub ioperm implementation for ARMv5 and later

Message ID 1449946774-18072-1-git-send-email-aurelien@aurel32.net
State New, archived
Headers

Commit Message

Aurelien Jarno Dec. 12, 2015, 6:59 p.m. UTC
  The ioperm, iopl, in{b,w,l} and out{b,w,l} functions only make sense
on ARMv4. This patch replaces them by stubs when the libc is built
for ARMv5 or later.
---
 ChangeLog                                   | 10 ++++
 sysdeps/arm/preconfigure                    |  5 ++
 sysdeps/arm/preconfigure.ac                 |  4 ++
 sysdeps/unix/sysv/linux/arm/armv5/ioperm.c  | 91 +++++++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/arm/armv6/Implies   |  2 +
 sysdeps/unix/sysv/linux/arm/armv6t2/Implies |  2 +
 sysdeps/unix/sysv/linux/arm/armv7/Implies   |  2 +
 7 files changed, 116 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/arm/armv5/ioperm.c
 create mode 100644 sysdeps/unix/sysv/linux/arm/armv6/Implies
 create mode 100644 sysdeps/unix/sysv/linux/arm/armv6t2/Implies
 create mode 100644 sysdeps/unix/sysv/linux/arm/armv7/Implies
  

Comments

Phil Blundell Dec. 12, 2015, 8:26 p.m. UTC | #1
On Sat, 2015-12-12 at 19:59 +0100, Aurelien Jarno wrote:
> The ioperm, iopl, in{b,w,l} and out{b,w,l} functions only make sense
> on ARMv4.

This statement isn't strictly true.  There is no inherent relationship
between the architecture level and the availability or not of the
port-mapped I/O functions.  And, in fact, a quick survey suggests that
there is at least one counterexample in the kernel: CONFIG_ARCH_VIPER is
a PXA25x board (hence ARMv5TE) and seems to include ISA.  So, I am not
all that keen on this patch. 

If you wanted to clean up the ioperm code a bit then it would probably
make sense to remove all the legacy stuff that grubs around
in /proc/cpuinfo and /etc/arm_systype to figure out what sort of machine
it's on, since this is only needed for kernels that are so old as to
lack the CTL_BUS_ISA sysctls.

p.
  
Aurelien Jarno Dec. 12, 2015, 8:45 p.m. UTC | #2
On 2015-12-12 20:26, Phil Blundell wrote:
> On Sat, 2015-12-12 at 19:59 +0100, Aurelien Jarno wrote:
> > The ioperm, iopl, in{b,w,l} and out{b,w,l} functions only make sense
> > on ARMv4.
> 
> This statement isn't strictly true.  There is no inherent relationship
> between the architecture level and the availability or not of the
> port-mapped I/O functions.  And, in fact, a quick survey suggests that
> there is at least one counterexample in the kernel: CONFIG_ARCH_VIPER is
> a PXA25x board (hence ARMv5TE) and seems to include ISA.  So, I am not
> all that keen on this patch. 

This patch was actually in answer to the remark from Arnd Bergmann on
the other ioperm patch. I admit I should have checked more in details
about that.

> If you wanted to clean up the ioperm code a bit then it would probably
> make sense to remove all the legacy stuff that grubs around
> in /proc/cpuinfo and /etc/arm_systype to figure out what sort of machine
> it's on, since this is only needed for kernels that are so old as to
> lack the CTL_BUS_ISA sysctls.

OK, I'll have a look at that someday.

Aurelien
  
Arnd Bergmann Dec. 12, 2015, 8:57 p.m. UTC | #3
On Saturday 12 December 2015 20:26:27 Phil Blundell wrote:
> On Sat, 2015-12-12 at 19:59 +0100, Aurelien Jarno wrote:
> > The ioperm, iopl, in{b,w,l} and out{b,w,l} functions only make sense
> > on ARMv4.
> 
> This statement isn't strictly true.  There is no inherent relationship
> between the architecture level and the availability or not of the
> port-mapped I/O functions.  And, in fact, a quick survey suggests that
> there is at least one counterexample in the kernel: CONFIG_ARCH_VIPER is
> a PXA25x board (hence ARMv5TE) and seems to include ISA.  So, I am not
> all that keen on this patch. 

It was my mistake when I misread the code as relying on the machine lookup
in addition to the sysctl, when in fact only one of the two is needed.

However, the sysctl method is also just supported in two platforms
in the kernel: the footbridge that is in the lookup table and the
ARM Integrator machine if PCI is enabled. The Integrator is even
rarer than footbridge, but there is a CPU module called CM1136JF-S
for it, that has an ARMv6 based ARM1136.

No other machines call register_isa_ports(), and I would very much
expect this to stay that way, so we could still disable the code for
ARMv7. I agree it's a less obvious optimization than what I was
suggesting initially, so we can also just leave it in place.

> If you wanted to clean up the ioperm code a bit then it would probably
> make sense to remove all the legacy stuff that grubs around
> in /proc/cpuinfo and /etc/arm_systype to figure out what sort of machine
> it's on, since this is only needed for kernels that are so old as to
> lack the CTL_BUS_ISA sysctls.

Agreed.

	Arnd
  
Phil Blundell Dec. 12, 2015, 10:40 p.m. UTC | #4
On Sat, 2015-12-12 at 21:57 +0100, Arnd Bergmann wrote:
> No other machines call register_isa_ports(), and I would very much
> expect this to stay that way, so we could still disable the code for
> ARMv7. 

I agree that it seems very unlikely anybody will be using these
functions on ARMv7.  But, if we remove the legacy /proc/cpuinfo-based
stuff (which is certainly obsolete) as I suggested to Aurelien earlier,
the remaining cost of this code will be only a couple of hundred bytes
of binary size.  At that point I think the potential benefit from
eliminating it on ARMv7 will be so small that isn't worth taking any
risk at all to do so, and we should just leave it alone.

Obviously we should refrain from adding these functions to aarch64 :-)

p.
  
Mike Frysinger Dec. 12, 2015, 11:04 p.m. UTC | #5
On 12 Dec 2015 22:40, Phil Blundell wrote:
> On Sat, 2015-12-12 at 21:57 +0100, Arnd Bergmann wrote:
> > No other machines call register_isa_ports(), and I would very much
> > expect this to stay that way, so we could still disable the code for
> > ARMv7. 
> 
> I agree that it seems very unlikely anybody will be using these
> functions on ARMv7.  But, if we remove the legacy /proc/cpuinfo-based
> stuff (which is certainly obsolete) as I suggested to Aurelien earlier,
> the remaining cost of this code will be only a couple of hundred bytes
> of binary size.  At that point I think the potential benefit from
> eliminating it on ARMv7 will be so small that isn't worth taking any
> risk at all to do so, and we should just leave it alone.
> 
> Obviously we should refrain from adding these functions to aarch64 :-)

we can force people to fix new builds too by making the symbols visible
only at runtime.  if new code fails to link with missing symbols, we kwow
they'll get noticed & fixed by at least distros.
-mike
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 13be8a8..c76a5b3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@ 
 2015-12-11  Aurelien Jarno  <aurelien@aurel32.net>
 
+	* sysdeps/arm/preconfigure: Detect ARMv5.
+	* sysdeps/arm/preconfigure.ac: Regenerate.
+	* sysdeps/unix/sysv/linux/arm/armv5/ioperm.c: New file. Provide
+	stub functions for the ioperm functions family.
+	* sysdeps/unix/sysv/linux/arm/armv6/Implies: New file.
+	* sysdeps/unix/sysv/linux/arm/armv6t2/Implies: New file.
+	* sysdeps/unix/sysv/linux/arm/armv7/Implies: New file.
+
+2015-12-11  Aurelien Jarno  <aurelien@aurel32.net>
+
 	* sysdeps/unix/sysv/linux/arm/ioperm.c: Do not include
 	<linux/version.h>.
 	[LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,23)]: Remove
diff --git a/sysdeps/arm/preconfigure b/sysdeps/arm/preconfigure
index 33e9501..67259c5 100644
--- a/sysdeps/arm/preconfigure
+++ b/sysdeps/arm/preconfigure
@@ -44,6 +44,11 @@  $as_echo "$as_me: Found compiler is configured for $machine" >&6;}
     { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for $machine" >&5
 $as_echo "$as_me: Found compiler is configured for $machine" >&6;}
     ;;
+  x__ARM_ARCH_5*__)
+    machine=armv5
+    { $as_echo "$as_me:${as_lineno-$LINENO}: Found compiler is configured for $machine" >&5
+$as_echo "$as_me: Found compiler is configured for $machine" >&6;}
+    ;;
   *)
     machine=arm
     { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: arm/preconfigure: Did not find ARM architecture type; using default" >&5
diff --git a/sysdeps/arm/preconfigure.ac b/sysdeps/arm/preconfigure.ac
index 20de5bc..a140edc 100644
--- a/sysdeps/arm/preconfigure.ac
+++ b/sysdeps/arm/preconfigure.ac
@@ -40,6 +40,10 @@  arm*)
     machine=armv6
     AC_MSG_NOTICE([Found compiler is configured for $machine])
     ;;
+  x__ARM_ARCH_5*__)
+    machine=armv5
+    AC_MSG_NOTICE([Found compiler is configured for $machine])
+    ;;
   *)
     machine=arm
     AC_MSG_WARN([arm/preconfigure: Did not find ARM architecture type; using default])
diff --git a/sysdeps/unix/sysv/linux/arm/armv5/ioperm.c b/sysdeps/unix/sysv/linux/arm/armv5/ioperm.c
new file mode 100644
index 0000000..a18d436
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/armv5/ioperm.c
@@ -0,0 +1,91 @@ 
+/* Access to hardware I/O ports. ARMv5 and later stub version.
+   Copyright (C) 1998-2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+
+int
+_ioperm (unsigned long int from, unsigned long int num, int turn_on)
+{
+  __set_errno (EINVAL);
+  return -1;
+}
+stub_warning (ioperm)
+weak_alias (_ioperm, ioperm)
+
+
+int
+_iopl (unsigned int level)
+{
+  __set_errno (EINVAL);
+  return -1;
+}
+stub_warning (iopl)
+weak_alias (_iopl, iopl)
+
+
+/* All the functions below should theoretically generate a segmentation
+   fault as ioperm and iopl always fail.  We don't simulate that.  */
+void
+_outb (unsigned char b, unsigned long int port)
+{
+}
+stub_warning (outb)
+weak_alias (_outb, outb)
+
+
+void
+_outw (unsigned short b, unsigned long int port)
+{
+}
+stub_warning (outw)
+weak_alias (_outw, outw)
+
+
+void
+_outl (unsigned int b, unsigned long int port)
+{
+}
+stub_warning (outl)
+weak_alias (_outl, outl)
+
+
+unsigned int
+_inb (unsigned long int port)
+{
+  return 0;
+}
+stub_warning (inb)
+weak_alias (_inb, inb);
+
+
+unsigned int
+_inw (unsigned long int port)
+{
+  return 0;
+}
+stub_warning (inw)
+weak_alias (_inw, inw);
+
+
+unsigned int
+_inl (unsigned long int port)
+{
+  return 0;
+}
+stub_warning (inl)
+weak_alias (_inl, inl);
diff --git a/sysdeps/unix/sysv/linux/arm/armv6/Implies b/sysdeps/unix/sysv/linux/arm/armv6/Implies
new file mode 100644
index 0000000..15a7e3d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/armv6/Implies
@@ -0,0 +1,2 @@ 
+# We can do everything that 5 can
+unix/sysv/linux/arm/armv5
diff --git a/sysdeps/unix/sysv/linux/arm/armv6t2/Implies b/sysdeps/unix/sysv/linux/arm/armv6t2/Implies
new file mode 100644
index 0000000..d256156
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/armv6t2/Implies
@@ -0,0 +1,2 @@ 
+# We can do everything that 6 can
+unix/sysv/linux/arm/armv6
diff --git a/sysdeps/unix/sysv/linux/arm/armv7/Implies b/sysdeps/unix/sysv/linux/arm/armv7/Implies
new file mode 100644
index 0000000..a7ce12a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/armv7/Implies
@@ -0,0 +1,2 @@ 
+# We can do everything that 6T2 can
+unix/sysv/linux/arm/armv6t2