NetBSD Problem Report #51663
From www@NetBSD.org Sun Nov 27 08:47:01 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 879017A316
for <gnats-bugs@gnats.NetBSD.org>; Sun, 27 Nov 2016 08:47:01 +0000 (UTC)
Message-Id: <20161127084700.309167A33D@mollari.NetBSD.org>
Date: Sun, 27 Nov 2016 08:47:00 +0000 (UTC)
From: rokuyama@rk.phys.keio.ac.jp
Reply-To: rokuyama@rk.phys.keio.ac.jp
To: gnats-bugs@NetBSD.org
Subject: crash dump related bugs in x68k/machdep.c
X-Send-Pr-Version: www-1.0
>Number: 51663
>Category: port-x68k
>Synopsis: crash dump related bugs in x68k/machdep.c
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: tsutsui
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Nov 27 08:50:00 +0000 2016
>Closed-Date: Sun Feb 05 10:24:48 +0000 2017
>Last-Modified: Thu Feb 11 13:05:02 +0000 2021
>Originator: Rin Okuyama
>Release: 7.99.42
>Organization:
Faculty of Science and Technology, Keio University
>Environment:
NetBSD x68k 7.99.42 NetBSD 7.99.42 (GENERIC) #4: Sat Nov 26 17:03:01 JST 2016 rin@xxx:xxx x68k
>Description:
On x68k, savecore(8) complains "no core dump (invalid dumplo)" even
though there is enough room for crash dump in the dump device. This is
because of a bug in x68k/machdep.c:
https://nxr.netbsd.org/source/xref/src/sys/arch/x68k/x68k/machdep.c#602
602 /*
603 * X68k has multiple RAM segments on some models.
604 */
605 m->ram_segs[0].start = lowram;
606 m->ram_segs[0].size = mem_size - lowram;
607 for (i = 1; i < vm_nphysseg; i++) {
608 m->ram_segs[i].start =
609 ctob(VM_PHYSMEM_PTR(i)->start);
610 m->ram_segs[i].size =
611 ctob(VM_PHYSMEM_PTR(i)->end - VM_PHYSMEM_PTR(i)->start);
612 }
Since mem_size is sum of the base and extended RAM segments, the amount
of extended RAM segments is double-counted. In addition, this code
assumes that vm_physmem[0], [1], and [2] correspond to the base, 1st,
and 2nd extended RAM segments, respectively. But this is not correct;
the order is inverted actually. In the attached patch, this part is
modified as follows:
604 /*
605 * X68k has multiple RAM segments on some models.
606 */
607 m->ram_segs[0].start = phys_basemem_seg.start;
608 m->ram_segs[0].size = phys_basemem_seg.end;
609 #ifdef EXTENDED_MEMORY
610 seg = 1;
611 for (i = 0; i < EXTMEM_SEGS; i++) {
612 size = phys_extmem_seg[i].end - phys_extmem_seg[i].start;
613 if (size == 0)
614 continue;
615 m->ram_segs[seg].start = phys_extmem_seg[i].start;
616 m->ram_segs[seg].size = size;
617 seg++;
618 }
619 #endif
Here, ram_segs[0] correctly describes the base RAM segment. ram_segs[1]
and [2] includes area not used by VM subsystem. However, it does not
cause any problems except for small increase in crash dump, and more
importantly, it is free from unnecessary dependent on the internal
structure of VM subsystem.
With this patch, crash dump is successfully generated, and saved by
savecore(8). I have checked ps(1), vmstat(1), and gdb(1) work well with
saved crash dumps.
>How-To-Repeat:
Use savecore(8) or sync in ddb(4) on x68k.
>Fix:
--- src/sys/arch/x68k/x68k/machdep.c.orig 2016-11-25 16:03:30.017435756 +0900
+++ src/sys/arch/x68k/x68k/machdep.c 2016-11-26 17:02:22.570386277 +0900
@@ -553,7 +553,9 @@
{
cpu_kcore_hdr_t *h = &cpu_kcore_hdr;
struct m68k_kcore_hdr *m = &h->un._m68k;
- int i;
+#ifdef EXTENDED_MEMORY
+ int i, seg, size;
+#endif
memset(&cpu_kcore_hdr, 0, sizeof(cpu_kcore_hdr));
@@ -602,14 +604,19 @@
/*
* X68k has multiple RAM segments on some models.
*/
- m->ram_segs[0].start = lowram;
- m->ram_segs[0].size = mem_size - lowram;
- for (i = 1; i < vm_nphysseg; i++) {
- m->ram_segs[i].start =
- ctob(VM_PHYSMEM_PTR(i)->start);
- m->ram_segs[i].size =
- ctob(VM_PHYSMEM_PTR(i)->end - VM_PHYSMEM_PTR(i)->start);
+ m->ram_segs[0].start = phys_basemem_seg.start;
+ m->ram_segs[0].size = phys_basemem_seg.end;
+#ifdef EXTENDED_MEMORY
+ seg = 1;
+ for (i = 0; i < EXTMEM_SEGS; i++) {
+ size = phys_extmem_seg[i].end - phys_extmem_seg[i].start;
+ if (size == 0)
+ continue;
+ m->ram_segs[seg].start = phys_extmem_seg[i].start;
+ m->ram_segs[seg].size = size;
+ seg++;
}
+#endif
}
/*
@@ -806,6 +813,7 @@
}
}
printf("succeeded\n");
+ delay(5000000); /* 5 seconds */
}
void
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: port-x68k-maintainer->tsutsui
Responsible-Changed-By: tsutsui@NetBSD.org
Responsible-Changed-When: Mon, 28 Nov 2016 06:38:23 +0000
Responsible-Changed-Why:
I'll take a look later.
From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: gnats-bugs@NetBSD.org, tsutsui@NetBSD.org
Cc:
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Mon, 28 Nov 2016 22:38:39 +0900
Let me explain a little more.
cpu_kcore_hdr.un._m68k.ram_segs[i].{start,size} describe start address
and size of the i-th segment of physical memory.
On the kernel side, dumpsys(9) dumps their contents:
https://nxr.netbsd.org/source/xref/src/sys/arch/x68k/x68k/machdep.c#702
702 void
703 dumpsys(void)
704 {
...
717 seg = 0;
...
750 for (pg = 0; pg < dumpsize; pg++) {
...
762 while (maddr >=
763 (m->ram_segs[seg].start + m->ram_segs[seg].size)) {
764 if (++seg >= M68K_NPHYS_RAM_SEGS ||
765 m->ram_segs[seg].size == 0) {
766 error = EINVAL; /* XXX ?? */
767 goto bad;
768 }
769 maddr = m->ram_segs[seg].start;
770 }
771 pmap_enter(pmap_kernel(), (vaddr_t)vmmap, maddr,
772 VM_PROT_READ, VM_PROT_READ|PMAP_WIRED);
773 pmap_update(pmap_kernel());
774
775 error = (*dump)(dumpdev, blkno, vmmap, PAGE_SIZE);
776 bad:
777 switch (error) {
778 case 0:
779 maddr += PAGE_SIZE;
780 blkno += btodb(PAGE_SIZE);
781 break;
...
806 }
807 }
808 printf("succeeded\n");
809 }
On the userland side, libkvm converts physical address into offset of
a crash dump:
https://nxr.netbsd.org/source/xref/src/lib/libkvm/kvm_m68k_cmn.c#158
158 /*
159 * Translate a physical address to a file-offset in the crash dump.
160 */
161 off_t
162 _kvm_cmn_pa2off(kvm_t *kd, u_long pa)
163 {
164 cpu_kcore_hdr_t *h = kd->cpu_data;
165 struct m68k_kcore_hdr *m = &h->un._m68k;
166 phys_ram_seg_t *rsp;
167 off_t off;
168 int i;
169
170 off = 0;
171 rsp = m->ram_segs;
172 for (i = 0; i < M68K_NPHYS_RAM_SEGS && rsp[i].size != 0; i++) {
173 if (pa >= rsp[i].start &&
174 pa < (rsp[i].start + rsp[i].size)) {
175 pa -= rsp[i].start;
176 break;
177 }
178 off += rsp[i].size;
179 }
180 return (kd->dump_off + off + pa);
181 }
(This function silently returns bogus offset when invalid address is
passed. This is a dangerous behavior and should be fixed later.)
cpu_kcore_hdr.un._m68k.ram_segs[0] must cover whole the base memory,
including areas not registered into the free list of UVM. This is
because there are global variables and msgbuf below and above the region
managed by UVM.
On the other hand, regions in extended memory not managed by UVM can
probably be excluded from ram_segs[i] (i >= 1). However, in my patch,
such regions are also included for the reasons explained before.
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: rokuyama@rk.phys.keio.ac.jp
Cc: gnats-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Wed, 30 Nov 2016 00:56:45 +0900
> Let me explain a little more.
Thank you for your explanation. Your analysis is correct.
> cpu_kcore_hdr.un._m68k.ram_segs[i].{start,size} describe start address
> and size of the i-th segment of physical memory.
:
I see. Actually I didn't understand what values ram_segs[] should have,
and I should have checked cpu_init_kcore_hdr() more carefully
when I changed mem_exists() for PR/45915:
http://mail-index.netbsd.org/port-x68k/2013/10/19/msg000039.html
> cpu_kcore_hdr.un._m68k.ram_segs[0] must cover whole the base memory,
> including areas not registered into the free list of UVM. This is
> because there are global variables and msgbuf below and above the region
> managed by UVM.
(BTW, mem_size for ram_segs[0] seems incorrect even before
my mem_exist() changes..)
> On the other hand, regions in extended memory not managed by UVM can
> probably be excluded from ram_segs[i] (i >= 1). However, in my patch,
> such regions are also included for the reasons explained before.
Maybe we should clarify the definitions of the following regions
in some m68k docs:
a) VM_PHYSMEM_PTR() (formerly vm_physmem[]) registered by
uvm_page_physload(9)
b) phys_ram_seg_t ram_segs[] for crashdump
The region a) includes memories that are managed by uvm(9).
The region b) includes memories that should be accessed
on gdb "target kvm" debug using crashdump via savecore(8).
The main difference is that b) includes kernel itself and msgbuf
but a) should not have them.
b) would exclude memory regions that are not managed by kernel
(for example, PROM reserved area on news68k and sun3x etc.)
as you mentioned, but IIRC x68k uses whole extended memory regions.
Forthermore, it would be harmless to register unmanaged regions
into b) (unless the region includes some sensitive data),
so we don't have to consider it strictly if the port doesn't
have useful info (variables) about the "unmanaged regions."
(Note news68k PROM excludes its reserved area from "memsize" info)
Your patch seems fine (maybe "size" should be psize_t rather than int).
I'll commit it after I also confirm it on my x68k.
Thanks,
---
Izumi Tsutsui
State-Changed-From-To: open->analyzed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Tue, 29 Nov 2016 16:02:48 +0000
State-Changed-Why:
From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@NetBSD.org
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Wed, 30 Nov 2016 21:16:26 +0900
Thank you very much for your careful analysis and review of my patch.
Also, I appreciate your detailed explanation. I learned a lot from you.
Rin
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: rokuyama@rk.phys.keio.ac.jp
Cc: gnats-bugs@NetBSD.org, tsutsui@ceres.dti.ne.jp
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Thu, 1 Dec 2016 06:49:02 +0900
I slightly modified your patch and have confirmed it on my X68030
(which has no highmem, so it just means "no bad side effect").
Changes are:
- use psize_t (instead of int) for "size" of memory regions
- explicitly calculate size of basemem_seg (for future readers)
even though phys_basemem_seg.start (== lowram) is zero on x68k
Could you also confirm? (sorry I'm so lazy not to setup XM6i)
---
Index: x68k/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x68k/x68k/machdep.c,v
retrieving revision 1.193
diff -u -p -d -r1.193 machdep.c
--- x68k/machdep.c 14 Jun 2016 07:51:10 -0000 1.193
+++ x68k/machdep.c 30 Nov 2016 21:47:54 -0000
@@ -553,7 +553,10 @@ cpu_init_kcore_hdr(void)
{
cpu_kcore_hdr_t *h = &cpu_kcore_hdr;
struct m68k_kcore_hdr *m = &h->un._m68k;
- int i;
+ psize_t size;
+#ifdef EXTENDED_MEMORY
+ int i, seg;
+#endif
memset(&cpu_kcore_hdr, 0, sizeof(cpu_kcore_hdr));
@@ -602,14 +605,20 @@ cpu_init_kcore_hdr(void)
/*
* X68k has multiple RAM segments on some models.
*/
- m->ram_segs[0].start = lowram;
- m->ram_segs[0].size = mem_size - lowram;
- for (i = 1; i < vm_nphysseg; i++) {
- m->ram_segs[i].start =
- ctob(VM_PHYSMEM_PTR(i)->start);
- m->ram_segs[i].size =
- ctob(VM_PHYSMEM_PTR(i)->end - VM_PHYSMEM_PTR(i)->start);
+ size = phys_basemem_seg.end - phys_basemem_seg.start;
+ m->ram_segs[0].start = phys_basemem_seg.start;
+ m->ram_segs[0].size = size;
+#ifdef EXTENDED_MEMORY
+ seg = 1;
+ for (i = 0; i < EXTMEM_SEGS; i++) {
+ size = phys_extmem_seg[i].end - phys_extmem_seg[i].start;
+ if (size == 0)
+ continue;
+ m->ram_segs[seg].start = phys_extmem_seg[i].start;
+ m->ram_segs[seg].size = size;
+ seg++;
}
+#endif
}
/*
---
Izumi Tsutsui
From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@NetBSD.org
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Thu, 1 Dec 2016 12:38:27 +0900
On 2016/12/01 6:49, Izumi Tsutsui wrote:
> I slightly modified your patch and have confirmed it on my X68030
> (which has no highmem, so it just means "no bad side effect").
>
> Changes are:
> - use psize_t (instead of int) for "size" of memory regions
> - explicitly calculate size of basemem_seg (for future readers)
> even though phys_basemem_seg.start (== lowram) is zero on x68k
>
> Could you also confirm? (sorry I'm so lazy not to setup XM6i)
Thank you for your revision. It seems very reasonable to me. I've
tested your patch on XM6i with 12MB of base memory and the following
layouts of extended memory:
(1) 16MB at 0x01000000 (TS-6BE16 mode)
(2) 128MB at 0x10000000 (060turbo mode)
(3) 240MB at 0x01000000 and 768MB at 0x01000000
dmesg, ps, vmstat, and gdb work just fine for the all layouts.
Thanks,
Rin
From: Rin Okuyama <rokuyama@rk.phys.keio.ac.jp>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: gnats-bugs@NetBSD.org
Subject: Re: port-x68k/51663 (crash dump related bugs in x68k/machdep.c)
Date: Thu, 1 Dec 2016 12:42:09 +0900
Sorry, I made typo.
On 2016/12/01 12:38, Rin Okuyama wrote:
> (3) 240MB at 0x01000000 and 768MB at 0x01000000
This one should be
> (3) 240MB at 0x01000000 and 768MB at 0x10000000
From: "Izumi Tsutsui" <tsutsui@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51663 CVS commit: src/sys/arch/x68k/x68k
Date: Fri, 2 Dec 2016 12:43:07 +0000
Module Name: src
Committed By: tsutsui
Date: Fri Dec 2 12:43:07 UTC 2016
Modified Files:
src/sys/arch/x68k/x68k: machdep.c
Log Message:
Fix crashdump on machines with EXTENDED_MEMORY.
PR port-x68k/51663 from Rin Okuyama.
To generate a diff of this commit:
cvs rdiff -u -r1.193 -r1.194 src/sys/arch/x68k/x68k/machdep.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51663 CVS commit: [netbsd-7] src/sys/arch/x68k/x68k
Date: Tue, 6 Dec 2016 06:46:02 +0000
Module Name: src
Committed By: snj
Date: Tue Dec 6 06:46:02 UTC 2016
Modified Files:
src/sys/arch/x68k/x68k [netbsd-7]: machdep.c
Log Message:
Pull up following revision(s) (requested by tsutsui in ticket #1282):
sys/arch/x68k/x68k/machdep.c: revision 1.194
Fix crashdump on machines with EXTENDED_MEMORY.
PR port-x68k/51663 from Rin Okuyama.
To generate a diff of this commit:
cvs rdiff -u -r1.191.4.1 -r1.191.4.2 src/sys/arch/x68k/x68k/machdep.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: analyzed->closed
State-Changed-By: tsutsui@NetBSD.org
State-Changed-When: Sun, 05 Feb 2017 10:24:48 +0000
State-Changed-Why:
Committed and pulled up to netbsd-7.
From: "Izumi Tsutsui" <tsutsui@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51663 CVS commit: src/sys/arch/x68k/x68k
Date: Sun, 7 Feb 2021 15:51:11 +0000
Module Name: src
Committed By: tsutsui
Date: Sun Feb 7 15:51:11 UTC 2021
Modified Files:
src/sys/arch/x68k/x68k: machdep.c
Log Message:
Restore fixes for PR/51663 lost in r1.195 (uvm_hotplug(9) merge).
The kernel crashdump and savecore(8) on NetBSD/x68k have been broken
(even without EXTNEDED_MEMORY) since NetBSD 8.0. Oops.
Should be pulled up to netbsd-9 and netbsd-8.
To generate a diff of this commit:
cvs rdiff -u -r1.203 -r1.204 src/sys/arch/x68k/x68k/machdep.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51663 CVS commit: [netbsd-9] src/sys/arch/x68k/x68k
Date: Thu, 11 Feb 2021 13:02:05 +0000
Module Name: src
Committed By: martin
Date: Thu Feb 11 13:02:05 UTC 2021
Modified Files:
src/sys/arch/x68k/x68k [netbsd-9]: machdep.c
Log Message:
Pull up following revision(s) (requested by tsutsui in ticket #1205):
sys/arch/x68k/x68k/machdep.c: revision 1.204
Restore fixes for PR/51663 lost in r1.195 (uvm_hotplug(9) merge).
The kernel crashdump and savecore(8) on NetBSD/x68k have been broken
(even without EXTNEDED_MEMORY) since NetBSD 8.0. Oops.
Should be pulled up to netbsd-9 and netbsd-8.
To generate a diff of this commit:
cvs rdiff -u -r1.200 -r1.200.4.1 src/sys/arch/x68k/x68k/machdep.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51663 CVS commit: [netbsd-8] src/sys/arch/x68k/x68k
Date: Thu, 11 Feb 2021 13:04:01 +0000
Module Name: src
Committed By: martin
Date: Thu Feb 11 13:04:01 UTC 2021
Modified Files:
src/sys/arch/x68k/x68k [netbsd-8]: machdep.c
Log Message:
Pull up following revision(s) (requested by tsutsui in ticket #1654):
sys/arch/x68k/x68k/machdep.c: revision 1.204
Restore fixes for PR/51663 lost in r1.195 (uvm_hotplug(9) merge).
The kernel crashdump and savecore(8) on NetBSD/x68k have been broken
(even without EXTNEDED_MEMORY) since NetBSD 8.0. Oops.
Should be pulled up to netbsd-9 and netbsd-8.
To generate a diff of this commit:
cvs rdiff -u -r1.196 -r1.196.6.1 src/sys/arch/x68k/x68k/machdep.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home