diff --git a/Makefile b/Makefile index 78929e0..a936bad 100644 --- a/Makefile +++ b/Makefile @@ -531,7 +531,26 @@ kernel: $(BUILD)/kernel/kernel # clobber; leaving it enabled corrupts locals at random. # -fshort-wchar makes L"..." UTF-16, which is the only string type UEFI has. # ----------------------------------------------------------------------- -UEFI_CFLAGS = --target=x86_64-unknown-windows -ffreestanding -fshort-wchar -mno-red-zone -fno-stack-protector -Wall -Wextra -O1 -I./include +# Debug builds. +# +# make DEBUG=1 +# +# What it turns on is the *boot menu's* remote-debugging entry, and nothing +# else -- see SCHBOOT_DEBUG_FLAG in include/schboot.h. A release build has no +# way to switch a network debugger on, because the code that would set the flag +# is not compiled into it. That is the point: a menu item that is present and +# refuses is an invitation to find out how to make it not refuse. +# +# It is deliberately not a kernel-wide -DDEBUG. Nothing in the kernel changes +# behaviour on it, so a debug kernel and a release kernel are the same kernel, +# and a bug that only reproduces on one of them cannot be this flag's fault. +ifeq ($(DEBUG),1) +UEFI_DEBUG_FLAGS = -DSCHOS_DEBUG +else +UEFI_DEBUG_FLAGS = +endif + +UEFI_CFLAGS = $(UEFI_DEBUG_FLAGS) --target=x86_64-unknown-windows -ffreestanding -fshort-wchar -mno-red-zone -fno-stack-protector -Wall -Wextra -O1 -I./include # The three headers at the end are the ones boot/common/ui.c actually includes, # and leaving them out meant a new logo did not rebuild the loader: the splash diff --git a/boot/uefi/main.c b/boot/uefi/main.c index 616b939..c0de95c 100644 --- a/boot/uefi/main.c +++ b/boot/uefi/main.c @@ -106,6 +106,12 @@ static int SPLASH_UP; * BIOS loader writes, so a verbose boot means the same thing either way. */ static int VERBOSE_BOOT; +// Remote debugging, off unless somebody picks it out of the boot menu -- and +// the entry that picks it only exists in a build made with DEBUG=1. See +// SCHBOOT_DEBUG_FLAG in include/schboot.h for why the consent is asked for at +// this particular moment. +static int DEBUG_BOOT; + static void serial_putc(char c) { @@ -1101,23 +1107,38 @@ boot_menu(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop) res_row(gop, resbuf, sizeof resbuf); - const char *items[5]; + // Six entries in a debug build, five in a release one. + // + // The debugger entry is compiled out rather than hidden, because a + // menu item that is present and refuses is an invitation to work out + // how to make it not refuse. An ISO built for anybody else has no way + // to turn a network debugger on, because the code that would set the + // flag is not in it. + const char *items[6]; + int nitems = 5; items[0] = "Boot SchOS"; items[1] = "Boot SchOS (verbose)"; items[2] = resbuf; items[3] = "System Information"; items[4] = "Reboot to firmware"; +#ifdef SCHOS_DEBUG + items[5] = DEBUG_BOOT ? "Remote debugging: on (schdbgd, port 4444)" + : "Remote debugging: off"; + nitems = 6; +#endif - ui_menu("Boot Manager", items, 5, selected, - "W/S or 1-5 to choose, Enter to select"); + ui_menu("Boot Manager", items, nitems, selected, + nitems == 6 ? "W/S or 1-6 to choose, Enter to select" + : "W/S or 1-5 to choose, Enter to select"); // The same menu on the serial line: the whole thing once, then one // line per *move*. Reprinting on every keypress would be correct and // unreadable -- a serial terminal has no cursor to paint over, so an // unrecognised key would scroll the selection past again for nothing. if (echoed < 0) { - serial_menu("Boot Manager", items, 5, selected, - "W/S or 1-5 to choose, Enter to select", 1); + serial_menu("Boot Manager", items, nitems, selected, + nitems == 6 ? "W/S or 1-6 to choose, Enter to select" + : "W/S or 1-5 to choose, Enter to select", 1); } else if (echoed != selected) { serial_item(items, selected, selected, 1); } @@ -1135,17 +1156,23 @@ boot_menu(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop) BS->Stall(10000); } - if (scan == 0x01) { selected = (selected + 4) % 5; continue; } /* up */ - if (scan == 0x02) { selected = (selected + 1) % 5; continue; } /* down */ - if (c == L'w' || c == L'W') { selected = (selected + 4) % 5; continue; } + if (scan == 0x01) { selected = (selected + nitems - 1) % nitems; continue; } + if (scan == 0x02) { selected = (selected + 1) % nitems; continue; } + if (c == L'w' || c == L'W') { selected = (selected + nitems - 1) % nitems; continue; } if (c == L's' || c == L'S') { selected = (selected + 1) % 5; continue; } - if (c >= L'1' && c <= L'5') { selected = (int)(c - L'1'); continue; } + if (c >= L'1' && c <= L'0' + nitems) { selected = (int)(c - L'1'); continue; } if (c == L'\r' || c == L'\n') { if (selected == 0) return 0; if (selected == 1) return 1; if (selected == 2) { res_choose(gop); continue; } if (selected == 3) { sys_info(gop); continue; } +#ifdef SCHOS_DEBUG + // A toggle rather than an action: picking it changes what the + // entry says and leaves you in the menu, so the state you booted + // with is the state you can see before you boot. + if (selected == 5) { DEBUG_BOOT = !DEBUG_BOOT; echoed = -1; continue; } +#endif if (selected == 4) { ui_clear(); ui_text_centered((int)ui_height() / 2, "Halted -- reset to reach firmware setup", @@ -1772,6 +1799,7 @@ EfiMain(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) *(UINT32 *)SCHBOOT_E820_COUNT = nent; *(UINT64 *)SCHBOOT_RSDP_ADDR = rsdp; *(UINT8 *)SCHBOOT_VERBOSE_FLAG = (UINT8)VERBOSE_BOOT; + *(UINT8 *)SCHBOOT_DEBUG_FLAG = (UINT8)DEBUG_BOOT; // The page tables mboot_entry would have built. entry64low's first act is // a movabs to 0xffff8000...., so the high half must be mapped before the diff --git a/cmd/schinitd/main.c b/cmd/schinitd/main.c index 4fb7bf1..42ef852 100644 --- a/cmd/schinitd/main.c +++ b/cmd/schinitd/main.c @@ -102,6 +102,11 @@ out(const char *s) // silent supervisor is worse than a chatty one. static int verbose_boot; +// Whether the boot menu was asked for a network debugger. See +// SCHBOOT_DEBUG_FLAG in include/schboot.h -- the kernel only publishes it, and +// this is what acts on it, because what it turns on is a job. +static int debug_boot; + static void read_bootmode(void) { @@ -109,13 +114,23 @@ read_bootmode(void) int fd = open("/proc/bootmode", O_RDONLY); int n; - if (fd < 0) - return; // no procfs: assume quiet - n = read(fd, b, sizeof(b) - 1); - close(fd); - if (n > 0) { - b[n] = 0; - verbose_boot = (b[0] == 'v'); + if (fd >= 0) { + n = read(fd, b, sizeof(b) - 1); + close(fd); + if (n > 0) { + b[n] = 0; + verbose_boot = (b[0] == 'v'); + } + } + + fd = open("/proc/bootdebug", O_RDONLY); + if (fd >= 0) { + n = read(fd, b, sizeof(b) - 1); + close(fd); + if (n > 0) { + b[n] = 0; + debug_boot = (b[0] == 'o' && b[1] == 'n'); + } } } @@ -635,6 +650,19 @@ main(void) // greeting below -- see the comment on it. for (int i = 0; i < njobs; i++) { struct job *j = &jobs[i]; + // The one job a boot-menu choice can un-disable. + // + // Deliberately by label rather than by a manifest key like + // `EnableIfDebug`: a key would be a general mechanism, and the general + // version of "the boot menu can start things that ship switched off" + // is a worse idea than the specific one. One job, named here, where + // anybody reading this file can see which. + if (debug_boot && j->disabled && seq(j->label, "os.schos.schdbgd")) { + out("schinitd: remote debugging was chosen at boot; " + "starting schdbgd\n"); + j->disabled = 0; + } + if (j->disabled || !j->run_at_load || j->own_tty) continue; if (j->pid) diff --git a/cmd/tone/main.c b/cmd/tone/main.c index a0207c0..be72fa0 100644 --- a/cmd/tone/main.c +++ b/cmd/tone/main.c @@ -15,6 +15,7 @@ #include "user.h" #include "fcntl.h" #include "audio.h" +#include "errno.h" // A quarter-amplitude sine, from a table rather than a libm that does not // exist here. 64 points is audibly a sine and arithmetically exact. @@ -86,8 +87,24 @@ main(int argc, char *argv[]) // Ask, then believe the answer. These are read/write on purpose: the // device writes back what it actually set, and a program that ignores // that plays at the wrong speed and blames the driver. - if (ioctl(fd, SNDCTL_DSP_SETFMT, &fmt) < 0 || fmt != AFMT_S16_LE) { - say("tone: the device will not take 16-bit samples\n"); + // + // The two failures are told apart, because they were not and the message + // was a lie: a machine with no sound card at all reported "the device will + // not take 16-bit samples", which sends you looking at the format code + // instead of at the empty PCI slot. The node exists whether or not any + // hardware is behind it -- that is deliberate, and it is why the *ioctl* + // is where the absence shows up. + int r = ioctl(fd, SNDCTL_DSP_SETFMT, &fmt); + if (r == -ENODEV) { + say("tone: there is no audio device on this machine\n"); + say(" (/dev/dsp exists on every machine; the hardware does not)\n"); + close(fd); + return 1; + } + if (r < 0 || fmt != AFMT_S16_LE) { + say("tone: the device will not take 16-bit samples (ioctl said "); + sayn(r); say(")\n"); + close(fd); return 1; } ioctl(fd, SNDCTL_DSP_CHANNELS, &channels); diff --git a/docs/architecture/debugging.md b/docs/architecture/debugging.md index 407a148..809af56 100644 --- a/docs/architecture/debugging.md +++ b/docs/architecture/debugging.md @@ -20,6 +20,51 @@ watchpoints. --- +## Turning it on at the boot menu + +`schdbgd` ships `Disabled`, and the reason is in its manifest: a debugger that +can read the process table should not be running on a machine whose owner never +asked for one. That is the right default and it has one bad consequence — +**the machines that most need debugging are the ones you cannot get a shell on +to enable it.** A laptop with no serial port that stops before it can draw has +nowhere to say why, and `schinitd enable` needs a prompt that does not exist. + +So a build made with `make DEBUG=1` has a sixth entry in the F8 menu: + +``` + > 6. Remote debugging: off +``` + +Picking it toggles the entry, booting carries the choice through +`SCHBOOT_DEBUG_FLAG`, the kernel publishes it as `/proc/bootdebug`, and +`schinitd` starts the one job that flag is allowed to un-disable. Plug the +machine into a router, pick the entry, and connect from the laptop next to it: + +```sh +nc 192.168.1.50 4444 +dma +``` + +Four decisions worth stating, because this is a network-reachable debugger: + +**It is compiled out, not hidden.** A release build has no way to set the flag, +because the code that would set it is not in the binary. A menu item that is +present and refuses is an invitation to work out how to make it not refuse. + +**The consent is asked for at the machine.** Somebody standing at the keyboard +during boot is the strongest consent this system can ask for, and the weakest +one that is any use on hardware that will not boot. + +**One job, named in `schinitd`.** Not a manifest key like `EnableIfDebug` — the +general version of "the boot menu can start things that ship switched off" is a +worse idea than the specific one, and this way the list of what the flag can +start is one line anybody can read. + +**The kernel does nothing with the flag but publish it.** A kernel that changed +behaviour on a debug flag would be a kernel whose bugs could depend on it, and +a debug build and a release build would stop being the same kernel. + + ## The rule the shape follows **It observes; it does not control.** No stepping, no software breakpoints, no diff --git a/etc/schinit/schdbgd.job b/etc/schinit/schdbgd.job index 1e4c6dd..982daf5 100644 --- a/etc/schinit/schdbgd.job +++ b/etc/schinit/schdbgd.job @@ -8,6 +8,11 @@ # # schinitd enable os.schos.schdbgd (or: set Disabled = no here) # +# There is now a third way, and it is the one for a machine that will not boot: +# pick "Remote debugging" in the F8 menu of a build made with DEBUG=1, and +# schinitd starts this job despite the line below. That entry does not exist in +# an ordinary build -- see SCHBOOT_DEBUG_FLAG in include/schboot.h. +# # Off rather than loopback-only-and-on, even though loopback-only is already the # default bind, because "a listener nobody asked for" is a different kind of # thing from "a listener bound somewhere narrow". A debugger that can read the @@ -19,7 +24,12 @@ Label = os.schos.schdbgd Program = /bin/schdbgd -Args = schdbgd +# Bound to every interface, *because* the only ways to start it are deliberate. +# A debugger reachable only from the machine being debugged is no use when the +# machine being debugged is the one that will not talk to you -- which is the +# whole reason the boot-menu entry exists. Plug the machine into a router, pick +# the entry, and connect to port 4444 from the laptop next to it. +Args = schdbgd -a 0.0.0.0 Disabled = yes RunAtLoad = yes KeepAlive = yes diff --git a/include/defs.h b/include/defs.h index 8a0461d..e6826f2 100644 --- a/include/defs.h +++ b/include/defs.h @@ -68,6 +68,7 @@ void cprintf(char*, ...); // See the note above kinfo() in kernel/core/console.c for which is which. void kinfo(char*, ...); extern int boot_verbose; +extern int boot_debug; // A log line: the serial port only, never the display. // diff --git a/include/schboot.h b/include/schboot.h index e0ca267..096b7ac 100644 --- a/include/schboot.h +++ b/include/schboot.h @@ -59,6 +59,25 @@ // reads through ATAPI rather than ATA; 0 means "not stated", which is what the // UEFI loader leaves, since it does not read the kernel through either. #define SCHBOOT_MEDIA_FLAG (SCHBOOT_BASE + 0x101) + +// Remote debugging, chosen at the boot menu. +// +// A byte, set by the loader when somebody picks it out of the F8 menu, read by +// the kernel and published as /proc/bootdebug so that schinit can act on it. +// +// **It exists because the machines that need debugging are the ones that +// cannot be debugged.** A laptop has no serial port, so a kernel that stops +// before it can draw has nowhere to say why; the answer is a debugger reachable +// over the network, and the answer to "when should that be listening" cannot be +// "always" -- schdbgd can read the process table, and a listener nobody asked +// for is not a thing to ship. Choosing it at the boot menu is a deliberate act +// by somebody standing at the machine, which is the strongest consent this +// system can ask for and the weakest one that is any use on hardware that will +// not boot. +// +// Zero on every ordinary boot, including every boot of a build made without +// DEBUG=1 -- where the menu entry does not exist and nothing can set it. +#define SCHBOOT_DEBUG_FLAG (SCHBOOT_BASE + 0x102) #define SCHBOOT_MEDIA_CD 1 // Real-mode spelling of the same byte, for cdboot.S which has no 32-bit diff --git a/kernel/core/main.c b/kernel/core/main.c index c129e22..69f80c1 100644 --- a/kernel/core/main.c +++ b/kernel/core/main.c @@ -22,6 +22,14 @@ extern char end[]; // first address after kernel loaded from ELF file // before anything else, so subsystem init can log its own steps. int boot_verbose; +// Remote debugging, chosen at the boot menu. See SCHBOOT_DEBUG_FLAG. +// +// The kernel does nothing with it beyond publishing /proc/bootdebug: the thing +// that acts on it is schinitd, because what it turns on is a *job*. A kernel +// that changed its own behaviour on a debug flag would be a kernel whose bugs +// could depend on the flag. +int boot_debug; + // Must match SCHBOOT_VERBOSE_FLAG in boot/bios/stage2main.c exactly: just past // the VBE info block, not 0x7000 (startothers() below uses that address for // the AP boot trampoline). @@ -40,6 +48,7 @@ kmain(uint32 fb_addr) int fb_early_ok = 0, fb_attached = 0; boot_verbose = *(uchar*)P2V(SCHBOOT_VERBOSE_FLAG); + boot_debug = *(uchar*)P2V(SCHBOOT_DEBUG_FLAG); // On a verbose boot, put the kernel log on the screen from the first line. // diff --git a/kernel/dev/procfs.c b/kernel/dev/procfs.c index 7b455fd..6e4ce6f 100644 --- a/kernel/dev/procfs.c +++ b/kernel/dev/procfs.c @@ -345,6 +345,7 @@ gen_filesystems(struct sbuf *sb) #define PROC_KMSG 17 #define PROC_RESOLV 18 #define PROC_BOOTMODE 19 +#define PROC_BOOTDEBUG 30 #define PROC_SLAB 20 // Render global file `id` (a PROC_* constant) into `buf`. Returns its length. @@ -393,6 +394,12 @@ proc_gen_global(int id, char *buf, int cap) case PROC_BOOTMODE: sb_puts(&sb, boot_verbose ? "verbose\n" : "quiet\n"); break; + // Whether the boot menu was asked for a network debugger. Its own file + // rather than a second line in bootmode, because bootmode's whole promise + // is that reading it is a string compare and not a parser. + case PROC_BOOTDEBUG: + sb_puts(&sb, boot_debug ? "on\n" : "off\n"); + break; case PROC_DEVICES: gen_devices(&sb); break; default: return 0; } diff --git a/kernel/fs/procfs.c b/kernel/fs/procfs.c index fc6af7e..c0d1bd3 100644 --- a/kernel/fs/procfs.c +++ b/kernel/fs/procfs.c @@ -100,6 +100,7 @@ static const struct global_file globals[] = { { "resolv.conf", 18 }, // "quiet" or "verbose" -- how this machine was booted. See cmd/schinitd. { "bootmode", 19 }, + { "bootdebug", 30 }, // What the slab allocator is holding. See include/kmem.h. { "slab", 20 }, { "kmsg", 17 },