Re: [PATCH v5 3/3] gdbserver: Add RISC-V/Linux support

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Maciej W. Rozycki <macro@wdc.com> [2020-02-10 09:01:53 +0000]:

> Andrew,
> 
> > I took a look through, and I'm happy with the changes in
> > gdb/gdbserver/*.  But I wondered if we could try something slightly
> > different for the gdb/arch/* and gdb/nat/* changes.
> > 
> > I'll follow up to this mail with a patch that I'd like to apply before
> > we apply your patch #3, it tweaks the target description lookup API
> > slightly.  The result is that your patch #3 would drop all changes to
> > gdb/arch/ and gdb/nat/, then the riscv_arch_setup function would look
> > like this:
> > 
> >   static void
> >   riscv_arch_setup ()
> >   {
> >     static const char *expedite_regs[] = { "sp", "pc", NULL };
> > 
> >     const riscv_gdbarch_features features
> >       = riscv_linux_read_features (lwpid_of (current_thread));
> >     target_desc *tdesc = riscv_create_target_description (features);
> > 
> >     if (!tdesc->expedite_regs)
> >       init_target_desc (tdesc, expedite_regs);
> >     current_process ()->tdesc = tdesc;
> >   }
> > 
> > The feature lookup and target description fetch are now separate
> > calls.
> > 
> > The benefit of this is that GDB gets to retain the const in its API,
> > while gdbserver gets a non-const API to call.
> 
>  Fine with me.
> 
> > If you're happy with this approach then feel free to push both these
> > patches, or let me know and I'll push them both.
> 
>  Thank you for your review and rework.
> 
>  We've got a mid-air collision with Tom's `gdbserver' directory structure 
> rearrangement however, so I had to adjust v6 2/2 accordingly and I'm 
> currently rerunning verification as I had to update the sources.
> 
>  I'll post v7 once that has completed, which may take several hours yet.  
> Changes are mechanical, applying to configury/Makefile structure only, so 
> I'll assume your approval for this updated version with a reasonable 
> amount of time allowed to chime in.

That sounds like a good plan.

>
>  NB what's the `Change-Id' included with the change descriptions of your 
> patches; is that needed/useful for anything or just internal bookkeeping?

The Change-Id is nothing important, I should get rid of it.  It's left
over from when we tried using gerrit, and is added automatically by my
git setup.

Thanks,
Andrew