Re: [PATCH procd] utils: fix get_active_console when kernel returns multiple values

2022-01-31 Thread Mathew McBride
Hi Daniel,
On Mon, Jan 31, 2022, at 10:58 AM, Daniel Golle wrote:
> Hi Mathew,
> 
> On Tue, Jan 25, 2022 at 03:12:08AM +, Mathew McBride wrote:
> > /sys/class/tty/console/active may return multiple values on
> > kernels where framebuffer support is enabled but the system
> > is supposed to be using a serial console.
> > e.g
> > $ cat /sys/class/tty/console/active
> > tty0 ttyS0
> 
> On BPi-R2 (which got HDMI with console) I'm seeing this:
> root@OpenWrt:~# cat /sys/class/tty/console/active
> ttyS2 tty1
Doh! Thanks for letting me know. That ruins my assumptions.

So this issue appears to be confined to platforms where the dummy 
framebuffer/console is loaded in the absence of a graphical console.
[snip]
> 
> While I agree that we need to somehow unify and solve this, just
> changing the current behavior will break serial console access at least
> on the targets listed above.
Fair enough, I will see if I can come up with another solution.

> 
> So either you should suggest to change the affected target to ship
> /etc/inittab or supply console= kernel parameter, or we change all the
> other targets above (and maybe more where console= parameter originates
> from existing non-OpenWrt-built bootchain).

> Does our current behaviour (using first active console) violate any
> universal convention we should obey?

The motivation for this is to successfully detect the console(s) on systems 
using EFI to boot (so we don't need to create separate images just to specify a 
different console= and/or inittab). The original console detection patch came 
from this but broke certain boards when graphical support was enabled.

Systemd manages to set up consoles correctly on these systems (without 
console=) so it would be nice for OpenWrt/procd to do the same.

(Specifically, "standards compliant" ARM boards, see this pull that adds EFI to 
armvirt: https://github.com/openwrt/openwrt/pull/4996 )

I guess the best approach now may be to generate (or act as if) an inittab 
based on /sys/class/tty/console/active? When inittab is not present and no 
console= is specified, of course.

(If there was a way to detect if tty0 was a dummy from sysfs I would do that 
instead, but there does not appear to be)

Cheers,
Matt

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH procd] utils: fix get_active_console when kernel returns multiple values

2022-01-30 Thread Daniel Golle
Hi Mathew,

On Tue, Jan 25, 2022 at 03:12:08AM +, Mathew McBride wrote:
> /sys/class/tty/console/active may return multiple values on
> kernels where framebuffer support is enabled but the system
> is supposed to be using a serial console.
> e.g
> $ cat /sys/class/tty/console/active
> tty0 ttyS0

On BPi-R2 (which got HDMI with console) I'm seeing this:
root@OpenWrt:~# cat /sys/class/tty/console/active
ttyS2 tty1

And I suspect it will be the same at least for

linux/tegra/image/generic-bootscript:setenv bootargs "root=PARTUUID=${ptuuid} 
rw rootwait console=ttyS0,115200 console=tty0"
linux/bcm27xx/image/cmdline.txt:console=serial0,115200 console=tty1 
root=/dev/mmcblk0p2 rootfstype=squashfs,ext4 rootwait
linux/rockchip/image/mmc.bootscript:setenv bootargs "console=ttyS2,150 
console=tty1 earlycon=uart8250,mmio32,0xff1a root=PARTUUID=${uuid} rw 
rootwait"

as the order of console in that sysfs file apparently corresponds to
the order of console= arguments passed via kernel cmdline.

bcm27xx, mt7623 and tegra enable the frame-buffer console via inittab,
while rockchip doesn't ship /etc/inittab.

> 
> On such systems, tty0 is a dummy console:
> [0.008273] Console: colour dummy device 80x25
> [0.012742] printk: console [tty0] enabled
> 
> The serial console doesn't appear until much later:
> [0.092468] 21c0500.serial: ttyS0 at MMIO 0x21c0500
> [1.713893] printk: console [ttyS0] enabled
> 
> So far this only appears to happen on systems using
> device tree.
> 
> In these circumstances, use the last console device
> shown in the sysfs active file.

While I agree that we need to somehow unify and solve this, just
changing the current behavior will break serial console access at least
on the targets listed above.

So either you should suggest to change the affected target to ship
/etc/inittab or supply console= kernel parameter, or we change all the
other targets above (and maybe more where console= parameter originates
from existing non-OpenWrt-built bootchain).

Does our current behaviour (using first active console) violate any
universal convention we should obey?


Cheers


Daniel

> 
> Fixes: 2cfc26f ("inittab: detect active console from kernel if no console= 
> specified")
> Signed-off-by: Mathew McBride 
> ---
>  utils/utils.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/utils/utils.c b/utils/utils.c
> index f0c4a90..43b24c6 100644
> --- a/utils/utils.c
> +++ b/utils/utils.c
> @@ -138,6 +138,10 @@ blobmsg_list_equal(struct blobmsg_list *l1, struct 
> blobmsg_list *l2)
>  char *get_active_console(char *out, int len)
>  {
>   char line[CMDLINE_SIZE + 1];
> + char *sptr, *token;
> + char *console = NULL;
> +
> + memset(line, 0, sizeof(line));
>   int fd = open("/sys/class/tty/console/active", O_RDONLY);
>   ssize_t r;
>  
> @@ -152,15 +156,22 @@ char *get_active_console(char *out, int len)
>   if (r <= 0)
>   return NULL;
>  
> - /* The active file is terminated by a newline which we need to strip */
> - char *newline = strtok(line, "\n");
> -
> - if (newline != NULL) {
> - strncpy(out, newline, len);
> - return out;
> + /* There may be multiple 'active' consoles.
> +  * On kernels that support both graphical and
> +  * serial consoles, Linux may create a 'dummy'
> +  * framebuffer console on tty0 if no other console
> +  * device has been probed yet. Often a serial
> +  * driver (e.g ttyS0) might only be probed later
> +  * in the boot process.
> +  */
> + for (token = strtok_r(line, " \t\n", ); token;
> +  token = strtok_r(NULL, " \t\n", )) {
> + strncpy(out, token, len);
> + console = out;
>   }
> + out[len-1] = '\0';
>  
> - return NULL;
> + return console;
>  }
>  
>  char* get_cmdline_val(const char* name, char* out, int len)
> -- 
> 2.30.1
> 
> 
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH procd] utils: fix get_active_console when kernel returns multiple values

2022-01-24 Thread Mathew McBride
/sys/class/tty/console/active may return multiple values on
kernels where framebuffer support is enabled but the system
is supposed to be using a serial console.
e.g
$ cat /sys/class/tty/console/active
tty0 ttyS0

On such systems, tty0 is a dummy console:
[0.008273] Console: colour dummy device 80x25
[0.012742] printk: console [tty0] enabled

The serial console doesn't appear until much later:
[0.092468] 21c0500.serial: ttyS0 at MMIO 0x21c0500
[1.713893] printk: console [ttyS0] enabled

So far this only appears to happen on systems using
device tree.

In these circumstances, use the last console device
shown in the sysfs active file.

Fixes: 2cfc26f ("inittab: detect active console from kernel if no console= 
specified")
Signed-off-by: Mathew McBride 
---
 utils/utils.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/utils/utils.c b/utils/utils.c
index f0c4a90..43b24c6 100644
--- a/utils/utils.c
+++ b/utils/utils.c
@@ -138,6 +138,10 @@ blobmsg_list_equal(struct blobmsg_list *l1, struct 
blobmsg_list *l2)
 char *get_active_console(char *out, int len)
 {
char line[CMDLINE_SIZE + 1];
+   char *sptr, *token;
+   char *console = NULL;
+
+   memset(line, 0, sizeof(line));
int fd = open("/sys/class/tty/console/active", O_RDONLY);
ssize_t r;
 
@@ -152,15 +156,22 @@ char *get_active_console(char *out, int len)
if (r <= 0)
return NULL;
 
-   /* The active file is terminated by a newline which we need to strip */
-   char *newline = strtok(line, "\n");
-
-   if (newline != NULL) {
-   strncpy(out, newline, len);
-   return out;
+   /* There may be multiple 'active' consoles.
+* On kernels that support both graphical and
+* serial consoles, Linux may create a 'dummy'
+* framebuffer console on tty0 if no other console
+* device has been probed yet. Often a serial
+* driver (e.g ttyS0) might only be probed later
+* in the boot process.
+*/
+   for (token = strtok_r(line, " \t\n", ); token;
+token = strtok_r(NULL, " \t\n", )) {
+   strncpy(out, token, len);
+   console = out;
}
+   out[len-1] = '\0';
 
-   return NULL;
+   return console;
 }
 
 char* get_cmdline_val(const char* name, char* out, int len)
-- 
2.30.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel