[PATCH 1/1] Initial support for OpenRISC

Stafford Horne shorne@gmail.com
Fri May 22 21:31:23 GMT 2020


Hello,

Thanks for the review.

On Fri, May 22, 2020 at 08:56:08PM +0000, Joseph Myers wrote:
> On Fri, 22 May 2020, Stafford Horne via Libc-alpha wrote:
> 
> > This patch includes the OpenRISC glibc support for linux.
> 
> This is missing a build-many-glibcs.py update.  You always need to include 
> such an update with a new port, which must build at least one 
> configuration for each ABI supported by the port (and may build any major 
> non-ABI variants that seem relevant).  build-many-glibcs.py results for 
> the port must be clean (i.e. no failures in the compilation part of the 
> testsuite), at least when building with GCC and binutils master (it's OK 
> if you need features or bug fixes not in the most recent release branches, 
> although distributors might prefer such fixes to be backported).  
> Similarly, there should be NEWS and README updates.

OK.

> New 32-bit ports should preferably be set up to support only 64-bit time 
> and 64-bit offsets (see ARC and RV32 patch postings for examples).

OK.

> > diff --git a/sysdeps/or1k/bits/atomic.h b/sysdeps/or1k/bits/atomic.h
> 
> There should be no such file.  bits/ is only for installed headers, not 
> internal ones.  See commit de071d199a8578055edf2722114788ae749823aa ("Move 
> bits/atomic.h to atomic-machine.h (bug 14912).", 11 Sep 2015).

OK.

> > diff --git a/sysdeps/or1k/nptl/bits/semaphore.h b/sysdeps/or1k/nptl/bits/semaphore.h
> 
> Do you really need this file?  See commit 
> 1270fbaaeebe642db335fccaaf98c82e6647cc0d ("semaphore: consolidate arch 
> headers into a generic one", 5 May 2020).

Likely, not, I will review.

> > +  # Needed for relro detection
> > +  libc_commonpagesize=0x2000
> > +  libc_relro_required=yes
> 
> See commit cb403c34c6f6e1cce5018864485958cfc2e28906 ("Remove relro 
> configure test.", 27 Jun 2014).

OK.

> When you have an old out-of-tree port, it's a good idea to go through 
> cross-architecture commits from when the port was first created onwards to 
> identify such global changes that need to be applied to that port.

I did try to do this, but it started in 2014 it was hard to pick up everything.
I also payed close attention to the riscv and csky ports.

> > diff --git a/sysdeps/unix/sysv/linux/or1k/bits/mman.h b/sysdeps/unix/sysv/linux/or1k/bits/mman.h
> 
> You shouldn't have this file.  See the comment in 
> sysdeps/unix/sysv/linux/bits/mman.h (and commit 
> d3a43e49f342c4663af0fff9d95000cfe26867c3, "Unify many bits/mman.h 
> headers.", 18 Sep 2018, and commit 
> 61d8b5feeed36e242a043befe9b11f7f8c294f58, "Share MAP_* flags between more 
> architectures.", 26 Sep 2018):
> 
> /* These definitions are appropriate for architectures that, in the
>    Linux kernel, either have no uapi/asm/mman.h, or have one that
>    includes asm-generic/mman.h without any changes or additions
>    relevant to glibc.  If there are additions relevant to glibc, an
>    architecture-specific bits/mman.h is needed.  */

OK.

> > diff --git a/sysdeps/unix/sysv/linux/or1k/configure.ac b/sysdeps/unix/sysv/linux/or1k/configure.ac
> > new file mode 100644
> > index 0000000000..505530d5c3
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/or1k/configure.ac
> > @@ -0,0 +1,4 @@
> > +GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
> > +# Local configure fragment for sysdeps/unix/sysv/linux/or1k.
> > +
> > +arch_minimum_kernel=3.4.0
> 
> You'll probably need a newer minimum kernel when requiring 64-bit time 
> support, until all the fallback for 64-bit time on 32-bit kernels without 
> the 64-bit-time syscalls is implemented.

OK, noted.

> > +#undef __ASSUME_SET_ROBUST_LIST
> 
> Are you sure this is never supported on this architecture?  
> arch/openrisc/include/asm/futex.h appears to have gained 
> futex_atomic_cmpxchg_inatomic in 4.11, so I'd expect the #undef only to be 
> for kernels older than that.

Sorry, I overlooked this thanks for catching.  I added the atomic support to the
kernel.

> > diff --git a/sysdeps/unix/sysv/linux/or1k/sys/procfs.h b/sysdeps/unix/sysv/linux/or1k/sys/procfs.h
> 
> See commit d62f9ec0cce26a275ec68f4564814041a33395b1 ("Complete 
> sys/procfs.h unification.", 25 Sep 2018).  You shouldn't have an 
> architecture-specific version of this file; add whatever bits/ headers are 
> needed instead.

OK.

> In general, each architecture-specific file needs a justification that 
> either no generic version exists or the generic one is unsuitable or 
> suboptimal for this architecture; the default is that we want to use 
> unified implementations of as many files as possible.

This has also been my goal for OpenRISC (use as much generic code as possible),
but when going over the original port I missed some of these.  I noticed in the
last few months since I started fixing up the port that many more architecture
specific parts were moved to generic code and I did those moves.  I seem to
missed everything from before 2019 when I started the port.

Thank you for pointing these out.

I will work on cleaning this up, testing and posting a new version in a week or
two.

-Stafford


More information about the Libc-alpha mailing list