FreeBSD source tree https://cgit-dev.freebsd.org/src-test2/atom?h=release%2F11.3.0 2018-03-29T02:50:57Z 2018-03-29T02:50:57Z Eitan Adler eadler@FreeBSD.org 2018-03-29T02:50:57Z urn:sha1:4ab2e064d7950be84256d671a7ae93f87cc6aa36 This was intended to be a non-functional change. It wasn't. The commit message was thus wrong. In addition it broke arm, and merged crypto related code. Revert with prejudice. This revert skips files touched in r316370 since that commit was since MFCed. This revert also skips files that require $FreeBSD$ property changes. Thank you to those who helped me get out of this mess including but not limited to gonzo, kevans, rgrimes. Requested by: gjb (re) 2018-03-14T07:58:11Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:58:11Z urn:sha1:7e15b3029fa085f8db7e2304dda012018f2bbc89 The switch to kernel terminal context needs to update more than the cursor position. Especially the screen size, and potentially everything except the input state and attributes. Do this by changing the cursor position setting method to a general syncing method. Use proper constructors instead of copying to create kernel terminal contexts. We really want clones and not new instances, but there is no method for cloning and there is nothing in the active instance that needs to be cloned exactly. Add proper destructors for kernel terminal contexts. I doubt that the destructor code has every been reached, but if it was then it leaked the memory of the clones. Remove freeing of statically allocated memory for the non-kernel terminal context for the same terminal as the kernel. This is in the nearly unreachable code. This used to not happen because delicate context swapping made the user context use the dynamic memory and kernel context the static memory. I didn't restore this swapping since it would have been unnatural to have all kernel contexts except 1 dynamic. The constructor for terminal context has bad layering for reasons related to the bug. It has to return static memory early before malloc() works. Callers also can't allocate memory until after the first constructor selects an emulator and tells upper layers the size of its context. After that, the cloning hack required the cloning code to allocate the memory, but for all other constructors it would be better for the terminal layer to allocate and deallocate the memory in all cases. Zero the memory when allocating terminal contexts dynamically. 2018-03-14T07:55:29Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:55:29Z urn:sha1:8463765810cad4b4df38de193b05de05bab32506 Restore switching to a separate kernel terminal "input" state and extend it to a separate state for each CPU. Terminal "input" is user or kernel output. Its state includes the current parser state for escape sequences and multi-byte characters, and some results of previous parsing (mainly attributes), and in teken the cursor position, but not completed output. This state must be switched for kernel output since the kernel can preempt anything, including itself, and this must not affect the preempted state more than necessary. Since vty0 is shared, it is necessary to affect the frame buffer and cursor position and history, but escape sequences must not be affected and attributes for further output must not be affected. This used to work. The syscons terminal state contained mainly the parser state for escape sequences and attributes, but not the cursor position, and was switched. This was first broken by SMP and/or preemptive kernels. Then there should really be a separate state for each thread, and one more for ddb, or locking to prevent preemption. Serialization of printf() helps. But it is arcane that full syscons escape sequences mostly work in kernel printf(), and I have never seen them used except by me to test this fix. They worked perfectly except for the races, since "input" from the kernel was not special in any way. This was broken to use teken. The general switch was removed, and the kernel normal attribute was switched specially. The kernel reverse attribute (config option SC_CONS_REVERSE_ATTR) became unused, and is still unusable because teken doesn't support default reverse attributes (it used to only be used via the ANSI escape sequence to set reverse video). The only new difficulty for using teken seems to be that the cursor position is in the "input" state, so it must be updated in the active input state for each half of the switch. Do this to complete the restoration. The per-CPU state is mainly to make per-CPU coloring work cleanly, at a cost of some space. Each CPU gets its own full set of attribute (not just the current attribute) maintained in the usual way. This also reduces races from unserialized printf()s. However, this gives races for serialized printf()s that otherwise have none. Nothing prevents the CPU doing the a printf() changing in the middle of an escape sequence. 2018-03-14T07:39:28Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:39:28Z urn:sha1:e899f9e540445dd8b3eb4473d1cbc8d53f29a601 Rename scteken_revattr() to scteken_sc_to_te_attr(). scteken_revattr() looked like it might handle reverse attributes, but it actually handles conversion of attributes in the direction indicated by the new name. Reverse attributes are just broken. Rename scteken_attr() to scteken_te_to_sc_attr(). scteken_attr() looked like it might give teken attributes, but it actually gives sc attributes. Change scteken_te_to_sc_attr() to return int instead of unsigned int. u_char would be enough, and it promotes to int, and syscons uses int or u_short for its attributes everywhere else (u_short holds a shifted form and it promotes to int too). Add a scteken_set_cursor() (sc to teken) method and use it to fix some cases of initialization and resetting of the teken cursor position. (This bad name is consistent with others, but it is too easy to confuse with scteken_cursor() which goes in the opposite direction.) The following cases were broken: - for booting without a syscons console, the teken and sc positions for ttyv0 were (0, 0), but are supposed to be somewhere in the middle of the screen (after carefully preserved BIOS and loader messages) (at least if there is no mode switch that loses the messages). - after mode switches, the screen is cleared and the cursor is supposed to be moved to (0, 0), but it was only moved there for sc. The following case was hacked to work: - for booting with a syscons console, it was arranged that scteken_init() for the console could see a nonzero cursor position and adjust, although this broke the sc seeing it in the non-console case above. Fix the attribute for scteken_clear() (change it back from the user user default normal attribute to the current attribute). This change only fixes a logic error. scterm_clear() used to be used for terminal reset, but teken uses a general fill function for that, leaving scterm_clear() only used for initialization and mode change, when using the user default attribute is correct. It is not really a terminal function, but needs to sync its changes with the terminal layer. Syncing of the attribute is currently broken for terminal reset, but works for initialization and mode change. 2018-03-14T07:32:48Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:32:48Z urn:sha1:b8e0e623b437b42dbb88083c4df25e4c0cd63bbb Start fixing some bugs in attribute handling. This change just does cleanups missed in r56043 17 years ago. The default attributes were still stored in structs for the purpose of changing them and passing around pointers to the defaults, but r56043 added another layer that made the defaults invariant and only used for initialization and reset. Just use the defaults directly. This was already done for the kernel defaults. The defaults for reverse attributes aren't actually used, but are ignored in layers that no longer support them. 2018-03-14T07:30:58Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:30:58Z urn:sha1:7491dcd03b921cf7b367f7d9b90a9e54a70ec73a Colorize syscons kernel console output according to a table indexed by the CPU number. This was originally for debugging near-deadlock conditions where multiple CPUs either deadlock or scramble each other's output trying to report the problem, but I found it interesting and sometimes useful for ordinary kernel messages. Ordinary kernel messages shouldn't be interleaved, but if they are then the colorization makes them readable even if the interleaving is for every character (provided the CPU printing each message doesn't change). The default colors are 8-15 starting at 15 (bright white on black) for CPU 0 and repeating every 8 CPUs. This works best with 8 CPUs. Non-bright colors and nonzero background colors need special configuration to avoid unreadable and ugly combinations so are not configured by default. The next bright color after 15 is 8 (bright black = dark gray) is not very readable but is the only other color used with 2 CPUs. After that the next bright color is 9 (bright blue) which is not much brighter than bright black, but is used with 3+ CPUs. Other bright colors are brighter. Colorization is configured by default so that it gets tested. It can only be turned off by configuring SC_KERNEL_CONS_ATTR to anything other than FG_WHITE. After booting, all colors can be changed using the syscons.kattr sysctl. This is a SYSCTL_OPAQUE, and no utility is provided to change it (sysctl only displays it). The default colors work in all VGA modes that I could test. In 2-color graphics modes, all 8 bright colors are displayed as bright white, so the colorization has no effect, but anything with a nonzero background gives white on white unless the foreground is zero. I don't have an mono or VGA grayscale hardware to test on. Support for mono mode seems to have never worked right in syscons (I think bright white gives white underline with either bold or bright), but VGA grayscale should work better than 2-color graphics. Implement ec_putc() (emergency kernel [syscons] console putc()) and use it in emergency in sc_cnputc(). Locking fixes in sc_cnputc() previously turned off normal output in near-deadlock conditions and added deferred output which might never be completed. Emergency output goes to the frame buffer using sufficiently atomic non-blocking writes if the console is in text mode (in graphics mode, nothing is done, modulo races setting the graphics mode bit). Screen updates overwrite the emergency output if the emergency condition clears enough to reach them. ec_putc() also works for "early" console output in normal x86 text mode as soon as this mode is initialized (if ever). This uses a hard-coded x86 frame buffer address before cninit() and a hopefully MI address after cninit(). But non-x86 is more likely to not support text mode, when ec_putc() will be null. ec_putc() has no dependencies of syscons before cninit(), and only has them later to track syscons' mode changes. This commit doesn't attach ec_putc() for early use. To test emergency use, put a breakpoint in central syscons output code like sc_puts() and do some user output. The system used to race or deadlock in ddb output soon after entry to ddb. The locking fixes deferred the output until after leaving ddb, so ddb was unusable and you had to try typing c[ontinue] blindly until it exited, or better use a serial console in parallel. Now the output goes to a window in the middle 2/3 of the screen. Scrolling is circular and there is no cursor, but otherwise ec_putc() provides full dumb terminal functionality and very fast output that hides artificates from dumb overwrites. 2018-03-14T07:16:29Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:16:29Z urn:sha1:343a494480e765b4483fc709c913a8eeb0f36a27 Add some locking to sc_cngetc(). Keyboard input needs Giant locking, and that is not possible to do correctly here. Use mtx_trylock() and proceed unlocked as before if we can't acquire Giant (non-recursively), except in kdb mode don't even try to acquire Giant. Everything here is a hack, but it often works. Even if mtx_trylock() succeeds, this might be a LOR. Keyboard input also needs screen locking, to handle screen updates and switches. Add this, using the same simplistic screen locking as for sc_cnputc(). Giant must be acquired before the screen lock, and the screen lock must be dropped when calling the keyboard driver (else it would get a harmless LOR if it tries to acquire Giant). It was intended that sc cn open/close hide the locking calls, and they do for i/o functions functions except for this complication. Non-console keyboard input is still only Giant-locked, with screen locking in some called functions. This is correct for the keyboard parts only. When Giant cannot be acquired properly, atkbd and kbdmux tend to race and work (they assume that the caller acquired Giant properly and don't try to acquire it again or check that it has been acquired, and the races rarely matter), while ukbd tends to deadlock or panic (since it does the opposite, and has other usb threads to deadlock with). The keyboard (Giant) locking here does very little, but the screen locking completes screen locking for console mode except for not detecting or handling deadlock. The log message for the previous commit didn't mention the most the important detail that sc_cngetc() now opens and closes the keyboard on every call again. This was moved from sc_cngetc() to scn_cngrab/ ungrab() in r228644, but the change wasn't quite complete. After fixes for nesting in kbdd_poll() in ukbd and kbdmux, these opens and closes should have no significant effect if done while grabbed. They fix unusual cases when cngetc() is called while not grabbed. This commit is the main fix for screen locking in sc_cnputc(): detect deadlock or likely-deadlock and handle it by buffering the output atomically and printing it later if the deadlock condition clears (and sc_cnputc() is called). The most common deadlock is when the screen lock is held by ourself. Then it would be safe to acquire the lock recursively if the console driver is calling printf() in a safe context, but we don't know when that is. It is not safe to ignore the lock even in kdb or panic mode. But ignore it in panic mode. The only other known case of deadlock is when another thread holds the lock but is running on a stopped CPU. Detect that case approximately by using trylock and retrying for 1000 usec. On a 4 GHz CPU, 100 usec is almost long enough -- screen switches take slightly longer than that. Not retrying at all is good enough except for stress tests, and planned future versions will extend the timeout so that the stress tests work better. To see the behaviour when deadlock is detected, single step through sctty_outwakeup() (or sc_puts() to start with deadlock). Another (serial) console is needed to the buffered-only output, but the keyboard works in this context to continue or step out of the deadlocked region. The buffer is not large enough to hold all the output for this. 2018-03-14T07:08:46Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:08:46Z urn:sha1:39c94c504940d5d4e2002b77483882ee7b2ce830 Start adding locking to sc_cngetc(). Restore an splx() lost in r228644. We aren't nearly ready to remove spl's. They give hints about missing locking. This lost one was misplaced. Dropping it early for convenience gave race windows for accesses to the fkey buffer. Giant locking accidentally fixed this for non-console cases. Put the spl's around the whole function. Since there are many returns that would need splx() just before them for a direct fix, split the function into a wrapper that does the spl's and a "locked" function that does the work. Return earlier when no keyboard is attached to match the ordering in a planned version. This breaks the dubious feature of returning keys from the fkey buffer after the keyboard has gone away. Losing the keys wouldn't matter, but we keep them too long now. 2018-03-14T07:08:02Z Eitan Adler eadler@FreeBSD.org 2018-03-14T07:08:02Z urn:sha1:62804a5a5ba03b0db0c340fa17693d505815e889 Add screen locking calls to sc cn grab and ungrab. The locking functions just use the same mutex locking as sc cn putc so they have the same defects. The locking calls to acquire the lock are actually in sc cn open and close. Ungrab has to unlock, although this opens a race window. Change the direct mutex lock calls in sc cn putc to the new locking functions via the open and close functions. Putc also has to unlock, but doesn't keep the screen open like grab. Screen open and close reduce to locking, except screen open for grab also attempts to switch the screen. Keyboard locking is more difficult and still null, even when keyboard input calls screen functions, except some of the functions have locks too deep to work right. This organization gives a single place to fix some of the locking. 2018-03-14T03:19:51Z Eitan Adler eadler@FreeBSD.org 2018-03-14T03:19:51Z urn:sha1:be5d0b9566b13fdf8cabebb63334cbec12bfc409 These changes are incomplete but are making it difficult to determine what other changes can/should be merged. No objections from: pfg