9:04 a.m.
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(a)os.amperecomputing.com>
---
v1 -->v2:
1.) more powerful get_value_vmcoreinfo().
2.) I only replace the code in arm64, since I do not have
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 */
+};
+
+/* 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
--
2.40.1
9:04 a.m.
New subject: [PATCH V2 2/2] arm64: Add vmemmap support
If the kernel exports the vmmemap then we can use that symbol in
crash to optimize access. vmmemap is just an array of page structs
after all.
This patch tries to get the "vmemmap" from the vmcore file.
If we can use the "vmemmap", we implement the arm64_vmemmap_is_page_ptr
and set it to machdep->is_page_ptr.
we implement the fast page_to_pfn code in arm64_vmemmap_is_page_ptr.
Test result:
Test the patch: "[PATCH v3] add "files -n" command for an inode"
https://lists.crash-utility.osci.io/archives/list/
devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/
Without the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed about 162 seconds.
With the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed less then 1 second.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
v1-->v2:
1.) Copy most of the code from x86_64.
2.) Tested with /proc/kcore.
---
arm64.c | 25 +++++++++++++++++++++++++
defs.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/arm64.c b/arm64.c
index e131102..42746ed 100644
--- a/arm64.c
+++ b/arm64.c
@@ -116,6 +116,28 @@ static void arm64_calc_kernel_start(void)
ms->kimage_end = (sp ? sp->value : 0);
}
+static int
+arm64_vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
+{
+ ulong size = SIZE(page);
+ ulong pfn, nr;
+
+
+ if (IS_SPARSEMEM() && (machdep->flags & VMEMMAP) &&
+ (addr >= VMEMMAP_VADDR && addr <= VMEMMAP_END) &&
+ !((addr - VMEMMAP_VADDR) % size)) {
+
+ pfn = (addr - vt->vmemmap) / size;
+ nr = pfn_to_section_nr(pfn);
+ if (valid_section_nr(nr)) {
+ if (phys)
+ *phys = PTOB(pfn);
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
/*
* Do all necessary machine-specific setup here. This is called several times
* during initialization.
@@ -385,6 +407,9 @@ arm64_init(int when)
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
+ /* If vmemmap exists, it means kernel enabled CONFIG_SPARSEMEM_VMEMMAP */
+ if (get_value_vmcoreinfo("vmemmap", &vt->vmemmap, SYMBOL_TYPE))
+ machdep->is_page_ptr = arm64_vmemmap_is_page_ptr;
machdep->uvtop = arm64_uvtop;
machdep->is_uvaddr = arm64_is_uvaddr;
diff --git a/defs.h b/defs.h
index 282fcc1..5646fa9 100644
--- a/defs.h
+++ b/defs.h
@@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data */
ulong vma_cache_fills;
void *mem_sec;
char *mem_section;
+ ulong vmemmap;
int ZONE_HIGHMEM;
ulong *node_online_map;
int node_online_map_len;
--
2.40.1
6:35 a.m.
New subject: [PATCH V2 2/2] arm64: Add vmemmap support
On 12/1/23 23:04, Huang Shijie wrote:
If the kernel exports the vmmemap then we can use that symbol in
crash to optimize access. vmmemap is just an array of page structs
after all.
This patch tries to get the "vmemmap" from the vmcore file.
If we can use the "vmemmap", we implement the arm64_vmemmap_is_page_ptr
and set it to machdep->is_page_ptr.
we implement the fast page_to_pfn code in arm64_vmemmap_is_page_ptr.
Test result:
Test the patch: "[PATCH v3] add "files -n" command for an inode"
https://lists.crash-utility.osci.io/archives/list/
devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/
Without the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed about 162 seconds.
With the this patch:
#files -n xxx (xxx is the inode of vmlinux which is 441M)
This costed less then 1 second.
Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com>
---
v1-->v2:
1.) Copy most of the code from x86_64.
2.) Tested with /proc/kcore.
---
arm64.c | 25 +++++++++++++++++++++++++
defs.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/arm64.c b/arm64.c
index e131102..42746ed 100644
--- a/arm64.c
+++ b/arm64.c
@@ -116,6 +116,28 @@ static void arm64_calc_kernel_start(void)
ms->kimage_end = (sp ? sp->value : 0);
}
+static int
+arm64_vmemmap_is_page_ptr(ulong addr, physaddr_t *phys)
+{
+ ulong size = SIZE(page);
+ ulong pfn, nr;
+
+
+ if (IS_SPARSEMEM() && (machdep->flags & VMEMMAP) &&
+ (addr >= VMEMMAP_VADDR && addr <= VMEMMAP_END) &&
+ !((addr - VMEMMAP_VADDR) % size)) {
+
+ pfn = (addr - vt->vmemmap) / size;
+ nr = pfn_to_section_nr(pfn);
+ if (valid_section_nr(nr)) {
+ if (phys)
+ *phys = PTOB(pfn);
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
/*
* Do all necessary machine-specific setup here. This is called several times
* during initialization.
@@ -385,6 +407,9 @@ arm64_init(int when)
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
+ /* If vmemmap exists, it means kernel enabled CONFIG_SPARSEMEM_VMEMMAP */
+ if (get_value_vmcoreinfo("vmemmap", &vt->vmemmap, SYMBOL_TYPE))
+ machdep->is_page_ptr = arm64_vmemmap_is_page_ptr;
machdep->uvtop = arm64_uvtop;
machdep->is_uvaddr = arm64_is_uvaddr;
diff --git a/defs.h b/defs.h
index 282fcc1..5646fa9 100644
--- a/defs.h
+++ b/defs.h
@@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data */
ulong vma_cache_fills;
void *mem_sec;
char *mem_section;
+ ulong vmemmap;
int ZONE_HIGHMEM;
ulong *node_online_map;
int node_online_map_len;
Although I haven't encountered compatibility issues due to such changes,
I would prefer it to be added to the end of this table.
In addition, could you please add the new variable to vm_table dumping?
Maybe the help -v can print the value. eg. dump_vm_table().
Thanks
Lianbo
7:55 p.m.
New subject: [PATCH V2 2/2] arm64: Add vmemmap support
Hi Lianbo,
在 2023/12/12 20:35, Lianbo Jiang 写道:
> void *mem_sec;
> char *mem_section;
> + ulong vmemmap;
> int ZONE_HIGHMEM;
> ulong *node_online_map;
> int node_online_map_len;
Although I haven't encountered compatibility issues due to such
changes, I would prefer it to be added to the end of this table.
No problem.
(I just put the relative fields togeter, the vmemmap and mem_section.)
In addition, could you please add the new variable to vm_table
dumping? Maybe the help -v can print the value. eg. dump_vm_table().
No problem.
Thanks
Huang Shijie
6:18 a.m.
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(a)os.amperecomputing.com>
---
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
7:49 p.m.
Hi Lianbo,
在 2023/12/12 20:18, Lianbo Jiang 写道:
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(a)os.amperecomputing.com>
> ---
> 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.
The benefit is :
4 files changed, 113 insertions(+), 189 deletions(-)
The code is reduced by 76 lines (I did not change other archs, such as
RISCV..)
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.
It is okay to use the base. But how to handle the "label"?
In the vmcoreinfo, there are several types:
"NUMBER(XX)=="
"OFFSET(XX)=="
"SYMBO(XX)=="
"LENGTH(XX)=="
"SIZE(XX)=="
...
Do you have any better solution to handle them?
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.
In the current kernel, the code in /kernel/crash_core.c creates the
NUMBER(XX)=%ld
But some ARCHs , such as arch/arm64/kernel/crash_core.c
NUMBER(XX)=%lx
So we need two different types for the "NUMBER" type.
Thanks
Huang Shijie
10:10 p.m.
On 12/13/23 09:49, Shijie Huang wrote:
Hi Lianbo,
在 2023/12/12 20:18, Lianbo Jiang 写道:
> 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(a)os.amperecomputing.com>
>> ---
>> 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.
The benefit is :
4 files changed, 113 insertions(+), 189 deletions(-)
The code is reduced by 76 lines (I did not change other archs, such as
RISCV..)
>
> 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.
It is okay to use the base. But how to handle the "label"?
Please do not touch the "label", it keeps the same key-value pairs in
the VMCOREINFO data. It is easy to debug if there are any changes, and
is more readable to me.
In the vmcoreinfo, there are several types:
"NUMBER(XX)=="
"OFFSET(XX)=="
"SYMBO(XX)=="
"LENGTH(XX)=="
"SIZE(XX)=="
...
Do you have any better solution to handle them?
Let me give an example, just an idea.
Try to extend the existing arm64_get_vmcoreinfo_ul() as below:
static int arm64_get_vmcoreinfo_ul(unsigned long *vaddr, const char*
label, int base)
{
int err = 0;
char *string = pc->read_vmcoreinfo(label);
if (!string)
return FALSE;
switch(base)
{
case NUM_HEX:
*vaddr = strtoul(string, NULL, 16);
break;
case NUM_DEC:
*vaddr = strtoul(string, NULL, 10);
break;
default:
err++;
error(INFO, "Unknown type: 0x%x, (NUM_HEX|NUM_DEC)\n",
base);
}
free(string);
return (err > 0) ? FALSE : TRUE;
}
And it can be invoked in arm64 code, for example:
@@ -160,11 +159,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)
+ arm64_get_vmcoreinfo_ul(&ms->kimage_voffset, "NUMBER(kimage_voffset)",
NUM_HEX);
if (ms->kimage_voffset ||
(ACTIVE() &&
(symbol_value_from_proc_kallsyms("kimage_voffset") != BADVAL))) {
@@ -185,11 +181,8 @@ arm64_init(int when)
if (kernel_symbol_exists("kimage_voffset"))
machdep->flags |= NEW_VMEMMAP;
- if (!machdep->pagesize &&
- (string = pc->read_vmcoreinfo("PAGESIZE"))) {
- machdep->pagesize = atoi(string);
- free(string);
- }
+ if (!machdep->pagesize)
+ arm64_get_vmcoreinfo_ul((ulong*)&machdep->pagesize, "PAGESIZE",
NUM_DEC);
Furthermore, this way can be extended to other arches, but it can be
done in another patch, make it become generic code. Anyway, you might
need to try it.
>
> 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.
In the current kernel, the code in /kernel/crash_core.c creates the
NUMBER(XX)=%ld
arm64_get_vmcoreinfo_ul(&value, "NUMBER(XX)", NUM_DEC);
But some ARCHs , such as arch/arm64/kernel/crash_core.c
NUMBER(XX)=%lx
arm64_get_vmcoreinfo_ul(&value, "NUMBER(XX)", NUM_HEX);
As I mentioned, we can know if the string is a decimal string or
hexadecimal string according to the kernel code.
Thanks.
Lianbo
So we need two different types for the "NUMBER" type.
Thanks
Huang Shijie
10:53 p.m.
Hi Lianbo,
在 2023/12/14 12:10, Lianbo Jiang 写道:
[EXTERNAL EMAIL NOTICE: This email originated from an external
sender.
Please be mindful of safe email handling and proprietary information
protection practices.]
On 12/13/23 09:49, Shijie Huang wrote:
> Hi Lianbo,
>
> 在 2023/12/12 20:18, Lianbo Jiang 写道:
>> 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(a)os.amperecomputing.com>
>>> ---
>>> 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.
>
> The benefit is :
>
> 4 files changed, 113 insertions(+), 189 deletions(-)
>
> The code is reduced by 76 lines (I did not change other archs, such as
> RISCV..)
>
>
>>
>> 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.
>
> It is okay to use the base. But how to handle the "label"?
>
Please do not touch the "label", it keeps the same key-value pairs in
the VMCOREINFO data. It is easy to debug if there are any changes, and
is more readable to me.
okay.
> In the vmcoreinfo, there are several types:
>
> "NUMBER(XX)=="
>
> "OFFSET(XX)=="
>
> "SYMBO(XX)=="
>
> "LENGTH(XX)=="
>
> "SIZE(XX)=="
>
> ...
>
> Do you have any better solution to handle them?
>
Let me give an example, just an idea.
Try to extend the existing arm64_get_vmcoreinfo_ul() as below:
static int arm64_get_vmcoreinfo_ul(unsigned long *vaddr, const char*
label, int base)
{
int err = 0;
char *string = pc->read_vmcoreinfo(label);
if (!string)
return FALSE;
switch(base)
{
case NUM_HEX:
*vaddr = strtoul(string, NULL, 16);
break;
case NUM_DEC:
*vaddr = strtoul(string, NULL, 10);
break;
default:
err++;
error(INFO, "Unknown type: 0x%x, (NUM_HEX|NUM_DEC)\n",
base);
}
free(string);
return (err > 0) ? FALSE : TRUE;
}
And it can be invoked in arm64 code, for example:
@@ -160,11 +159,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)
+ arm64_get_vmcoreinfo_ul(&ms->kimage_voffset,
"NUMBER(kimage_voffset)",
NUM_HEX);
if (ms->kimage_voffset ||
(ACTIVE() &&
(symbol_value_from_proc_kallsyms("kimage_voffset") != BADVAL))) {
@@ -185,11 +181,8 @@ arm64_init(int when)
if (kernel_symbol_exists("kimage_voffset"))
machdep->flags |= NEW_VMEMMAP;
- if (!machdep->pagesize &&
- (string = pc->read_vmcoreinfo("PAGESIZE"))) {
- machdep->pagesize = atoi(string);
- free(string);
- }
+ if (!machdep->pagesize)
+ arm64_get_vmcoreinfo_ul((ulong*)&machdep->pagesize, "PAGESIZE",
NUM_DEC);
Furthermore, this way can be extended to other arches, but it can be
done in another patch, make it become generic code. Anyway, you might
need to try it.
Okay, I will keep the changes in the arm64 code, and do not change the
generic 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.
>
> In the current kernel, the code in /kernel/crash_core.c creates the
>
> NUMBER(XX)=%ld
>
arm64_get_vmcoreinfo_ul(&value, "NUMBER(XX)", NUM_DEC);
> But some ARCHs , such as arch/arm64/kernel/crash_core.c
>
> NUMBER(XX)=%lx
>
arm64_get_vmcoreinfo_ul(&value, "NUMBER(XX)", NUM_HEX);
As I mentioned, we can know if the string is a decimal string or
hexadecimal string according to the kernel code.
Got it.
Thanks
Huang Shijie
348
days inactive
361
days old
devel@lists.crash-utility.osci.io
7 comments
3 participants
participants (3)
-
Huang Shijie -
Lianbo Jiang -
Shijie Huang