From: Jaydeep Patil <Jaydeep.Patil@imgtec.com>
To: Rich Felker <dalias@libc.org>
Cc: Szabolcs Nagy <nsz@port70.net>,
"musl@lists.openwall.com" <musl@lists.openwall.com>,
Andre McCurdy <armccurdy@gmail.com>
Subject: RE: [MUSL] microMIPS32R2 O32 port
Date: Tue, 25 Apr 2017 04:45:29 +0000 [thread overview]
Message-ID: <BD7773622145634B952E5B54ACA8E349DAE2D9AB@PUMAIL01.pu.imgtec.org> (raw)
In-Reply-To: <20170424134808.GQ17319@brightrain.aerifal.cx>
>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>Sent: 24 April 2017 PM 07:18
>To: Jaydeep Patil
>Cc: Szabolcs Nagy; musl@lists.openwall.com; Andre McCurdy
>Subject: Re: [musl] [MUSL] microMIPS32R2 O32 port
>
>On Mon, Apr 24, 2017 at 05:30:34AM +0000, Jaydeep Patil wrote:
>> >-----Original Message-----
>> >From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>> >Sent: 21 April 2017 PM 07:03
>> >To: Jaydeep Patil
>> >Cc: Szabolcs Nagy; musl@lists.openwall.com; Andre McCurdy
>> >Subject: Re: [musl] [MUSL] microMIPS32R2 O32 port
>> >
>> >On Thu, Apr 13, 2017 at 10:37:05AM +0000, Jaydeep Patil wrote:
>> >> Hi Szabolcs,
>> >>
>> >> Please find the attached patch.
>> >
>> >I still don't follow the motivation of all the changes in this patch.
>> >See comments below:
>> >
>> >> [...]
>> >> diff --git a/arch/mips/crt_arch.h b/arch/mips/crt_arch.h index
>> >> 9fc50d7..78832b0 100644
>> >> --- a/arch/mips/crt_arch.h
>> >> +++ b/arch/mips/crt_arch.h
>> >> @@ -1,6 +1,7 @@
>> >> __asm__(
>> >> ".set push\n"
>> >> ".set noreorder\n"
>> >> +".set nomicromips\n"
>> >> ".text \n"
>> >> ".global _" START "\n"
>> >> ".global " START "\n"
>> >
>> >Is there a need for crt1.o (or other users of this file like
>> >dlstart/rcrt1) to be built as plain mips rather than micromips? If
>> >so, please explain. Arbitrary changes like this without an
>> >explanation of why they're being made (what was broken before, and
>> >why this is the correct fix) are not acceptable.
>>
>> The crt1.o and __cp_begin are built as pure MIPS functions rather than
>> microMIPS as they are reading data using $ra register (with ISA bit
>> set). We can clear the ISA bit (using INS) and choose to compile these
>> functions as microMIPS.
>
>I see. So all the .s files are assembled as plain (32-bit) MIPS, and these files are
>just affected because the asm is included in .c files that might be compiled in
>microMIPS mode. That makes sense. I think it's probably just better to make
>the code microMIPS-compatible though by clearing the low bits.
Ok, I will remove the "nomicromips" restriction and clear the ISA bit instead.
> But syscall_cp.s needs some care because saved instruction pointer values are
>compared against these labels. In micromips mode, do the labels evaluate
>with the +1 low bit offset?
Yes, in microMIPS mode, ISA bit (0th bit) is set for labels. However I don't see any issue with following comparison
pc >= (uintptr_t)__cp_begin && pc < (uintptr_t)__cp_end
The ISA bit will be set even for PC in the saved context.
>> >> diff --git a/arch/mips/reloc.h b/arch/mips/reloc.h index
>> >> b3d59a4..772b3aa 100644
>> >> --- a/arch/mips/reloc.h
>> >> +++ b/arch/mips/reloc.h
>> >> @@ -36,15 +36,23 @@
>> >> #define CRTJMP(pc,sp) __asm__ __volatile__( \
>> >> "move $sp,%1 ; jr %0" : : "r"(pc), "r"(sp) : "memory" )
>> >>
>> >> +/*
>> >> + * When compiled for microMIPS, .align makes sure that .gpword
>> >> + * is placed at word boundary. $ra must point to first .gpword.
>> >> + * ISA bit of $ra must be cleared for microMIPS before using it
>> >> + * as a base address. For MIPS, ISA bit is always zero.
>> >> +*/
>> >> #define GETFUNCSYM(fp, sym, got) __asm__ ( \
>> >> ".hidden " #sym "\n" \
>> >> ".set push \n" \
>> >> ".set noreorder \n" \
>> >> + " .align 2 \n" \
>> >> " bal 1f \n" \
>> >> " nop \n" \
>> >> " .gpword . \n" \
>> >> " .gpword " #sym " \n" \
>> >> - "1: lw %0, ($ra) \n" \
>> >> + "1: ins $ra, $0, 0, 1 \n" \
>> >> + " lw %0, ($ra) \n" \
>> >> " subu %0, $ra, %0 \n" \
>> >> " lw $ra, 4($ra) \n" \
>> >> " addu %0, %0, $ra \n" \
>> >
>> >By this, do you mean that .gpword is producing a value that's
>> >relative to the actual byte position of the directive rather than the
>> >logical (ISA bit set) value of "." at the point of .gpword?
>>
>> ISA bit is set for all microMIPS symbols (including the dot symbol).
>> The GETFUNCSYM macro cannot be compiled as pure MIPS code as it might
>> be expanded in a microMIPS function. We need to clear the ISA bit of
>> $ra before using it as a base address in load/store instructions.
>
>OK. And the same appproach would work for crt_arch.h and other places,
>right?
Yes.
>> >> diff --git a/src/thread/mips/syscall_cp.s
>> >> b/src/thread/mips/syscall_cp.s index d284626..9c5f55e 100644
>> >> --- a/src/thread/mips/syscall_cp.s
>> >> +++ b/src/thread/mips/syscall_cp.s
>> >> @@ -1,5 +1,5 @@
>> >> .set noreorder
>> >> -
>> >> +.set nomicromips
>> >> .global __cp_begin
>> >> .hidden __cp_begin
>> >> .type __cp_begin,@function
>> >
>> >I'm also unclear on the motivation of this one. Before (v1) you had a
>> >lot of changes to replace .s files with something
>> >micromips-compatible (removing branch delay slots); now (v2) those
>> >changes are not included. So are .s files even being built as
>> >micromips at all? If not, why is the above needed? If so, how do the files
>with delay slots work?
>>
>> Branch delay slots are removed (called as compact instructions) in the
>> newer MIPS/microMIPS cores (in development).
>> The MIPS/microMIPS R2-R6 still support instructions with delay slot.
>> Assembler takes care of converting a BRANCH + NOP to its appropriate
>> compact instruction (BEQ + NOP to BEQC).
>> With the v1 branch I was trying to create generic hand-written
>> assembly which can be used for newer cores with the compact
>> instructions.
>> However I realized that it would appropriate to create a new arch
>> instead of creating generic assembly.
>> Thus in v2 branch I modified only those functions which would create
>> issues when compiled with interlinking on.
>
>Based on the discussions so far, I don't think pure-micromips qualifies as a
>new arch. If it would be possible to take a program compiled as micromips-
>only, and run it with the libc.so/ldso built for plain mips on a machine that
>supports both forms of code, then it's not a separate arch, and as I
>understand it this should be possible.
Yes, in the context of miroMIPSR2-R5, we don't need to create a new arch.
>Rich
I will create v3 if you are OK with this approach.
Thanks,
Jaydeep
next prev parent reply other threads:[~2017-04-25 4:45 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-05 6:33 Jaydeep Patil
2017-04-06 16:18 ` dalias
2017-04-07 6:47 ` Jaydeep Patil
2017-04-07 14:19 ` Rich Felker
2017-04-12 11:54 ` Jaydeep Patil
2017-04-12 19:25 ` Szabolcs Nagy
2017-04-12 20:27 ` Rich Felker
2017-04-12 21:47 ` Andre McCurdy
2017-04-13 4:29 ` Jaydeep Patil
2017-04-13 9:00 ` Szabolcs Nagy
2017-04-13 10:37 ` Jaydeep Patil
2017-04-21 9:40 ` Jaydeep Patil
2017-04-21 13:33 ` Rich Felker
2017-04-24 5:30 ` Jaydeep Patil
2017-04-24 13:48 ` Rich Felker
2017-04-25 4:45 ` Jaydeep Patil [this message]
2017-04-25 16:52 ` Rich Felker
2017-04-26 7:14 ` Jaydeep Patil
2017-05-11 3:25 ` Jaydeep Patil
2017-05-17 8:28 ` Jaydeep Patil
2017-05-26 3:46 ` Jaydeep Patil
2017-05-28 2:00 ` Rich Felker
2017-05-31 13:11 ` Rich Felker
2017-06-01 4:21 ` Jaydeep Patil
2017-04-21 13:26 ` Rich Felker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BD7773622145634B952E5B54ACA8E349DAE2D9AB@PUMAIL01.pu.imgtec.org \
--to=jaydeep.patil@imgtec.com \
--cc=armccurdy@gmail.com \
--cc=dalias@libc.org \
--cc=musl@lists.openwall.com \
--cc=nsz@port70.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).