_ioperm support for Arm

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

_ioperm support for Arm

Florian Weimer-5
Do we still need this?  I'm not sure how many kernels/boards support the
ISA bus today.

In the kernel, I only see a call to register_isa_ports in
arch/arm/mach-footbridge/dc21285.c.  Apparently, this is a chip on
Netwinder devices.  Those seem to be armv4l.  Do we even support that
architecture in glibc?  At least originally, there was no EABI support.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Phil Blundell-3
On Wed, May 29, 2019 at 02:11:42PM +0200, Florian Weimer wrote:
> In the kernel, I only see a call to register_isa_ports in
> arch/arm/mach-footbridge/dc21285.c.

Right, I think that functionality was only ever supported on
DC21285 platforms.  There were more of those than just NetWinder,
but they were all StrongARM (ARMv4).

You're correct that it isn't practical for an ARMv4 machine to be
EABI conformant because the EABI mandates interworking and ARMv4
doesn't have BX/BLX, but last time I looked glibc did still have
all the right conditional guards in place to allow for compilation
on an ARMv4 platform.  I haven't checked recently though and it's
possible that it might have bit-rotted.  If we did want to make a
policy decision that glibc doesn't support older ARM architectures
anymore then there are probably quite a lot of #ifdefs that could
be eliminated.

All that said, I don't imagine anybody would notice if the
ioperm() support in glibc were to go away, and in the unlikely
event that there is some application somewhere that relies on it,
it wouldn't be very difficult to patch ioperm() into the
application itself.  It doesn't interact with anything else
inside glibc.

Is there a particular reason for wanting to remove it, or is it
just that it seems like useless cruft we could do without?

p.
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Florian Weimer-5
* Phil Blundell:

> On Wed, May 29, 2019 at 02:11:42PM +0200, Florian Weimer wrote:
>> In the kernel, I only see a call to register_isa_ports in
>> arch/arm/mach-footbridge/dc21285.c.
>
> Right, I think that functionality was only ever supported on
> DC21285 platforms.  There were more of those than just NetWinder,
> but they were all StrongARM (ARMv4).
>
> You're correct that it isn't practical for an ARMv4 machine to be
> EABI conformant because the EABI mandates interworking and ARMv4
> doesn't have BX/BLX, but last time I looked glibc did still have
> all the right conditional guards in place to allow for compilation
> on an ARMv4 platform.  I haven't checked recently though and it's
> possible that it might have bit-rotted.  If we did want to make a
> policy decision that glibc doesn't support older ARM architectures
> anymore then there are probably quite a lot of #ifdefs that could
> be eliminated.

There's this bug:

  Remove ARM old-ABI support
  <https://sourceware.org/bugzilla/show_bug.cgi?id=13556>

I believe this removed core support for non-EABI platforms.

All the target triplets we test upstream have gnueabi in them, and these
days, anything that is not built by build-many-glibcs.py in at least
variant is not considered supported, I think.

> All that said, I don't imagine anybody would notice if the
> ioperm() support in glibc were to go away, and in the unlikely
> event that there is some application somewhere that relies on it,
> it wouldn't be very difficult to patch ioperm() into the
> application itself.  It doesn't interact with anything else
> inside glibc.
>
> Is there a particular reason for wanting to remove it, or is it
> just that it seems like useless cruft we could do without?

It's an internal consumer of the sysctl facility.  The sysctl facility
has been removed on the Linux side and is not enabled by default.  I
looked at the _ioperm code and wondered whether it makes sense to patch
it to issue the system call directly (rather than using the sysctl
wrapper).  But when I started looking where kernel support for the
specific sysctl subtrees comes from, I noticed the Netwinder dependency.

We should get rid of sysctl support because it is really confusing to
developers.  The current attempt at deprecation didn't work out.  Both
RISC-V and C-SKY were added after the supposed deprecation, but
accidentally received a stub implementation of the sysctl function in
glibc.  Even on some older platforms (notably AArch64), there has never
been kernel support.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Phil Blundell-3
On Wed, May 29, 2019 at 04:03:26PM +0200, Florian Weimer wrote:
> There's this bug:
>
>   Remove ARM old-ABI support
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=13556>
>
> I believe this removed core support for non-EABI platforms.

That's not quite the same thing.  The old ABI differed from the EABI
in several ways, including the stack alignment rules and the way
that system calls were done.  I think the bug you mentioned above
was to remove the support in glibc for those specific "old ABI"
mechanisms.

What you currently get if you build glibc for ARMv4 is an "almost-EABI"
configuration which does everything in the same way as the EABI except
that there is no interworking support.  I think we still have a certain
amount of #ifdef __ARM_ARCH_4__ for that purpose.  If we wanted to clean
things up there then removing support for the half-baked V4T interworking
would probably be more of a win since I think that accounts for a larger
amount of #ifdef scar tissue.

> All the target triplets we test upstream have gnueabi in them, and these
> days, anything that is not built by build-many-glibcs.py in at least
> variant is not considered supported, I think.

For the reasons above, even when building glibc for armv4 you still put
"gnueabi" in the triplet.  But I think you're still correct that
build-many-glibcs.py doesn't actually test anything older than ARMv5TE.

> It's an internal consumer of the sysctl facility.

Fair enough.  I would be supportive of removing it, anyway.

p.

Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Adhemerval Zanella-2
In reply to this post by Florian Weimer-5


On 29/05/2019 11:03, Florian Weimer wrote:

> * Phil Blundell:
>
>> On Wed, May 29, 2019 at 02:11:42PM +0200, Florian Weimer wrote:
>>> In the kernel, I only see a call to register_isa_ports in
>>> arch/arm/mach-footbridge/dc21285.c.
>>
>> Right, I think that functionality was only ever supported on
>> DC21285 platforms.  There were more of those than just NetWinder,
>> but they were all StrongARM (ARMv4).
>>
>> You're correct that it isn't practical for an ARMv4 machine to be
>> EABI conformant because the EABI mandates interworking and ARMv4
>> doesn't have BX/BLX, but last time I looked glibc did still have
>> all the right conditional guards in place to allow for compilation
>> on an ARMv4 platform.  I haven't checked recently though and it's
>> possible that it might have bit-rotted.  If we did want to make a
>> policy decision that glibc doesn't support older ARM architectures
>> anymore then there are probably quite a lot of #ifdefs that could
>> be eliminated.
>
> There's this bug:
>
>   Remove ARM old-ABI support
>   <https://sourceware.org/bugzilla/show_bug.cgi?id=13556>
>
> I believe this removed core support for non-EABI platforms.
>
> All the target triplets we test upstream have gnueabi in them, and these
> days, anything that is not built by build-many-glibcs.py in at least
> variant is not considered supported, I think.
>
>> All that said, I don't imagine anybody would notice if the
>> ioperm() support in glibc were to go away, and in the unlikely
>> event that there is some application somewhere that relies on it,
>> it wouldn't be very difficult to patch ioperm() into the
>> application itself.  It doesn't interact with anything else
>> inside glibc.
>>
>> Is there a particular reason for wanting to remove it, or is it
>> just that it seems like useless cruft we could do without?
>
> It's an internal consumer of the sysctl facility.  The sysctl facility
> has been removed on the Linux side and is not enabled by default.  I
> looked at the _ioperm code and wondered whether it makes sense to patch
> it to issue the system call directly (rather than using the sysctl
> wrapper).  But when I started looking where kernel support for the
> specific sysctl subtrees comes from, I noticed the Netwinder dependency.
>
> We should get rid of sysctl support because it is really confusing to
> developers.  The current attempt at deprecation didn't work out.  Both
> RISC-V and C-SKY were added after the supposed deprecation, but
> accidentally received a stub implementation of the sysctl function in
> glibc.  Even on some older platforms (notably AArch64), there has never
> been kernel support.

Also, from previous Cauldron discussion, we should make the deprecation
in longer phases.  It also seems that compiler warning that symbol will
be deprecated in future releases is not a complete way to warn developers
to add configure build options take this consideration (the libgo example
with uname come to my mind). So maybe we use a different strategy for
sysctl, by deprecating in 2/3 releases and make it a compat symbol after
3/4 releases.
Reply | Threaded
Open this post in threaded view
|

[PATCH] Remove ioperm etc. support for arm (was: Re: _ioperm support for Arm)

Florian Weimer-5
In reply to this post by Phil Blundell-3
* Phil Blundell:

> On Wed, May 29, 2019 at 04:03:26PM +0200, Florian Weimer wrote:
>> There's this bug:
>>
>>   Remove ARM old-ABI support
>>   <https://sourceware.org/bugzilla/show_bug.cgi?id=13556>
>>
>> I believe this removed core support for non-EABI platforms.
>
> That's not quite the same thing.  The old ABI differed from the EABI
> in several ways, including the stack alignment rules and the way
> that system calls were done.  I think the bug you mentioned above
> was to remove the support in glibc for those specific "old ABI"
> mechanisms.
>
> What you currently get if you build glibc for ARMv4 is an "almost-EABI"
> configuration which does everything in the same way as the EABI except
> that there is no interworking support.  I think we still have a certain
> amount of #ifdef __ARM_ARCH_4__ for that purpose.  If we wanted to clean
> things up there then removing support for the half-baked V4T interworking
> would probably be more of a win since I think that accounts for a larger
> amount of #ifdef scar tissue.
>
>> All the target triplets we test upstream have gnueabi in them, and these
>> days, anything that is not built by build-many-glibcs.py in at least
>> variant is not considered supported, I think.
>
> For the reasons above, even when building glibc for armv4 you still put
> "gnueabi" in the triplet.  But I think you're still correct that
> build-many-glibcs.py doesn't actually test anything older than ARMv5TE.

For libc.so.6 on arm-linux-gnueabi (as built by build-many-glibcs.py),
readelf shows this:

Attribute Section: aeabi
File Attributes
  Tag_CPU_name: "5T"
  Tag_CPU_arch: v5T
  Tag_ARM_ISA_use: Yes
  Tag_THUMB_ISA_use: Thumb-1
  Tag_ABI_PCS_wchar_t: 4
  Tag_ABI_FP_rounding: Needed
  Tag_ABI_FP_denormal: Needed
  Tag_ABI_FP_exceptions: Needed
  Tag_ABI_FP_user_exceptions: Needed
  Tag_ABI_FP_number_model: IEEE 754
  Tag_ABI_align_needed: 8-byte
  Tag_ABI_align_preserved: 8-byte, except leaf SP
  Tag_ABI_enum_size: int

So it looks like to me like a later architectue version.

I would welcome comments on the commit message.  The patch itself has
been “tested” with build-many-glibcs.py on all 32-bit Arm architectures.

Thanks,
Florian

arm: Remove ioperm/iopl/inb/inw/inl/outb/outw/outl support

Linux only supports the required ISA sysctls on StrongARM devices,
which are armv4 and no longer tested during glibc development
and probably bit-rotted by this point.  (No reported test results,
and the last discussion of armv4 support was in the glibc 2.19
release notes.)

2019-05-29  Florian Weimer  <[hidden email]>

        arm: Remove ioperm/iopl/inb/inw/inl/outb/outw/outl support.
        * sysdeps/unix/sysv/linux/arm/Makefile
        [$(subdir) == misc] (sysdep_headers): Remove sys/io.h.
        * sysdeps/unix/sysv/linux/arm/sys/io.h: Remove file.
        * sysdeps/unix/sysv/linux/arm/ioperm.c: Rewrite file.
        (ioperm, iopl, inb, inw, inl, outb, outw, outl): Turn into
        compatibility symbols.

diff --git a/NEWS b/NEWS
index c885b960ca..31ead24851 100644
--- a/NEWS
+++ b/NEWS
@@ -58,6 +58,9 @@ Deprecated and removed features, and other changes affecting compatibility:
   configurations) has been removed, following the deprecation of this
   subarchitecture in version 8 of GCC, and its removal in version 9.
 
+* On 32-bit Arm, support for the port-based I/O emulation and the <sys/io.h>
+  header have been removed.
+
 Changes to build and runtime requirements:
 
 * GCC 6.2 or later is required to build the GNU C Library.
diff --git a/sysdeps/unix/sysv/linux/arm/Makefile b/sysdeps/unix/sysv/linux/arm/Makefile
index 4adc35de04..d7a2f6a8a7 100644
--- a/sysdeps/unix/sysv/linux/arm/Makefile
+++ b/sysdeps/unix/sysv/linux/arm/Makefile
@@ -5,7 +5,7 @@ endif
 
 ifeq ($(subdir),misc)
 sysdep_routines += ioperm
-sysdep_headers += sys/elf.h sys/io.h
+sysdep_headers += sys/elf.h
 endif
 
 ifeq ($(subdir),signal)
diff --git a/sysdeps/unix/sysv/linux/arm/ioperm.c b/sysdeps/unix/sysv/linux/arm/ioperm.c
index 0e338c4504..e832a6605e 100644
--- a/sysdeps/unix/sysv/linux/arm/ioperm.c
+++ b/sysdeps/unix/sysv/linux/arm/ioperm.c
@@ -17,167 +17,71 @@
    License along with the GNU C Library.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-/* I/O port access on the ARM is something of a fiction.  What we do is to
-   map an appropriate area of /dev/mem into user space so that a program
-   can blast away at the hardware in such a way as to generate I/O cycles
-   on the bus.  To insulate user code from dependencies on particular
-   hardware we don't allow calls to inb() and friends to be inlined, but
-   force them to come through code in here every time.  Performance-critical
-   registers tend to be memory mapped these days so this should be no big
-   problem.  */
+#include <shlib-compat.h>
 
-/* Once upon a time this file used mprotect to enable and disable
-   access to particular areas of I/O space.  Unfortunately the
-   mprotect syscall also has the side effect of enabling caching for
-   the area affected (this is a kernel limitation).  So we now just
-   enable all the ports all of the time.  */
+#if SHLIB_COMPAT (libc, GLIBC_2_4, GLIBC_2_30)
+# include <errno.h>
 
-#include <errno.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <ctype.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-#include <sys/types.h>
-#include <sys/mman.h>
-
-#include <sys/sysctl.h>
-
-#define MAX_PORT 0x10000
-
-static struct {
-  unsigned long int base;
-  unsigned long int io_base;
-  unsigned int shift;
-  unsigned int initdone; /* since all the above could be 0 */
-} io;
-
-#define IO_ADDR(port) (io.base + ((port) << io.shift))
-
-/*
- * Initialize I/O system.  The io_bae and port_shift values are fetched
- * using sysctl (CTL_BUS, CTL_BUS_ISA, ISA_*).
- */
-
-static int
-init_iosys (void)
+int
+ioperm (unsigned long int from, unsigned long int num, int turn_on)
 {
-  static int iobase_name[] = { CTL_BUS, CTL_BUS_ISA, BUS_ISA_PORT_BASE };
-  static int ioshift_name[] = { CTL_BUS, CTL_BUS_ISA, BUS_ISA_PORT_SHIFT };
-  size_t len = sizeof (io.base);
-
-  if (! __sysctl (iobase_name, 3, &io.io_base, &len, NULL, 0)
-      && ! __sysctl (ioshift_name, 3, &io.shift, &len, NULL, 0))
-    {
-      io.initdone = 1;
-      return 0;
-    }
-
-  /* sysctl has failed... */
-  __set_errno (ENODEV);
+  __set_errno (ENOSYS);
   return -1;
 }
+compat_symbol (libc, ioperm, ioperm, GLIBC_2_4);
 
 int
-_ioperm (unsigned long int from, unsigned long int num, int turn_on)
+iopl (unsigned int level)
 {
-  if (! io.initdone && init_iosys () < 0)
-    return -1;
-
-  /* this test isn't as silly as it may look like; consider overflows! */
-  if (from >= MAX_PORT || from + num > MAX_PORT)
-    {
-      __set_errno (EINVAL);
-      return -1;
-    }
-
-  if (turn_on)
-    {
-      if (! io.base)
- {
-  int fd;
-
-  fd = __open ("/dev/mem", O_RDWR);
-  if (fd < 0)
-    return -1;
-
-  io.base =
-    (unsigned long int) __mmap (0, MAX_PORT << io.shift,
- PROT_READ | PROT_WRITE,
- MAP_SHARED, fd, io.io_base);
-  __close (fd);
-  if ((long) io.base == -1)
-    return -1;
- }
-    }
-
-  return 0;
+  __set_errno (ENOSYS);
+  return -1;
 }
+compat_symbol (libc, iopl, iopl, GLIBC_2_4);
 
 
-int
-_iopl (unsigned int level)
-{
-    if (level > 3)
-      {
- __set_errno (EINVAL);
- return -1;
-      }
-    if (level)
-      {
- return _ioperm (0, MAX_PORT, 1);
-      }
-    return 0;
-}
-
+/* The remaining functions do not have any way to indicate failure.
+   However, it is only valid to call them after calling ioperm/iopl,
+   which will have indicated failure.  */
 
 void
-_outb (unsigned char b, unsigned long int port)
+outb (unsigned char b, unsigned long int port)
 {
-  *((volatile unsigned char *)(IO_ADDR (port))) = b;
 }
-
+compat_symbol (libc, outb, outb, GLIBC_2_4);
 
 void
-_outw (unsigned short b, unsigned long int port)
+outw (unsigned short b, unsigned long int port)
 {
-  *((volatile unsigned short *)(IO_ADDR (port))) = b;
 }
-
+compat_symbol (libc, outw, outw, GLIBC_2_4);
 
 void
-_outl (unsigned int b, unsigned long int port)
+outl (unsigned int b, unsigned long int port)
 {
-  *((volatile unsigned long *)(IO_ADDR (port))) = b;
 }
-
+compat_symbol (libc, outl, outl, GLIBC_2_4);
 
 unsigned int
-_inb (unsigned long int port)
+inb (unsigned long int port)
 {
-  return *((volatile unsigned char *)(IO_ADDR (port)));
+  return 0;
 }
+compat_symbol (libc, inb, inb, GLIBC_2_4);
 
 
 unsigned int
-_inw (unsigned long int port)
+inw (unsigned long int port)
 {
-  return *((volatile unsigned short *)(IO_ADDR (port)));
+  return 0;
 }
+compat_symbol (libc, inw, inw, GLIBC_2_4);
 
 
 unsigned int
-_inl (unsigned long int port)
+inl (unsigned long int port)
 {
-  return *((volatile unsigned long *)(IO_ADDR (port)));
+  return 0;
 }
+compat_symbol (libc, inl, inl, GLIBC_2_4);
 
-weak_alias (_ioperm, ioperm);
-weak_alias (_iopl, iopl);
-weak_alias (_inb, inb);
-weak_alias (_inw, inw);
-weak_alias (_inl, inl);
-weak_alias (_outb, outb);
-weak_alias (_outw, outw);
-weak_alias (_outl, outl);
+#endif /* SHLIB_COMAT */
diff --git a/sysdeps/unix/sysv/linux/arm/sys/io.h b/sysdeps/unix/sysv/linux/arm/sys/io.h
deleted file mode 100644
index 6f5ae5079a..0000000000
--- a/sysdeps/unix/sysv/linux/arm/sys/io.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/* Copyright (C) 1996-2019 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/>.  */
-
-#ifndef _SYS_IO_H
-
-#define _SYS_IO_H 1
-#include <features.h>
-
-__BEGIN_DECLS
-
-/* If TURN_ON is TRUE, request for permission to do direct i/o on the
-   port numbers in the range [FROM,FROM+NUM-1].  Otherwise, turn I/O
-   permission off for that range.  This call requires root privileges.  */
-extern int ioperm (unsigned long int __from, unsigned long int __num,
-   int __turn_on) __THROW;
-
-/* Set the I/O privilege level to LEVEL.  If LEVEL is nonzero,
-   permission to access any I/O port is granted.  This call requires
-   root privileges. */
-extern int iopl (int __level) __THROW;
-
-/* The functions that actually perform reads and writes.  */
-extern unsigned char inb (unsigned long int __port) __THROW;
-extern unsigned short int inw (unsigned long int __port) __THROW;
-extern unsigned long int inl (unsigned long int __port) __THROW;
-
-extern void outb (unsigned char __value, unsigned long int __port) __THROW;
-extern void outw (unsigned short __value, unsigned long int __port) __THROW;
-extern void outl (unsigned long __value, unsigned long int __port) __THROW;
-
-__END_DECLS
-
-#endif /* _SYS_IO_H */
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Florian Weimer-5
In reply to this post by Adhemerval Zanella-2
* Adhemerval Zanella:

> Also, from previous Cauldron discussion, we should make the deprecation
> in longer phases.  It also seems that compiler warning that symbol will
> be deprecated in future releases is not a complete way to warn developers
> to add configure build options take this consideration (the libgo example
> with uname come to my mind). So maybe we use a different strategy for
> sysctl, by deprecating in 2/3 releases and make it a compat symbol after
> 3/4 releases.

I think the ustat business was due to the fact that we did *not* issue a
warning beforehand.  Like sysctl, the deprecation is mainly captured in
the manual page for the system call.

The linker warnings happen only on some architectures right now (which
use the “generic” Linux system call ABI).

Once we've settled the ioperm matter for Arm, I will try to come up with
a simple patch that adds deprecation warnings for sysctl.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Adhemerval Zanella-2


On 29/05/2019 11:57, Florian Weimer wrote:

> * Adhemerval Zanella:
>
>> Also, from previous Cauldron discussion, we should make the deprecation
>> in longer phases.  It also seems that compiler warning that symbol will
>> be deprecated in future releases is not a complete way to warn developers
>> to add configure build options take this consideration (the libgo example
>> with uname come to my mind). So maybe we use a different strategy for
>> sysctl, by deprecating in 2/3 releases and make it a compat symbol after
>> 3/4 releases.
>
> I think the ustat business was due to the fact that we did *not* issue a
> warning beforehand.  Like sysctl, the deprecation is mainly captured in
> the manual page for the system call.

Right, this was raised on Cauldron as well. GCC developers also asked to
extend the deprecation phase a bit longer if I recall correctly.

>
> The linker warnings happen only on some architectures right now (which
> use the “generic” Linux system call ABI).

This is standard ENOSYS stub we emit for non-implemented symbols. I think
we will need both add __attribute_deprecated__ and create a new stub with
a different warning message (maybe "symbol is deprecated and it will be
removed in future versions").

>
> Once we've settled the ioperm matter for Arm, I will try to come up with
> a simple patch that adds deprecation warnings for sysctl.
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm (was: Re: _ioperm support for Arm)

Aaro Koskinen
In reply to this post by Florian Weimer-5
Hi,

On Wed, May 29, 2019 at 04:53:46PM +0200, Florian Weimer wrote:
> For libc.so.6 on arm-linux-gnueabi (as built by build-many-glibcs.py),
> readelf shows this:
>
> Attribute Section: aeabi
> File Attributes
>   Tag_CPU_name: "5T"
>   Tag_CPU_arch: v5T

Doesn't this depend on what is the default in the toolchain (GCC)
being used? Looking at this script it enforces arch for some variants,
but not for plain arm-linux-gnueabi.

A.
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Florian Weimer-5
In reply to this post by Adhemerval Zanella-2
* Adhemerval Zanella:

[sysctl deprecation]

>> The linker warnings happen only on some architectures right now (which
>> use the “generic” Linux system call ABI).
>
> This is standard ENOSYS stub we emit for non-implemented symbols. I think
> we will need both add __attribute_deprecated__ and create a new stub with
> a different warning message (maybe "symbol is deprecated and it will be
> removed in future versions").

We have an explicit sysdeps/unix/sysv/linux/generic/sysctl.c override,
though.  I don't know why it was done this way.  I think it predates the
attempt to deprecate sysctl with the addition of the x32 ABI.

Anyway, it doesn't matter for the upcoming deprecation.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm

Florian Weimer-5
In reply to this post by Aaro Koskinen
* Aaro Koskinen:

> Hi,
>
> On Wed, May 29, 2019 at 04:53:46PM +0200, Florian Weimer wrote:
>> For libc.so.6 on arm-linux-gnueabi (as built by build-many-glibcs.py),
>> readelf shows this:
>>
>> Attribute Section: aeabi
>> File Attributes
>>   Tag_CPU_name: "5T"
>>   Tag_CPU_arch: v5T
>
> Doesn't this depend on what is the default in the toolchain (GCC)
> being used?

Yes, I think so.

> Looking at this script it enforces arch for some variants, but not for
> plain arm-linux-gnueabi.

Has GCC ever changed default here?

Maybe we used to test ARMv4, and GCC changed under us without us
noticing?

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm

Aaro Koskinen
Hi,

On Wed, May 29, 2019 at 08:46:43PM +0200, Florian Weimer wrote:

> > On Wed, May 29, 2019 at 04:53:46PM +0200, Florian Weimer wrote:
> >> For libc.so.6 on arm-linux-gnueabi (as built by build-many-glibcs.py),
> >> readelf shows this:
> >>
> >> Attribute Section: aeabi
> >> File Attributes
> >>   Tag_CPU_name: "5T"
> >>   Tag_CPU_arch: v5T
> >
> > Doesn't this depend on what is the default in the toolchain (GCC)
> > being used?
>
> Yes, I think so.
>
> > Looking at this script it enforces arch for some variants, but not for
> > plain arm-linux-gnueabi.
>
> Has GCC ever changed default here?

Interesting, it seems that v5t is indeed what GCC defaults to. I thought
it would have been the lowest one currently supported (v4t).

> Maybe we used to test ARMv4, and GCC changed under us without us
> noticing?

Don't know if they have ever changed that, but it would probably make
sense to add flags to build-many-glibcs.py to build for the lowest
supported arch level.

ARMv4 is already deprecated in GCC, so probably it's OK to remove this
stuff from glibc. Personally I'm concerned about armv4t support as I'm
running such systems still with modern glibc.

A.
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Adhemerval Zanella-2
In reply to this post by Florian Weimer-5


On 29/05/2019 15:44, Florian Weimer wrote:

> * Adhemerval Zanella:
>
> [sysctl deprecation]
>
>>> The linker warnings happen only on some architectures right now (which
>>> use the “generic” Linux system call ABI).
>>
>> This is standard ENOSYS stub we emit for non-implemented symbols. I think
>> we will need both add __attribute_deprecated__ and create a new stub with
>> a different warning message (maybe "symbol is deprecated and it will be
>> removed in future versions").
>
> We have an explicit sysdeps/unix/sysv/linux/generic/sysctl.c override,
> though.  I don't know why it was done this way.  I think it predates the
> attempt to deprecate sysctl with the addition of the x32 ABI.

My understanding is the linux/generic is aiming to mimic the current Linux
UAPI, so a new port would require to just add the folder on its Implies
folder.

>
> Anyway, it doesn't matter for the upcoming deprecation.

I would suggest to add a new stub, stub_deprecate, which similar to
stub_warning emit a linker message when the symbol is linked to state
is not safe to rely on it and most likely it will be removed in future
glibc version.  Combined with __attribute_deprecated__ it should give
users enough warnings to fix it.

What I am not sure is for newer ports, do we really need to export
the symbol as ENOSYS?
 
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm (was: Re: _ioperm support for Arm)

Phil Blundell-3
In reply to this post by Florian Weimer-5
On Wed, May 29, 2019 at 04:53:46PM +0200, Florian Weimer wrote:
> So it looks like to me like a later architectue version.

Yes, right.  build-many-glibcs.py seems to explicitly call out ARMv5TE and
later, and it also builds at least one library with gcc's default
architecture settings (which you get to choose at the point you build
the compiler, and I think the default defaults have also changed at
various points in the past).  But there's no explicit selection of
anything older than ARMv5TE so I think we should assume it isn't getting
tested.

> I would welcome comments on the commit message.  The patch itself has
> been “tested” with build-many-glibcs.py on all 32-bit Arm architectures.

The commit message looks fine to me, or at least I couldn't think of
anything better to write.

>  void
> -_outb (unsigned char b, unsigned long int port)
> +outb (unsigned char b, unsigned long int port)

Does this cause symbols that were previously weak to become strong?  If
so, is that ok?  It would be a bit unfortunate if the act of deprecating
these functions caused them to suddenly start interposing something else,
or do symbol versions prevent that from happening?

>  unsigned int
> -_inl (unsigned long int port)
> +inl (unsigned long int port)
>  {
> -  return *((volatile unsigned long *)(IO_ADDR (port)));
> +  return 0;
>  }

The outcome of calling inl() etc without a previous successful call to
ioperm() would have been SIGSEGV, so it would be ok to replace this
with *((volatile unsigned long *)0).  But what you have is fine too.

p.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm

Florian Weimer-5
* Phil Blundell:

>>  void
>> -_outb (unsigned char b, unsigned long int port)
>> +outb (unsigned char b, unsigned long int port)
>
> Does this cause symbols that were previously weak to become strong?

Yes, it does.

> If so, is that ok?  It would be a bit unfortunate if the act of
> deprecating these functions caused them to suddenly start interposing
> something else, or do symbol versions prevent that from happening?

The SHLIB_COMPAT check returns false for static builds, so nothing ends
up in libc.a after this change.

The weak/strong alias difference is not supposed to matter for dynamic
linking.  (glibc used to be buggy, and there is the LD_DYNAMIC_WEAK
override to get the old behavior, but I haven't heard of anyone using
it.)  It's also hard to get the dynamic linker to search beyond
libc.so.6 in a different DSO because libc.so.6 obviously does not have a
DT_NEEDED dependency on some application-provided DSO.

>>  unsigned int
>> -_inl (unsigned long int port)
>> +inl (unsigned long int port)
>>  {
>> -  return *((volatile unsigned long *)(IO_ADDR (port)));
>> +  return 0;
>>  }
>
> The outcome of calling inl() etc without a previous successful call to
> ioperm() would have been SIGSEGV, so it would be ok to replace this
> with *((volatile unsigned long *)0).  But what you have is fine too.

Thanks.

Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for arm

Florian Weimer-5
In reply to this post by Aaro Koskinen
* Aaro Koskinen:

>> Maybe we used to test ARMv4, and GCC changed under us without us
>> noticing?
>
> Don't know if they have ever changed that, but it would probably make
> sense to add flags to build-many-glibcs.py to build for the lowest
> supported arch level.

Indeed.

> ARMv4 is already deprecated in GCC, so probably it's OK to remove this
> stuff from glibc. Personally I'm concerned about armv4t support as I'm
> running such systems still with modern glibc.

Interesting.  Could you perhaps suggest changes to build-many-glibcs.py
so that we can build an armv4t target as part of the regular runs?

(And I assume you aren't interested in ioperm support anymore. 8-)

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Remove ioperm etc. support for army

Phil Blundell-3
In reply to this post by Florian Weimer-5
On Wed, May 29, 2019 at 11:10:32PM +0200, Florian Weimer wrote:
> The SHLIB_COMPAT check returns false for static builds, so nothing ends
> up in libc.a after this change.
>
> The weak/strong alias difference is not supposed to matter for dynamic
> linking.

Right, makes sense.  In that case, I think this patch is OK.

Thanks

Phil

Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Florian Weimer-5
In reply to this post by Adhemerval Zanella-2
* Adhemerval Zanella:

> On 29/05/2019 15:44, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>> [sysctl deprecation]
>>
>>>> The linker warnings happen only on some architectures right now (which
>>>> use the “generic” Linux system call ABI).
>>>
>>> This is standard ENOSYS stub we emit for non-implemented symbols. I think
>>> we will need both add __attribute_deprecated__ and create a new stub with
>>> a different warning message (maybe "symbol is deprecated and it will be
>>> removed in future versions").
>>
>> We have an explicit sysdeps/unix/sysv/linux/generic/sysctl.c override,
>> though.  I don't know why it was done this way.  I think it predates the
>> attempt to deprecate sysctl with the addition of the x32 ABI.
>
> My understanding is the linux/generic is aiming to mimic the current Linux
> UAPI, so a new port would require to just add the folder on its Implies
> folder.

Right, and the sysctl stub there looks like a mistake in retrospect.

>> Anyway, it doesn't matter for the upcoming deprecation.
>
> I would suggest to add a new stub, stub_deprecate, which similar to
> stub_warning emit a linker message when the symbol is linked to state
> is not safe to rely on it and most likely it will be removed in future
> glibc version.  Combined with __attribute_deprecated__ it should give
> users enough warnings to fix it.

Interesting idea.  Maybe not stub_deprecate but
deprecated_linker_warning or something like that.  We already have
something like that form mktemp.

Is it possible to disable these warnings for people who do not want to
see them?  If they cannot be disabled, that would be an argument against
adding them.

> What I am not sure is for newer ports, do we really need to export
> the symbol as ENOSYS?

No, not really.  The removal with the compat symbol mechanism will take
care of that automatically for architectures with symbol baselines after
the compat symbol activation.

Until that, for new ports, we should just add an empty sysctl.c file.
(The x32 hack is unnecessarily complicated.)  Unfortunately, we missed
doing this for C-SKY at all.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Adhemerval Zanella-2


On 29/05/2019 18:17, Florian Weimer wrote:

> * Adhemerval Zanella:
>
>> On 29/05/2019 15:44, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>> [sysctl deprecation]
>>>
>>>>> The linker warnings happen only on some architectures right now (which
>>>>> use the “generic” Linux system call ABI).
>>>>
>>>> This is standard ENOSYS stub we emit for non-implemented symbols. I think
>>>> we will need both add __attribute_deprecated__ and create a new stub with
>>>> a different warning message (maybe "symbol is deprecated and it will be
>>>> removed in future versions").
>>>
>>> We have an explicit sysdeps/unix/sysv/linux/generic/sysctl.c override,
>>> though.  I don't know why it was done this way.  I think it predates the
>>> attempt to deprecate sysctl with the addition of the x32 ABI.
>>
>> My understanding is the linux/generic is aiming to mimic the current Linux
>> UAPI, so a new port would require to just add the folder on its Implies
>> folder.
>
> Right, and the sysctl stub there looks like a mistake in retrospect.
>
>>> Anyway, it doesn't matter for the upcoming deprecation.
>>
>> I would suggest to add a new stub, stub_deprecate, which similar to
>> stub_warning emit a linker message when the symbol is linked to state
>> is not safe to rely on it and most likely it will be removed in future
>> glibc version.  Combined with __attribute_deprecated__ it should give
>> users enough warnings to fix it.
>
> Interesting idea.  Maybe not stub_deprecate but
> deprecated_linker_warning or something like that.  We already have
> something like that form mktemp.

We could use link_warning instead, deprecated_linker_warning would be
just a easier way (which concatenate the name to pre-defined message).

>
> Is it possible to disable these warnings for people who do not want to
> see them?  If they cannot be disabled, that would be an argument against
> adding them.

I am not aware of any way to disable .gnu.warning. on ld, neither I could
find a way glancing binutils source code.  However I do not consider this
an argument against because the object will still be created by binutils
in the end.

>
>> What I am not sure is for newer ports, do we really need to export
>> the symbol as ENOSYS?
>
> No, not really.  The removal with the compat symbol mechanism will take
> care of that automatically for architectures with symbol baselines after
> the compat symbol activation.
>
> Until that, for new ports, we should just add an empty sysctl.c file.
> (The x32 hack is unnecessarily complicated.)  Unfortunately, we missed
> doing this for C-SKY at all.

Right, I think we have plan then.  How should we proceed regarding
deprecation warning and finally making it a compat symbol? Maybe 2 releases
with the warning and then finally moving to compat?
Reply | Threaded
Open this post in threaded view
|

Re: _ioperm support for Arm

Joseph Myers
In reply to this post by Phil Blundell-3
On Wed, 29 May 2019, Phil Blundell wrote:

> You're correct that it isn't practical for an ARMv4 machine to be
> EABI conformant because the EABI mandates interworking and ARMv4
> doesn't have BX/BLX, but last time I looked glibc did still have
> all the right conditional guards in place to allow for compilation
> on an ARMv4 platform.  I haven't checked recently though and it's

The EABI fully supports v4t (though I haven't tested glibc on v4t hardware
for a few years), but not v4.  There are two ways in which there can be
some limited support for the EABI on v4:

* --fix-v4bx can be passed to the assembler and linker (and GCC does so
automatically for v4).  When passed to the assembler, it results in
R_ARM_V4BX relocations on BX instructions; when passed to the linker, it
results in those instructions being rewritten into a form that works on v4
(so the .o files are interworking-safe but can also be used on v4 if the
linker option is used).  This is mainly useful in bare-metal contexts with
static linking only; it's not so good in a dynamic linking context where
linked Arm code in an executable might later get run with Thumb code in a
shared library on a newer processor.

* With --fix-v4bx-interworking passed to the linker, BX instructions
(marked with R_ARM_V4BX) are converted to branch to veneers (with the
caveats in the linker manual about how that may clobber condition flags
that aren't meant to be clobbered by branches, so isn't strictly
EABI-conforming).  This allows executables and shared libraries to be
produced that work on v4 and are interworking-safe on v4t.  This option
was added because there was some desire at the time for distributions -
the Debian EABI port, at least - to be able to work on v4 hardware while
still using the EABI, but I'm not sure if anyone ever actually made
serious use of it in the end.

The conditionals in glibc's sysdeps/arm/sysdep.h, and consequent BX macro,
do something simpler: allow code to build for v4 but without any attempt
to make it interworking-safe on later architecture versions.  It would not
surprise me at all if that support for almost-EABI-but-not-interworking
code supporting v4 is in fact bitrotten, and I've never tried to build
such a configuration at all.  I think removing the attempt to support v4
(as opposed to v4t) would be reasonable.

--
Joseph S. Myers
[hidden email]
12