[lldb][RISCV] Handle subsets of CSRs in RV32 core dump images #142932
Conversation
The standard RISC-V ISA sets aside a 12-bit encoding space for up to 4,096 CSRs. However, many of these may remain unutilized and needn't be saved in core dump images. To address this, we've come up with a new note, NT_CSREGMAP, that saves subsets of CSRs as key-value pairs. This change provisions support for handling the subsets of CSRs saved in 32-bit RISC-V core dump images by building the register information for GPRs, FPRs, and CSRs dynamically.
ayushsahay1837
requested review from
DavidSpickett,
jasonmolenda,
santhoshe447 and
tedwoodward
|
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-lldb Author: Ayush Sahay (ayushsahay1837) ChangesThe standard RISC-V ISA sets aside a 12-bit encoding space for up to 4,096 CSRs. However, many of these may remain unutilized and needn't be saved in core dump images. To address this, we've come up with a new note, NT_CSREGMAP, that saves subsets of CSRs as key-value pairs. This change provisions support for handling the subsets of CSRs saved in 32-bit RISC-V core dump images by building the register information for GPRs, FPRs, and CSRs dynamically. Kindly refer to the corresponding topic (Add RISC-V CSRs to core dumps) for additional details. Patch is 411.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142932.diff 12 Files Affected:
diff --git a/lldb/include/lldb/Target/DynamicRegisterInfo.h b/lldb/include/lldb/Target/DynamicRegisterInfo.h
index 43bba5038e537..558adefe2e151 100644
--- a/lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -95,6 +95,8 @@ class DynamicRegisterInfo {
template <typename T> T registers() = delete;
+ template <typename T> T registers() const = delete;
+
void ConfigureOffsets();
protected:
@@ -145,6 +147,12 @@ DynamicRegisterInfo::registers() {
return reg_collection_range(m_regs);
}
+template <>
+inline DynamicRegisterInfo::reg_collection_const_range
+DynamicRegisterInfo::registers() const {
+ return reg_collection_const_range(m_regs);
+}
+
void addSupplementaryRegister(std::vector<DynamicRegisterInfo::Register> ®s,
DynamicRegisterInfo::Register new_reg_info);
diff --git a/lldb/source/Plugins/Process/Utility/CMakeLists.txt b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
index fd3019613892a..53313e139c4e7 100644
--- a/lldb/source/Plugins/Process/Utility/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Utility/CMakeLists.txt
@@ -60,6 +60,7 @@ add_lldb_library(lldbPluginProcessUtility
RegisterInfoPOSIX_ppc64le.cpp
RegisterInfoPOSIX_riscv32.cpp
RegisterInfoPOSIX_riscv64.cpp
+ RegisterInfoPOSIXDynamic_riscv32.cpp
StopInfoMachException.cpp
ThreadMemory.cpp
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.cpp
new file mode 100644
index 0000000000000..950c0c3d3cae8
--- /dev/null
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.cpp
@@ -0,0 +1,87 @@
+//===-- RegisterInfoPOSIXDynamic_riscv32.cpp ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+
+#include <lldb/Utility/Flags.h>
+#include <stddef.h>
+
+#include "lldb/lldb-defines.h"
+#include "llvm/Support/Compiler.h"
+
+#include "RegisterInfoPOSIXDynamic_riscv32.h"
+
+RegisterInfoPOSIXDynamic_riscv32::RegisterInfoPOSIXDynamic_riscv32(
+ const lldb_private::ArchSpec &target_arch)
+ : lldb_private::RegisterInfoAndSetInterface(target_arch),
+ m_target_arch(target_arch) {}
+
+uint32_t RegisterInfoPOSIXDynamic_riscv32::GetRegisterCount() const {
+ return m_dyn_reg_infos.GetNumRegisters();
+}
+
+size_t RegisterInfoPOSIXDynamic_riscv32::GetGPRSize() const {
+ for (uint32_t set_idx = 0; set_idx < GetRegisterSetCount(); ++set_idx) {
+ const lldb_private::RegisterSet *set =
+ m_dyn_reg_infos.GetRegisterSet(set_idx);
+ if (lldb_private::ConstString(set->name) == "GPR")
+ return set->num_registers;
+ }
+ return 0;
+}
+
+size_t RegisterInfoPOSIXDynamic_riscv32::GetFPRSize() const {
+ for (uint32_t set_idx = 0; set_idx < GetRegisterSetCount(); ++set_idx) {
+ const lldb_private::RegisterSet *set =
+ m_dyn_reg_infos.GetRegisterSet(set_idx);
+ if (lldb_private::ConstString(set->name) == "FPR")
+ return set->num_registers;
+ }
+ return 0;
+}
+
+const lldb_private::RegisterInfo *
+RegisterInfoPOSIXDynamic_riscv32::GetRegisterInfo() const {
+ return &*m_dyn_reg_infos
+ .registers<lldb_private::DynamicRegisterInfo::
+ reg_collection_const_range>()
+ .begin();
+}
+
+size_t RegisterInfoPOSIXDynamic_riscv32::GetRegisterSetCount() const {
+ return m_dyn_reg_infos.GetNumRegisterSets();
+}
+
+size_t RegisterInfoPOSIXDynamic_riscv32::GetRegisterSetFromRegisterIndex(
+ uint32_t reg_index) const {
+ for (size_t set_index = 0; set_index < m_dyn_reg_infos.GetNumRegisterSets();
+ ++set_index) {
+ const lldb_private::RegisterSet *reg_set =
+ m_dyn_reg_infos.GetRegisterSet(set_index);
+ for (uint32_t idx = 0; idx < reg_set->num_registers; ++idx)
+ if (reg_set->registers[idx] == reg_index)
+ return set_index;
+ }
+ return LLDB_INVALID_REGNUM;
+}
+
+const lldb_private::RegisterSet *
+RegisterInfoPOSIXDynamic_riscv32::GetRegisterSet(size_t set_index) const {
+ if (set_index < GetRegisterSetCount())
+ return m_dyn_reg_infos.GetRegisterSet(set_index);
+ return nullptr;
+}
+
+size_t RegisterInfoPOSIXDynamic_riscv32::SetRegisterInfo(
+ std::vector<lldb_private::DynamicRegisterInfo::Register> regs) {
+ return m_dyn_reg_infos.SetRegisterInfo(std::move(regs), m_target_arch);
+}
+
+const lldb_private::RegisterInfo *
+RegisterInfoPOSIXDynamic_riscv32::GetRegisterInfo(
+ llvm::StringRef reg_name) const {
+ return m_dyn_reg_infos.GetRegisterInfo(reg_name);
+}
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.h b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.h
new file mode 100644
index 0000000000000..b1f9b75e72df9
--- /dev/null
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIXDynamic_riscv32.h
@@ -0,0 +1,50 @@
+//===-- RegisterInfoPOSIXDynamic_riscv32.h ----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIXDYNAMIC_RISCV32_H
+#define LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIXDYNAMIC_RISCV32_H
+
+#include "RegisterInfoAndSetInterface.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Utility/Flags.h"
+#include "lldb/lldb-private.h"
+#include <map>
+
+class RegisterInfoPOSIXDynamic_riscv32
+ : public lldb_private::RegisterInfoAndSetInterface {
+public:
+ RegisterInfoPOSIXDynamic_riscv32(const lldb_private::ArchSpec &target_arch);
+
+ size_t GetGPRSize() const override;
+
+ size_t GetFPRSize() const override;
+
+ const lldb_private::RegisterInfo *GetRegisterInfo() const override;
+
+ uint32_t GetRegisterCount() const override;
+
+ const lldb_private::RegisterSet *
+ GetRegisterSet(size_t reg_set) const override;
+
+ size_t GetRegisterSetCount() const override;
+
+ size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
+
+ size_t SetRegisterInfo(
+ std::vector<lldb_private::DynamicRegisterInfo::Register> regs);
+
+ const lldb_private::RegisterInfo *
+ GetRegisterInfo(llvm::StringRef reg_name) const;
+
+private:
+ lldb_private::DynamicRegisterInfo m_dyn_reg_infos;
+ const lldb_private::ArchSpec m_target_arch;
+};
+
+#endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_REGISTERINFOPOSIXDYNAMIC_RISCV32_H
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv32.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv32.cpp
index e213b4a4a1820..e3093d52ff4ae 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv32.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv32.cpp
@@ -16,6 +16,9 @@
#define GPR_OFFSET(idx) ((idx) * 4 + 0)
#define FPR_OFFSET(idx) ((idx) * 4 + sizeof(RegisterInfoPOSIX_riscv32::GPR))
+#define CSR_OFFSET(idx) \
+ ((idx) * 4 + sizeof(RegisterInfoPOSIX_riscv32::GPR) + \
+ sizeof(RegisterInfoPOSIX_riscv32::FPR))
#define REG_CONTEXT_SIZE \
(sizeof(RegisterInfoPOSIX_riscv32::GPR) + \
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv32.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv32.h
index ab6fec829bbce..1d1ad379d27db 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv32.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv32.h
@@ -8,13 +8,14 @@
#ifdef DECLARE_REGISTER_INFOS_RISCV32_STRUCT
-#include "Utility/RISCV_DWARF_Registers.h"
-#include "lldb-riscv-register-enums.h"
+#include <stddef.h>
+
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private.h"
-#include <stddef.h>
+#include "Utility/RISCV_DWARF_Registers.h"
+#include "lldb-riscv-register-enums.h"
#ifndef GPR_OFFSET
#error GPR_OFFSET must be defined before including this header file
@@ -24,56 +25,76 @@
#error FPR_OFFSET must be defined before including this header file
#endif
+#ifndef CSR_OFFSET
+#error CSR_OFFSET must be defined before including this header file
+#endif
+
using namespace riscv_dwarf;
// clang-format off
-// I suppose EHFrame and DWARF are the same.
+// Assuming register numbers seen in eh_frame and DWARF to be the same.
#define KIND_HELPER(reg, generic_kind) \
{ \
riscv_dwarf::dwarf_##reg, riscv_dwarf::dwarf_##reg, generic_kind, \
- LLDB_INVALID_REGNUM, reg##_riscv \
+ LLDB_INVALID_REGNUM, reg##_riscv \
}
-// Generates register kinds array for vector registers
+// Generates RegisterInfo::kinds for GPRs.
#define GPR32_KIND(reg, generic_kind) KIND_HELPER(reg, generic_kind)
-// FPR register kinds array for vector registers
+// Generates RegisterInfo::kinds for FPRs.
#define FPR32_KIND(reg, generic_kind) KIND_HELPER(reg, generic_kind)
-// VPR register kinds array for vector registers
+// Generates RegisterInfo::kinds for VPRs.
#define VPR_KIND(reg, generic_kind) KIND_HELPER(reg, generic_kind)
-// Defines a 32-bit general purpose register
+// Generates RegisterInfo::kinds for CSRs.
+#define CSR_KIND(reg, generic_kind) KIND_HELPER(reg, generic_kind)
+
+// Defines a 32-bit GPR.
#define DEFINE_GPR32(reg, generic_kind) DEFINE_GPR32_ALT(reg, reg, generic_kind)
-// Defines a 32-bit general purpose register
+// Defines a 32-bit GPR.
#define DEFINE_GPR32_ALT(reg, alt, generic_kind) \
{ \
#reg, #alt, 4, GPR_OFFSET(gpr_##reg##_riscv - gpr_first_riscv), \
- lldb::eEncodingUint, lldb::eFormatHex, \
- GPR32_KIND(gpr_##reg, generic_kind), nullptr, nullptr, nullptr, \
+ lldb::eEncodingUint, lldb::eFormatHex, \
+ GPR32_KIND(gpr_##reg, generic_kind), nullptr, nullptr, nullptr, \
}
-#define DEFINE_FPR32(reg, generic_kind) DEFINE_FPR32_ALT(reg, reg, generic_kind)
-
-#define DEFINE_FPR32_ALT(reg, alt, generic_kind) DEFINE_FPR_ALT(reg, alt, 4, generic_kind)
+// Defines a 32-bit FPR.
+#define DEFINE_FPR32_ALT(reg, alt, generic_kind) \
+ DEFINE_FPR_ALT(reg, alt, 8, generic_kind)
+// Defines a 32-bit FPR.
#define DEFINE_FPR_ALT(reg, alt, size, generic_kind) \
{ \
#reg, #alt, size, FPR_OFFSET(fpr_##reg##_riscv - fpr_first_riscv), \
- lldb::eEncodingUint, lldb::eFormatHex, \
- FPR32_KIND(fpr_##reg, generic_kind), nullptr, nullptr, nullptr, \
+ lldb::eEncodingIEEE754, lldb::eFormatHex, \
+ FPR32_KIND(fpr_##reg, generic_kind), nullptr, nullptr, nullptr, \
}
+// Defines a 32-bit VPR.
#define DEFINE_VPR(reg, generic_kind) DEFINE_VPR_ALT(reg, reg, generic_kind)
-// Defines a scalable vector register, with default size 128 bits
-// The byte offset 0 is a placeholder, which should be corrected at runtime.
+// Defines a scalable vector register with default size of 128 bits.
+// The byte offset of 0 is a placeholder and should be corrected at runtime.
+// Defines a 32-bit VPR.
#define DEFINE_VPR_ALT(reg, alt, generic_kind) \
{ \
#reg, #alt, 16, 0, lldb::eEncodingVector, lldb::eFormatVectorOfUInt8, \
- VPR_KIND(vpr_##reg, generic_kind), nullptr, nullptr, nullptr \
+ VPR_KIND(vpr_##reg, generic_kind), nullptr, nullptr, nullptr \
+ }
+
+// Defines a 32-bit CSR.
+#define DEFINE_CSR32(reg, generic_kind) DEFINE_CSR32_ALT(reg, reg, generic_kind)
+
+// Defines a 32-bit CSR.
+#define DEFINE_CSR32_ALT(reg, alt, generic_kind) \
+ { \
+ #reg, #alt, 4, 0, lldb::eEncodingUint, lldb::eFormatHex, \
+ CSR_KIND(csr_##reg, generic_kind), nullptr, nullptr, nullptr \
}
// clang-format on
@@ -182,4 +203,4193 @@ static lldb_private::RegisterInfo g_register_infos_riscv32_le[] = {
DEFINE_VPR(v31, LLDB_INVALID_REGNUM),
};
+static lldb_private::RegisterInfo g_register_infos_riscv32_gpr[] = {
+ DEFINE_GPR32(pc, LLDB_REGNUM_GENERIC_PC),
+ DEFINE_GPR32_ALT(ra, x1, LLDB_REGNUM_GENERIC_RA),
+ DEFINE_GPR32_ALT(sp, x2, LLDB_REGNUM_GENERIC_SP),
+ DEFINE_GPR32_ALT(gp, x3, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(tp, x4, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t0, x5, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t1, x6, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t2, x7, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(fp, x8, LLDB_REGNUM_GENERIC_FP),
+ DEFINE_GPR32_ALT(s1, x9, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(a0, x10, LLDB_REGNUM_GENERIC_ARG1),
+ DEFINE_GPR32_ALT(a1, x11, LLDB_REGNUM_GENERIC_ARG2),
+ DEFINE_GPR32_ALT(a2, x12, LLDB_REGNUM_GENERIC_ARG3),
+ DEFINE_GPR32_ALT(a3, x13, LLDB_REGNUM_GENERIC_ARG4),
+ DEFINE_GPR32_ALT(a4, x14, LLDB_REGNUM_GENERIC_ARG5),
+ DEFINE_GPR32_ALT(a5, x15, LLDB_REGNUM_GENERIC_ARG6),
+ DEFINE_GPR32_ALT(a6, x16, LLDB_REGNUM_GENERIC_ARG7),
+ DEFINE_GPR32_ALT(a7, x17, LLDB_REGNUM_GENERIC_ARG8),
+ DEFINE_GPR32_ALT(s2, x18, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s3, x19, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s4, x20, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s5, x21, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s6, x22, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s7, x23, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s8, x24, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s9, x25, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s10, x26, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(s11, x27, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t3, x28, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t4, x29, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t5, x30, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(t6, x31, LLDB_INVALID_REGNUM),
+ DEFINE_GPR32_ALT(zero, x0, LLDB_INVALID_REGNUM),
+};
+
+static lldb_private::RegisterInfo g_register_infos_riscv32_fpr[] = {
+ DEFINE_FPR32_ALT(ft0, f0, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft1, f1, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft2, f2, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft3, f3, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft4, f4, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft5, f5, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft6, f6, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft7, f7, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs0, f8, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs1, f9, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa0, f10, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa1, f11, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa2, f12, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa3, f13, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa4, f14, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa5, f15, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa6, f16, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fa7, f17, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs2, f18, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs3, f19, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs4, f20, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs5, f21, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs6, f22, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs7, f23, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs8, f24, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs9, f25, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs10, f26, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(fs11, f27, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft8, f28, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft9, f29, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft10, f30, LLDB_INVALID_REGNUM),
+ DEFINE_FPR32_ALT(ft11, f31, LLDB_INVALID_REGNUM),
+};
+
+static lldb_private::RegisterInfo g_register_infos_riscv32_vpr[] = {
+ DEFINE_VPR(v0, LLDB_INVALID_REGNUM), DEFINE_VPR(v1, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v2, LLDB_INVALID_REGNUM), DEFINE_VPR(v3, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v4, LLDB_INVALID_REGNUM), DEFINE_VPR(v5, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v6, LLDB_INVALID_REGNUM), DEFINE_VPR(v7, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v8, LLDB_INVALID_REGNUM), DEFINE_VPR(v9, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v10, LLDB_INVALID_REGNUM), DEFINE_VPR(v11, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v12, LLDB_INVALID_REGNUM), DEFINE_VPR(v13, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v14, LLDB_INVALID_REGNUM), DEFINE_VPR(v15, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v16, LLDB_INVALID_REGNUM), DEFINE_VPR(v17, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v18, LLDB_INVALID_REGNUM), DEFINE_VPR(v19, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v20, LLDB_INVALID_REGNUM), DEFINE_VPR(v21, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v22, LLDB_INVALID_REGNUM), DEFINE_VPR(v23, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v24, LLDB_INVALID_REGNUM), DEFINE_VPR(v25, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v26, LLDB_INVALID_REGNUM), DEFINE_VPR(v27, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v28, LLDB_INVALID_REGNUM), DEFINE_VPR(v29, LLDB_INVALID_REGNUM),
+ DEFINE_VPR(v30, LLDB_INVALID_REGNUM), DEFINE_VPR(v31, LLDB_INVALID_REGNUM),
+};
+
+static lldb_private::RegisterInfo g_register_infos_riscv32_csr[] = {
+ DEFINE_CSR32(0, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(fflags, 1, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(frm, 2, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(fcsr, 3, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(4, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(5, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(6, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(7, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(vstart, 8, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(vxsat, 9, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(vxrm, 10, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(11, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(12, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(13, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(14, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32_ALT(vcsr, 15, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(16, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(17, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(18, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(19, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(20, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(21, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(22, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(23, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(24, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(25, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(26, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(27, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(28, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(29, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(30, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(31, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(32, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(33, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(34, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(35, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(36, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(37, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(38, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(39, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(40, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(41, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(42, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(43, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(44, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(45, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(46, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(47, LLDB_INVALID_REGNUM),
+ DEFINE_CSR32(48, LLDB_INVALID_REGNUM),
...
[truncated]
|
DavidSpickett
left a comment
DavidSpickett
left a comment
There was a problem hiding this comment.
I'll leave the proper review to the RISC-V interested parties, just some high level points:
- The csrs that have names, are all the ones here from the RISC-V standard? Do they include any names allocated for custom extensions or do you plan to do anything like that? (I assume custom extensions can also make use of some csr space, but you could just make people type the generic name for these).
- LLDB's design is unfortunately forcing you to enumerate everything up front, I doubt there's much we can do about that without refactoring the others. In some places you might be able to consteveal something, but it probably works out the same if not more in compile time.
- You have this riscv-32-dynamic now, is the existing riscv-32 just the dynamic one with no extra registers, can we share the two implementations?
- You're treating bare metal core files as if they are Linux, I would like to know if Linux cores themselves work and how far we are from supporting those. This will become a popular use case. You can raise an issue with what you find, I'm not expecting you to do the work.
| return llvm::make_error<llvm::StringError>( | ||
| "Don't know how to parse core file. Unsupported OS.", | ||
| llvm::inconvertibleErrorCode()); | ||
| // Treat bare-metal 32-bit RISC-V like Linux. |
There was a problem hiding this comment.
I have been wondering if Linux cores work or not. If they do, I would appreciate one being added as a test case, in a separate PR. If they don't work, you don't have to make them work.
|
@AlexeyMerzlyakov worked on RV64 core file support in the past, might have some comments. |
DavidSpickett
left a comment
DavidSpickett
left a comment
There was a problem hiding this comment.
Also, test cases?
I already let one RISC-V core file patch go in without test cases, I'm not happy about another.
If you want to know how to make them, some way that documents the format of the note at the same time would be great. If you were able to yaml2obj perhaps.
(because otherwise the C++ code is the only documentation)
Maybe Python could be used to generate YAML of various sparse or not sparse notes and substitute those into a template for testing.
|
Also we need a comment somewhere, I suggest in this PR's description for now, that describes the note's format in detail. It will also help review. (as there's no natural place for it, if the Linux kernel was generating them, it'd go there, but bare metal, could be anyone generating them) |
Our intention is to provide support in this PR for all the standard CSRs, and in a later PR for the CSRs in the Xqci extension, and use that as an example for others to implement their own extensions. These should be gated by some discovery mechanism, like ELF attributes, since we don't want to collide if 2 extensions use, say, CSR 50. The existing riscv-32 is static. I think we'll need to replace that (and the 64 bit static implementation, to add CSR support) with dynamic. Downstream we have only dynamic in our 20.0 release. We don't have a running Linux, so I can't comment on "real" Linux core files. In theory, they're the same, but you know about theory vs practice...
I'm currently negotiating with our users on this. Like @JDevlieghere , I can't publish a core dump with internal code in it. But if I can learn how to dump core with an arbitrary testcase, then I can make a core file to load and test. We'll see.
I agree - we should publish a comment with the note's format. I think a comment in the sources would be a more natural place to look than the commit message. |
|
Thanks for reviewing this, David!
The named CSRs in this PR are limited to those listed in Table 4 (Currently allocated RISC-V unprivileged CSR addresses) of the RISC-V Instruction Set Manual (Volume II). Qualcomm is in the process of upstreaming implementations of Xqci. We'll be looking to include named CSRs therein in the future.
This corresponds to the third phase listed here.
IIUC, then the existing support for the postmortem debug of 32-bit RISC-V core dump images looks to facilitate the enablement/disablement of optional register sets in their entirety; it doesn't support handling subsets of register sets. This change looks to provision support for handling subsets of register sets in addition to handling entire register sets.
We've tweaked the existing implementation to facilitate the coexistence of both the schemes. However, the implementation proposed in this PR switches to building register information for GPRs, FPRs, and CSRs dynamically unconditionally, rendering the existing scheme unexercised.
As Ted has mentioned, we're working on devising/procuring a test that we could upstream. I just thought that it'll be good to get the community's initial thoughts on the proposed scheme in the meanwhile.
Sure, I'll add a description of the note's format to the source, and post it here as well. |
|
Ok I'm happy with everything aside from testing. Thanks for explaining. For tests, I don't care if the core file is full of interesting stuff, it can be hello world. I'm thinking you could take some vanilla QEMU example of bare metal RISC-V and use GDB's ability to dump a core file. Assuming that extends to bare metal. Then use https://llvm.org/docs/CommandGuide/llvm-objcopy.html#cmdoption-llvm-objcopy-add-section to add this specially formatted note with data you concoct from some script. Then you can be sure there's zero proprietary IP in there. And in fact...you could write the test such that it uses the normal core file as a template and invokes llvm-objcopy each time to add a new CSR section. One that's sparse, one that's not, etc, etc. Which means at least the CSR bit is programatically generated and better documents the section format in the process. ...which makes me wonder if you can patch this section into a Linux rv32 core file. The kernel knows how to write those and you're treating bare metal core files as Linux anyway. So:
The other way is to go contribute support for this to GDB and/or the Linux kernel, or add ELF core authoring to LLDB. Which are most welcome, but tasks in their own right. |
4 participants
The standard RISC-V ISA sets aside a 12-bit encoding space for up to 4,096 CSRs. However, many of these may remain unutilized and needn't be saved in core dump images. To address this, we've come up with a new note, NT_CSREGMAP, that saves subsets of CSRs as key-value pairs. This change provisions support for handling the subsets of CSRs saved in 32-bit RISC-V core dump images by building the register information for GPRs, FPRs, and CSRs dynamically.
Kindly refer to the corresponding topic (Add RISC-V CSRs to core dumps) for additional details.