Message ID | 20250304035447.3138221-4-adamsimonelli@gmail.com |
---|---|
State | New |
Headers | show |
Series | Optionally allow ttynull to be selected as a default console | expand |
On Tue, Mar 4, 2025 at 5:55 AM <adamsimonelli@gmail.com> wrote: > > From: Adam Simonelli <adamsimonelli@gmail.com> > > If CONFIG_NULL_TTY_DEFAULT_CONSOLE is enabled, and CONFIG_VT is disabled, > ttynull will become the default primary console device, based on the link > order. > > Many distributions ship with CONFIG_VT enabled. On tested desktop hardware > if CONFIG_VT is disabled, the default console device falls back to > /dev/ttyS0 instead of /dev/tty. > > This could cause issues in user space, and hardware problems: > > 1. The user space issues include the case where /dev/ttyS0 is > disconnected, and the TCGETS ioctl, which some user space libraries use > as a probe to determine if a file is a tty, is called on /dev/console and > fails. Programs that call isatty() on /dev/console and get an incorrect > false value may skip expected logging to /dev/console Missing period at the end. > 2. The hardware issues include the case if a user has a science instrument > or other device connected to the /dev/ttyS0 port, and they were to upgrade > to a kernel that is disabling the CONFIG_VT option, kernel logs will then be > sent to the device connected to /dev/ttyS0 unless they edit their kernel > command line manually. > > The new CONFIG_NULL_TTY_CONSOLE option will give users and distribution > maintainers an option to avoid this. Disabling CONFIG_VT and enabling > CONFIG_NULL_TTY_CONSOLE will ensure the default kernel console behavior > is not dependant on hardware configuration by default, and avoid > unexpected new behavior on devices connected to the /dev/ttyS0 serial > port. ... > obj-y += vt/ + blank line. > +# If ttynull is configured to be a console by default, ensure that it is linked > +# earlier before a real one is selected. > +obj-$(CONFIG_NULL_TTY_DEFAULT_CONSOLE) \ > + += ttynull.o Here is the question: are you sure that all console drivers that exist in the kernel happen to be here? Have you grepped the source tree for checking this? ... > +# If ttynull is enabled, but not as a boot console, it is linked and used later > +# after the real ones. > +ifneq ($(CONFIG_NULL_TTY_DEFAULT_CONSOLE),y) Also can be written as ifeq ($(...),) but it might be less explicit. Up to you. > obj-$(CONFIG_NULL_TTY) += ttynull.o > +endif
On Tuesday, March 4, 2025 1:51:52 AM EST Andy Shevchenko wrote: > On Tue, Mar 4, 2025 at 5:55 AM <adamsimonelli@gmail.com> wrote: > > > > From: Adam Simonelli <adamsimonelli@gmail.com> > > > > If CONFIG_NULL_TTY_DEFAULT_CONSOLE is enabled, and CONFIG_VT is disabled, > > ttynull will become the default primary console device, based on the link > > order. > > > > Many distributions ship with CONFIG_VT enabled. On tested desktop hardware > > if CONFIG_VT is disabled, the default console device falls back to > > /dev/ttyS0 instead of /dev/tty. > > > > This could cause issues in user space, and hardware problems: > > > > 1. The user space issues include the case where /dev/ttyS0 is > > disconnected, and the TCGETS ioctl, which some user space libraries use > > as a probe to determine if a file is a tty, is called on /dev/console and > > fails. Programs that call isatty() on /dev/console and get an incorrect > > false value may skip expected logging to /dev/console > > Missing period at the end. > > > 2. The hardware issues include the case if a user has a science instrument > > or other device connected to the /dev/ttyS0 port, and they were to upgrade > > to a kernel that is disabling the CONFIG_VT option, kernel logs will then be > > sent to the device connected to /dev/ttyS0 unless they edit their kernel > > command line manually. > > > > The new CONFIG_NULL_TTY_CONSOLE option will give users and distribution > > maintainers an option to avoid this. Disabling CONFIG_VT and enabling > > CONFIG_NULL_TTY_CONSOLE will ensure the default kernel console behavior > > is not dependant on hardware configuration by default, and avoid > > unexpected new behavior on devices connected to the /dev/ttyS0 serial > > port. > > ... > > > obj-y += vt/ > > + blank line. > > > +# If ttynull is configured to be a console by default, ensure that it is linked > > +# earlier before a real one is selected. > > +obj-$(CONFIG_NULL_TTY_DEFAULT_CONSOLE) \ > > + += ttynull.o > > Here is the question: are you sure that all console drivers that exist > in the kernel happen to be here? Have you grepped the source tree for > checking this? > Grepping for console_initcall, the only other places I see outside of drivers/tty/ is arch/mips/fw/arc/arc_con.c arch/mips/sibyte/common/cfe_console.c arch/powerpc/kernel/legacy_serial.c arch/powerpc/kernel/udbg.c arch/powerpc/platforms/powermac/setup.c arch/um/drivers/stderr_console.c arch/xtensa/platforms/iss/console.c drivers/s390/char/con3215.c drivers/s390/char/con3270.c drivers/s390/char/sclp_con.c drivers/s390/char/sclp_vt220.c > ... > > > +# If ttynull is enabled, but not as a boot console, it is linked and used later > > +# after the real ones. > > +ifneq ($(CONFIG_NULL_TTY_DEFAULT_CONSOLE),y) > > Also can be written as > ifeq ($(...),) > but it might be less explicit. Up to you. > > > obj-$(CONFIG_NULL_TTY) += ttynull.o > > +endif > >
On Thu 2025-03-06 09:10:29, Andy Shevchenko wrote: > On Thu, Mar 6, 2025 at 6:22 AM Adam Simonelli <adamsimonelli@gmail.com> wrote: > > > > On Wednesday, March 5, 2025 1:52:00 PM EST Andy Shevchenko wrote: > > > On Wed, Mar 5, 2025 at 4:06 AM Adam Simonelli <adamsimonelli@gmail.com> wrote: > > > > On Tuesday, March 4, 2025 1:51:52 AM EST Andy Shevchenko wrote: > > > > > On Tue, Mar 4, 2025 at 5:55 AM <adamsimonelli@gmail.com> wrote: > > ... > > > > > > > obj-y += vt/ > > > > > > > > > > + blank line. > > > > > > > > > > > +# If ttynull is configured to be a console by default, ensure that it is linked > > > > > > +# earlier before a real one is selected. > > > > > > +obj-$(CONFIG_NULL_TTY_DEFAULT_CONSOLE) \ > > > > > > + += ttynull.o > > > > > > > > > > Here is the question: are you sure that all console drivers that exist > > > > > in the kernel happen to be here? Have you grepped the source tree for > > > > > checking this? > > > > > > > > > Grepping for console_initcall, the only other places I see outside of > > > > drivers/tty/ is > > > > > > > > arch/mips/fw/arc/arc_con.c > > > > arch/mips/sibyte/common/cfe_console.c > > > > arch/powerpc/kernel/legacy_serial.c > > > > arch/powerpc/kernel/udbg.c > > > > arch/powerpc/platforms/powermac/setup.c > > > > arch/um/drivers/stderr_console.c > > > > arch/xtensa/platforms/iss/console.c > > > > drivers/s390/char/con3215.c > > > > drivers/s390/char/con3270.c > > > > drivers/s390/char/sclp_con.c > > > > drivers/s390/char/sclp_vt220.c > > > > > > Which means you need to test your stuff on those cases, to see how the > > > linker order is done there. It might be that your change wouldn't work > > > there as expected (quick workaround is to mark the new option as > > > depends on !S390 && !PPC and so on. > > > It will be difficult to test other arches, I mean I guess it is possible with > > QEMU, and cross-building, though I did do an experimental test on x86: > > > > Making it temporarily adding an architecture specific console like > > powerpc/some mips/s390/arches with Xen enabled. > > Thanks. Make sure the summary of this gets into the commit message. > Also consider updating the relevant documentation under > Documentation/, if any. Honestly, I am not sure what is the preferred behavior here. I am not familiar with the arch-specific consoles. But it made me think and investigate the various console drivers. It is a kind of a mess and I am not sure that I understand it correctly. Anyway, I suggested to use "add_preferred_console()" in console_initcall() in the v5 review, see https://lore.kernel.org/r/Z73teICMWNx7BiHT@pathway.suse.cz And I expected that it would be enough and the hack with linking order won't be needed anymore. Now, I see that I was wrong. The problem is that many console drivers call register_console() in console_initcall(). Such consoles would be registered by default when their register_console() is called before the ttynull.c calls add_preferred_console(). By other words, the hack with the linking order is still needed to call add_preferred_console() in time. Resume: From my POV, the solution with add_preferred_console() in console_initcall() still relies on the linking order. So, v6 is not better than v5. IMHO, v6 is actually be worse, see below. > > ------------------------------------------------------------------------------- > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > > index 05c5aa951da7..bcd248c44fc8 100644 > > --- a/arch/x86/kernel/setup.c > > +++ b/arch/x86/kernel/setup.c > > @@ -1159,6 +1159,8 @@ void __init setup_arch(char **cmdline_p) > > > > e820__setup_pci_gap(); > > > > + add_preferred_console("ttyS", 0, NULL); > > + > > #ifdef CONFIG_VT > > #if defined(CONFIG_VGA_CONSOLE) > > if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY)) > > ------------------------------------------------------------------------------- > > > > to see what /proc/consoles will look like, to pretend that x86 is an arch that > > sets a console somewhere, and I get: > > > > ttynull0 --- (EC ) 242:0 > > ttyS0 -W- (E p a) 4:64 > > > > and I got console messages to ttyS0 with no issue. > > > > which in my mind is acceptable I would think. ttynull is first in the list, > > which is desired effect of CONFIG_NULL_TTY_DEFAULT_CONSOLE, it doesn't have to > > be _exclusive_ AFAIK, especially if there are long-time default consoles that. > > users or the hardware expects. > > > > > > The only arch that seems to _unconditionally_ add a console without some other > > circumstance, like boot loader env var, and command line option, or firmware > > flag, or suboption (like CONFIG_SERIAL_PMACZILOG_CONSOLE) is Jazz. > > > > Like platforms/powernv adds it if CONFIG_HVC_OPAL is disabled, or the firmware > > is missing "FW_FEATURE_OPAL". I would assume that a user of this situation > > turning on CONFIG_NULL_TTY_DEFAULT_CONSOLE in addition, will just get ttynull > > and hvc in /proc/consoles instead of just hvc. Could that cause something to > > break? > > > Correct me if I am wrong, I could very very very well be wrong. > > I leave this to Petr to comment as I'm not that expert in the area. It depends on the expectations. I see two possibilities: 1. view: CONFIG_NULL_TTY_DEFAULT_CONSOLE is disabled by default. We could assume that people would enable it only when they really want to use ttynull for /dev/console. And they do not mind when some other platform-specific console is enabled as well. Note that we do not need the hack with the linking order for this. A solution is to call add_preferred_console() directly in console_init(). It has already been used in an earlier version, see https://lore.kernel.org/all/10194425.EvYhyI6sBW@nerdopolis2/ To make it clear. I consider this as a cleaner and more reliable solution than using the linking order hack. 2. view: The new config option wants to prefer "ttynull" when CONFIG_CONSOLE_VT is disabled. So, it is an alternative for "ttyX" from "vt". A conservative approach would be to enable it by default in the same situations where "ttyX" from "vt" is enabled by default. To make "ttynull" behave the same as "vt" would require using: + register_console() in console_initcall(). + the same console_initcall() ordering => move ttynull.o linking order => going back to v5 approach. I would normally prefer the conservative approach. But I hate the dependency on the linking order. It is so non-intuitive and fragile. So, I personally prefer to call add_preffed_console() directly in console_init() in the end. I guess that CONFIG_NULL_TTY_DEFAULT_CONSOLE won't be used on architectures with a better native console. So, it will be good enough in practice. Best Regards, Petr PS: I am sorry that you have already sent two more versions in the meantime. But I was not able to answer this quickly. I needed to find time and understand the various aspects.
diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 07aca5184a55..a4caf32a2014 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -11,6 +11,11 @@ obj-$(CONFIG_N_HDLC) += n_hdlc.o obj-$(CONFIG_N_GSM) += n_gsm.o obj-y += vt/ +# If ttynull is configured to be a console by default, ensure that it is linked +# earlier before a real one is selected. +obj-$(CONFIG_NULL_TTY_DEFAULT_CONSOLE) \ + += ttynull.o + obj-$(CONFIG_HVC_DRIVER) += hvc/ obj-y += serial/ obj-$(CONFIG_SERIAL_DEV_BUS) += serdev/ @@ -20,7 +25,13 @@ obj-$(CONFIG_AMIGA_BUILTIN_SERIAL) += amiserial.o obj-$(CONFIG_MOXA_INTELLIO) += moxa.o obj-$(CONFIG_MOXA_SMARTIO) += mxser.o obj-$(CONFIG_NOZOMI) += nozomi.o + +# If ttynull is enabled, but not as a boot console, it is linked and used later +# after the real ones. +ifneq ($(CONFIG_NULL_TTY_DEFAULT_CONSOLE),y) obj-$(CONFIG_NULL_TTY) += ttynull.o +endif + obj-$(CONFIG_SYNCLINK_GT) += synclink_gt.o obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o