Add get_aie_coredump_elf() API for generating coredump (ET_CORE) ELF by sayyanna · Pull Request #9997 · Xilinx/XRT

sayyanna · 2026-08-24T23:43:02Z

Problem solved by the commit

Add get_aie_coredump_elf() API for generating coredump (ET_CORE) ELF

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

Add get_aie_coredump_elf() API for generating coredump (ET_CORE) ELF. Currently AIE coredump is generated as a raw bin file, but this PR enables generated of coredump in ELF format. The existing get_aie_coredump() raw blob API will be deprecated.

How problem was solved, alternative solutions (if any) and why they were rejected

Added new API that package a raw AIE coredump blob into an ET_CORE ELF using the AIEBU coredump ELF writer:
std::vector
get_aie_coredump_elf(std::optionalxrt::aie::coredump_meta meta = std::nullopt) const;

The AIE architecture is derived from the ELF loaded in this context. Metadata (timestamp, versions, device info, uuid) is always embedded. If @meta is provided it is used as-is; otherwise metadata is built internally from the hw_context and device.
The coredump ELF packaging logic lives in xrt_elf.cpp and is exposed internally via xrt_core::elf_int::make_aie_coredump_elf, keeping xrt_hw_context.cpp free of any direct aiebu dependency.

Packaging the dump as an ET_CORE ELF solves:

Self-describing: the ELF header encodes the AIE architecture via the OS/ABI byte, so debuggers and analysis tools can identify the target without extra context
Metadata-rich: a dedicated ELF note section carries timestamp, firmware version, device info, ELF UUID, and context state at capture time — critical for correlating a dump with the exact firmware image and system state
Integration: callers like XDP can call get_aie_coredump_elf() directly and hand off a self-contained artifact without needing to know anything about the underlying architecture

Risks (if any) associated the changes in the commit

None

What has been tested and how, request additional testing if necessary

Added below setting in xrt.ini
[Runtime]
aie_coredump_file = aie_coredump.elf

Enabled coredump: xrt-smi configure --advanced --auto-coredump --enable
then triggered ERT_COM_STATE_TIMEOUT, coredump is ELF generated.

Documentation impact (if any)

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-24T23:48:58Z

clang-tidy review says "All clean, LGTM! 👍"

stsoe

Please comment the code, structs, etc.

stsoe · 2026-08-25T03:40:49Z

+  case aiebu::osabi_aie2p:  return aiebu::aiebu_assembler::buffer_type::coredump_aie2p;
+  case aiebu::osabi_aie2ps: return aiebu::aiebu_assembler::buffer_type::coredump_aie2ps;
+  case aiebu::osabi_aie4:   return aiebu::aiebu_assembler::buffer_type::coredump_aie4;
+  case aiebu::osabi_aie4a:  return aiebu::aiebu_assembler::buffer_type::coredump_aie4a;
+  case aiebu::osabi_aie4z:  return aiebu::aiebu_assembler::buffer_type::coredump_aie4z;
+  default: throw std::runtime_error("AIE coredump not supported for this ELF architecture");


Impossible to debug, please new line after case

stsoe · 2026-08-25T03:42:17Z

+        throw std::runtime_error("AIE coredump ELF not available: no ELF loaded in this context");
+      // All ELFs in one hw_context target the same AIE partition — any entry yields correct OS/ABI


stsoe · 2026-08-25T03:42:56Z

+        throw std::runtime_error("AIE coredump ELF not available: no ELF loaded in this context");
+      // All ELFs in one hw_context target the same AIE partition — any entry yields correct OS/ABI


stsoe · 2026-08-25T03:46:02Z

+  auto buf_type = osabi_to_coredump_type(elf.get_handle()->get_os_abi());
+  aiebu::aiebu_assembler a(buf_type, blob, aiebu::aiebu_assembler::no_meta);
+  return a.get_elf();


What makes this XRT code? Should it be in aiebu. Comments may help explain why it is here.

stsoe · 2026-08-25T03:47:37Z

+#include <vector>
+
+namespace xrt::aie {
+


This is a public API, these structs need extensive comments.
I am not sure about the aie namespace within this file. I need to think about this.

stsoe · 2026-08-25T13:19:44Z

One more question, who is the consumer of these new APIs? If it is XDP, I would argue that they should add this code themselves. E.g. first get the raw coredump from XRT, then re-package it using aiebu.

larry9523

A few high level comments

please add some detail in the PR description about why are we doing this.
I would suggest we only provide one public API get_aie_coredump_elf() internally it will create an ELF coredump with metadata. It does not hurt to add metadata into ELF. (Thanks @stsoe for the input)
I think there is no consumer of xrt.ini path to create aie coredump (I confirm this with @IshitaGhosh and @jvillarre). So while we keep our xrt::hw_context::get_aie_coredump() API, we should generate ELF format coredump with metadata for the xrt,ini path.

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-27T18:55:09Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-27T23:43:15Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-28T07:18:11Z

clang-tidy review says "All clean, LGTM! 👍"

stsoe · 2026-08-28T14:19:51Z

+    for (const auto& entry : data) {
+      if (std::stoi(entry.metadata.id) == islot) {
+        meta.context_status = entry.is_suspended
+            ? aiebu::aie_context_status::idle
+            : aiebu::aie_context_status::running;
+        break;
+      }


One less scope with early-continue

Suggested change

for (const auto& entry : data) {

if (std::stoi(entry.metadata.id) == islot) {

meta.context_status = entry.is_suspended

? aiebu::aie_context_status::idle

: aiebu::aie_context_status::running;

break;

}

for (const auto& entry : data) {

if (std::stoi(entry.metadata.id) != islot)

continue;

meta.context_status = entry.is_suspended

? aiebu::aie_context_status::idle

: aiebu::aie_context_status::running;

break;

}

stsoe · 2026-08-28T14:21:59Z

+namespace {
+
+aiebu::aiebu_assembler::buffer_type
+osabi_to_coredump_type(uint8_t os_abi)
+{
+  switch (os_abi) {
+  case aiebu::osabi_aie2p:
+    return aiebu::aiebu_assembler::buffer_type::coredump_aie2p;
+  case aiebu::osabi_aie2ps:
+    return aiebu::aiebu_assembler::buffer_type::coredump_aie2ps;
+  case aiebu::osabi_aie4:
+    return aiebu::aiebu_assembler::buffer_type::coredump_aie4;
+  case aiebu::osabi_aie4a:
+    return aiebu::aiebu_assembler::buffer_type::coredump_aie4a;
+  case aiebu::osabi_aie4z:
+    return aiebu::aiebu_assembler::buffer_type::coredump_aie4z;
+  default:
+    throw std::runtime_error("AIE coredump not supported for this ELF architecture");
+  }
+}
+
+} // namespace


Don't scope the anonymous namespace inside a named namespace, move it out, or make this a static function within current namespace to keep the proximity to where it is used.

stsoe · 2026-08-28T14:36:15Z

      if (!ostr)
        throw std::runtime_error("Could not open '" + file + "' for writing");
      ostr.write(core.data(), static_cast<std::streamsize>(core.size()));
-      std::abort();


I think there was a mention that we should not abort on coredump, but the original spec said we should. I want to make sure remove std::abort() is indeed correct.

We should keep the std::abort() to make sure driver clean up hw_context after timeout. We don't have to get the exception message and coredump() at one run. When usesrs set xrt.ini to save aie coredump, they already see the timeout and exception message.

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-28T22:05:11Z

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

github-actions · 2026-08-29T00:31:19Z

clang-tidy review says "All clean, LGTM! 👍"

stsoe

Looks good. Minor corrections.

stsoe · 2026-08-29T00:53:11Z

    : "";
 }

+aiebu::aiebu_assembler::buffer_type


It was previously in a nested unnamed namespace, I suggested either moving the namespace or making the function static,

Suggested change

aiebu::aiebu_assembler::buffer_type

static aiebu::aiebu_assembler::buffer_type

stsoe · 2026-08-29T00:57:09Z

+      for (const auto& kv : *drivers) {
+        const auto& drv = kv.second;


It is generally preferred to use structured bindings for anything return tuples.

Suggested change

for (const auto& kv : *drivers) {

const auto& drv = kv.second;

for (const auto& [dummy, drv] : *drivers) {

stsoe · 2026-08-29T01:01:14Z

+    return xrt_core::elf_int::make_aie_coredump_elf(
+        elf, get_aie_coredump(), m_core_device.get(), m_hdl->get_slotidx(), "");


You defaulted the uuid string in make_aie_coredump_elf, which I think is fine, but then this function should not mention anything about UUID and not call with the empty string.

Add XRT api for generating core dump ELF

627ddc0

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

sayyanna requested review from rbramand-xilinx, stsoe and uday610 as code owners August 24, 2026 23:43

sayyanna requested a review from larry9523 August 24, 2026 23:43

stsoe requested changes Aug 25, 2026

View reviewed changes

larry9523 requested changes Aug 25, 2026

View reviewed changes

Address PR comments

ec9db67

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

sayyanna requested review from larry9523 and stsoe August 27, 2026 19:03

stsoe reviewed Aug 27, 2026

View reviewed changes

Comment thread src/runtime_src/core/include/xrt/xrt_hw_context.h Outdated

stsoe reviewed Aug 27, 2026

View reviewed changes

Comment thread src/runtime_src/core/common/api/xrt_hw_context.cpp

stsoe reviewed Aug 27, 2026

View reviewed changes

Comment thread src/runtime_src/core/include/xrt/experimental/xrt_aie_coredump.h Outdated

Address PR comments

4faa92d

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

fix driver info in metadata

6eb994f

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

stsoe reviewed Aug 28, 2026

View reviewed changes

Address PR comments

a5c4ec7

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

sayyanna requested a review from stsoe August 28, 2026 21:51

add new line

ca1e98e

Signed-off-by: Sri Latha Ayyannagari <SriLatha.Ayyannagari@amd.com>

stsoe reviewed Aug 29, 2026

View reviewed changes

		throw std::runtime_error("AIE coredump ELF not available: no ELF loaded in this context");
		// All ELFs in one hw_context target the same AIE partition — any entry yields correct OS/ABI

	aiebu::aiebu_assembler::buffer_type
	static aiebu::aiebu_assembler::buffer_type

		for (const auto& kv : *drivers) {
		const auto& drv = kv.second;

	for (const auto& kv : *drivers) {
	const auto& drv = kv.second;
	for (const auto& [dummy, drv] : *drivers) {

		return xrt_core::elf_int::make_aie_coredump_elf(
		elf, get_aie_coredump(), m_core_device.get(), m_hdl->get_slotidx(), "");

Uh oh!

Conversation

sayyanna commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

Uh oh!

github-actions Bot commented Aug 24, 2026

Uh oh!

stsoe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stsoe commented Aug 25, 2026

Uh oh!

larry9523 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stsoe Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 29, 2026

Uh oh!

stsoe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sayyanna commented Aug 24, 2026 •

edited

Loading

stsoe Aug 28, 2026 •

edited

Loading