riscv-software-src
Query Compiler Features for Better Disassembling
This is the same patchset submitted to the OpenSBI mailing list as:
- PATCH v1: http://lists.infradead.org/pipermail/opensbi/2022-November/003659.html
- PATCH v2: http://lists.infradead.org/pipermail/opensbi/2022-November/003671.html
(warning detection on PATCH 2/3 got more robust)
Still, cover letter is a bit enhanced than that.
Description
This patchset makes the resulting ELF files (e.g. fw_dynamic.elf) disassembled correctly (as possible) on the latest toolchain, when compiled and assembled with the latest GNU toolchain (especially on Binutils master [going to be 2.40?] but some changes are also effective on 2.39).
In this post, I will call master commit of GNU Binutils as 2.40.
Before the Patchset: GNU Binutils 2.39 and 2.40
Note that some annotations are added.
000000008000a0d0 <__sbi_hfence_gvma_gpa>:
8000a0d0: 62050073 .word 0x62050073
^^^^^^^^^^^^^^^^^^
Printed as data (actually, "hfence.gvma a0" instruction)
8000a0d4: 8082 ret
8000a0d6: 0001 nop
# (quote from csr_read_num)
800047b6: 3c002573 csrr a0,0x3c0
^^^^^
Raw CSR number (pmpaddr16, a CSR from priv. spec. v1.12)
800047ba: 8082 ret
PATCH 1/3: lib: sbi: Optionally use .insn instead of .word
This is partially effective on GNU Binutils 2.39 or later.
It optionally replaces .word (which emits a 32-bit "data" word) in lib/sbi/sbi_hfence.S with .insn with raw value (which emits an "instruction" word) when supported by the assembler.
Before this commit and on GNU Binutils, an ELF mapping symbol $d (from here, data follows) is inserted before .word. Even if the hfence.* instruction itself is recognized, .words are recognized as data, not instructions (due to the existence of mapping symbol).
Instead, .insn emits (possibly variable length) instruction word and inserts $x (from here, instruction follows) mapping symbol where necessary.
Note that, not all assemblers support .insn with raw instruction value (.insn [LENGTH,] RAW_VALUE is second .insn assembler directive format after .insn INSN_FORMAT ... like .insn r 0x33,0,0,a0,a1,a2 turns into add a0,a1,a2. LLVM and older GNU Binutils support only the latter).
This commit checks .insn with raw value support.
After that, objdump result of __sbi_hfence_gvma_gpa will look like this (for PATCH 2-3/3, GNU Binutils 2.39 output will be the same):
000000008000a0d0 <__sbi_hfence_gvma_gpa>:
8000a0d0: 62050073 .4byte 0x62050073
^^^^^^^^^^^^^^^^^^
Printed as an instruction
(but not recognized as hfence.gvma yet)
8000a0d4: 8082 ret
8000a0d6: 0001 nop
PATCH 2/3: lib: sbi: Optionally use .option arch,+h
This is effective on GNU Binutils 2.40.
.option arch,+h assembler directive enables H extension support in the section (may be the file scope depending on the Binutils version [in specific, until Nelson's upcoming patchset, it will be the file scope]).
It will be reflected to the ELF mapping symbols $x[arch] (where [arch] is an ISA string) on GNU Binutils 2.40 and H extension can be partially enabled (only on lib/sbi/sbi_hfence.o).
This commit queries existence of .option arch and enables this feature when available.
At first, I intended this to be fully effective on GNU Binutils 2.39 (where section-level architecture change will be reflected to file scope and H extension dependency is merged into final ELF files).
However, due to a bug in the linker (to be specific, riscv_merge_std_ext function in BFD), it did not happen on 2.39. The fix to this linker issue is submitted as a separate patchset to GNU Binutils (now merged for 2.40):
- https://github.com/a4lg/binutils-gdb/wiki/riscv_ld_merge_h_ext
- https://sourceware.org/pipermail/binutils/2022-November/124679.html
After that, objdump result of __sbi_hfence_gvma_gpa in GNU Binutils 2.40 will be:
000000008000a0d0 <__sbi_hfence_gvma_gpa>:
8000a0d0: 62050073 hfence.gvma a0
^^^^^^^^^^^^^^^^^^
Printed as a hfence.gvma instruction
8000a0d4: 8082 ret
8000a0d6: 0001 nop
PATCH 3/3: Makefile: Optionally specify latest privileged specification
This is effective on GNU Binutils 2.39 or later.
OpenSBI uses some privileged architecture version 1.12 CSRs and this commit reflects this version to the final ELF file. As a result, 1.12 CSRs are correctly named (instead of raw numbers).
This commit queries availability of an assembler option -mpriv-spec=1.12.
If this option exists, Makefile adds -Wa,-mpriv-spec=1.12 to CFLAGS and ASFLAGS.
Note: actually, it separates 1.12 to the separate variable: OPENSBI_PRIV_SPEC.
After that, objdump result of csr_read_num will look like this (both on GNU Binutils 2.39 and 2.40):
# (quote from csr_read_num)
800047b6: 3c002573 csrr a0,pmpaddr16
^^^^^^^^^
CSR name new in v1.12
800047ba: 8082 ret
Thanks but you don't need to raise PR since you have already sent patches to the mailing list.
We have mailing based review and patches are picked from the mailing list only.
Regards, Anup
Understood. This issue is free to close but if it didn't, I'll close this depending on the consequence of the mailing list-based review.