port-x68k/51663: crash dump related bugs in x68k/machdep.c

[rokuyama%rk.phys.keio.ac.jp@localhost]

>Number:         51663
>Category:       port-x68k
>Synopsis:       crash dump related bugs in x68k/machdep.c
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    port-x68k-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 27 08:50:00 +0000 2016
>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