Please see inline comments highlighted in red. On Wed, Mar 30, 2011 at 12:04 AM, Andreas Färber <andreas.faer...@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 >> 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 >
