diff --git a/include/core/defs.h b/include/core/defs.h index e05d5fc..c7cbd0c 100644 --- a/include/core/defs.h +++ b/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); diff --git a/include/gfx/qr.h b/include/gfx/qr.h index a2151d1..a261855 100644 --- a/include/gfx/qr.h +++ b/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 -// card -- until PSOGM mode wanted the log itself off the machine, a page of it -// 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. +// If a payload ever needs more than 134 bytes, the honest fix is to shorten the +// report -- not to reach for version 10 and its two extra tables. -#define QR_MAX_VERSION 40 -#define QR_MAX_SIZE (17 + 4 * QR_MAX_VERSION) // 177 modules per side -#define QR_MAX_BYTES 2953 - -// 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 +#define QR_MAX_VERSION 6 +#define QR_MAX_SIZE (17 + 4 * QR_MAX_VERSION) // 41 modules per side +#define QR_MAX_BYTES 134 struct qr { int size; // modules per side; 0 if unencoded diff --git a/kernel/arch/x86_64/ioperm.c b/kernel/arch/x86_64/ioperm.c index b27bcb4..cf6d238 100644 --- a/kernel/arch/x86_64/ioperm.c +++ b/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 diff --git a/kernel/arch/x86_64/lapic.c b/kernel/arch/x86_64/lapic.c index 2c06bd0..5830d05 100644 --- a/kernel/arch/x86_64/lapic.c +++ b/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) diff --git a/kernel/arch/x86_64/trap.c b/kernel/arch/x86_64/trap.c index 46ecde5..6c90fa4 100644 --- a/kernel/arch/x86_64/trap.c +++ b/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: diff --git a/kernel/core/panic.c b/kernel/core/panic.c index cedd853..b29939c 100644 --- a/kernel/core/panic.c +++ b/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. diff --git a/kernel/core/qr.c b/kernel/core/qr.c index a3e6421..6d70fbf 100644 --- a/kernel/core/qr.c +++ b/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 -// local, or static. +// what is broken. Every buffer here is either on the caller's struct or a +// 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 -// transcribed from ISO/IEC 18004 table 9 -- EC codewords per block, and how -// many blocks -- and everything else is computed: the symbol's total codeword -// count from its geometry, and the block lengths from the total. Above -// version 5 the blocks are not all one length; the first ones are a codeword -// shorter, and interleaving has to step around the gap. -static const unsigned char ecc_per_block[QR_MAX_VERSION + 1] = { - 0, 7, 10, 15, 20, 26, 18, 20, 24, 30, 18, 20, 24, 26, 30, 22, 24, 28, 30, 28, - 28, 28, 28, 30, 30, 26, 28, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, 30, - 30, +static const struct { + short total; // total codewords in the symbol + unsigned char ecc; // EC codewords per block + unsigned char blocks; // number of RS blocks +} vinfo[QR_MAX_VERSION + 1] = { + { 0, 0, 0 }, // version 0 does not exist; keeps indexing natural + { 26, 7, 1 }, + { 44, 10, 1 }, + { 70, 15, 1 }, + { 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, - // every combination drawn except the three that would sit on a finder. The - // list starts at 6, ends at n-7, and steps evenly (in even numbers) between, - // except version 32, which the standard spaces by hand. - int v = (n - 17) / 4; - 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); - } - } + // For versions 2-6 the alignment centres are {6, n-7}, and three of those + // four combinations collide with a finder. Only the bottom-right survives, + // which is why there is no position table here. Version 7 would need one. + if (q->size > 21) + draw_alignment(q, n - 7, n - 7); 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 ecclen = ecc_per_block[v]; - int total = total_codewords(v); - int datacw = data_codewords(v); - int nshort = blocks - total % blocks; // blocks one data codeword short - int shortlen = total / blocks - ecclen; // their data length + int total = vinfo[v].total; + int ecclen = vinfo[v].ecc; + int blocks = vinfo[v].blocks; + int datacw = total - ecclen * blocks; + + // 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 --------------------------------------- - // - // 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]; + unsigned char cw[172]; // >= max total codewords 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[BSTART(b) + i]; + out[o++] = cw[b * blocklen + 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)); diff --git a/tools/python/qrcheck.py b/tools/python/qrcheck.py index 87ef221..e034439 100644 --- a/tools/python/qrcheck.py +++ b/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 -- -# written out rather than computed, so it checks qr.c instead of agreeing with -# it. The exact-fill cases below are built from it, and a disagreement shows up +# Byte capacity per version: total - ecc*blocks - 2. Mirrors the table in qr.c; +# the exact-fill cases below are built from it, and a disagreement would show up # as a version mismatch rather than passing quietly. -CAPACITY = dict(enumerate([ - 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)) +CAPACITY = {1: 17, 2: 32, 3: 53, 4: 78, 5: 106, 6: 134} 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 -# blocks from 6, blocks of two lengths from 6 too, the version information -# from 7, the 16-bit character count from 10. +# padded and unpadded paths are covered at every version -- including 6, which +# is the only one with more than one Reed-Solomon block. for v, cap in CAPACITY.items(): CASES.append(alpha(cap)) CASES.append(alpha(cap - 1)) diff --git a/tools/qrtest.c b/tools/qrtest.c index 65ecee2..ba2b64f 100644 --- a/tools/qrtest.c +++ b/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 -// the blocks are no more than one codeword apart, and the symbol has room for -// exactly the codewords the geometry formula claims. +// reasoning, so check what can be checked: block sizes must divide evenly, and +// the symbol must have room for exactly the codewords claimed. static void check_tables(void) { printf("version table\n"); for (int v = 1; v <= QR_MAX_VERSION; v++) { - int total = total_codewords(v); - int data = data_codewords(v); + int total = vinfo[v].total; + int data = total - vinfo[v].ecc * vinfo[v].blocks; char msg[128]; - snprintf(msg, sizeof msg, "v%d: %d data codewords in %d blocks", - v, data, ecc_blocks[v]); - check(data > 0 && data / ecc_blocks[v] > 0, msg); + snprintf(msg, sizeof msg, "v%d: %d data codewords divide into %d blocks", + v, data, vinfo[v].blocks); + 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(qr_capacity(40) == 2953 && qr_capacity(10) == 271 && qr_capacity(9) == 230, - "capacities match the standard at v9, v10 and v40"); + check(q.size == 41, "and picks version 6"); 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);