Re: [RFC PATCH v3 07/12] C-SKY: Linux ABI

[Mao Han <han_mao at c-sky dot com>]

On Fri, Jun 29, 2018 at 04:53:23PM +0000, Joseph Myers wrote:
> On Fri, 29 Jun 2018, Mao Han wrote:
> 
> > +/* ipcs ctl commands */
> > +# define SHM_STAT 	13
> > +# define SHM_INFO 	14
> 
> This is out of date (missing SHM_STAT_ANY).  You need to watch out for 
> global changes to many or all ports (any such changes since the glibc 
> version with which you started your port) and make sure you keep 
> architecture-specific headers up to date with such changes.
>

OK. I'll watch out for those changes.
 
> > diff --git a/sysdeps/unix/sysv/linux/csky/shlib-versions b/sysdeps/unix/sysv/linux/csky/shlib-versions
> > new file mode 100644
> > index 0000000..15d5d7e
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/csky/shlib-versions
> > @@ -0,0 +1,9 @@
> > +DEFAULT			GLIBC_2.28
> > +
> > +%if CSKYABI == 2 && CSKY_HARD_FLOAT == 0
> > +ld=ld-linux-cskyv2-sfp-le.so.1
> > +%elif CSKYABI == 2 && CSKY_HARD_FLOAT == 1
> > +ld=ld-linux-cskyv2-hfp-le.so.1
> > +%else
> > +%error cannot determine ABI
> > +%endif
> 
> Since you support both BE and LE (conditionals elsewhere providing 
> substantive support for both rather than just errors for BE), I'd expect 
> BE dynamic linker names as well.

I was intended to support big endian in the patchset, but my colleage
who is responsible for linux do not have this on his short term plan.
I'll remove all the big endian support in the next patch.
> 
> > diff --git a/sysdeps/unix/sysv/linux/csky/sigcontextinfo.h b/sysdeps/unix/sysv/linux/csky/sigcontextinfo.h
> > new file mode 100644
> > index 0000000..a9f55f6
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/csky/sigcontextinfo.h
> 
> See my commit a44061398c3b531b37e134a6a97accb2251fa28a.  There are macros 
> in this header I'd expect to be removed (again, review many-ports changes 
> since you started the port to make sure you are up to date with such 
> changes).

OK. I'll watch out for those changes.
> 
> > +typedef struct
> > +  {
> > +    unsigned long __ctx(tls);
> > +    unsigned long __ctx(lr);
> > +    unsigned long __ctx(pc);
> > +    unsigned long __ctx(sr);
> > +    unsigned long __ctx(usp);
> 
> As a new port I'd like to suggest you name all your ucontext fields with 
> leading __ unconditionally, so you don't need __ctx to make the names 
> depend on __USE_MISC.

OK
> 
> > +struct user_regs {
> > +#if defined(__ck807__) || defined(__ck810__) || defined(__ck860__)
> > +  unsigned long int uregs[34];	/* CSKY V2 has 32 general rgister */
> > +#else
> > +  unsigned long int uregs[18];	/* CSKY V1 has 16 general rgister */
> 
> "register", missing "." (and second space) at end of comment.  Is V1 
> something different from ABIv1 which isn't being supported initially?

I'll check all the comment to make sure they meet the convention.
Although, some of the cpu under abiv2 got only 16 general registers,
they are not design for linux system. So only this line will be needed.
unsigned long int uregs[34];    /* CSKY V2 has 32 general registers.  */

Best Regards,
Mao Han