This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [Patch] Microblaze: Port of Linux gdbserver
- From: Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>
- To: Michael Eager <eager at eagercon dot com>, Pedro Alves <palves at redhat dot com>, Joel Brobecker <brobecker at adacore dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, Vinod Kathail <vinodk at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, "Nagaraju Mekala" <nmekala at xilinx dot com>
- Date: Thu, 9 Oct 2014 18:54:30 +0000
- Subject: RE: [Patch] Microblaze: Port of Linux gdbserver
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 149.199.60.83) smtp dot mailfrom=ajit dot kumar dot agarwal at xilinx dot com;
- References: <25de23b98e054fd291ea232d10f2800c at BN1BFFO11FD018 dot protection dot gbl> <5436B7D0 dot 9060004 at eagercon dot com>
-----Original Message-----
From: Michael Eager [mailto:eager@eagercon.com]
Sent: Thursday, October 09, 2014 9:59 PM
To: Ajit Kumar Agarwal; Pedro Alves; Joel Brobecker
Cc: gdb-patches@sourceware.org; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch] Microblaze: Port of Linux gdbserver
On 10/08/14 06:51, Ajit Kumar Agarwal wrote:
> Please find the updated patch with feedback comments incorporated.
>>Please don't top post and please trim email.
To send the patches after incorporating the comments, Is there any other way of sending the patches without top post?
I will make sure that I will trim the mail.
>>Please respond to questions/comments inline. This makes it easier to tell whether all issues have been addressed.
Sure. Please find my comments inlined below.
> Microblaze: Port of Linux gdbserver
>
> This patch is the port of Linux gdbserver.
> Tested with gdb regression testsuite with this patch of
> gdbserver.
>
> gdb/:
> 2014-10-08 Ajit Agarwal <ajitkum@xilinx.com>
>
> * configure.tgt (build_gdbserver): New Definition.
>
> gdb/gdbserver/:
>
> * gdbserver/Makefile.in (microblaze-linux.c): New target.
> * gdbserver/configure.srv (microblaze*-*-linux*): New target.
> * gdbserver/linux-microblaze-low.c: New file.
+#define microblaze_breakpoint_len 4
>>Use CAPS for macros.
The MIPS and the ARM gdbserver code does not use the CAPS for the above macro defined.
+ (*the_target->read_memory) (where, (unsigned char *) &insn, 4);
+
+ if (insn == microblaze_breakpoint)
>>Why use the explicit length rather than the macro you just defined?
>>Why not use sizeof (insn)?
To match up with the MIPS target and ARM target they have not used the macro defined. In the Mips 4 is used and in the ARM target for the THUMB_ARM 2 is used and for the ARM Mode code 4 is used.
Pedro:
> I'd much prefer if we had that patch in the tree before accepting further patches that tweak things around register names, etc. Could you send that (as an independent patch, in a new thread).
>>Please address issues with previous patches before moving on to submit dependent patches.
I have already send the patch related to the above Pedro's comment. I have also send the patch after incorporating the Pedro feedback comments.
Pedro:
> Did this kernel port make it upstream without PTRACE_GETREGSET?
> If there's support for that, can you please switch to using it?
>>Please answer all questions.
Sure. The Kernel code(ptrace.h) for Microblaze doesn't have upstream code without PTRACE_GETREGSET.
Pedro:
> PTRACE_GETREGS is supposed to an old way of doing things...
>>And address all comments.
The Microblaze Kernel code PTRACE_GETREGS is always defined and there is no conditional compilation which is without the PTRACE_GETREGS. So I agree with Pedro comment of not using #ifdef PTRACE_GETREGS and in the patch submitted I have removed if #ifdef PTRACE_GETREGS which is not required.
Pedro:
>diff --git a/gdb/regformats/microblaze-with-stack-protect.dat
...
> Please send a preparatory, independent, patch that updates
> features/Makefile instead and generates this file, in a new thread,
> with self-contained description, following the
> checklist:
> https://sourceware.org/gdb/wiki/ContributionChecklist
>>Preparatory means that the patch should be submitted before the current patch.
I will be sending this patch soon.
Thanks & Regards
Ajit
--
Michael Eager eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077