From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48509 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8Vl7-00011G-DT for qemu-devel@nongnu.org; Sat, 09 Apr 2011 06:51:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q8Vl5-0002Zl-Tf for qemu-devel@nongnu.org; Sat, 09 Apr 2011 06:51:17 -0400 Received: from mail-qw0-f45.google.com ([209.85.216.45]:56393) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q8Vl5-0002Zg-PN for qemu-devel@nongnu.org; Sat, 09 Apr 2011 06:51:15 -0400 Received: by qwj8 with SMTP id 8so2978026qwj.4 for ; Sat, 09 Apr 2011 03:51:15 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <37559859-44A6-4DC2-AE65-9383EB1C8D55@web.de> References: <37559859-44A6-4DC2-AE65-9383EB1C8D55@web.de> Date: Sat, 9 Apr 2011 15:51:15 +0500 Message-ID: Subject: Re: [Qemu-devel] QEMU development for MIPS64 user mode From: Khansa Butt Content-Type: multipart/alternative; boundary=00235433362e1aa1b104a07a1fc7 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Andreas_F=E4rber?= Cc: qemu-devel@nongnu.org --00235433362e1aa1b104a07a1fc7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Please see inline comments highlighted in red. On Wed, Mar 30, 2011 at 12:04 AM, Andreas F=E4rber = wrote: > Hi, > > Am 29.03.2011 um 08:49 schrieb Khansa Butt: > > > I have added support for MIPS64 user mode emulation in QEMU and email gi= t >> patch to the qemu-devel mailing list >> but I got no any response yet. My Patch mail has the following subject >> line >> MIPS64 user mode emulation Patch >> please verify that this patch mail is not neglected or guide me towards >> the proper way of patch submitting. >> > > You should use git-send-email to submit it (marking it as [PATCH]) so tha= t > it can be applied with git-am, see > http://wiki.qemu.org/Contribute/SubmitAPatch and the list archives. > Also don't forget to cc the maintainer(s) - Aurelien for mips and Riku fo= r > linux-user IIRC. > > A description of how to test it may be helpful. Maybe you have links to > mips64 binaries that work? > > Usually, the subject line of the commit message is prefixed with the topi= c > (linux-user) or architecture (mips). > If all the people you name contributed to this patch, you should probably > add their SoBs before yours. > The patch is rather large - is it possible to split it up into a patch > series with at least a linux-user and a (target-)mips part? > > TARGET_OCTEON looks rather uncommon to me... > > Your patch contains a "Nasty hack". Please elaborate on that - what's the > problem, do you intend to fix it later, etc. > > linux-user/mips64/syscall.h +/* Nasty hack: define a fake errno value for use by sigreturn. */ +#define TARGET_QEMU_ESIGRETURN 255 + The above lines has been copied from linux-user/mips32/syscall.h, in order to define the constant TARGET_QEMU_ESIGRETURN(as it is needed in main.c:cpu_loop()) > You simply comment out a #warning that signal handling is not implemented > for mipsn64. Why didn't you implement it? Don't you need it? > The signal handling for Mips64 is same as for other architectures. qemu handles signals which comes from a program and actual handling is done by host operating system. we follow the same convention. why this warning is generated initially? > Similarly you comment out a sign extension. Please elaborate. If it's a b= ug > and definitely wrong, it should be moved to its own patch, explaining wha= t > goes wrong and fully removing it instead. > > resolved this sign extension problem > In CPUMIPSState, the surrounding struct members use lowercase characters. > > Some spaces missing after if. > > Thanks for your contribution and for taking the time to go through the > review process. > > Regards, > Andreas > --00235433362e1aa1b104a07a1fc7 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Please see inline comments=A0highlighted=A0in red.

On Wed, Mar 30, 2011 at 12:04 AM, Andreas F=E4rber <andreas.faerber@web.de> wrote:
Hi,

Am 29.03.2011 um 08:49 schrieb Khansa Butt:


I have added support for MIPS64 user mode emulation in QEMU and email git p= atch to the qemu-devel mailing list
but I got no any response yet. My Patch mail has the following subject line=
MIPS64 user mode emulation Patch
please verify that this patch mail is not neglected or guide me towards the= proper way of patch submitting.

You should use git-send-email to submit it (marking it as [PATCH]) so that = it can be applied with git-am, see http://wiki.qemu.org/Contribute/SubmitAP= atch and the list archives.
Also don't forget to cc the maintainer(s) - Aurelien for mips and Riku = for linux-user IIRC.

A description of how to test it may be helpful. Maybe you have links to mip= s64 binaries that work?

Usually, the subject line of the commit message is prefixed with the topic = (linux-user) or architecture (mips).
If all the people you name contributed to this patch, you should probably a= dd their SoBs before yours.
The patch is rather large - is it possible to split it up into a patch seri= es with at least a linux-user and a (target-)mips part?

TARGET_OCTEON looks rather uncommon to me...

Your patch contains a "Nasty hack". Please elaborate on that - wh= at's the problem, do you intend to fix it later, etc.


linux-user/mips64/syscall.h
+/* Nasty hack: define a fake errno= value for use by sigreturn. =A0*/
+#define TARGET_QEM= U_ESIGRETURN 255
+
The above lines has been=A0copied=A0from linux-user/mips32/sys= call.h, in order to define the constant=A0TARGET_QEMU_ESIGRETURN(as it is n= eeded in main.c:cpu_loop())
=A0
You simply comment out a #warning that signal handling is not implemented f= or mipsn64. Why didn't you implement it? Don't you need it?

The signal handling for Mips64 is same as for other architectures. =A0= qemu handles signals which comes from a program and actual handling is done= by host operating system. we follow the same convention. why this warning = is generated initially?

=A0
Similarly you comment out a sign extension. Please elaborate. If it's a= bug and definitely wrong, it should be moved to its own patch, explaining = what goes wrong and fully removing it instead.


resolved this sign extension problem
=A0
In CPUMIPSState, the surrounding struct members use lowercase characters.
Some spaces missing after if.

Thanks for your contribution and for taking the time to go through the revi= ew process.

Regards,
Andreas

--00235433362e1aa1b104a07a1fc7--