|
Message-ID: <20160203233657.GL9349@brightrain.aerifal.cx> Date: Wed, 3 Feb 2016 18:36:57 -0500 From: Rich Felker <dalias@...c.org> To: Mahesh Bodapati <Mahesh.Bodapati@...tec.com> Cc: Szabolcs Nagy <nsz@...t70.net>, Jaydeep Patil <Jaydeep.Patil@...tec.com>, Anand Takale <Anand.Takale@...tec.com>, musl@...ts.openwall.com Subject: Re: mips n64 porting review On Wed, Feb 03, 2016 at 03:41:13PM +0000, Mahesh Bodapati wrote: > Hi Rich, > I have attached the patch which has all the MIPS n64 porting work. I > have created mips64port remote branch on GitHub and the repository > is https://github.com/MaheshBodapati/musl/tree/mips64port which has > the broken down patches and the base revision on which I have > prepared patch is v1.1.12-41-g3abb094. Some preliminary review: The relocation support commit still needs a lot of work: https://github.com/MaheshBodapati/musl/commit/55147891bc0caf757bb0e00a81d550aadc71750c The good news is, I think it's mostly -'s that it needs. I was puzzled at first by all the Elf64_Mips_Rel[a] stuff, and found it seems to be have been copied verbatim from uClibc source (and possibly indirectly from glibc) which would be a big problem, but I don't think it's needed anyway. Here's the relevant comment: https://git.uclibc.org/uClibc/tree/ldso/ldso/mips/dl-sysdep.h?id=966adfe8ed0364ae32cc80d375d736298972d031#n13 In short, somebody's mental model of how MIPS64 relocations work (and this might actually have relevance for relocations used in .o files) was that each relocation slot stores up to 3 actual relocations. In reality, however, the only place where "more than one relocation" appears in the same lot for dynamic relocations is R_MIPS_REL32 followed by R_MIPS_64, which you're actually treating as one relocation: #define REL_SYM_OR_REL 4611 /* (R_MIPS_64 << 8) | R_MIPS_REL32 */ and this latter treatment makes a whole lot more sense. In short, I think you could remove all of the Elf64_Mips_Rel[a] stuff and the associated #ifdef __mips64 and just use the standard ELF reloc processing code paths, and everything should work. If not, it should only be minor details keeping it from working and it should be easy to fix them in a cleaner way. Second thing I noticed was the invasive way you're dealing with the broken kernel stat structure for mips. This should be done the same way it's done on 32-bit mips, with the fixup function in syscall_arch.h. Third, I suspect socket.h needs to be checked. The kernel usually has the wrong types for 64-bit archs, and some endian-specific padding is required to work around it. See how x86_64 and aarch64 do it. Fourth, I suspect ksigaction.h is mildly wrong ([4] should be [2]) and this is why the #ifdefs to use _NSIG/8 instead of sizeof were needed. Fixing that should eliminate the need to change non-arch-specific files. Fifth, I don't see why there's a mips64-specific definition for RLIM_INFINITY; it's the same value as the generic definition. There are a couple other things that you've got #ifdefs for that are probably a matter of code that's changed upstream: pthread_cancel.c should be using the MC_PC macro, which pthread_arch.h should define, to access the member of mcontext_t containing the program counter. The definition from 32-bit mips should work. A recent commit also replaced the DYNAMIC_IS_RO macro with DT_DEBUG_INDIRECT which was part of making gdb work right on mips. I suspect the same solution works on mips64. The bits deduplication patch removed the need for a lot of the bits headers. The ones that are no longer needed should be left out when we merge. The .sub system for subarchs (soft float) was removed and replaced by support for .S and .c files in arch dirs, so this also needs to be updated when merging. I haven't gotten a chance to review everything in full detail yet but that should be the bulk of what needs to be changed. I'll follow up with anything else I find. Thanks for all your work on this! Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.