This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/5] RISC-V: Add native linux support.


On Wed, Aug 8, 2018 at 8:58 AM, Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>> +class riscv_linux_nat_target final : public linux_nat_target
>
> This probably needs a comment.

OK.

>> +void
>> +supply_gregset_regnum (struct regcache *regcache, const prgregset_t *gregs,
>> +                    int regnum)
>
> Can this be static?

Yes.

>> +void
>> +supply_fpregset_regnum (struct regcache *regcache, const prfpregset_t *fpregs,
>> +                     int regnum)
>
> Can this be static?

Yes.

>> +  if (regnum == RISCV_CSR_MISA_REGNUM)
>
> Should this also have a 'regnum == -1' check?

I was going to worry about CSR support later, but it is harmless to
add this now.  I did so.

>> +  /* Access to other CSRs has potential security issues, don't support them for
>> +     now.  */
>
> Should we add a warning if regnum is NOT -1, but IS for a CSR?  Would
> this warning end up triggering all the time?  Maybe if it does it
> should just trigger once?
>
> I'm just thinking if a user specifically asks for a csr register, it
> might be nice if GDB would say "can't" instead of silently failing...

Gdb is tracking register info via regcache, so it knows that the
register was not updated.  I get for instance

(gdb) print $mtvec
$1 = <unavailable>
(gdb) print $mtvec = 0x180
$2 = <unavailable>

So I don't think we need anything in the riscv-linux-nat.c file to cover this.

Jim


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]