| Message ID | 20230915155835.51140-3-arbab@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/petitboot: misc enhancement | expand |
On 15/09/2023 17:58, Reza Arbab wrote: > Install a udev rule to run the petitboot UI on commonly-known consoles > instead of getty. To prevent the kernel's console output from trampling > the UI, also add a sysctl file to reduce the kernel log levels to > KERN_ALERT. > > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> > --- > v2: > * Add ttyGF* to list of commonly-known consoles. > * Don't select BR2_TARGET_GENERIC_GETTY by default. > > package/petitboot/petitboot-console-ui.rules | 5 +++++ > package/petitboot/petitboot.mk | 4 ++++ > package/petitboot/sysctl.conf | 1 + > system/Config.in | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > create mode 100644 package/petitboot/petitboot-console-ui.rules > create mode 100644 package/petitboot/sysctl.conf > > diff --git a/package/petitboot/petitboot-console-ui.rules b/package/petitboot/petitboot-console-ui.rules > new file mode 100644 > index 000000000000..e5c188387bc5 > --- /dev/null > +++ b/package/petitboot/petitboot-console-ui.rules > @@ -0,0 +1,5 @@ > +# spawn a petitboot UI on common user-visible interface devices > +SUBSYSTEM=="tty", KERNEL=="hvc*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="tty0", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyGF*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyS*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" In general, it's quite likely that there are some serial ports where some device is connected rather that a console (e.g. GPS, Bluetooth, cell modem, ...). So starting pb-console on "anything that looks ike a serial port" is a bad idea. In Buildroot, we by default assume that the console is on /dev/console - which is why that is the default for BR2_TARGET_GENERIC_GETTY_PORT. If it's needed to run pb-console on a different port or ports than the default /dev/console, then that's a system-specific configuration. I see no way we can have this generic assumption of "all ttys need pb-console" in Buildroot. It _does_ make sense, however, to put pb-console on /dev/console. However, wouldn't it make more sense to do that in inittab instead of with a udev rule - like the getty line? BTW, I don't understand why you even have a KERNEL filter. Anything that is a tty could be the console, no? > diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk > index 8d220f88f45a..556d41230237 100644 > --- a/package/petitboot/petitboot.mk > +++ b/package/petitboot/petitboot.mk > @@ -55,6 +55,10 @@ define PETITBOOT_POST_INSTALL > $(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb > $(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \ > $(TARGET_DIR)/etc/init.d/S15pb-discover > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/petitboot-console-ui.rules \ > + $(TARGET_DIR)/etc/udev/rules.d/petitboot-console-ui.rules > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/sysctl.conf \ > + $(TARGET_DIR)/etc/sysctl.d/petitboot.conf > mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > ln -sf /usr/sbin/pb-udhcpc \ > $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > diff --git a/package/petitboot/sysctl.conf b/package/petitboot/sysctl.conf > new file mode 100644 > index 000000000000..02ab8e3275b5 > --- /dev/null > +++ b/package/petitboot/sysctl.conf > @@ -0,0 +1 @@ > +kernel.printk = 1 1 1 1 I'm not sure how useful this is... There is still output from the init scripts coming on the console, so why block the kernel output but not those? In addition, by the time we get to S10udev (or definitely by the time we get to the inittab stuff), most kernel output has already passed. > diff --git a/system/Config.in b/system/Config.in > index 24798dc06803..9587dd9ce4db 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH > > menuconfig BR2_TARGET_GENERIC_GETTY > bool "Run a getty (login prompt) after boot" > - default y > + default y if !BR2_PACKAGE_PETITBOOT Ugh, this is super ugly to have to do something petitboot-specific in the system menu. But I don't readily know a better way to do this. Regards, Arnout > > if BR2_TARGET_GENERIC_GETTY > config BR2_TARGET_GENERIC_GETTY_PORT
On Fri, 15 Sept 2023 at 15:58, Reza Arbab <arbab@linux.ibm.com> wrote: > > Install a udev rule to run the petitboot UI on commonly-known consoles > instead of getty. To prevent the kernel's console output from trampling > the UI, also add a sysctl file to reduce the kernel log levels to > KERN_ALERT. > > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > v2: > * Add ttyGF* to list of commonly-known consoles. > * Don't select BR2_TARGET_GENERIC_GETTY by default. > > package/petitboot/petitboot-console-ui.rules | 5 +++++ > package/petitboot/petitboot.mk | 4 ++++ > package/petitboot/sysctl.conf | 1 + > system/Config.in | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > create mode 100644 package/petitboot/petitboot-console-ui.rules > create mode 100644 package/petitboot/sysctl.conf > > diff --git a/package/petitboot/petitboot-console-ui.rules b/package/petitboot/petitboot-console-ui.rules > new file mode 100644 > index 000000000000..e5c188387bc5 > --- /dev/null > +++ b/package/petitboot/petitboot-console-ui.rules > @@ -0,0 +1,5 @@ > +# spawn a petitboot UI on common user-visible interface devices > +SUBSYSTEM=="tty", KERNEL=="hvc*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="tty0", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyGF*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyS*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk > index 8d220f88f45a..556d41230237 100644 > --- a/package/petitboot/petitboot.mk > +++ b/package/petitboot/petitboot.mk > @@ -55,6 +55,10 @@ define PETITBOOT_POST_INSTALL > $(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb > $(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \ > $(TARGET_DIR)/etc/init.d/S15pb-discover > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/petitboot-console-ui.rules \ > + $(TARGET_DIR)/etc/udev/rules.d/petitboot-console-ui.rules > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/sysctl.conf \ > + $(TARGET_DIR)/etc/sysctl.d/petitboot.conf > mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > ln -sf /usr/sbin/pb-udhcpc \ > $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > diff --git a/package/petitboot/sysctl.conf b/package/petitboot/sysctl.conf > new file mode 100644 > index 000000000000..02ab8e3275b5 > --- /dev/null > +++ b/package/petitboot/sysctl.conf > @@ -0,0 +1 @@ > +kernel.printk = 1 1 1 1 > diff --git a/system/Config.in b/system/Config.in > index 24798dc06803..9587dd9ce4db 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH > > menuconfig BR2_TARGET_GENERIC_GETTY > bool "Run a getty (login prompt) after boot" > - default y > + default y if !BR2_PACKAGE_PETITBOOT > > if BR2_TARGET_GENERIC_GETTY > config BR2_TARGET_GENERIC_GETTY_PORT > -- > 2.39.3 >
Le 15/09/2023 à 17:58, Reza Arbab a écrit : > Install a udev rule to run the petitboot UI on commonly-known consoles > instead of getty. To prevent the kernel's console output from trampling > the UI, also add a sysctl file to reduce the kernel log levels to > KERN_ALERT. > > Signed-off-by: Reza Arbab <arbab@linux.ibm.com> > --- > v2: > * Add ttyGF* to list of commonly-known consoles. > * Don't select BR2_TARGET_GENERIC_GETTY by default. > > package/petitboot/petitboot-console-ui.rules | 5 +++++ > package/petitboot/petitboot.mk | 4 ++++ > package/petitboot/sysctl.conf | 1 + > system/Config.in | 2 +- > 4 files changed, 11 insertions(+), 1 deletion(-) > create mode 100644 package/petitboot/petitboot-console-ui.rules > create mode 100644 package/petitboot/sysctl.conf > > diff --git a/package/petitboot/petitboot-console-ui.rules b/package/petitboot/petitboot-console-ui.rules > new file mode 100644 > index 000000000000..e5c188387bc5 > --- /dev/null > +++ b/package/petitboot/petitboot-console-ui.rules > @@ -0,0 +1,5 @@ > +# spawn a petitboot UI on common user-visible interface devices > +SUBSYSTEM=="tty", KERNEL=="hvc*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="tty0", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyGF*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > +SUBSYSTEM=="tty", KERNEL=="ttyS*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk > index 8d220f88f45a..556d41230237 100644 > --- a/package/petitboot/petitboot.mk > +++ b/package/petitboot/petitboot.mk > @@ -55,6 +55,10 @@ define PETITBOOT_POST_INSTALL > $(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb > $(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \ > $(TARGET_DIR)/etc/init.d/S15pb-discover > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/petitboot-console-ui.rules \ > + $(TARGET_DIR)/etc/udev/rules.d/petitboot-console-ui.rules > + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/sysctl.conf \ > + $(TARGET_DIR)/etc/sysctl.d/petitboot.conf > mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > ln -sf /usr/sbin/pb-udhcpc \ > $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ > diff --git a/package/petitboot/sysctl.conf b/package/petitboot/sysctl.conf > new file mode 100644 > index 000000000000..02ab8e3275b5 > --- /dev/null > +++ b/package/petitboot/sysctl.conf > @@ -0,0 +1 @@ > +kernel.printk = 1 1 1 1 > diff --git a/system/Config.in b/system/Config.in > index 24798dc06803..9587dd9ce4db 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH > > menuconfig BR2_TARGET_GENERIC_GETTY > bool "Run a getty (login prompt) after boot" > - default y > + default y if !BR2_PACKAGE_PETITBOOT > > if BR2_TARGET_GENERIC_GETTY > config BR2_TARGET_GENERIC_GETTY_PORT Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On Sun, Sep 17, 2023 at 08:44:00PM +0200, Arnout Vandecappelle wrote: >>--- /dev/null >>+++ b/package/petitboot/petitboot-console-ui.rules >>@@ -0,0 +1,5 @@ >>+# spawn a petitboot UI on common user-visible interface devices >>+SUBSYSTEM=="tty", KERNEL=="hvc*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" >>+SUBSYSTEM=="tty", KERNEL=="tty0", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" >>+SUBSYSTEM=="tty", KERNEL=="ttyGF*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" >>+SUBSYSTEM=="tty", KERNEL=="ttyS*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" > > In general, it's quite likely that there are some serial ports where >some device is connected rather that a console (e.g. GPS, Bluetooth, >cell modem, ...). So starting pb-console on "anything that looks ike a >serial port" is a bad idea. > > In Buildroot, we by default assume that the console is on >/dev/console - which is why that is the default for >BR2_TARGET_GENERIC_GETTY_PORT. If it's needed to run pb-console on a >different port or ports than the default /dev/console, then that's a >system-specific configuration. > > I see no way we can have this generic assumption of "all ttys need >pb-console" in Buildroot. > > It _does_ make sense, however, to put pb-console on /dev/console. Fair point. The intention is just to replace the login prompt with a petitboot menu, so if buildroot only assumes /dev/console for getty by default, then this should too. As you say, anything more can be system-specific. >However, wouldn't it make more sense to do that in inittab instead of >with a udev rule - like the getty line? This part I'm not sure about. In the petitboot docs it does say "it is useful to have this called by udev", but not why that way specifically. http://open-power.github.io/petitboot/dev/linux-image.html > BTW, I don't understand why you even have a KERNEL filter. Anything >that is a tty could be the console, no? The filters are very broad, but I guess still an attempt to limit things to the enumerated "common user-visible interface devices" instead of being totally open-ended. >>--- /dev/null >>+++ b/package/petitboot/sysctl.conf >>@@ -0,0 +1 @@ >>+kernel.printk = 1 1 1 1 > > I'm not sure how useful this is... There is still output from the >init scripts coming on the console, so why block the kernel output but >not those? In addition, by the time we get to S10udev (or definitely >by the time we get to the inittab stuff), most kernel output has >already passed. Boot output has passed, but pb-discover is in the background populating the boot menu with bootable images via disk and network, and I think there can be enough kernel noise at runtime to motivate minimizing it. Thanks for the review. I'll reevaluate what changes are appropriate to carry in buildroot vs elsewhere.
diff --git a/package/petitboot/petitboot-console-ui.rules b/package/petitboot/petitboot-console-ui.rules new file mode 100644 index 000000000000..e5c188387bc5 --- /dev/null +++ b/package/petitboot/petitboot-console-ui.rules @@ -0,0 +1,5 @@ +# spawn a petitboot UI on common user-visible interface devices +SUBSYSTEM=="tty", KERNEL=="hvc*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" +SUBSYSTEM=="tty", KERNEL=="tty0", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" +SUBSYSTEM=="tty", KERNEL=="ttyGF*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" +SUBSYSTEM=="tty", KERNEL=="ttyS*", RUN+="/usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 $name linux" diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk index 8d220f88f45a..556d41230237 100644 --- a/package/petitboot/petitboot.mk +++ b/package/petitboot/petitboot.mk @@ -55,6 +55,10 @@ define PETITBOOT_POST_INSTALL $(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb $(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \ $(TARGET_DIR)/etc/init.d/S15pb-discover + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/petitboot-console-ui.rules \ + $(TARGET_DIR)/etc/udev/rules.d/petitboot-console-ui.rules + $(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/sysctl.conf \ + $(TARGET_DIR)/etc/sysctl.d/petitboot.conf mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ ln -sf /usr/sbin/pb-udhcpc \ $(TARGET_DIR)/usr/share/udhcpc/default.script.d/ diff --git a/package/petitboot/sysctl.conf b/package/petitboot/sysctl.conf new file mode 100644 index 000000000000..02ab8e3275b5 --- /dev/null +++ b/package/petitboot/sysctl.conf @@ -0,0 +1 @@ +kernel.printk = 1 1 1 1 diff --git a/system/Config.in b/system/Config.in index 24798dc06803..9587dd9ce4db 100644 --- a/system/Config.in +++ b/system/Config.in @@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH menuconfig BR2_TARGET_GENERIC_GETTY bool "Run a getty (login prompt) after boot" - default y + default y if !BR2_PACKAGE_PETITBOOT if BR2_TARGET_GENERIC_GETTY config BR2_TARGET_GENERIC_GETTY_PORT
Install a udev rule to run the petitboot UI on commonly-known consoles instead of getty. To prevent the kernel's console output from trampling the UI, also add a sysctl file to reduce the kernel log levels to KERN_ALERT. Signed-off-by: Reza Arbab <arbab@linux.ibm.com> --- v2: * Add ttyGF* to list of commonly-known consoles. * Don't select BR2_TARGET_GENERIC_GETTY by default. package/petitboot/petitboot-console-ui.rules | 5 +++++ package/petitboot/petitboot.mk | 4 ++++ package/petitboot/sysctl.conf | 1 + system/Config.in | 2 +- 4 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 package/petitboot/petitboot-console-ui.rules create mode 100644 package/petitboot/sysctl.conf