Hi, Shijie
Thank you for the update.
On 12/1/23 23:04, Huang Shijie wrote:
Add get_value_vmcoreinfo() to get the value for @name in vmcoreinfo.
1.) add macro GET_SYMBOL/GET_OFFSET/GET_SIZE to simplify the code.
2.) use get_value_vmcoreinfo to simplify the arm64 code.
Signed-off-by: Huang Shijie <shijie@xxxxxxxxxxxxxxxxxxxxxx>
---
v1 -->v2:
1.) more powerful get_value_vmcoreinfo().
Refactoring the code looks like a good idea, but I did not see much
benefit from the current changes.
Is it possible to extend the existing arm64_get_vmcoreinfo_ul() as below?
static int arm64_get_vmcoreinfo_ul(unsigned long *vaddr, const char*
label, int base)
And the variable 'base' may be 10 or 16 for now, because we can know
whether the string is a decimal string or hexadecimal string before
invoking it, that could be handled in this function, and this won't
touch other arches code.
Any thoughts?
2.) I only replace the code in arm64, since I do not have
It seems that the generic code has been changed, eg. kernel.c and symbols.c.
boards for other ARCHs, such as x86/ppc.
---
arm64.c | 87 ++++++------------------
defs.h | 10 +++
kernel.c | 198 ++++++++++++++++++++++--------------------------------
symbols.c | 7 +-
4 files changed, 113 insertions(+), 189 deletions(-)
diff --git a/arm64.c b/arm64.c
index 2b6b0e5..e131102 100644
--- a/arm64.c
+++ b/arm64.c
@@ -160,11 +160,8 @@ arm64_init(int when)
if (!ms->kimage_voffset && STREQ(pc->live_memsrc, "/dev/crash"))
ioctl(pc->mfd, DEV_CRASH_ARCH_DATA, &ms->kimage_voffset);
- if (!ms->kimage_voffset &&
- (string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) {
- ms->kimage_voffset = htol(string, QUIET, NULL);
- free(string);
- }
+ if (!ms->kimage_voffset)
+ get_value_vmcoreinfo("kimage_voffset", &ms->kimage_voffset, NUMBER_H_TYPE);
if (ms->kimage_voffset ||
(ACTIVE() && (symbol_value_from_proc_kallsyms("kimage_voffset") != BADVAL))) {
@@ -443,9 +440,8 @@ arm64_init(int when)
arm64_get_section_size_bits();
if (!machdep->max_physmem_bits) {
- if ((string = pc->read_vmcoreinfo("NUMBER(MAX_PHYSMEM_BITS)"))) {
- machdep->max_physmem_bits = atol(string);
- free(string);
+ if (get_value_vmcoreinfo("MAX_PHYSMEM_BITS", &machdep->max_physmem_bits, NUMBER_TYPE)) {
+ /* nothing */;
} else if (machdep->machspec->VA_BITS == 52) /* guess */
machdep->max_physmem_bits = _MAX_PHYSMEM_BITS_52;
else if (THIS_KERNEL_VERSION >= LINUX(3,17,0))
@@ -572,19 +568,6 @@ static int arm64_get_struct_page_max_shift(struct machine_specific *ms)
return (int)ceil(log2(ms->struct_page_size));
}
-/* Return TRUE if we succeed, return FALSE on failure. */
-static int arm64_get_vmcoreinfo_ul(unsigned long *vaddr, const char* label)
-{
- char *string = pc->read_vmcoreinfo(label);
-
- if (!string)
- return FALSE;
-
- *vaddr = strtoul(string, NULL, 0);
- free(string);
- return TRUE;
-}
-
/*
* The change is caused by the kernel patch since v5.18-rc1:
* "arm64: crash_core: Export MODULES, VMALLOC, and VMEMMAP ranges"
@@ -594,21 +577,21 @@ static struct kernel_range *arm64_get_range_v5_18(struct machine_specific *ms)
struct kernel_range *r = &tmp_range;
/* Get the MODULES_VADDR ~ MODULES_END */
- if (!arm64_get_vmcoreinfo_ul(&r->modules_vaddr, "NUMBER(MODULES_VADDR)"))
+ if (!get_value_vmcoreinfo("MODULES_VADDR", &r->modules_vaddr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->modules_end, "NUMBER(MODULES_END)"))
+ if (!get_value_vmcoreinfo("MODULES_END", &r->modules_end, NUMBER_H_TYPE))
return NULL;
/* Get the VMEMMAP_START ~ VMEMMAP_END */
- if (!arm64_get_vmcoreinfo_ul(&r->vmemmap_vaddr, "NUMBER(VMEMMAP_START)"))
+ if (!get_value_vmcoreinfo("VMEMMAP_START", &r->vmemmap_vaddr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->vmemmap_end, "NUMBER(VMEMMAP_END)"))
+ if (!get_value_vmcoreinfo("VMEMMAP_END", &r->vmemmap_end, NUMBER_H_TYPE))
return NULL;
/* Get the VMALLOC_START ~ VMALLOC_END */
- if (!arm64_get_vmcoreinfo_ul(&r->vmalloc_start_addr, "NUMBER(VMALLOC_START)"))
+ if (!get_value_vmcoreinfo("VMALLOC_START", &r->vmalloc_start_addr, NUMBER_H_TYPE))
return NULL;
- if (!arm64_get_vmcoreinfo_ul(&r->vmalloc_end, "NUMBER(VMALLOC_END)"))
+ if (!get_value_vmcoreinfo("VMALLOC_END", &r->vmalloc_end, NUMBER_H_TYPE))
return NULL;
return r;
@@ -888,12 +871,7 @@ range_failed:
/* Get the size of struct page {} */
static void arm64_get_struct_page_size(struct machine_specific *ms)
{
- char *string;
-
- string = pc->read_vmcoreinfo("SIZE(page)");
- if (string)
- ms->struct_page_size = atol(string);
- free(string);
+ get_value_vmcoreinfo("page", &ms->struct_page_size, SIZE_TYPE);
}
/*
@@ -1469,16 +1447,12 @@ arm64_calc_phys_offset(void)
physaddr_t paddr;
ulong vaddr;
struct syment *sp;
- char *string;
if ((machdep->flags & NEW_VMEMMAP) &&
ms->kimage_voffset && (sp = kernel_symbol_search("memstart_addr"))) {
if (pc->flags & PROC_KCORE) {
- if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
- ms->phys_offset = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("PHYS_OFFSET", &ms->phys_offset, NUMBER_H_TYPE))
return;
- }
vaddr = symbol_value_from_proc_kallsyms("memstart_addr");
if (vaddr == BADVAL)
vaddr = sp->value;
@@ -1560,9 +1534,8 @@ arm64_get_section_size_bits(void)
} else
machdep->section_size_bits = _SECTION_SIZE_BITS;
- if ((string = pc->read_vmcoreinfo("NUMBER(SECTION_SIZE_BITS)"))) {
- machdep->section_size_bits = atol(string);
- free(string);
+ if (get_value_vmcoreinfo("SECTION_SIZE_BITS", &machdep->section_size_bits, NUMBER_TYPE)) {
+ /* nothing */
} else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
if ((ret = get_kernel_config("CONFIG_MEMORY_HOTPLUG", NULL)) == IKCONFIG_Y) {
if ((ret = get_kernel_config("CONFIG_HOTPLUG_SIZE_BITS", &string)) == IKCONFIG_STR)
@@ -1581,15 +1554,11 @@ arm64_get_section_size_bits(void)
static int
arm64_kdump_phys_base(ulong *phys_offset)
{
- char *string;
struct syment *sp;
physaddr_t paddr;
- if ((string = pc->read_vmcoreinfo("NUMBER(PHYS_OFFSET)"))) {
- *phys_offset = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("PHYS_OFFSET", phys_offset, NUMBER_H_TYPE))
return TRUE;
- }
if ((machdep->flags & NEW_VMEMMAP) &&
machdep->machspec->kimage_voffset &&
@@ -4592,10 +4561,9 @@ static int
arm64_set_va_bits_by_tcr(void)
{
ulong value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(TCR_EL1_T1SZ)")) ||
- (string = pc->read_vmcoreinfo("NUMBER(tcr_el1_t1sz)"))) {
+ if (get_value_vmcoreinfo("TCR_EL1_T1SZ", &value, NUMBER_H_TYPE) ||
+ get_value_vmcoreinfo("tcr_el1_t1sz", &value, NUMBER_H_TYPE)) {
/* See ARMv8 ARM for the description of
* TCR_EL1.T1SZ and how it can be used
* to calculate the vabits_actual
@@ -4604,10 +4572,9 @@ arm64_set_va_bits_by_tcr(void)
* Basically:
* vabits_actual = 64 - T1SZ;
*/
- value = 64 - strtoll(string, NULL, 0);
+ value = 64 - value;
if (CRASHDEBUG(1))
fprintf(fp, "vmcoreinfo : vabits_actual: %ld\n", value);
- free(string);
machdep->machspec->VA_BITS_ACTUAL = value;
machdep->machspec->VA_BITS = value;
machdep->machspec->VA_START = _VA_START(machdep->machspec->VA_BITS_ACTUAL);
@@ -4623,13 +4590,8 @@ arm64_calc_VA_BITS(void)
int bitval;
struct syment *sp;
ulong vabits_actual, value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(VA_BITS)"))) {
- value = atol(string);
- free(string);
- machdep->machspec->CONFIG_ARM64_VA_BITS = value;
- }
+ get_value_vmcoreinfo("VA_BITS", &machdep->machspec->CONFIG_ARM64_VA_BITS, NUMBER_TYPE);
if (kernel_symbol_exists("vabits_actual")) {
if (pc->flags & PROC_KCORE) {
@@ -4754,10 +4716,8 @@ arm64_calc_virtual_memory_ranges(void)
ulong PUD_SIZE = UNINITIALIZED;
if (!machdep->machspec->CONFIG_ARM64_VA_BITS) {
- if ((string = pc->read_vmcoreinfo("NUMBER(VA_BITS)"))) {
- value = atol(string);
- free(string);
- machdep->machspec->CONFIG_ARM64_VA_BITS = value;
+ if (get_value_vmcoreinfo("VA_BITS", &ms->CONFIG_ARM64_VA_BITS, NUMBER_TYPE)) {
+ /* nothing */
} else if (kt->ikconfig_flags & IKCONFIG_AVAIL) {
if ((ret = get_kernel_config("CONFIG_ARM64_VA_BITS",
&string)) == IKCONFIG_STR)
@@ -4852,11 +4812,8 @@ arm64_swp_offset(ulong pte)
static void arm64_calc_KERNELPACMASK(void)
{
ulong value;
- char *string;
- if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
- value = htol(string, QUIET, NULL);
- free(string);
+ if (get_value_vmcoreinfo("KERNELPACMASK", &value, NUMBER_H_TYPE)) {
machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
if (CRASHDEBUG(1))
fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n", value);
diff --git a/defs.h b/defs.h
index 1fe2d0b..282fcc1 100644
--- a/defs.h
+++ b/defs.h
@@ -6055,6 +6055,16 @@ int hide_offline_cpu(int);
int get_highest_cpu_online(void);
int get_highest_cpu_present(void);
int get_cpus_to_display(void);
+enum vmcore_type {
+ SYMBOL_TYPE,
+ OFFSET_TYPE,
+ NUMBER_TYPE,
+ SIZE_TYPE,
+ LENGTH_TYPE,
+ NUMBER_H_TYPE,
+ MAX_TYPE,
+};
+bool get_value_vmcoreinfo(const char *name, ulong *v, enum vmcore_type type);
void get_log_from_vmcoreinfo(char *file);
int in_cpu_map(int, int);
void paravirt_init(void);
diff --git a/kernel.c b/kernel.c
index 6dcf414..3f3d908 100644
--- a/kernel.c
+++ b/kernel.c
@@ -104,6 +104,46 @@ static void check_vmcoreinfo(void);
static int is_pvops_xen(void);
static int get_linux_banner_from_vmlinux(char *, size_t);
+static char *type_names[MAX_TYPE] = {
+ "SYMBOL",
+ "OFFSET",
+ "NUMBER", /* for NUMBER_TYPE */
+ "SIZE",
+ "LENGTH",
+ "NUMBER", /* for NUMBER_H_TYPE */
+};
+
This confused me a bit.
Thanks.
Lianbo
+/* Return TRUE if we succeed, return FALSE on failure. */
+bool
+get_value_vmcoreinfo(const char *name, ulong *v, enum vmcore_type type)
+{
+ char buf[64] = {};
+ char *string;
+
+ if (type >= MAX_TYPE)
+ return FALSE;
+
+ sprintf(buf, "%s(%s)", type_names[type], name);
+ string = pc->read_vmcoreinfo(buf);
+ if (!string)
+ return FALSE;
+
+ switch (type) {
+ case SYMBOL_TYPE:
+ case NUMBER_H_TYPE:
+ *v = htol(string, RETURN_ON_ERROR, NULL);
+ break;
+ case OFFSET_TYPE:
+ case NUMBER_TYPE:
+ case SIZE_TYPE:
+ *v = dtol(string, RETURN_ON_ERROR, NULL);
+ break;
+ }
+
+ free(string);
+ return TRUE;
+}
+
/*
* popuplate the global kernel table (kt) with kernel version
* information parsed from UTSNAME/OSRELEASE string
@@ -10984,6 +11024,22 @@ hypervisor_init(void)
fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
}
+#define GET_SYMBOL(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), SYMBOL_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "SYMBOL(" s "): %lx\n", v); \
+ }
+#define GET_OFFSET(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), OFFSET_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "OFFSET(" s "): %lx\n", v); \
+ }
+#define GET_SIZE(s,v) \
+ if (get_value_vmcoreinfo((s), &(v), SIZE_TYPE)) { \
+ if (CRASHDEBUG(1)) \
+ fprintf(fp, "SIZE(" s "): %lx\n", v); \
+ }
+
/*
* Get and display the kernel log buffer using the vmcoreinfo
* data alone without the vmlinux file.
@@ -11024,125 +11080,29 @@ get_log_from_vmcoreinfo(char *file)
} else
error(FATAL, "VMCOREINFO: cannot determine page size\n");
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
- vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf): %lx\n",
- vmc->log_buf_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
- vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_end): %lx\n",
- vmc->log_end_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
- vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
- vmc->log_buf_len_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
- vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(logged_chars): %lx\n",
- vmc->logged_chars_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
- vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
- vmc->log_first_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
- vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
- vmc->log_next_idx_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
- vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(phys_base): %lx\n",
- vmc->phys_base_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SYMBOL(_stext): %lx\n",
- vmc->_stext_SYMBOL);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.ts_nsec)"))) {
- vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.ts_nsec): %ld\n",
- vmc->log_ts_nsec_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.ts_nsec)"))) {
- vmc->log_ts_nsec_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.ts_nsec): %ld\n",
- vmc->log_ts_nsec_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.len)"))) {
- vmc->log_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.len): %ld\n",
- vmc->log_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.len)"))) {
- vmc->log_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.len): %ld\n",
- vmc->log_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.text_len)"))) {
- vmc->log_text_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.text_len): %ld\n",
- vmc->log_text_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.text_len)"))) {
- vmc->log_text_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.text_len): %ld\n",
- vmc->log_text_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("OFFSET(log.dict_len)"))) {
- vmc->log_dict_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(log.dict_len): %ld\n",
- vmc->log_dict_len_OFFSET);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("OFFSET(printk_log.dict_len)"))) {
- vmc->log_dict_len_OFFSET = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "OFFSET(printk_log.dict_len): %ld\n",
- vmc->log_dict_len_OFFSET);
- free(string);
- }
- if ((string = pc->read_vmcoreinfo("SIZE(log)"))) {
- vmc->log_SIZE = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SIZE(log): %ld\n", vmc->log_SIZE);
- free(string);
- } else if ((string = pc->read_vmcoreinfo("SIZE(printk_log)"))) {
- vmc->log_SIZE = dtol(string, RETURN_ON_ERROR, NULL);
- if (CRASHDEBUG(1))
- fprintf(fp, "SIZE(printk_log): %ld\n", vmc->log_SIZE);
- free(string);
- }
+ GET_SYMBOL("log_buf", vmc->log_buf_SYMBOL);
+ GET_SYMBOL("log_end", vmc->log_end_SYMBOL);
+ GET_SYMBOL("log_buf_len", vmc->log_buf_len_SYMBOL);
+ GET_SYMBOL("logged_chars", vmc->logged_chars_SYMBOL);
+ GET_SYMBOL("log_first_idx", vmc->log_first_idx_SYMBOL);
+ GET_SYMBOL("log_next_idx", vmc->log_next_idx_SYMBOL);
+ GET_SYMBOL("phys_base", vmc->phys_base_SYMBOL);
+ GET_SYMBOL("_stext", vmc->_stext_SYMBOL);
+
+ GET_OFFSET("log.ts_nsec", vmc->log_ts_nsec_OFFSET)
+ else GET_OFFSET("printk_log.ts_nsec", vmc->log_ts_nsec_OFFSET);
+
+ GET_OFFSET("log.len", vmc->log_len_OFFSET)
+ else GET_OFFSET("printk_log.len", vmc->log_len_OFFSET);
+
+ GET_OFFSET("log.text_len", vmc->log_text_len_OFFSET)
+ else GET_OFFSET("printk_log.text_len", vmc->log_text_len_OFFSET);
+
+ GET_OFFSET("log.dict_len", vmc->log_dict_len_OFFSET)
+ else GET_OFFSET("printk_log.dict_len", vmc->log_dict_len_OFFSET);
+
+ GET_SIZE("log", vmc->log_SIZE)
+ else GET_SIZE("printk_log", vmc->log_SIZE);
/*
* The per-arch VTOP() macro must be functional.
diff --git a/symbols.c b/symbols.c
index 8e8b4c3..20e598d 100644
--- a/symbols.c
+++ b/symbols.c
@@ -632,11 +632,8 @@ kaslr_init(void)
!machine_type("S390X") && !machine_type("RISCV64")) || (kt->flags & RELOC_SET))
return;
- if (!kt->vmcoreinfo._stext_SYMBOL &&
- (string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
- kt->vmcoreinfo._stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
- free(string);
- }
+ if (!kt->vmcoreinfo._stext_SYMBOL)
+ get_value_vmcoreinfo("_stext", &kt->vmcoreinfo._stext_SYMBOL, SYMBOL_TYPE);
/*
* --kaslr=auto
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
