Re: RISCV NFPREG?

[Palmer Dabbelt <palmer at sifive dot com>]

On Tue, 08 Oct 2019 22:41:48 PDT (-0700), macro@wdc.com wrote:
> Palmer,
>
>  We have this:
>
> #define ELF_NFPREG	NFPREG
>
> in sysdeps/unix/sysv/linux/riscv/bits/procfs.h, however NFPREG is nowhere
> defined.  This has come from your initial commit 7f33b09c65e3 ("RISC-V:
> Linux ABI").  Do you happen to remember what the intent was here if any?
>
>  We also have:
>
> #define ELF_NGREG	NGREG
>
> (in the same place) and then:
>
> # define NGREG	32
>
> in sysdeps/unix/sysv/linux/riscv/sys/ucontext.h, however under __USE_MISC
> only.
>
>  We don't use any of these macros and the way they have been defined means
> they're mostly useless to user programs.  All the other architectures
> define them unconditionally and then do use them one way or another in
> public headers.
>
>  Unfortunately I'll have to work it around now in GDB for backwards
> compatibility, but has there been any plan here that was interrupted and
> never completed?
>
>  If not, I'll make some patches to fix it up, by defining __NGREG and
> __NFPREG unconditionally as some targets do and use these to fix up the
> mess.

That seems fine.  This looks like cruft that's left over from before we 
properly sorted out the multilib stuff.

On Thu, 10 Oct 2019 15:12:31 PDT (-0700), Jim Wilson wrote:
> On Thu, Oct 10, 2019 at 7:41 AM Maciej W. Rozycki <macro@wdc.com> wrote:
> >  Please observe however that for ELF we actually have 33 uniform FPR slots
> > in the NT_PRFPREG core file note, because this is how Linux regsets work,
> > even though the FCSR slot is only partially occupied; consequently this
> > also applies to PTRACE_GETREGSET/PTRACE_SETREGSET ptrace(2) requests.
>
> I wrote this linux kernel ptrace support.  I did it this way because I
> didn't see a better way to do it.  But if you look at riscv_fpr_get
> and riscv_fpr_set, you will see that I'm using two copyout/copyin
> calls to copy the two different types of registers.  I'm not making
> any assumptions about the number of registers there.  We probably do
> have to accept that we have 33 FP registers, but we need to be careful
> about what exactly this means, which is why I suggested adding
> comments to document that this is really 32 FP regs and 1 FCSR which
> is not the same thing as 33 FP regs.  I'm not an expert with the linux
> kernel/glibc interface, so I don't understand all the implications
> here.  I'm not objecting to your changes.  I'm just trying to provide
> a little history.

That's the right way to do it.  The ptrace stuff matches mcontext, and none of 
that has anything to do with these #defines any more -- IIRC, before we sorted 
out multilib we had different mcontexts for the different ABIs, but we've since 
fixed that as it doesn't work.