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

Jim Wilson
2018-08-08T23:36:40Z

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