Hi, On Mon, May 4, 2020 at 4:53 AM Daniel Thompson <daniel.thompson@xxxxxxxxxx> wrote: > > On Fri, May 01, 2020 at 10:36:14AM -0700, Doug Anderson wrote: > > Hi, > > > > On Fri, May 1, 2020 at 6:32 AM Daniel Thompson > > <daniel.thompson@xxxxxxxxxx> wrote: > > > > > > On Fri, May 01, 2020 at 12:49:43PM +0100, Daniel Thompson wrote: > > > > On Thu, Apr 30, 2020 at 09:59:09AM -0700, Douglas Anderson wrote: > > > > > The original implementation of kgdboc_earlycon included a comment > > > > > about how it was impossible to get notified about the boot console > > > > > going away without making changes to the Linux core. Since folks > > > > > often don't want to change the Linux core for kgdb's purposes, the > > > > > kgdboc_earlycon implementation did a bit of polling to figure out when > > > > > the boot console went away. > > > > > > > > > > It turns out, though, that it is possible to get notified about the > > > > > boot console going away. The solution is either clever or a hack > > > > > depending on your viewpoint. ...or, perhaps, a clever hack. All we > > > > > need to do is head-patch the "exit" routine of the boot console. We > > > > > know that "struct console" must be writable because it has a "next" > > > > > pointer in it, so we can just put our own exit routine in, do our > > > > > stuff, and then call back to the original. > > > > > > > > I think I'm in the hack camp on this one! > > > > > > > > > > > > > This works great to get notified about the boot console going away. > > > > > The (slight) problem is that in the context of the boot console's exit > > > > > routine we can't call tty_find_polling_driver(). > > > > > > > > I presume this is something to do with the tty_mutex? > > > > > We solve this by > > > > > kicking off some work on the system_wq when we get notified and this > > > > > works pretty well. > > > > > > > > There are some problems with the workqueue approach. > > > > > > ... so did a couple of experiments to avoid the workqueue. > > > > > > It occured to me that, since we have interfered with deinit() then the > > > console hasn't actually been uninitialized meaning we could still use it. > > > This does exposes us to risks similar to keep_bootcon but in exchange > > > there is no window where kgdb is broken (and no need to panic). > > > > > > My prototype is minimal but I did wonder about ripping out all the > > > code to defend against removal of the earlycon and simply keep the > > > earlycon around until a new kgdbio handler is installed. > > > > It took me a little while, but I finally see what you're saying. > > You're saying that we keep calling into the boot console even though > > it's no longer in the list of consoles. Then we temporarily disable > > the boot console's exit routine until kgdb_earlycon() is done. (side > > note: the exit routine was recently added and probably most consoles > > don't use it). > > Certainly none of the devices with a read() method have an exit(). > > > > OK, that doesn't seem totally insane. It actually works OK for you? > > I tested on qemu/x86-64 (8250) and qemu/arm64 (pl011). In both cases it > works well. > > > > It's probably at least worth a warning in the log if we detect that > > we're using the boot console and it's not in the console list anymore. > > Then if kgdb starts misbehaving someone might have a clue. > > Yes, I'll add that. > > > > If your solution is OK we might also want to remove the call to > > cleanup_earlycon_if_invalid() in configure_kgdboc() too. > > That's what I thought, yes. Although it might be best to handle that > by ripping it out of the original patch set. I've incorporated ideas from my patch and yours into a v4 patchset of the original series. Note that I didn't include your earlycon deferral patchset [1] in my series which means it'll need to be rebased. Hopefully this is OK since I think the rebase will be easy, but yell if you want an extra pair of eyes on it. [1] https://lore.kernel.org/r/20200430161741.1832050-1-daniel.thompson@xxxxxxxxxx -Doug
