Home Original page

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

Palmer Dabbelt via binutils binutils@sourceware.org
Mon Feb 10 18:12:00 GMT 2020 
On Fri, 31 Jan 2020 16:24:29 PST (-0800), Andrew Waterman wrote:
> On Fri, Jan 31, 2020 at 4:19 PM Jim Wilson <jimw@sifive.com> wrote:
>>
>> 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?
>
> It seems inadvisable to incorporate draft standards into the FSF
> binutils port.  In particular, the hypervisor ISA is virtually certain
> to change in backwards-incompatible ways prior to being frozen.  It
> seems to me that pushing this stuff upstream at this time will cause
> more problems than it solves.
>
> We could consider maintaining these patches on branches in the RISC-V
> github repositories, and push them to the FSF tree at the appropriate
> time.  That way, we can support software development against these
> draft standards while reducing the scope of backwards
> incompatibilities.

That's the approach we're taking everywhere else, and I think it's the right
way to go here.  Sorry, I must have been off by one when I was looking.

>
>>
>>
>> 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
More information about the Binutils mailing list