Re: [Qemu-devel] QEMU development for MIPS64 user mode

[Khansa Butt]

Please see inline comments highlighted in red.

On Wed, Mar 30, 2011 at 12:04 AM, Andreas Färber <address@hidden> 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 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 that 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 for 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 topic (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 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

 

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