-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libdrgn: add support for remote debugging via OpenOCD #338
base: main
Are you sure you want to change the base?
Changes from 1 commit
b0c9017
1ba5c70
48217d2
c171f8f
4daccce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -380,7 +380,8 @@ static void linux_kernel_pgtable_iterator_init_aarch64(struct drgn_program *prog | |
{ | ||
struct pgtable_iterator_aarch64 *it = | ||
container_of(_it, struct pgtable_iterator_aarch64, it); | ||
if (it->it.pgtable == prog->vmcoreinfo.swapper_pg_dir) { | ||
if (it->it.pgtable == prog->vmcoreinfo.swapper_pg_dir && | ||
it->it.pgtable_phys == prog->vmcoreinfo.swapper_pg_dir_phys) { | ||
it->va_range_min = UINT64_MAX << prog->vmcoreinfo.va_bits; | ||
it->va_range_max = UINT64_MAX; | ||
} else { | ||
|
@@ -408,7 +409,7 @@ linux_kernel_pgtable_iterator_next_aarch64(struct drgn_program *prog, | |
int level; | ||
uint16_t num_entries = it->last_level_num_entries; | ||
uint64_t table = it->it.pgtable; | ||
bool table_physical = false; | ||
bool table_physical = it->it.pgtable_phys; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't delved deep into the pgtable code, so I was not aware that I had a hack where I fooled drgn into using a physical There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Longer term I was thinking that we might want to try removing the |
||
|
||
if (virt_addr < it->va_range_min || | ||
virt_addr > it->va_range_max) { | ||
|
@@ -466,6 +467,141 @@ linux_kernel_pgtable_iterator_next_aarch64(struct drgn_program *prog, | |
} | ||
} | ||
|
||
static struct drgn_error * | ||
linux_kernel_init_vmcoreinfo_from_phys_aarch64(struct drgn_program *prog, | ||
Elf *vmlinux, GElf_Ehdr *ehdr, | ||
uint64_t text_pa) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having We may find ourselves needing to take a page out of crash's book, which allows you to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
To me it seems that there should be different APIs for "starting from a vmcoreinfo note" and "starting from a physical address". In each case the information discovery mechanism is very different. For example, initialization from PA doesn't even require the kernel to be configured with
Possibly. I suppose that another option would be to get any information we need added to the vmlinux file in some form, and then we can read it from there. The page size in the arm64 kernel header is one example. You might also have noticed changes in the code to tolerate missing information (i.e. changes to |
||
{ | ||
Comment on lines
+470
to
+474
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a clever way of getting the important information into Drgn without needing to have a more architectural shift to understand "bare-metal" targets explicitly. But the downside is that faking vmcoreinfo data is a time-honored tradition in Linux core dump debugging which has been the source of several bugs (e.g. makedumpfile and azure-linux-utils both do it, badly). That said, I don't have a specific issue I'm worried about in this case -- especially given that this hook is only used by the aarch64 OpenOCD backend (for now). I guess the thing that would make me more comfortable is not tying this directly to the "vmcoreinfo" concept, which is Linux-specific. If instead we could have a way of representing that this is a bare metal target (i.e. one where we need to handle all virtual address translations ourselves, and need to bootstrap those translations with physical address information), then we can separate this from the linux kernel / vmcoreinfo specific code. That way, in the future we could still have a target which is both bare-metal and Linux kernel, where we can find the vmcoreinfo via the equivalent of That said, I don't know if it's worth blocking on these concerns or if we should try to update the architecture as it becomes an issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think it would be worth trying to remove dependencies on Linux-isms such as vmcoreinfo as drgn starts to become useful for debugging bare-metal programs other than the Linux kernel. I personally don't think it needs to be done immediately though. |
||
struct drgn_error *err = NULL; | ||
Elf_Data *symtab = NULL; | ||
Elf_Data *head_text = NULL; | ||
size_t shstrndx; | ||
uint64_t strtab; | ||
uint64_t numsyms; | ||
|
||
uint64_t swapper_pg_dir_rva = 0; | ||
uint64_t text_rva = 0; | ||
uint64_t image_flags; | ||
uint64_t kimage_vaddr_rva = 0; | ||
uint64_t kimage_vaddr; | ||
uint64_t vabits_actual_rva = 0; | ||
uint64_t vabits_actual; | ||
|
||
bool bswap; | ||
err = drgn_program_bswap(prog, &bswap); | ||
if (err) | ||
return err; | ||
|
||
if (elf_getshdrstrndx(vmlinux, &shstrndx)) | ||
return drgn_error_libelf(); | ||
|
||
for (int i = 0; i != ehdr->e_shnum; ++i) { | ||
Elf_Scn *section = elf_getscn(vmlinux, i); | ||
GElf_Shdr shdr_mem, *shdr; | ||
|
||
if (!section) | ||
continue; | ||
|
||
shdr = gelf_getshdr(section, &shdr_mem); | ||
if (!shdr) | ||
continue; | ||
|
||
if (shdr->sh_type == SHT_SYMTAB) { | ||
symtab = elf_getdata(section, NULL); | ||
strtab = shdr->sh_link; | ||
numsyms = shdr->sh_size / shdr->sh_entsize; | ||
continue; | ||
} | ||
|
||
const char *name = elf_strptr(vmlinux, shstrndx, shdr->sh_name); | ||
if (strcmp(name, ".head.text") == 0) { | ||
head_text = elf_getdata(section, NULL); | ||
} | ||
} | ||
|
||
if (!symtab || !head_text) | ||
return drgn_error_format(DRGN_ERROR_MISSING_DEBUG_INFO, | ||
"could not find required section"); | ||
|
||
for (int i = 1; i != numsyms; ++i) { | ||
GElf_Sym sym_mem, *sym; | ||
const char *name; | ||
|
||
sym = gelf_getsym(symtab, i, &sym_mem); | ||
if (!sym) | ||
continue; | ||
|
||
name = elf_strptr(vmlinux, strtab, sym->st_name); | ||
|
||
if (strcmp(name, "swapper_pg_dir") == 0) | ||
swapper_pg_dir_rva = sym->st_value; | ||
if (strcmp(name, "_text") == 0) | ||
text_rva = sym->st_value; | ||
if (strcmp(name, "kimage_vaddr") == 0) | ||
kimage_vaddr_rva = sym->st_value; | ||
if (strcmp(name, "vabits_actual") == 0) | ||
vabits_actual_rva = sym->st_value; | ||
} | ||
|
||
if (!swapper_pg_dir_rva || !text_rva || !kimage_vaddr_rva) | ||
return drgn_error_format(DRGN_ERROR_MISSING_DEBUG_INFO, | ||
"could not find required symbol"); | ||
|
||
|
||
/* Read the flags field, from which we infer the page size. */ | ||
image_flags = *(uint64_t *)(((char *)head_text->d_buf) + 24); | ||
if (!HOST_LITTLE_ENDIAN) | ||
image_flags = bswap_64(image_flags); | ||
if ((image_flags & 6) == 0) | ||
return drgn_error_format(DRGN_ERROR_MISSING_DEBUG_INFO, | ||
"unknown page size"); | ||
prog->vmcoreinfo.page_shift = (image_flags & 6) + 10; | ||
prog->vmcoreinfo.page_size = 1 << prog->vmcoreinfo.page_shift; | ||
|
||
if (vabits_actual_rva) { | ||
err = drgn_program_read_memory( | ||
prog, &prog->vmcoreinfo.va_bits, | ||
text_pa + (vabits_actual_rva - text_rva), 8, true); | ||
if (err) | ||
return err; | ||
if (bswap) | ||
prog->vmcoreinfo.va_bits = bswap_64(prog->vmcoreinfo.va_bits); | ||
} else { | ||
/* | ||
* Assumes that kernel RVAs start in the middle of the kernel | ||
* address space. | ||
*/ | ||
for (int bit = 63; bit != 0; --bit) { | ||
if (!(text_rva & (1ULL << bit))) { | ||
prog->vmcoreinfo.va_bits = bit + 2; | ||
break; | ||
} | ||
} | ||
if (!prog->vmcoreinfo.va_bits) { | ||
return drgn_error_format(DRGN_ERROR_MISSING_DEBUG_INFO, | ||
"could not infer VA_BITS"); | ||
} | ||
} | ||
/* | ||
* This is correct for newer kernels which set TCR_EL1.TBID1, but it | ||
* doesn't hurt to mask the top byte in older kernels as well. | ||
*/ | ||
prog->aarch64_insn_pac_mask = ~((1ULL << prog->vmcoreinfo.va_bits) - 1); | ||
|
||
err = drgn_program_read_memory(prog, &kimage_vaddr, | ||
text_pa + (kimage_vaddr_rva - text_rva), | ||
8, true); | ||
if (err) | ||
return err; | ||
if (bswap) | ||
kimage_vaddr = bswap_64(kimage_vaddr); | ||
prog->vmcoreinfo.kaslr_offset = kimage_vaddr - text_rva; | ||
prog->vmcoreinfo.swapper_pg_dir = | ||
swapper_pg_dir_rva + text_pa - text_rva; | ||
prog->vmcoreinfo.swapper_pg_dir_phys = true; | ||
return NULL; | ||
} | ||
|
||
static uint64_t untagged_addr_aarch64(uint64_t addr) | ||
{ | ||
/* Apply TBI by sign extending bit 55 into bits 56-63. */ | ||
|
@@ -494,5 +630,7 @@ const struct drgn_architecture_info arch_info_aarch64 = { | |
linux_kernel_pgtable_iterator_init_aarch64, | ||
.linux_kernel_pgtable_iterator_next = | ||
linux_kernel_pgtable_iterator_next_aarch64, | ||
.linux_kernel_init_vmcoreinfo_from_phys = | ||
linux_kernel_init_vmcoreinfo_from_phys_aarch64, | ||
.untagged_addr = untagged_addr_aarch64, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny suggestion, Python allows you to shorten this:
Non-int args are printed to stderr and an error code of 1 is returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, I'll take care of that when rebasing.