Re: [Crash-utility] [PATCH] Determine the ARM64 kernel's Pointer Authentication mask value by reading the new KERNELPACMASK vmcoreinfo entry.
Hi Dave,
On 4/7/20 9:17 PM, Dave Anderson wrote:
Hi Amit,
I have a few suggestions and a question regarding your patch.
First, these two warnings need to be addressed:
$ make warn
...
cc -c -g -DARM64 -DLZO -DSNAPPY -DGDB_7_6 arm64.c -Wall -O2
-Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security
arm64.c: In function ‘arm64_calc_KERNELPACMASK’:
arm64.c:4090:2: warning: format ‘%llx’ expects argument of type ‘long long
unsigned int’, but argument 3 has type ‘ulong’ [-Wformat=]
fprintf(fp, " got NUMBER(KERNELPACMASK) =%llx\n", value);
^
arm64.c:4090:9: warning: ‘value’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
fprintf(fp, " got NUMBER(KERNELPACMASK) =%llx\n", value);
^
...
The message should be moved inside the if statement, and also should be gated
with CRASHDEBUG(1) to prevent it from being displayed unconditionally:
static void
arm64_calc_KERNELPACMASK(void)
{
ulong value;
char *string;
if ((string = pc->read_vmcoreinfo("NUMBER(KERNELPACMASK)"))) {
value = htol(string, QUIET, NULL);
free(string);
machdep->machspec->CONFIG_ARM64_KERNELPACMASK = value;
if (CRASHDEBUG(1))
fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: %lx\n",
value);
}
}
Yes thanks for pointing this out. I just posted v2 version and added
your suggestion.
And the CONFIG_ARM64_KERNELPACMASK value should be displayed in arm64_dump_machdep_table().
Ok.
But given that the patch only modifies text return addresses on the kernel
stack is here:
@@ -1932,6 +1936,9 @@ arm64_print_stackframe_entry(struct bt_info *bt, int
level, struct arm64_stackfr
* See, for example, "bl schedule" before ret_to_user().
*/
branch_pc = frame->pc - 4;
+ if (ms->CONFIG_ARM64_KERNELPACMASK)
+ branch_pc |= ms->CONFIG_ARM64_KERNELPACMASK;
+
name = closest_symbol(branch_pc);
name_plus_offset = NULL;
I'm wondering how all of the other places that check addresses found
on the kernel stack will work? For example, all of these places
check whether an address found on the stack is a kernel text address:
$ grep is_kernel_text arm64.c
is_kernel_text(regs->pc) &&
is_kernel_text(regs->regs[30])) {
if (is_kernel_text(*up)) { <= from
arm64_print_text_symbols()
if (is_kernel_text(frame->pc) ||
if (is_kernel_text(frame->pc) &&
if (is_kernel_text(regs->pc) &&
if (is_kernel_text(LR) &&
if (is_kernel_text(regs->pc) && (bt->flags & BT_LINE_NUMBERS)) {
$
Except for the call arm64_print_text_symbols(), these are checking register
values found in exception frames. Can you confirm that they will still be
unmodified kernel text addresses?
The call from arm64_print_text_symbols() is for "bt -[tT]", which just
scours a kernel stack for text addresses and dumps them. So presumably
that needs to apply the mask to each stack value as you've done in
arm64_print_stackframe_entry()? (while still recognizing unmodified text
addresses found in exception frames)
I added PAC mask changes in most of the is_kernel_text function except
arm64_get_stack_frame as it is the first function so PC should not have
any PAC details. Thanks for your detailed suggestion. It was helpful in
fixing the missing implementation.
Thanks,
Amit Daniel
Thanks,
Dave
- Original Message -
This value is used to mask the PAC bits and generate correct backtrace.
(amit.kach...@arm.com)
---
The kernel version for the corresponding vmcoreinfo entry is posted here[1].
[1]: https://lore.kernel.org/patchwork/patch/1211981/
arm64.c | 20
defs.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/arm64.c b/arm64.c
index 09b1b76..55e084f 100644
--- a/arm64.c
+++ b/arm64.c
@@ -84,6 +84,7 @@ static int arm64_get_kvaddr_ranges(struct vaddr_range *);
static void arm64_get_crash_notes(void);
static void arm64_calc_VA_BITS(void);
static int arm64_is_uvaddr(ulong, struct task_context *);
+static void arm64_calc_KERNELPACMASK(void);
/*
@@ -213,6 +214,7 @@ arm64_init(int when)
machdep->pagemask = ~((ulonglong)machdep->pageoffset);
arm64_calc_VA_BITS();
+ arm64_calc_KERNELPACMASK();
ms = machdep->machspec;
if (ms->VA_BITS_ACTUAL) {
ms->page_offset = ARM64_PAGE_OFFSET_ACTUAL;
@@ -472,6 +474,7 @@ arm64_init(int when)
case LOG_ONLY:
machdep->machspec = _machine_specific;
arm64_calc_VA_BITS();
+ arm64_calc_KERNELPACMASK();
arm64_calc_phys_offset();
machdep->machspec->page_offset = ARM64_PAGE_OFFSET;
break;
@@ -1925,6
[Crash-utility] [PATCH v2] Determine the ARM64 kernel's Pointer Authentication mask value by reading the new KERNELPACMASK vmcoreinfo entry.
This value is used to mask the PAC bits and generate correct backtrace
and symbol name.
(amit.kach...@arm.com)
---
Changes since v1:
* Moved PAC mask code from arm64_print_stackframe_entry to
arm64_unwind_frame.
* PAC mask check on all kernel text during complete stack parsing
with bt -t command.
* dump_machdep_table now prints CONFIG_ARM64_KERNELPACMASK.
The kernel version for the corresponding vmcoreinfo entry is posted here[1].
[1]: https://lore.kernel.org/patchwork/patch/1211981/
arm64.c | 52 ++--
defs.h | 1 +
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/arm64.c b/arm64.c
index 7662d71..326b8e0 100644
--- a/arm64.c
+++ b/arm64.c
@@ -84,6 +84,7 @@ static int arm64_get_kvaddr_ranges(struct vaddr_range *);
static void arm64_get_crash_notes(void);
static void arm64_calc_VA_BITS(void);
static int arm64_is_uvaddr(ulong, struct task_context *);
+static void arm64_calc_KERNELPACMASK(void);
/*
@@ -213,6 +214,7 @@ arm64_init(int when)
machdep->pagemask = ~((ulonglong)machdep->pageoffset);
arm64_calc_VA_BITS();
+ arm64_calc_KERNELPACMASK();
ms = machdep->machspec;
if (ms->VA_BITS_ACTUAL) {
ms->page_offset = ARM64_PAGE_OFFSET_ACTUAL;
@@ -472,6 +474,7 @@ arm64_init(int when)
case LOG_ONLY:
machdep->machspec = _machine_specific;
arm64_calc_VA_BITS();
+ arm64_calc_KERNELPACMASK();
arm64_calc_phys_offset();
machdep->machspec->page_offset = ARM64_PAGE_OFFSET;
break;
@@ -659,6 +662,11 @@ arm64_dump_machdep_table(ulong arg)
fprintf(fp, "%ld\n", ms->VA_BITS_ACTUAL);
else
fprintf(fp, "(unused)\n");
+ fprintf(fp, "CONFIG_ARM64_KERNELPACMASK: ");
+ if (ms->CONFIG_ARM64_KERNELPACMASK)
+ fprintf(fp, "%lx\n", ms->CONFIG_ARM64_KERNELPACMASK);
+ else
+ fprintf(fp, "(unused)\n");
fprintf(fp, " userspace_top: %016lx\n", ms->userspace_top);
fprintf(fp, " page_offset: %016lx\n", ms->page_offset);
fprintf(fp, "vmalloc_start_addr: %016lx\n", ms->vmalloc_start_addr);
@@ -1774,13 +1782,14 @@ static int
arm64_is_kernel_exception_frame(struct bt_info *bt, ulong stkptr)
{
struct arm64_pt_regs *regs;
+ struct machine_specific *ms = machdep->machspec;
regs = (struct arm64_pt_regs
*)>stackbuf[(ulong)(STACK_OFFSET_TYPE(stkptr))];
if (INSTACK(regs->sp, bt) && INSTACK(regs->regs[29], bt) &&
!(regs->pstate & (0xULL | PSR_MODE32_BIT)) &&
- is_kernel_text(regs->pc) &&
- is_kernel_text(regs->regs[30])) {
+ is_kernel_text(regs->pc | ms->CONFIG_ARM64_KERNELPACMASK) &&
+ is_kernel_text(regs->regs[30] | ms->CONFIG_ARM64_KERNELPACMASK)) {
switch (regs->pstate & PSR_MODE_MASK)
{
case PSR_MODE_EL1t:
@@ -1924,6 +1933,7 @@ arm64_print_stackframe_entry(struct bt_info *bt, int
level, struct arm64_stackfr
* See, for example, "bl schedule" before ret_to_user().
*/
branch_pc = frame->pc - 4;
+
name = closest_symbol(branch_pc);
name_plus_offset = NULL;
@@ -2135,7 +2145,7 @@ arm64_unwind_frame(struct bt_info *bt, struct
arm64_stackframe *frame)
unsigned long stack_mask;
unsigned long irq_stack_ptr, orig_sp;
struct arm64_pt_regs *ptregs;
- struct machine_specific *ms;
+ struct machine_specific *ms = machdep->machspec;
stack_mask = (unsigned long)(ARM64_STACK_SIZE) - 1;
fp = frame->fp;
@@ -2149,6 +2159,8 @@ arm64_unwind_frame(struct bt_info *bt, struct
arm64_stackframe *frame)
frame->sp = fp + 0x10;
frame->fp = GET_STACK_ULONG(fp);
frame->pc = GET_STACK_ULONG(fp + 8);
+ if (is_kernel_text(frame->pc | ms->CONFIG_ARM64_KERNELPACMASK))
+ frame->pc |= ms->CONFIG_ARM64_KERNELPACMASK;
if ((frame->fp == 0) && (frame->pc == 0))
return FALSE;
@@ -2200,7 +2212,6 @@ arm64_unwind_frame(struct bt_info *bt, struct
arm64_stackframe *frame)
* irq_stack_ptr = IRQ_STACK_PTR(raw_smp_processor_id());
* orig_sp = IRQ_STACK_TO_TASK_STACK(irq_stack_ptr); (pt_regs
pointer on process stack)
*/
- ms = machdep->machspec;
irq_stack_ptr = ms->irq_stacks[bt->tc->processor] + ms->irq_stack_size
- 16;
if (frame->sp == irq_stack_ptr) {
@@ -2802,6 +2813,8 @@ arm64_print_text_symbols(struct bt_info *bt, struct
arm64_stackframe *frame, FIL
char buf2[BUFSIZE];
char *name;
ulong start;
+ ulong val;
+ struct machine_specific *ms = machdep->machspec;
if (bt->flags & BT_TEXT_SYMBOLS_ALL)
start = bt->stackbase;
@@ -2816,8 +2829,10 @@
[Crash-utility] [PATCH v2] add log -T option to display the message text with human readable timestamp
Sometimes, we need to know the accurate time of the log, which
helps us analyze the problem.
add -T option(like dmesg -T command) for log command to display
the message text with human readable timestamp.
Signed-off-by: Wang Long
---
defs.h | 2 ++
help.c | 28 +++-
kernel.c | 23 +--
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/defs.h b/defs.h
index d8eda5e..4e57a56 100644
--- a/defs.h
+++ b/defs.h
@@ -763,6 +763,7 @@ struct kernel_table { /* kernel data */
} vmcoreinfo;
ulonglong flags2;
char *source_tree;
+ struct timespec boot_date;
};
/*
@@ -5577,6 +5578,7 @@ void dump_log(int);
#define SHOW_LOG_DICT (0x2)
#define SHOW_LOG_TEXT (0x4)
#define SHOW_LOG_AUDIT (0x8)
+#define SHOW_LOG_CTIME (0x10)
void set_cpu(int);
void clear_machdep_cache(void);
struct stack_hook *gather_text_list(struct bt_info *);
diff --git a/help.c b/help.c
index c443cad..1ee70f7 100644
--- a/help.c
+++ b/help.c
@@ -3892,12 +3892,13 @@ NULL
char *help_log[] = {
"log",
"dump system message buffer",
-"[-tdma]",
+"[-Ttdma]",
" This command dumps the kernel log_buf contents in chronological order.
The",
" command supports the older log_buf formats, which may or may not contain a",
" timestamp inserted prior to each message, as well as the newer
variable-length",
" record format, where the timestamp is contained in each log entry's
header.",
" ",
+"-T Display the message text with human readable timestamp.",
"-t Display the message text without the timestamp; only applicable to
the",
"variable-length record format.",
"-d Display the dictionary of key/value pair properties that are
optionally",
@@ -4031,6 +4032,31 @@ char *help_log[] = {
"type=1307 audit(1489384479.809:4346): cwd=\"/proc\"",
"...",
" ",
+" Display the message text with human readable timestamp.\n"
+"%s> log -T",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0x-0x0009fbff] usable",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0x0009fc00-0x0009] reserved",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0x000f-0x000f] reserved",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0x0010-0xdffe] usable",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0xdfff-0xdfff] ACPI data",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0xfec0-0xfec00fff] reserved",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0xfee0-0xfee00fff] reserved",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0xfffc-0x] reserved",
+"[Sat Apr 4 07:41:09 2020] BIOS-e820: [mem
0x0001-0x00011fff] usable",
+"[Sat Apr 4 07:41:09 2020] NX (Execute Disable) protection: active",
+"[Sat Apr 4 07:41:09 2020] SMBIOS 2.5 present.",
+"[Sat Apr 4 07:41:09 2020] DMI: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006",
+"[Sat Apr 4 07:41:09 2020] Hypervisor detected: KVM",
+"[Sat Apr 4 07:41:09 2020] kvm-clock: Using msrs 4b564d01 and 4b564d00",
+"[Sat Apr 4 07:41:09 2020] kvm-clock: cpu 0, msr 6de01001, primary cpu
clock",
+"[Sat Apr 4 07:41:09 2020] kvm-clock: using sched offset of 11838753697
cycles",
+"[Sat Apr 4 07:41:09 2020] clocksource: kvm-clock: mask:
0x max_cycles: 0x1cd42e4dffb, max_idle_ns: 881590591483 ns",
+"[Sat Apr 4 07:41:09 2020] e820: update [mem 0x-0x0fff]
usable ==> reserved",
+"[Sat Apr 4 07:41:09 2020] e820: remove [mem 0x000a-0x000f]
usable",
+"[Sat Apr 4 07:41:09 2020] last_pfn = 0x12 max_arch_pfn =
0x4",
+"[Sat Apr 4 07:41:09 2020] MTRR default type: uncachable",
+"[Sat Apr 4 07:41:09 2020] MTRR variable ranges disabled:",
+"...",
NULL
};
diff --git a/kernel.c b/kernel.c
index 7604fac..7e68e6d 100644
--- a/kernel.c
+++ b/kernel.c
@@ -4912,9 +4912,12 @@ cmd_log(void)
msg_flags = 0;
-while ((c = getopt(argcnt, args, "tdma")) != EOF) {
+while ((c = getopt(argcnt, args, "Ttdma")) != EOF) {
switch(c)
{
+ case 'T':
+ msg_flags |= SHOW_LOG_CTIME;
+ break;
case 't':
msg_flags |= SHOW_LOG_TEXT;
break;
@@ -4936,6 +4939,15 @@ cmd_log(void)
if (argerrs)
cmd_usage(pc->curcmd, SYNOPSIS);
+ if (kt->boot_date.tv_sec == 0) {
+ ulonglong uptime_jiffies;
+ ulong uptime_sec;
+ get_uptime(NULL, _jiffies);
+ uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz;
+ kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec;
+ kt->boot_date.tv_nsec = 0;
+ }
Re: [Crash-utility] [PATCH] add log -T option to display the message text with human readable timestamp
On 20/4/2020 11:28 pm, Dave Anderson wrote: - Original Message - On 20/4/2020 3:48 am, Dave Anderson wrote: FWIW, I tried it on another RHEL7 machine running live, but then also on a RHEL8 kernel dumpfile, and they all hang: I apply this patch on RHEL7 virtual machine(VirtualBbox) and it work ok. and on a RHEL7 kernel dumpfile, I found it hang after I send the patch. and I debug it and found the machdep->hz == 0 on the following: get_uptime(NULL, _jiffies); uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz; kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; kt->boot_date.tv_nsec = 0; because machdep-> hz has not been initialized here. divide by zero make the cpu spinning at 100%. I thought two solutions: (1) add misc_init function after machdep_init(POST_INIT) call, and calculate the value of kt-> boot_date in it. read_in_kernel_config(IKCFG_INIT); kernel_init(); machdep_init(POST_GDB); vm_init(); machdep_init(POST_VM); module_init(); help_init(); task_init(); vfs_init(); net_init(); dev_init(); machdep_init(POST_INIT); + misc_init(); (2) calculate the value of kt-> boot_date on cmd_log function, when we call log command. Dave, Which one do you like? Definitely option #2. Since it's not required unless your new command option is run, you can simply check whether the new boot_date structure is still zero-filled, and do your initialization at that time. And BTW, please move the boot_date structure to the end of the kernel_table to prevent any possible breakage of previously-compiled extension modules that use the kernel_table. And also can you please display the new structure's contents in dump_kernel_table()? You can put the display under the current "date" display. OK, I will send the v2 patch. Thanks Thanks, Dave -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
- Original Message -
> It's just a risk,i found this risk when i try to fix crash-utility launch
> error with arm64 in 5.4.
> i made the fix patch the almost same as Vinayak's,As a supplement, I make
> these two suggestion(vmemmap_start _offset).
> If the advice is reasonable, you can take it
Let's wait for Vinayak to consolidate everything in his v2 patch update. When
he
posts it, please review and give your ACK/NAK.
Thanks,
Dave
>
>
> From: crash-utility-boun...@redhat.com on
> behalf of Dave Anderson
> Sent: Monday, April 20, 2020 22:54
> To: Discussion list for crash utility usage,maintenance and development
> Subject: [营销邮件] Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re:
> [External Mail][] Re: ramdump support for va_bits_actual
>
> - Original Message -
> > In fact,vmemmap not easy to calculated in crash-utility,if
> > CONFIG_RANDOMIZE_BASE is configured,memstart_addr will be changed since
> > below codes:
> > [arm64_memblock_init]
> > 348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
> > PAGE_SHIFT));
> > ...
> > 413 if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > 414 extern u16 memstart_offset_seed;
> > 415 u64 range = linear_region_size -
> > 416 (memblock_end_of_DRAM() -
> > memblock_start_of_DRAM());
> > 417
> > 418 /*
> > 419 * If the size of the linear region exceeds, by a
> > sufficient
> > 420 * margin, the size of the region that the available
> > physical
> > 421 * memory spans, randomize the linear region as well.
> > 422 */
> > 423 if (memstart_offset_seed > 0 && range >=
> > ARM64_MEMSTART_ALIGN) {
> > 424 range /= ARM64_MEMSTART_ALIGN;
> > 425 memstart_addr -= ARM64_MEMSTART_ALIGN *
> > 426 ((range *
> > memstart_offset_seed) >> 16);
> > 427 }
> > 428 }
>
> OK.
>
> >
> > the reason i showed the "address_markers " is just to prove vmemmap and
> > ms->vmemmap_start is wrong.we'd better to do below change.
> > - vmemmap_start = (-vmemmap_size);
> > + vmemmap_start = (-vmemmap_size - MEGABYTES(2));
>
>
>
> >
> >$ crash vmlinux vmcore
> >
> >crash 7.2.9rc13
> >Copyright (C) 2002-2020 Red Hat, Inc.
> >Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation
> >Copyright (C) 1999-2006 Hewlett-Packard Co
> >Copyright (C) 2005, 2006, 2011, 2012 Fujitsu Limited
> >Copyright (C) 2006, 2007 VA Linux Systems Japan K.K.
> >Copyright (C) 2005, 2011 NEC Corporation
> >Copyright (C) 1999, 2002, 2007 Silicon Graphics, Inc.
> >Copyright (C) 1999, 2000, 2001, 2002 Mission Critical Linux, Inc.
> >This program is free software, covered by the GNU General Public
> >License,
> >and you are welcome to change it and/or distribute copies of it under
> >certain conditions. Enter "help copying" to see the conditions.
> >This program has absolutely no warranty. Enter "help warranty" for
> >details.
> >
> >GNU gdb (GDB) 7.6
> >Copyright (C) 2013 Free Software Foundation, Inc.
> >License GPLv3+: GNU GPL version 3 or later
> >
> >This is free software: you are free to change and redistribute it.
> >There is NO WARRANTY, to the extent permitted by law. Type "show
> >copying"
> >and "show warranty" for details.
> >This GDB was configured as "x86_64-unknown-linux-gnu"...
> >
> >WARNING: kernel relocated [950MB]: patching 94929 gdb minimal_symbol
> >values
> >
> >crash: start patch...
> >
> >
> >
> > - Original Message -
> >>
> >>
> >> - Original Message -
> >>> Sometimes, we need to know the accurate time of the log, which
> >>> helps us analyze the problem.
> >>>
> >>> add -T option(like dmesg -T command) for log command to display
> >>> the message text with human readable timestamp.
> >>>
> >>> Signed-off-by: Wang Long
> >>
> >> Did you attempt this patch on a live system? Because your patch to
> >> kernel_init() hangs the session. I didn't bother to investigate beyond
> >> adding these two debug statements around your addition to kernel_init():
> >>
> >>error(INFO, "start patch...\n");
> >>get_uptime(NULL, _jiffies);
> >>uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz;
> >>kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec;
> >>kt->boot_date.tv_nsec = 0;
> >>error(INFO, "end patch...\n");
> >>
> >> And that's where it hangs:
> >>
> >>$ ./crash
> >>
> >>crash 7.2.9rc13
> >>Copyright (C) 2002-2020 Red Hat, Inc.
> >>Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation
> >>Copyright (C) 1999-2006 Hewlett-Packard Co
> >>
Re: [Crash-utility] [PATCH] add log -T option to display the message text with human readable timestamp
- Original Message - > > > On 20/4/2020 3:48 am, Dave Anderson wrote: > > > > FWIW, I tried it on another RHEL7 machine running live, > > but then also on a RHEL8 kernel dumpfile, and they all hang: > > I apply this patch on RHEL7 virtual machine(VirtualBbox) and it work ok. > and on a RHEL7 kernel dumpfile, I found it hang after I send the patch. > > > and I debug it and found the machdep->hz == 0 on the following: > >get_uptime(NULL, _jiffies); >uptime_sec = (uptime_jiffies)/(ulonglong)machdep->hz; >kt->boot_date.tv_sec = kt->date.tv_sec - uptime_sec; >kt->boot_date.tv_nsec = 0; > > because machdep-> hz has not been initialized here. divide by zero make > the cpu spinning at 100%. > > I thought two solutions: > > (1) add misc_init function after machdep_init(POST_INIT) call, and > calculate the value of kt-> boot_date in it. > read_in_kernel_config(IKCFG_INIT); > kernel_init(); > machdep_init(POST_GDB); > vm_init(); > machdep_init(POST_VM); > module_init(); > help_init(); > task_init(); > vfs_init(); > net_init(); > dev_init(); > machdep_init(POST_INIT); > + misc_init(); > > (2) calculate the value of kt-> boot_date on cmd_log function, when we > call log command. > > > Dave, Which one do you like? Definitely option #2. Since it's not required unless your new command option is run, you can simply check whether the new boot_date structure is still zero-filled, and do your initialization at that time. And BTW, please move the boot_date structure to the end of the kernel_table to prevent any possible breakage of previously-compiled extension modules that use the kernel_table. And also can you please display the new structure's contents in dump_kernel_table()? You can put the display under the current "date" display. Thanks, Dave -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
It's just a risk,i found this risk when i try to fix crash-utility launch error
with arm64 in 5.4.
i made the fix patch the almost same as Vinayak's,As a supplement, I make these
two suggestion(vmemmap_start _offset).
If the advice is reasonable, you can take it
From: crash-utility-boun...@redhat.com on
behalf of Dave Anderson
Sent: Monday, April 20, 2020 22:54
To: Discussion list for crash utility usage,maintenance and development
Subject: [营销邮件] Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re:
[External Mail][] Re: ramdump support for va_bits_actual
- Original Message -
> In fact,vmemmap not easy to calculated in crash-utility,if
> CONFIG_RANDOMIZE_BASE is configured,memstart_addr will be changed since
> below codes:
> [arm64_memblock_init]
> 348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
> PAGE_SHIFT));
> ...
> 413 if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> 414 extern u16 memstart_offset_seed;
> 415 u64 range = linear_region_size -
> 416 (memblock_end_of_DRAM() -
> memblock_start_of_DRAM());
> 417
> 418 /*
> 419 * If the size of the linear region exceeds, by a
> sufficient
> 420 * margin, the size of the region that the available
> physical
> 421 * memory spans, randomize the linear region as well.
> 422 */
> 423 if (memstart_offset_seed > 0 && range >=
> ARM64_MEMSTART_ALIGN) {
> 424 range /= ARM64_MEMSTART_ALIGN;
> 425 memstart_addr -= ARM64_MEMSTART_ALIGN *
> 426 ((range * memstart_offset_seed)
> >> 16);
> 427 }
> 428 }
OK.
>
> the reason i showed the "address_markers " is just to prove vmemmap and
> ms->vmemmap_start is wrong.we'd better to do below change.
> - vmemmap_start = (-vmemmap_size);
> + vmemmap_start = (-vmemmap_size - MEGABYTES(2));
This looks correct, although I've never seen a problem using the current
setting on 5.4 and later kernels. What happens on your system? Is your
system's memstart_addr within that low 2MB?
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
#/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from XIAOMI,
which is intended only for the person or entity whose address is listed above.
Any use of the information contained herein in any way (including, but not
limited to, total or partial disclosure, reproduction, or dissemination) by
persons other than the intended recipient(s) is prohibited. If you receive this
e-mail in error, please notify the sender by phone or email immediately and
delete it!**/#
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] ramdump support for va_bits_actual
- Original Message -
> Hi Dave,
>
> On Sat, Apr 18, 2020 at 2:08 PM Dave Anderson wrote:
> >
> >
> >
> > - Original Message -
> > > Hi Dave,
> > >
> > > Noticed that raw ramdumps of 5.4 kernel aren't working with crash tip.
> > > With the patches attached, I could get it working. Please take a look.
> > >
> > > Thanks,
> > > Vinayak
> > >
> >
> > Hi Vinayak,
> >
> > A couple quick questions come to mind...
> >
> > First, I haven't checked all possible READMEM plugins, but for example, if
> > this
> > function is run on a live system, the -1 file descriptor would cause the
> > READMEM()
> > call to fail:
>
>
> I changed it like this and it works for ramdump. I don't actually have
> a live setup to try this. Let me try
> to set up one.
>
> diff --git a/arm64.c b/arm64.c
> index 04efc13..fce3f8e 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -981,7 +981,7 @@ arm64_calc_physvirt_offset(void)
>
> if ((sp = kernel_symbol_search("physvirt_offset")) &&
> machdep->machspec->kimage_voffset) {
> - if (READMEM(-1, _offset, sizeof(physvirt_offset),
> + if (READMEM(pc->mfd, _offset,
> sizeof(physvirt_offset),
> sp->value, sp->value -
> machdep->machspec->kimage_voffset) > 0) {
> ms->physvirt_offset = physvirt_offset;
>
>
> >
> > static void
> > +arm64_calc_physvirt_offset(void)
> > +{
> > + struct machine_specific *ms = machdep->machspec;
> > + ulong physvirt_offset;
> > + struct syment *sp;
> > +
> > + ms->physvirt_offset = ms->phys_offset - ms->page_offset;
> > +
> > + if ((sp = kernel_symbol_search("physvirt_offset")) &&
> > + machdep->machspec->kimage_voffset) {
> > + if (READMEM(-1, _offset, sizeof(physvirt_offset),
> > + sp->value, sp->value -
> > + machdep->machspec->kimage_voffset) > 0) {
> > + ms->physvirt_offset = physvirt_offset;
> > + }
> > + }
> > +
> > + if (CRASHDEBUG(1))
> > + fprintf(fp, "using %lx as physvirt_offset\n",
> > ms->physvirt_offset);
> > +}
> >
> > And here -- are you missing some brackets? (run "make warn")
> >
>
> I did try "make warn" and it does not show any issues.Am I missing something?
I saw on a system provisioned with Fedora's latest and greatest gcc version.
I don't have the system available any more, but the warning message picked up
on the fact that your second if statement "was not guarded" by the if statement
above it.
>
> > But regardless of that, why are you setting it back to 48 if it's greater
> > than 48?
> >
>
>
> I did that because machspec->CONFIG_ARM64_VA_BITS is used for calculation of
> vmemmap size. In kernel vmemmap size is calculated using VA_BITS_MIN and it is
> defined like this
>
> #if VA_BITS > 48
> #define VA_BITS_MIN (48)
> #else
> #define VA_BITS_MIN (VA_BITS)
> #endif
>
> But I realize now that its not the right thing to do, because
> machspec->CONFIG_ARM64_VA_BITS
> is later used in arm64_calc_VA_BITS to verify machspec->VA_BITS. So
> what about this ?
>
> diff --git a/arm64.c b/arm64.c
> index 04efc13..a35a30e 100644
> --- a/arm64.c
> +++ b/arm64.c
> @@ -4023,8 +4023,6 @@ arm64_calc_virtual_memory_ranges(void)
> if ((ret = get_kernel_config("CONFIG_ARM64_VA_BITS",
> )) == IKCONFIG_STR)
> machdep->machspec->CONFIG_ARM64_VA_BITS =
> atol(string);
> - if (machdep->machspec->CONFIG_ARM64_VA_BITS >
> 48)
> -
> machdep->machspec->CONFIG_ARM64_VA_BITS = 48;
> }
> }
>
> @@ -4049,7 +4047,12 @@ arm64_calc_virtual_memory_ranges(void)
> #define STRUCT_PAGE_MAX_SHIFT 6
>
> if (ms->VA_BITS_ACTUAL) {
> - vmemmap_size = (1UL) << (ms->CONFIG_ARM64_VA_BITS -
> machdep->pageshift - 1 + STRUCT_PAGE_MAX_SHIFT);
> + ulong va_bits_min = 48;
> +
> + if (machdep->machspec->CONFIG_ARM64_VA_BITS < 48)
> + va_bits_min = ms->CONFIG_ARM64_VA_BITS;
> +
> + vmemmap_size = (1UL) << (va_bits_min - machdep->pageshift - 1
> + STRUCT_PAGE_MAX_SHIFT);
> vmalloc_end = (- PUD_SIZE - vmemmap_size - KILOBYTES(64));
> vmemmap_start = (-vmemmap_size);
> ms->vmalloc_end = vmalloc_end - 1;
>
Yeah, that looks reasonable. But what about the parallel discussion re:
vmemmap_start?
https://www.redhat.com/archives/crash-utility/2020-April/msg00064.html
Can you send in an updated patch set with all fixes applied?
Thanks,
Dave
Shouldn't it be
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
- Original Message -
> In fact,vmemmap not easy to calculated in crash-utility,if
> CONFIG_RANDOMIZE_BASE is configured,memstart_addr will be changed since
> below codes:
> [arm64_memblock_init]
> 348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
> PAGE_SHIFT));
> ...
> 413 if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> 414 extern u16 memstart_offset_seed;
> 415 u64 range = linear_region_size -
> 416 (memblock_end_of_DRAM() -
> memblock_start_of_DRAM());
> 417
> 418 /*
> 419 * If the size of the linear region exceeds, by a
> sufficient
> 420 * margin, the size of the region that the available
> physical
> 421 * memory spans, randomize the linear region as well.
> 422 */
> 423 if (memstart_offset_seed > 0 && range >=
> ARM64_MEMSTART_ALIGN) {
> 424 range /= ARM64_MEMSTART_ALIGN;
> 425 memstart_addr -= ARM64_MEMSTART_ALIGN *
> 426 ((range * memstart_offset_seed)
> >> 16);
> 427 }
> 428 }
OK.
>
> the reason i showed the "address_markers " is just to prove vmemmap and
> ms->vmemmap_start is wrong.we'd better to do below change.
> - vmemmap_start = (-vmemmap_size);
> + vmemmap_start = (-vmemmap_size - MEGABYTES(2));
This looks correct, although I've never seen a problem using the current
setting on 5.4 and later kernels. What happens on your system? Is your
system's memstart_addr within that low 2MB?
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] new printk ringbuffer interface
- Original Message - > Hello Dave, > > You may or may not be aware that we are working on replacing [0] the > Linux printk ringbuffer. Rather than a single buffer containing a single > struct type, the new ringbuffer makes use of several different structs. Yes, I am most definitely aware... > > I am writing to ask your advice about how this should be exported for > the crash utility. Should all struct sizes and field offsets be > exported? It would look something like this: > > VMCOREINFO_SYMBOL(prb); > > VMCOREINFO_STRUCT_SIZE(printk_ringbuffer); > VMCOREINFO_OFFSET(printk_ringbuffer, desc_ring); > VMCOREINFO_OFFSET(printk_ringbuffer, text_data_ring); > VMCOREINFO_OFFSET(printk_ringbuffer, dict_data_ring); > VMCOREINFO_OFFSET(printk_ringbuffer, fail); > > VMCOREINFO_STRUCT_SIZE(prb_desc_ring); > VMCOREINFO_OFFSET(prb_desc_ring, count_bits); > VMCOREINFO_OFFSET(prb_desc_ring, descs); > VMCOREINFO_OFFSET(prb_desc_ring, head_id); > VMCOREINFO_OFFSET(prb_desc_ring, tail_id); > > VMCOREINFO_STRUCT_SIZE(prb_desc); > VMCOREINFO_OFFSET(prb_desc, info); > VMCOREINFO_OFFSET(prb_desc, state_var); > VMCOREINFO_OFFSET(prb_desc, text_blk_lpos); > VMCOREINFO_OFFSET(prb_desc, dict_blk_lpos); > > VMCOREINFO_STRUCT_SIZE(prb_data_blk_lpos); > VMCOREINFO_OFFSET(prb_data_blk_lpos, begin); > VMCOREINFO_OFFSET(prb_data_blk_lpos, next); > > VMCOREINFO_STRUCT_SIZE(printk_info); > VMCOREINFO_OFFSET(printk_info, seq); > VMCOREINFO_OFFSET(printk_info, ts_nsec); > VMCOREINFO_OFFSET(printk_info, text_len); > VMCOREINFO_OFFSET(printk_info, dict_len); > VMCOREINFO_OFFSET(printk_info, caller_id); > > VMCOREINFO_STRUCT_SIZE(prb_data_ring); > VMCOREINFO_OFFSET(prb_data_ring, size_bits); > VMCOREINFO_OFFSET(prb_data_ring, data); > VMCOREINFO_OFFSET(prb_data_ring, head_id); > VMCOREINFO_OFFSET(prb_data_ring, tail_id); > > Or would it be enough to just recognize the new "prb" symbol and have > all the structures defined in the crash utility? If the latter is > preferred, should some sort of version number be exported? Or is the > kernel version number enough? > > I appreciate your feedback. > > John Ogness With respect to the crash utility, there are two answers. When running crash session normally, i.e. running "crash vmlinux vmcore", the runtime "log" command does not use any VMCOREINFO entries that happen to be attached to a dumpfile. Since crash has the vmlinux debuginfo data available, it uses its own interfaces to get all kernel symbol and structure related information. But there is a little-used capability where the the vmlinux file is not required, but rather just the vmcore, in its "crash --log vmcore" feature. That functionality does require the VMCOREINFO entries to extract/dump the log, and exit. Honestly I wish I had never even introduced that feature. And I wonder if it were deprecated, would anybody care? However, your question is highly relevant to the makedumpfile(8) facility for its "makedumpfile --dump-dmesg" option. Since it doesn't have the luxury of a vmlinux file, it needs all of the VMCOREINF_xxx items. Kazuhito Hagio is the makedumpfile maintainer, and since he is the primary customer of the VMCOREINFO entries, he would be a better person to answer your question. That being said, due the sheer number VMCOREINFO entries required, I like your idea of providing a single version number. But I defer to Kazu for his preference. Thanks, Dave -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
In fact,vmemmap not easy to calculated in crash-utility,if
CONFIG_RANDOMIZE_BASE is configured,memstart_addr will be changed since below
codes:
[arm64_memblock_init]
348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
PAGE_SHIFT));
...
413 if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
414 extern u16 memstart_offset_seed;
415 u64 range = linear_region_size -
416 (memblock_end_of_DRAM() -
memblock_start_of_DRAM());
417
418 /*
419 * If the size of the linear region exceeds, by a sufficient
420 * margin, the size of the region that the available
physical
421 * memory spans, randomize the linear region as well.
422 */
423 if (memstart_offset_seed > 0 && range >=
ARM64_MEMSTART_ALIGN) {
424 range /= ARM64_MEMSTART_ALIGN;
425 memstart_addr -= ARM64_MEMSTART_ALIGN *
426 ((range * memstart_offset_seed) >>
16);
427 }
428 }
the reason i showed the "address_markers " is just to prove vmemmap and
ms->vmemmap_start is wrong.we'd better to do below change.
- vmemmap_start = (-vmemmap_size);
+ vmemmap_start = (-vmemmap_size - MEGABYTES(2));
From: crash-utility-boun...@redhat.com on
behalf of Dave Anderson
Sent: Monday, April 20, 2020 21:13
To: Discussion list for crash utility usage,maintenance and development
Subject: [营销邮件] Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [External
Mail][] Re: ramdump support for va_bits_actual
- Original Message -
> Hi
> vmemmap and VMEMMAP_START are different,vmemmap begin from physical address
> 0x0 of page,but VMEMMAP_START start with memstart_addr.
>
> [mm/init.c]
> arm64_memblock_init
> 348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
> PAGE_SHIFT));
Right, so VMEMMAP_START can be calculated by reading vmemmap and memstart_addr.
>
> We can look at address_markers, this symbol indicates the entire memory map
> allocation
> check vmemmap and address_markers, we can see vmemmap has out of vmemmap range
It would be ideal if address_markers was guaranteed to be there, but it
only exists if CONFIG_PTDUMP_CORE was configured.
> crash> p -x vmemmap
> vmemmap = $1 = (struct page *) 0xfffefde0
>
> address_markers = $2 =
> {{
> start_address = 0xff80,
> name = 0xffd44b1fed8f "Linear Mapping start"
> ...
> }, {
> start_address = 0xfffeffe0,
> name = 0xffd44b2c5beb "vmemmap start"
> }, {
> start_address = 0xffe0,
> name = 0xffd44b1eb00c "vmemmap end"
> }, {
> start_address = 0x,
> name = 0x0
> }}
>
>
> > When the readmem() of symbol_value("physvirt_offset") is made, arm64_VTOP()
> > will
> > be called with its virtual address, right?
> Yes, arm64_VTOP to get physvirt_offset needs virtual address, and
> physvirt_offset is a kimage symbol,so just need kimage_voffset can translate
> to physical address,then get value of physvirt_offset.
Ah, you're right, it's a mapped kernel symbol and doesn't use it.
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
#/**本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from XIAOMI,
which is intended only for the person or entity whose address is listed above.
Any use of the information contained herein in any way (including, but not
limited to, total or partial disclosure, reproduction, or dissemination) by
persons other than the intended recipient(s) is prohibited. If you receive this
e-mail in error, please notify the sender by phone or email immediately and
delete it!**/#
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
- Original Message -
> Hi
> vmemmap and VMEMMAP_START are different,vmemmap begin from physical address
> 0x0 of page,but VMEMMAP_START start with memstart_addr.
>
> [mm/init.c]
> arm64_memblock_init
> 348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
> PAGE_SHIFT));
Right, so VMEMMAP_START can be calculated by reading vmemmap and memstart_addr.
>
> We can look at address_markers, this symbol indicates the entire memory map
> allocation
> check vmemmap and address_markers, we can see vmemmap has out of vmemmap range
It would be ideal if address_markers was guaranteed to be there, but it
only exists if CONFIG_PTDUMP_CORE was configured.
> crash> p -x vmemmap
> vmemmap = $1 = (struct page *) 0xfffefde0
>
> address_markers = $2 =
> {{
> start_address = 0xff80,
> name = 0xffd44b1fed8f "Linear Mapping start"
> ...
> }, {
> start_address = 0xfffeffe0,
> name = 0xffd44b2c5beb "vmemmap start"
> }, {
> start_address = 0xffe0,
> name = 0xffd44b1eb00c "vmemmap end"
> }, {
> start_address = 0x,
> name = 0x0
> }}
>
>
> > When the readmem() of symbol_value("physvirt_offset") is made, arm64_VTOP()
> > will
> > be called with its virtual address, right?
> Yes, arm64_VTOP to get physvirt_offset needs virtual address, and
> physvirt_offset is a kimage symbol,so just need kimage_voffset can translate
> to physical address,then get value of physvirt_offset.
Ah, you're right, it's a mapped kernel symbol and doesn't use it.
Dave
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] Crash-utility Digest, Vol 175, Issue 27
- Original Message - > Would you please take me off your list totally ridiculous Perhaps you should investigate who explicitly sent your email address for a subscription. You're removed now. -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
[Crash-utility] new printk ringbuffer interface
Hello Dave, You may or may not be aware that we are working on replacing [0] the Linux printk ringbuffer. Rather than a single buffer containing a single struct type, the new ringbuffer makes use of several different structs. I am writing to ask your advice about how this should be exported for the crash utility. Should all struct sizes and field offsets be exported? It would look something like this: VMCOREINFO_SYMBOL(prb); VMCOREINFO_STRUCT_SIZE(printk_ringbuffer); VMCOREINFO_OFFSET(printk_ringbuffer, desc_ring); VMCOREINFO_OFFSET(printk_ringbuffer, text_data_ring); VMCOREINFO_OFFSET(printk_ringbuffer, dict_data_ring); VMCOREINFO_OFFSET(printk_ringbuffer, fail); VMCOREINFO_STRUCT_SIZE(prb_desc_ring); VMCOREINFO_OFFSET(prb_desc_ring, count_bits); VMCOREINFO_OFFSET(prb_desc_ring, descs); VMCOREINFO_OFFSET(prb_desc_ring, head_id); VMCOREINFO_OFFSET(prb_desc_ring, tail_id); VMCOREINFO_STRUCT_SIZE(prb_desc); VMCOREINFO_OFFSET(prb_desc, info); VMCOREINFO_OFFSET(prb_desc, state_var); VMCOREINFO_OFFSET(prb_desc, text_blk_lpos); VMCOREINFO_OFFSET(prb_desc, dict_blk_lpos); VMCOREINFO_STRUCT_SIZE(prb_data_blk_lpos); VMCOREINFO_OFFSET(prb_data_blk_lpos, begin); VMCOREINFO_OFFSET(prb_data_blk_lpos, next); VMCOREINFO_STRUCT_SIZE(printk_info); VMCOREINFO_OFFSET(printk_info, seq); VMCOREINFO_OFFSET(printk_info, ts_nsec); VMCOREINFO_OFFSET(printk_info, text_len); VMCOREINFO_OFFSET(printk_info, dict_len); VMCOREINFO_OFFSET(printk_info, caller_id); VMCOREINFO_STRUCT_SIZE(prb_data_ring); VMCOREINFO_OFFSET(prb_data_ring, size_bits); VMCOREINFO_OFFSET(prb_data_ring, data); VMCOREINFO_OFFSET(prb_data_ring, head_id); VMCOREINFO_OFFSET(prb_data_ring, tail_id); Or would it be enough to just recognize the new "prb" symbol and have all the structures defined in the crash utility? If the latter is preferred, should some sort of version number be exported? Or is the kernel version number enough? I appreciate your feedback. John Ogness [0] lkml.kernel.org/r/20200128161948.8524-1-john.ogn...@linutronix.de -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] ramdump support for va_bits_actual
Hi Bhupesh, On Sat, Apr 18, 2020 at 1:00 PM Bhupesh Sharma wrote: > > Hi Vinayak, > > On Fri, Apr 17, 2020 at 8:34 PM vinayak menon wrote: > > > > Hi Dave, > > > > Noticed that raw ramdumps of 5.4 kernel aren't working with crash tip. > > With the patches attached, I could get it working. Please take a look. > > Can you share some details/message-logs of the issue you are seeing? The first issue was crash not resolving kimage_voffset, even with kaslr passed. WARNING: kimage_voffset cannot be determined from the dumpfile. Try using the command line option: --machdep kimage_voffset= One this was fixed, there were issue with VTOP for linear mapped addresses. crash: read error: kernel virtual address: ff817f2ee628 type: "IRQ stack pointer" crash: read error: kernel virtual address: ff817f312380 type: "memory section root table" Later, there were issues with PTOV during FILL_PMD (read error "pmd page") Thanks, Vinayak > > Thanks, > Bhupesh > > > -- > Crash-utility mailing list > Crash-utility@redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@redhat.com https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] ramdump support for va_bits_actual
Hi Dave,
On Sat, Apr 18, 2020 at 2:08 PM Dave Anderson wrote:
>
>
>
> - Original Message -
> > Hi Dave,
> >
> > Noticed that raw ramdumps of 5.4 kernel aren't working with crash tip.
> > With the patches attached, I could get it working. Please take a look.
> >
> > Thanks,
> > Vinayak
> >
>
> Hi Vinayak,
>
> A couple quick questions come to mind...
>
> First, I haven't checked all possible READMEM plugins, but for example, if
> this
> function is run on a live system, the -1 file descriptor would cause the
> READMEM()
> call to fail:
I changed it like this and it works for ramdump. I don't actually have
a live setup to try this. Let me try
to set up one.
diff --git a/arm64.c b/arm64.c
index 04efc13..fce3f8e 100644
--- a/arm64.c
+++ b/arm64.c
@@ -981,7 +981,7 @@ arm64_calc_physvirt_offset(void)
if ((sp = kernel_symbol_search("physvirt_offset")) &&
machdep->machspec->kimage_voffset) {
- if (READMEM(-1, _offset, sizeof(physvirt_offset),
+ if (READMEM(pc->mfd, _offset, sizeof(physvirt_offset),
sp->value, sp->value -
machdep->machspec->kimage_voffset) > 0) {
ms->physvirt_offset = physvirt_offset;
>
> static void
> +arm64_calc_physvirt_offset(void)
> +{
> + struct machine_specific *ms = machdep->machspec;
> + ulong physvirt_offset;
> + struct syment *sp;
> +
> + ms->physvirt_offset = ms->phys_offset - ms->page_offset;
> +
> + if ((sp = kernel_symbol_search("physvirt_offset")) &&
> + machdep->machspec->kimage_voffset) {
> + if (READMEM(-1, _offset, sizeof(physvirt_offset),
> + sp->value, sp->value -
> + machdep->machspec->kimage_voffset) > 0) {
> + ms->physvirt_offset = physvirt_offset;
> + }
> + }
> +
> + if (CRASHDEBUG(1))
> + fprintf(fp, "using %lx as physvirt_offset\n",
> ms->physvirt_offset);
> +}
>
> And here -- are you missing some brackets? (run "make warn")
>
I did try "make warn" and it does not show any issues.Am I missing something ?
> But regardless of that, why are you setting it back to 48 if it's greater
> than 48?
>
I did that because machspec->CONFIG_ARM64_VA_BITS is used for calculation of
vmemmap size. In kernel vmemmap size is calculated using VA_BITS_MIN and it is
defined like this
#if VA_BITS > 48
#define VA_BITS_MIN (48)
#else
#define VA_BITS_MIN (VA_BITS)
#endif
But I realize now that its not the right thing to do, because
machspec->CONFIG_ARM64_VA_BITS
is later used in arm64_calc_VA_BITS to verify machspec->VA_BITS. So
what about this ?
diff --git a/arm64.c b/arm64.c
index 04efc13..a35a30e 100644
--- a/arm64.c
+++ b/arm64.c
@@ -4023,8 +4023,6 @@ arm64_calc_virtual_memory_ranges(void)
if ((ret = get_kernel_config("CONFIG_ARM64_VA_BITS",
)) == IKCONFIG_STR)
machdep->machspec->CONFIG_ARM64_VA_BITS = atol(string);
- if
(machdep->machspec->CONFIG_ARM64_VA_BITS > 48)
-
machdep->machspec->CONFIG_ARM64_VA_BITS = 48;
}
}
@@ -4049,7 +4047,12 @@ arm64_calc_virtual_memory_ranges(void)
#define STRUCT_PAGE_MAX_SHIFT 6
if (ms->VA_BITS_ACTUAL) {
- vmemmap_size = (1UL) << (ms->CONFIG_ARM64_VA_BITS -
machdep->pageshift - 1 + STRUCT_PAGE_MAX_SHIFT);
+ ulong va_bits_min = 48;
+
+ if (machdep->machspec->CONFIG_ARM64_VA_BITS < 48)
+ va_bits_min = ms->CONFIG_ARM64_VA_BITS;
+
+ vmemmap_size = (1UL) << (va_bits_min -
machdep->pageshift - 1 + STRUCT_PAGE_MAX_SHIFT);
vmalloc_end = (- PUD_SIZE - vmemmap_size - KILOBYTES(64));
vmemmap_start = (-vmemmap_size);
ms->vmalloc_end = vmalloc_end - 1;
Thanks,
Vinayak
>
> Thanks,
> Dave
>
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
Re: [Crash-utility] [营销邮件] Re: [营销邮件] Re: [External Mail][????] Re: ramdump support for va_bits_actual
Hi
vmemmap and VMEMMAP_START are different,vmemmap begin from physical address 0x0
of page,but VMEMMAP_START start with memstart_addr.
[mm/init.c]
arm64_memblock_init
348 vmemmap = ((struct page *)VMEMMAP_START - (memstart_addr >>
PAGE_SHIFT));
We can look at address_markers,this symbol indicates the entire memory map
allocation
check vmemmap and address_markers, we can see vmemmap has out of vmemmap range
crash> p -x vmemmap
vmemmap = $1 = (struct page *) 0xfffefde0
address_markers = $2 =
{{
start_address = 0xff80,
name = 0xffd44b1fed8f "Linear Mapping start"
...
}, {
start_address = 0xfffeffe0,
name = 0xffd44b2c5beb "vmemmap start"
}, {
start_address = 0xffe0,
name = 0xffd44b1eb00c "vmemmap end"
}, {
start_address = 0x,
name = 0x0
}}
> When the readmem() of symbol_value("physvirt_offset") is made, arm64_VTOP()
> will
> be called with its virtual address, right?
Yes,arm64_VTOP to get physvirt_offset needs virtual address,and physvirt_offset
is a kimage symbol,so just need kimage_voffset can translate to physical
address,then get value of physvirt_offset.
physvirt_offset just two value in different kernel
version,"phys_offset-page_offset" or physvirt_offset,so after initialize
ms->physvirt_offset to different value,we can use ms->physvirt_offset for
vtop/ptov for Linner mapping.
From: crash-utility-boun...@redhat.com on
behalf of Dave Anderson
Sent: Sunday, April 19, 2020 23:30
To: Discussion list for crash utility usage,maintenance and development
Subject: [营销邮件] Re: [Crash-utility] [营销邮件] Re: [External Mail][] Re:
ramdump support for va_bits_actual
- Original Message -
> Hi,Dave
>
> I don't quite understand how you said to read vmemmap,VMEMMAP_START is a
> macro definition,or it's a constant,in crash-utility vmemmap_start is a
> wrong value,this may cause arm64_IS_VMALLOC_ADDR return wrong status
I mean the kernel symbol "vmemmap", here in "arch/arm64/mm/init.c":
struct page *vmemmap __ro_after_init;
EXPORT_SYMBOL(vmemmap);
Doesn't it contain the resolved starting address?
> We can get physvirt_offset earlier,in my patch,after calling
> arm64_calc_phys_offset we can initialize physvirt_offset,in this
> time,kimage_offset,page_offset and phys_offset are ready,such as:
>
> @@ -391,6 +391,13 @@ arm64_init(int when)
>
> ms = machdep->machspec;
>
> + if (kernel_symbol_exists("physvirt_offset") &&
> + readmem(symbol_value("physvirt_offset"), KVADDR,
> + , sizeof(ulong), "physvirt_offset",
> QUIET|RETURN_ON_ERROR))
> + ms->physvirt_offset = value;
> + else
> + ms->physvirt_offset = ms->phys_offset ->
> ms->page_offset;
When the readmem() of symbol_value("physvirt_offset") is made, arm64_VTOP() will
be called with its virtual address, right?
Dave
>
> From: crash-utility-boun...@redhat.com on
> behalf of Dave Anderson
> Sent: Sunday, April 19, 2020 1:03
> To: Discussion list for crash utility usage,maintenance and development
> Subject: [营销邮件] Re: [Crash-utility] [External Mail][] Re: ramdump support
> for va_bits_actual
>
> - Original Message -
> > Hi
> > I made almost the same patch to fix the problem with arm64 in version
> > 5.4...
> >
> > One very small change can merged together,vmemmap_start has a little error:
> > [arch/arm64/include/asm/memory.h]
> > 56 #define VMEMMAP_START (-VMEMMAP_SIZE - SZ_2M)
> > in crash arm64.c
> > - vmemmap_start = (-vmemmap_size);
> > + vmemmap_start = (-vmemmap_size - MEGABYTES(2));
>
> Can't we just read the value of "vmemmap"? If not, what is the difference
> between the calculated value above and the value of vmemmap?
>
> >
> >
> > BTW,in arm64_VTOP,it's easier to use the physvirt_offset directly
> > @@ -1148,8 +1155,7 @@ arm64_VTOP(ulong addr)
> >
> > }
> >
> > if (addr >= machdep->machspec->page_offset)
> > - return machdep->machspec->phys_offset
> > - + (addr - machdep->machspec->page_offset);
> > + return (addr + machdep->machspec->physvirt_offset);
>
> Unfortunately that's not possible, because there is at least one arm64_VTOP()
> call *before* the new machdep->machspec->physvirt_offset gets initialized,
> which I presume is why Vinayak's patch checks whether it's non-zero first.
>
> Dave
>
>
>
> >
> > From: crash-utility-boun...@redhat.com
> > on
> > behalf of Dave Anderson
> > Sent: Saturday, April 18, 2020 0:32
> > To: Discussion list for crash utility usage,maintenance and development
> > Subject: [External Mail][] Re: [Crash-utility] ramdump support for
> >
