Re: [PATCH v3 1/4] RISC-V: Update the CSR to privilege spec 1.12.

[Jim Wilson <jimw at sifive dot com>]

On Sun, Dec 15, 2019 at 9:22 PM Nelson Chu <nelson.chu@sifive.com> wrote:
>         gas/
>         * testsuite/gas/riscv/priv-reg.s: Rename to priv-reg-all.s  Update
>         the CSR to privilege spec 1.12.
>         * testsuite/gas/riscv/priv-reg.d: Likewise.
>         * testsuite/gas/riscv/bad-csr.s: Rename to priv-reg-fail-nonexistent.
>         * testsuite/gas/riscv/bad-csr.d: Likewise.
>         * testsuite/gas/riscv/bad-csr.l: Likewise.
>         * testsuite/gas/riscv/satp.s: Deleted.  Duplicate of priv-reg-all.s
>         * testsuite/gas/riscv/satp.d: Likewise.
>         * testsuite/gas/riscv/csr-dw-regnums.s: Updated.
>         * testsuite/gas/riscv/csr-dw-regnums.d: Likewise.
>
>         include/
>         * opcode/riscv-opc.h: Update the CSR to privilege spec 1.12.
>
>         gdb/
>         * features/riscv/32bit-csr.xml: Regenerated.
>         * features/riscv/64bit-csr.xml: Regenerated.

Privilege spec 1.11 is ratified, and spec 1.12 is in draft form.  Do
we want to implement a draft?

I see some hypervisor registers missing, e.g. hgeie and hgeip which
were added to the 1.12 draft on November 20.  Hie was added Oct 29,
and is listed as dropped in your patch.  It isn't clear to me which
version of the 1.12 draft you are using, but it seems to be an old
one.  If we are implementing
1.12 draft we should probably implement the current one.

Overall it looks reasonable, but we need to be clear which spec we are
implementing, so we can verify that the patch matches the spec.

I see that the csr-dw-regnums.s test doesn't have any of the csr
aliases, but that seems OK.  priv-reg-all.s is the important one and
does have all of them.

You are modifying gdb xml files.  These need to be sent to gdb-patches
for approval instead of the binutils list, and I don't have gdb patch
approval permission.  These are shared with qemu, and ideally should
be kept in sync.  As a practical matter though, gdb doesn't use the
csr xml files as yet.  And updating qemu means adding priv spec 1.12
draft support to qemu which is probably non-trivial, unless we can
ignore the as yet unsupported registers in the gdb stub which should
work.  I believe WDC is working on the qemu hypervisor support, so it
might actually be tracking the 1.12 draft already.  It is probably OK
to submit the gdb xml changes, and then file a bug somewhere asking
for gdb and qemu xml support to be synchronized.

Jim