include/core/defs.h

@@ -457,8 +457,6 @@ extern uint64   boot_epoch;
 int             cpunum(void);
 extern volatile uint*    lapic;
 void            lapiceoi(void);
-extern volatile int panic_stop;   // kernel/core/panic.c
-void            lapic_nmi_others(void);
 void            lapicinit(void);
 void            lapicstartap(uchar, uint);
 void            microdelay(int);

include/gfx/qr.h

@@ -8,26 +8,25 @@
 // no second machine on the desk. A phone camera is a working out-of-band
 // channel that needs no drivers on our side at all. See docs/architecture/graphics.md.
 //
-// Deliberately small, in two ways:
+// Deliberately small, in three ways:
 //
 //   - Byte mode only. The report is arbitrary bytes; alphanumeric mode would
 //     pack tighter but only for a restricted character set.
 //   - Error correction level L only. The screen is a clean, evenly lit,
 //     high-contrast source -- the failure mode QR's stronger levels protect
 //     against (dirt, print damage, curved packaging) does not apply.
+//   - Versions 1 to 6 only. Version 7 is where QR starts requiring an extra
+//     18-bit version-information block in two more places on the symbol, and
+//     version 10 is where the character count field grows and the alignment
+//     pattern grid becomes 2-dimensional. Stopping at 6 avoids both, and
+//     134 bytes is a large crash report.
 //
-// Every version, 1 to 40. It stopped at 6 -- 134 bytes, enough for the crash
+// If a payload ever needs more than 134 bytes, the honest fix is to shorten the
-// card -- until PSOGM mode wanted the log itself off the machine, a page of it
+// report -- not to reach for version 10 and its two extra tables.
-// per symbol: version 40 at level L carries 2,953 bytes. A struct qr is
-// therefore about 62 KB; keep one static, never on a stack.
 
-#define QR_MAX_VERSION 40
+#define QR_MAX_VERSION 6
-#define QR_MAX_SIZE    (17 + 4 * QR_MAX_VERSION)   // 177 modules per side
+#define QR_MAX_SIZE    (17 + 4 * QR_MAX_VERSION)   // 41 modules per side
-#define QR_MAX_BYTES   2953
+#define QR_MAX_BYTES   134
-
-// The crash card on the PSOGM stays small, so a phone reads it from across a
-// desk: a version-6 symbol at most.
-#define QR_CARD_BYTES  134
 
 struct qr {
   int size;                              // modules per side; 0 if unencoded

kernel/arch/x86_64/ioperm.c

@@ -56,14 +56,6 @@ struct tssblk {
 
 static struct tssblk tssblk[NCPU];
 
-// The stack an NMI arrives on: IST slot 1, one per CPU. Without it an NMI runs
-// on whatever stack was live, and for a few instructions at the top of
-// syscall_entry that is the user's -- at ring 0, so the CPU would push the
-// trap frame there, and a fault on that push is a double fault on the same
-// stack. Panic stops the other processors with an NMI; it must not be the
-// thing that resets the machine instead of the report.
-static uint8 nmistack[NCPU][4096] __attribute__((aligned(16)));
-
 // What each CPU currently has open. Kept per CPU rather than read back from the
 // process, because switching *away* has to close what was opened and the
 // process that opened it may be part-way through exit() by then.
@@ -97,7 +89,6 @@ ioperm_tss(int cpu, uint64 *limit)
   memset(b->map, 0xFF, sizeof(b->map));
   b->tail = 0xFF;
   b->ts.iomb = (uint16)__builtin_offsetof(struct tssblk, map);
-  b->ts.ist[0] = (uint64)(nmistack[cpu] + sizeof(nmistack[cpu]));
   installed[cpu].n = 0;
 
   // The limit is the offset of the last valid byte, and it has to include the

kernel/arch/x86_64/lapic.c

@@ -402,21 +402,6 @@ cpunum(void)
   panic("unknown apicid\n");
 }
 
-// An NMI to every processor but this one: how a panic stops the others. An
-// NMI is delivered with interrupts off too, so it reaches a processor spinning
-// on a lock with cli as surely as one running a program; trap() halts any that
-// takes one while panic_stop is set. Bounded, because the caller is dying.
-void
-lapic_nmi_others(void)
-{
-  if (!lapic)
-    return;
-  lapicw(ICRHI, 0);
-  lapicw(ICRLO, 0x000C0000 | 0x00000400 | ASSERT);   // all but self, NMI
-  for (int i = 0; i < 100000 && (lapic[ICRLO] & DELIVS); i++)
-    microdelay(1);
-}
-
 // Acknowledge interrupt.
 void
 lapiceoi(void)

kernel/arch/x86_64/trap.c

@@ -48,8 +48,6 @@ void tvinit(void)
   for (n = 0; n < 256; n++)
     mkgate(idt, n, vectors[n], 0);
   mkgate(idt, T_SYSCALL, vectors[T_SYSCALL], 3);
-  // NMI on its own stack, IST slot 1; see nmistack in ioperm.c.
-  idt[T_NMI * 4 + 1] |= 1;
 }
 
 uint
@@ -83,17 +81,6 @@ trapsig(uint64 trapno)
 void
 trap(struct trapframe *tf)
 {
-  // Another processor has panicked and wants this one stopped: a processor
-  // left running carries on with a kernel that is already dead, and on a
-  // laptop that showed as the report flashing up and being drawn over, and
-  // as fans that stayed loud until somebody held the power button. First, and
-  // before the canary check, because this stack may be what is broken.
-  if(tf->trapno == T_NMI && panic_stop)
-    for(;;){
-      cli();
-      hlt();
-    }
-
   kstack_check_canary("trap entry");
   switch(tf->trapno){
   case T_IRQ0 + IRQ_TIMER:

kernel/core/panic.c

@@ -326,7 +326,7 @@ text_at(int x, int y, const char *s, uint32 color)
 static int
 draw_qr_panel(int sw, int sh, int ch, char *reason, addr_t *pcs, int npcs)
 {
-  char payload[QR_CARD_BYTES];
+  char payload[QR_MAX_BYTES];
   int n = build_qr_payload(payload, sizeof(payload), reason, pcs, npcs);
 
   if (n <= 0 || qr_encode(&panic_qr, (unsigned char *)payload, n) != 0)
@@ -559,9 +559,6 @@ report_serial(char *reason, addr_t *pcs, int npcs)
 
 static void panic_blink(void) __attribute__((noreturn));
 
-volatile int panic_stop;
-static volatile int panic_owner = -1;
-
 __attribute__((noreturn))
 void
 panic(char *s)
@@ -569,21 +566,6 @@ panic(char *s)
   addr_t pcs[10];
 
   cli();
-
-  // One processor reports; any other that panics meanwhile -- the same broken
-  // state tends to reach several at once -- stops where it is. The owner may
-  // panic again while drawing, and that nested report is let through.
-  int me = cpunum();
-  int owner = __sync_val_compare_and_swap(&panic_owner, -1, me);
-  if (owner != -1 && owner != me)
-    for (;;) {
-      cli();
-      hlt();
-    }
-  // And the rest are stopped outright, see trap().
-  panic_stop = 1;
-  lapic_nmi_others();
-
   // Stop every other path from writing to the console. consputc checks this
   // and spins, which is what keeps another CPU from painting over the report
   // while it is being drawn.

kernel/core/qr.c

@@ -2,8 +2,8 @@
 //
 // Freestanding on purpose: no allocation, no floating point, no libc beyond
 // memset/memmove. It has to run in panic context, where the heap may be exactly
-// what is broken. Every buffer here is on the caller's struct, a bounded
+// what is broken. Every buffer here is either on the caller's struct or a
-// local, or static.
+// bounded local.
 //
 // The structure follows ISO/IEC 18004 in the order the standard describes it,
 // which is also the only order in which any of it can be checked:
@@ -40,66 +40,42 @@
 // every row below, because levels 1..6 at L happen to have equally sized
 // blocks. qr_encode() asserts exactly that at runtime, which is what turns a
 // mistyped number here into a visible failure instead of an unscannable symbol.
-// Error correction level L, every version. Two numbers per version are
+static const struct {
-// transcribed from ISO/IEC 18004 table 9 -- EC codewords per block, and how
+  short total;          // total codewords in the symbol
-// many blocks -- and everything else is computed: the symbol's total codeword
+  unsigned char ecc;    // EC codewords per block
-// count from its geometry, and the block lengths from the total. Above
+  unsigned char blocks; // number of RS blocks
-// version 5 the blocks are not all one length; the first ones are a codeword
+} vinfo[QR_MAX_VERSION + 1] = {
-// shorter, and interleaving has to step around the gap.
+  { 0,   0,  0 },   // version 0 does not exist; keeps indexing natural
-static const unsigned char ecc_per_block[QR_MAX_VERSION + 1] = {
+  { 26,  7,  1 },
-  0, 7, 10, 15, 20, 26, 18, 20, 24, 30, 18, 20, 24, 26, 30, 22, 24, 28, 30, 28,
+  { 44,  10, 1 },
-  28, 28, 28, 30, 30, 26, 28, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30,
+  { 70,  15, 1 },
-  30,
+  { 100, 20, 1 },
+  { 134, 26, 1 },
+  { 172, 18, 2 },
 };
-static const unsigned char ecc_blocks[QR_MAX_VERSION + 1] = {
-  0, 1, 1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 4, 4, 6, 6, 6, 6, 7,
-  8, 8, 9, 9, 10, 12, 12, 12, 13, 14, 15, 16, 17, 18, 19, 19, 20, 21, 22, 24,
-  25,
-};
-
-// Modules left for data once every function pattern is drawn: the whole
-// square, less finders and separators (192), timing (2 * (n - 16)), format
-// (31), alignment patterns and, from version 7, the two version blocks (36).
-static int
-raw_modules(int v)
-{
-  int r = (16 * v + 128) * v + 64;
-  if (v >= 2) {
-    int na = v / 7 + 2;
-    r -= (25 * na - 10) * na - 55;
-    if (v >= 7)
-      r -= 36;
-  }
-  return r;
-}
-
-static int
-total_codewords(int v)
-{
-  return raw_modules(v) / 8;
-}
-
-static int
-data_codewords(int v)
-{
-  return total_codewords(v) - ecc_per_block[v] * ecc_blocks[v];
-}
-
-// Byte mode's character count is 8 bits up to version 9 and 16 from 10.
-static int
-count_bits(int v)
-{
-  return v <= 9 ? 8 : 16;
-}
 
 int
 qr_capacity(int version)
 {
   if (version < 1 || version > QR_MAX_VERSION)
     return 0;
-  return (data_codewords(version) * 8 - 4 - count_bits(version)) / 8;
+  // Data codewords, less the two bytes of framing: a 4-bit mode indicator, an
+  // 8-bit character count (versions 1-9), and a 4-bit terminator come to
+  // exactly two codewords' worth of overhead.
+  int data = vinfo[version].total - vinfo[version].ecc * vinfo[version].blocks;
+  return data - 2;
 }
 
+// ---------------------------------------------------------------------------
+// GF(256) and Reed-Solomon
+// ---------------------------------------------------------------------------
+
+// Multiply in GF(256) modulo x^8 + x^4 + x^3 + x^2 + 1 (0x11D).
+//
+// Done by shift-and-add rather than log/exp tables. Two 256-byte tables plus
+// their initialisation is more code than this loop, and this needs no init step
+// at all -- which matters when the caller is a panic handler that cannot assume
+// any particular earlier function ran.
 static unsigned char
 gmul(unsigned char x, unsigned char y)
 {
@@ -255,37 +231,11 @@ draw_function_patterns(struct qr *q)
   draw_finder(q, n - 4, 3);
   draw_finder(q, 3, n - 4);
 
-  // Alignment patterns on a grid: the same list of centres on both axes,
+  // For versions 2-6 the alignment centres are {6, n-7}, and three of those
-  // every combination drawn except the three that would sit on a finder. The
+  // four combinations collide with a finder. Only the bottom-right survives,
-  // list starts at 6, ends at n-7, and steps evenly (in even numbers) between,
+  // which is why there is no position table here. Version 7 would need one.
-  // except version 32, which the standard spaces by hand.
+  if (q->size > 21)
-  int v = (n - 17) / 4;
+    draw_alignment(q, n - 7, n - 7);
-  if (v >= 2) {
-    int na = v / 7 + 2, pos[7];
-    int step = v == 32 ? 26 : (v * 4 + na * 2 + 1) / (na * 2 - 2) * 2;
-    pos[0] = 6;
-    for (int i = na - 1, p = n - 7; i >= 1; i--, p -= step)
-      pos[i] = p;
-    for (int i = 0; i < na; i++)
-      for (int j = 0; j < na; j++)
-        if (!((i == 0 && j == 0) || (i == 0 && j == na - 1) ||
-              (i == na - 1 && j == 0)))
-          draw_alignment(q, pos[i], pos[j]);
-  }
-
-  // From version 7 the version number is written out too, 6 bits and a
-  // BCH(18,6) check, in a 3x6 block beside each of the two far finders.
-  if (v >= 7) {
-    int rem = v;
-    for (int i = 0; i < 12; i++)
-      rem = (rem << 1) ^ ((rem >> 11) * 0x1F25);
-    long bits = ((long)v << 12) | rem;
-    for (int i = 0; i < 18; i++) {
-      int bit = (bits >> i) & 1, a = n - 11 + i % 3, b = i / 3;
-      set_fn(q, a, b, bit);
-      set_fn(q, b, a, bit);
-    }
-  }
 
   draw_format(q, 0);        // reserve; overwritten once a mask is chosen
 }
@@ -407,24 +357,25 @@ qr_encode(struct qr *q, const unsigned char *data, int len)
   if (v > QR_MAX_VERSION)
     return -1;
 
-  int blocks   = ecc_blocks[v];
+  int total    = vinfo[v].total;
-  int ecclen   = ecc_per_block[v];
+  int ecclen   = vinfo[v].ecc;
-  int total    = total_codewords(v);
+  int blocks   = vinfo[v].blocks;
-  int datacw   = data_codewords(v);
+  int datacw   = total - ecclen * blocks;
-  int nshort   = blocks - total % blocks;     // blocks one data codeword short
+
-  int shortlen = total / blocks - ecclen;     // their data length
+  // The table claims equal-sized blocks for every version this file supports.
+  // If that is ever untrue the interleave below is silently wrong, so check it
+  // rather than trust it.
+  if (datacw % blocks != 0)
+    return -1;
+  int blocklen = datacw / blocks;
 
   // --- 1. payload -> data codewords ---------------------------------------
-  //
+  unsigned char cw[172];            // >= max total codewords
-  // Static, not on the stack: version 40 is 3,706 codewords, and this runs on
-  // whatever stack a panic left. Not reentrant, which nothing here needs.
-  static unsigned char cw[3706], out[3706];
-  static unsigned char ec[25][30];
   memset(cw, 0, sizeof(cw));
 
   int pos = 0;
   put_bits(cw, &pos, 0x4, 4);       // byte mode
-  put_bits(cw, &pos, (unsigned)len, count_bits(v));
+  put_bits(cw, &pos, (unsigned)len, 8);
   for (int i = 0; i < len; i++)
     put_bits(cw, &pos, data[i], 8);
 
@@ -442,25 +393,20 @@ qr_encode(struct qr *q, const unsigned char *data, int len)
     cw[i] = (i - pos / 8) % 2 == 0 ? 0xEC : 0x11;
 
   // --- 2 and 3. error correction, then interleave -------------------------
-  //
-  // Block b starts at start(b): the short blocks first, then the long ones.
   unsigned char gen[30];
+  unsigned char ec[4][30];
   rs_divisor(ecclen, gen);
-#define BSTART(b) ((b) * shortlen + ((b) > nshort ? (b) - nshort : 0))
-#define BLEN(b)   (shortlen + ((b) >= nshort))
   for (int b = 0; b < blocks; b++)
-    rs_remainder(&cw[BSTART(b)], BLEN(b), gen, ecclen, ec[b]);
+    rs_remainder(&cw[b * blocklen], blocklen, gen, ecclen, ec[b]);
 
+  unsigned char out[172];
   int o = 0;
-  for (int i = 0; i <= shortlen; i++)
+  for (int i = 0; i < blocklen; i++)
     for (int b = 0; b < blocks; b++)
-      if (i < BLEN(b))
+      out[o++] = cw[b * blocklen + i];
-        out[o++] = cw[BSTART(b) + i];
   for (int i = 0; i < ecclen; i++)
     for (int b = 0; b < blocks; b++)
       out[o++] = ec[b][i];
-#undef BSTART
-#undef BLEN
 
   // --- 4. function patterns ------------------------------------------------
   memset(q, 0, sizeof(*q));

tools/python/qrcheck.py

@@ -117,15 +117,10 @@ def decode(m):
     return ""
 
 
-# Byte-mode capacity per version at level L, from ISO/IEC 18004 table 7 --
+# Byte capacity per version: total - ecc*blocks - 2. Mirrors the table in qr.c;
-# written out rather than computed, so it checks qr.c instead of agreeing with
+# the exact-fill cases below are built from it, and a disagreement would show up
-# it. The exact-fill cases below are built from it, and a disagreement shows up
 # as a version mismatch rather than passing quietly.
-CAPACITY = dict(enumerate([
+CAPACITY = {1: 17, 2: 32, 3: 53, 4: 78, 5: 106, 6: 134}
-    17, 32, 53, 78, 106, 134, 154, 192, 230, 271, 321, 367, 425, 458, 520,
-    586, 644, 718, 792, 858, 929, 1003, 1091, 1171, 1273, 1367, 1465, 1528,
-    1628, 1732, 1840, 1952, 2068, 2188, 2303, 2431, 2563, 2699, 2809, 2953,
-], start=1))
 
 alpha = lambda n: "".join(chr(65 + i % 26) for i in range(n))
 
@@ -138,9 +133,8 @@ CASES = [
     bytes(range(1, 40)).decode("latin-1"),          # non-printable payload
 ]
 # One payload that exactly fills each version, and one just under, so both the
-# padded and unpadded paths are covered at every version: several Reed-Solomon
+# padded and unpadded paths are covered at every version -- including 6, which
-# blocks from 6, blocks of two lengths from 6 too, the version information
+# is the only one with more than one Reed-Solomon block.
-# from 7, the 16-bit character count from 10.
 for v, cap in CAPACITY.items():
     CASES.append(alpha(cap))
     CASES.append(alpha(cap - 1))

tools/qrtest.c

@@ -155,26 +155,25 @@ check(int ok, const char *what)
 }
 
 // The version table is the one part of qr.c that is transcription rather than
-// reasoning, so check what can be checked: the data left over is positive and
+// reasoning, so check what can be checked: block sizes must divide evenly, and
-// the blocks are no more than one codeword apart, and the symbol has room for
+// the symbol must have room for exactly the codewords claimed.
-// exactly the codewords the geometry formula claims.
 static void
 check_tables(void)
 {
     printf("version table\n");
     for (int v = 1; v <= QR_MAX_VERSION; v++) {
-        int total = total_codewords(v);
+        int total = vinfo[v].total;
-        int data  = data_codewords(v);
+        int data  = total - vinfo[v].ecc * vinfo[v].blocks;
         char msg[128];
 
-        snprintf(msg, sizeof msg, "v%d: %d data codewords in %d blocks",
+        snprintf(msg, sizeof msg, "v%d: %d data codewords divide into %d blocks",
-                 v, data, ecc_blocks[v]);
+                 v, data, vinfo[v].blocks);
-        check(data > 0 && data / ecc_blocks[v] > 0, msg);
+        check(data > 0 && data % vinfo[v].blocks == 0, msg);
 
         // Count the modules a symbol of this version actually leaves for data,
-        // by building one and asking. If the formula's codeword count disagrees
+        // by building one and asking. If the table's codeword count disagrees
         // with the geometry, one of the two is wrong.
-        static struct qr q;
+        struct qr q;
         memset(&q, 0, sizeof q);
         q.size = 17 + 4 * v;
         draw_function_patterns(&q);
@@ -220,7 +219,7 @@ static void
 check_encode(void)
 {
     printf("encode\n");
-    static struct qr q;
+    struct qr q;
 
     check(qr_encode(&q, (const unsigned char *)"HELLO", 5) == 0, "short payload encodes");
     check(q.size == 21, "and picks version 1");
@@ -228,9 +227,7 @@ check_encode(void)
     unsigned char big[QR_MAX_BYTES];
     memset(big, 'A', sizeof big);
     check(qr_encode(&q, big, QR_MAX_BYTES) == 0, "maximum payload encodes");
-    check(q.size == QR_MAX_SIZE, "and picks version 40");
+    check(q.size == 41, "and picks version 6");
-    check(qr_capacity(40) == 2953 && qr_capacity(10) == 271 && qr_capacity(9) == 230,
-          "capacities match the standard at v9, v10 and v40");
     check(qr_encode(&q, big, QR_MAX_BYTES + 1) == -1, "one byte over is refused");
 
     // Every function module must still be a function module after masking --
@@ -273,7 +270,7 @@ main(int argc, char **argv)
     // codeword two encoders disagree about is the difference between a bug you
     // can see and one you can only stare at.
     if (argc > 2 && strcmp(argv[1], "-f") == 0) {
-        static struct qr q;
+        struct qr q;
         int n = (int)strlen(argv[2]);
         if (n > QR_MAX_BYTES || qr_encode(&q, (const unsigned char *)argv[2], n) != 0)
             return 1;
@@ -286,7 +283,7 @@ main(int argc, char **argv)
     }
 
     if (argc > 2 && strcmp(argv[1], "-m") == 0) {
-        static struct qr q;
+        struct qr q;
         int n = (int)strlen(argv[2]);
         if (n > QR_MAX_BYTES || qr_encode(&q, (const unsigned char *)argv[2], n) != 0)
             return 1;
@@ -311,7 +308,7 @@ main(int argc, char **argv)
     check_rs();
     check_encode();
 
-    static struct qr q;
+    struct qr q;
     int len = (int)strlen(text);
     if (len > QR_MAX_BYTES) {
         fprintf(stderr, "qrtest: payload is %d bytes, max %d\n", len, QR_MAX_BYTES);