| Message ID | 20230524200310.131510-1-rs@ti.com |
|---|---|
| State | Rejected |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | [meta-ti,master/kirkstone] conf: machine: k3: remove extra ttyS* definitions | expand |
On 5/24/23 3:03 PM, rs@ti.com wrote: > From: Randolph Sapp <rs@ti.com> > > Adding serial consoles to this variable makes systemd attempt to start a > Getty instance on them. Getty has no retry limit and no retry timout. > I'm sure you can guess where this is going. > > This fixes the endless start / stop Getty on ttyS0/ttyS3 messages that > will flood the log until it's full. > > In the case of Jailhouse demos, the getty systemd service should be > invoked upon demo start and stopped upon end of the demo. It should not > be listed as a system serial interface since it will not be present > during normal device operation. > > Before anyone asks, SERIAL_CONSOLES_CHECK only works on the sysVinit > script so it's not worth discussing as a solution here. > Sounds like the better solution then is to fix systemd to timeout on SERIAL_CONSOLES_CHECK. We are not the only board with multiple possible terminals in OE right now. Making systemd match sysv here is something worth doing. Not tested but a bbappends that changes the service reset might do: --- a/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service +++ b/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service @@ -32,7 +32,7 @@ Before=rescue.service Environment="TERM=@TERM@" ExecStart=-/sbin/agetty -8 -L %I @BAUDRATE@ $TERM Type=idle -Restart=always +Restart=on-success UtmpIdentifier=%I TTYPath=/dev/%I TTYReset=yes Andrew > Signed-off-by: Randolph Sapp <rs@ti.com> > --- > meta-ti-bsp/conf/machine/include/am62xx.inc | 3 --- > meta-ti-bsp/conf/machine/include/k3.inc | 2 +- > 2 files changed, 1 insertion(+), 4 deletions(-) > > diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc b/meta-ti-bsp/conf/machine/include/am62xx.inc > index c98da5f5..1274d3b5 100644 > --- a/meta-ti-bsp/conf/machine/include/am62xx.inc > +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc > @@ -25,6 +25,3 @@ TFA_BOARD = "lite" > TFA_K3_SYSTEM_SUSPEND = "1" > > OPTEEMACHINE = "k3-am62x" > - > -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use ttyS3, so try both > -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" > diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc > index c6b55239..97c6da94 100644 > --- a/meta-ti-bsp/conf/machine/include/k3.inc > +++ b/meta-ti-bsp/conf/machine/include/k3.inc > @@ -55,5 +55,5 @@ WKS_FILE ?= "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. > do_image_wic[depends] += "virtual/bootloader:do_deploy" > do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" > > -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" > +SERIAL_CONSOLES = "115200;ttyS2" > SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}"
On 5/30/23 13:03, Andrew Davis wrote: > On 5/24/23 3:03 PM, rs@ti.com wrote: >> From: Randolph Sapp <rs@ti.com> >> >> Adding serial consoles to this variable makes systemd attempt to start a >> Getty instance on them. Getty has no retry limit and no retry timout. >> I'm sure you can guess where this is going. >> >> This fixes the endless start / stop Getty on ttyS0/ttyS3 messages that >> will flood the log until it's full. >> >> In the case of Jailhouse demos, the getty systemd service should be >> invoked upon demo start and stopped upon end of the demo. It should not >> be listed as a system serial interface since it will not be present >> during normal device operation. >> >> Before anyone asks, SERIAL_CONSOLES_CHECK only works on the sysVinit >> script so it's not worth discussing as a solution here. >> > > Sounds like the better solution then is to fix systemd to timeout > on SERIAL_CONSOLES_CHECK. We are not the only board with multiple > possible terminals in OE right now. Making systemd match sysv here > is something worth doing. > No, if a user logs out the serial-getty instance is restarted. If the console isn't present when the service first starts then the service should be restarted until it is. It's functioning as intended, do not edit that service. > Not tested but a bbappends that changes the service reset might do: > > --- a/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service > +++ b/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service > @@ -32,7 +32,7 @@ Before=rescue.service > Environment="TERM=@TERM@" > ExecStart=-/sbin/agetty -8 -L %I @BAUDRATE@ $TERM > Type=idle > -Restart=always > +Restart=on-success > UtmpIdentifier=%I > TTYPath=/dev/%I > TTYReset=yes > > Andrew > >> Signed-off-by: Randolph Sapp <rs@ti.com> >> --- >> meta-ti-bsp/conf/machine/include/am62xx.inc | 3 --- >> meta-ti-bsp/conf/machine/include/k3.inc | 2 +- >> 2 files changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc >> b/meta-ti-bsp/conf/machine/include/am62xx.inc >> index c98da5f5..1274d3b5 100644 >> --- a/meta-ti-bsp/conf/machine/include/am62xx.inc >> +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc >> @@ -25,6 +25,3 @@ TFA_BOARD = "lite" >> TFA_K3_SYSTEM_SUSPEND = "1" >> OPTEEMACHINE = "k3-am62x" >> - >> -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use >> ttyS3, so try both >> -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" >> diff --git a/meta-ti-bsp/conf/machine/include/k3.inc >> b/meta-ti-bsp/conf/machine/include/k3.inc >> index c6b55239..97c6da94 100644 >> --- a/meta-ti-bsp/conf/machine/include/k3.inc >> +++ b/meta-ti-bsp/conf/machine/include/k3.inc >> @@ -55,5 +55,5 @@ WKS_FILE ?= >> "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. >> do_image_wic[depends] += "virtual/bootloader:do_deploy" >> do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" >> -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" >> +SERIAL_CONSOLES = "115200;ttyS2" >> SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}"
On 5/30/23 1:11 PM, Randolph Sapp wrote: > On 5/30/23 13:03, Andrew Davis wrote: >> On 5/24/23 3:03 PM, rs@ti.com wrote: >>> From: Randolph Sapp <rs@ti.com> >>> >>> Adding serial consoles to this variable makes systemd attempt to start a >>> Getty instance on them. Getty has no retry limit and no retry timout. >>> I'm sure you can guess where this is going. >>> >>> This fixes the endless start / stop Getty on ttyS0/ttyS3 messages that >>> will flood the log until it's full. >>> >>> In the case of Jailhouse demos, the getty systemd service should be >>> invoked upon demo start and stopped upon end of the demo. It should not >>> be listed as a system serial interface since it will not be present >>> during normal device operation. >>> >>> Before anyone asks, SERIAL_CONSOLES_CHECK only works on the sysVinit >>> script so it's not worth discussing as a solution here. >>> >> >> Sounds like the better solution then is to fix systemd to timeout >> on SERIAL_CONSOLES_CHECK. We are not the only board with multiple >> possible terminals in OE right now. Making systemd match sysv here >> is something worth doing. >> > > No, if a user logs out the serial-getty instance is restarted. If the console isn't present when the service first starts then the service should be restarted until it is. It's functioning as intended, do not edit that service. > If a user logs out it will return 0 and will be restarted "on-success". If the console is not present the service *should* fail as it was told to start on a non-existent console. If you want to handle the case of a late attaching tty device, you can have the service try again on attach, that functionality is part of systemd/udev already. The current behavior in this service does not match sysv nor what SERIAL_CONSOLES_CHECK is meant for, and so should be fixed. Andrew >> Not tested but a bbappends that changes the service reset might do: >> >> --- a/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >> +++ b/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >> @@ -32,7 +32,7 @@ Before=rescue.service >> Environment="TERM=@TERM@" >> ExecStart=-/sbin/agetty -8 -L %I @BAUDRATE@ $TERM >> Type=idle >> -Restart=always >> +Restart=on-success >> UtmpIdentifier=%I >> TTYPath=/dev/%I >> TTYReset=yes >> >> Andrew >> >>> Signed-off-by: Randolph Sapp <rs@ti.com> >>> --- >>> meta-ti-bsp/conf/machine/include/am62xx.inc | 3 --- >>> meta-ti-bsp/conf/machine/include/k3.inc | 2 +- >>> 2 files changed, 1 insertion(+), 4 deletions(-) >>> >>> diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc b/meta-ti-bsp/conf/machine/include/am62xx.inc >>> index c98da5f5..1274d3b5 100644 >>> --- a/meta-ti-bsp/conf/machine/include/am62xx.inc >>> +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc >>> @@ -25,6 +25,3 @@ TFA_BOARD = "lite" >>> TFA_K3_SYSTEM_SUSPEND = "1" >>> OPTEEMACHINE = "k3-am62x" >>> - >>> -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use ttyS3, so try both >>> -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" >>> diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc >>> index c6b55239..97c6da94 100644 >>> --- a/meta-ti-bsp/conf/machine/include/k3.inc >>> +++ b/meta-ti-bsp/conf/machine/include/k3.inc >>> @@ -55,5 +55,5 @@ WKS_FILE ?= "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. >>> do_image_wic[depends] += "virtual/bootloader:do_deploy" >>> do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" >>> -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" >>> +SERIAL_CONSOLES = "115200;ttyS2" >>> SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}" >
On 5/30/23 13:18, Andrew Davis wrote: > On 5/30/23 1:11 PM, Randolph Sapp wrote: >> On 5/30/23 13:03, Andrew Davis wrote: >>> On 5/24/23 3:03 PM, rs@ti.com wrote: >>>> From: Randolph Sapp <rs@ti.com> >>>> >>>> Adding serial consoles to this variable makes systemd attempt to >>>> start a >>>> Getty instance on them. Getty has no retry limit and no retry timout. >>>> I'm sure you can guess where this is going. >>>> >>>> This fixes the endless start / stop Getty on ttyS0/ttyS3 messages that >>>> will flood the log until it's full. >>>> >>>> In the case of Jailhouse demos, the getty systemd service should be >>>> invoked upon demo start and stopped upon end of the demo. It should not >>>> be listed as a system serial interface since it will not be present >>>> during normal device operation. >>>> >>>> Before anyone asks, SERIAL_CONSOLES_CHECK only works on the sysVinit >>>> script so it's not worth discussing as a solution here. >>>> >>> >>> Sounds like the better solution then is to fix systemd to timeout >>> on SERIAL_CONSOLES_CHECK. We are not the only board with multiple >>> possible terminals in OE right now. Making systemd match sysv here >>> is something worth doing. >>> >> >> No, if a user logs out the serial-getty instance is restarted. If the >> console isn't present when the service first starts then the service >> should be restarted until it is. It's functioning as intended, do not >> edit that service. >> > > If a user logs out it will return 0 and will be restarted "on-success". > If the console is not present the service *should* fail as it was told > to start on a non-existent console. > > If you want to handle the case of a late attaching tty device, you > can have the service try again on attach, that functionality is part > of systemd/udev already. > > The current behavior in this service does not match sysv nor what > SERIAL_CONSOLES_CHECK is meant for, and so should be fixed. > > Andrew In that case the better thing would be for yocto to not explicitly set any serial interfaces and instead rely on the built in systemd-getty-generator. Go argue it with upstream. I'm sure they had to override things for a reason... Whether or not that's still a valid reason it yet to be seen. > >>> Not tested but a bbappends that changes the service reset might do: >>> >>> --- >>> a/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >>> +++ >>> b/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >>> @@ -32,7 +32,7 @@ Before=rescue.service >>> Environment="TERM=@TERM@" >>> ExecStart=-/sbin/agetty -8 -L %I @BAUDRATE@ $TERM >>> Type=idle >>> -Restart=always >>> +Restart=on-success >>> UtmpIdentifier=%I >>> TTYPath=/dev/%I >>> TTYReset=yes >>> >>> Andrew >>> >>>> Signed-off-by: Randolph Sapp <rs@ti.com> >>>> --- >>>> meta-ti-bsp/conf/machine/include/am62xx.inc | 3 --- >>>> meta-ti-bsp/conf/machine/include/k3.inc | 2 +- >>>> 2 files changed, 1 insertion(+), 4 deletions(-) >>>> >>>> diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc >>>> b/meta-ti-bsp/conf/machine/include/am62xx.inc >>>> index c98da5f5..1274d3b5 100644 >>>> --- a/meta-ti-bsp/conf/machine/include/am62xx.inc >>>> +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc >>>> @@ -25,6 +25,3 @@ TFA_BOARD = "lite" >>>> TFA_K3_SYSTEM_SUSPEND = "1" >>>> OPTEEMACHINE = "k3-am62x" >>>> - >>>> -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use >>>> ttyS3, so try both >>>> -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" >>>> diff --git a/meta-ti-bsp/conf/machine/include/k3.inc >>>> b/meta-ti-bsp/conf/machine/include/k3.inc >>>> index c6b55239..97c6da94 100644 >>>> --- a/meta-ti-bsp/conf/machine/include/k3.inc >>>> +++ b/meta-ti-bsp/conf/machine/include/k3.inc >>>> @@ -55,5 +55,5 @@ WKS_FILE ?= >>>> "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. >>>> do_image_wic[depends] += "virtual/bootloader:do_deploy" >>>> do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" >>>> -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" >>>> +SERIAL_CONSOLES = "115200;ttyS2" >>>> SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}" >>
On 5/30/23 1:57 PM, Randolph Sapp wrote: > On 5/30/23 13:18, Andrew Davis wrote: >> On 5/30/23 1:11 PM, Randolph Sapp wrote: >>> On 5/30/23 13:03, Andrew Davis wrote: >>>> On 5/24/23 3:03 PM, rs@ti.com wrote: >>>>> From: Randolph Sapp <rs@ti.com> >>>>> >>>>> Adding serial consoles to this variable makes systemd attempt to start a >>>>> Getty instance on them. Getty has no retry limit and no retry timout. >>>>> I'm sure you can guess where this is going. >>>>> >>>>> This fixes the endless start / stop Getty on ttyS0/ttyS3 messages that >>>>> will flood the log until it's full. >>>>> >>>>> In the case of Jailhouse demos, the getty systemd service should be >>>>> invoked upon demo start and stopped upon end of the demo. It should not >>>>> be listed as a system serial interface since it will not be present >>>>> during normal device operation. >>>>> >>>>> Before anyone asks, SERIAL_CONSOLES_CHECK only works on the sysVinit >>>>> script so it's not worth discussing as a solution here. >>>>> >>>> >>>> Sounds like the better solution then is to fix systemd to timeout >>>> on SERIAL_CONSOLES_CHECK. We are not the only board with multiple >>>> possible terminals in OE right now. Making systemd match sysv here >>>> is something worth doing. >>>> >>> >>> No, if a user logs out the serial-getty instance is restarted. If the console isn't present when the service first starts then the service should be restarted until it is. It's functioning as intended, do not edit that service. >>> >> >> If a user logs out it will return 0 and will be restarted "on-success". >> If the console is not present the service *should* fail as it was told >> to start on a non-existent console. >> >> If you want to handle the case of a late attaching tty device, you >> can have the service try again on attach, that functionality is part >> of systemd/udev already. >> >> The current behavior in this service does not match sysv nor what >> SERIAL_CONSOLES_CHECK is meant for, and so should be fixed. >> >> Andrew > > In that case the better thing would be for yocto to not explicitly set any serial interfaces and instead rely on the built in systemd-getty-generator. Go argue it with upstream. I'm sure they had to override things for a reason... Whether or not that's still a valid reason it yet to be seen. > Looks like someone already did, support for systemd-getty-generator is just a PACKAGECONFIG option away now[0]. Might be time to switch Arago over. [0] https://github.com/openembedded/openembedded-core/commit/2a8d0df47c9d28f8ca7285861dee7a178273eae4 Andrew >> >>>> Not tested but a bbappends that changes the service reset might do: >>>> >>>> --- a/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >>>> +++ b/meta/recipes-core/systemd/systemd-serialgetty/serial-getty@.service >>>> @@ -32,7 +32,7 @@ Before=rescue.service >>>> Environment="TERM=@TERM@" >>>> ExecStart=-/sbin/agetty -8 -L %I @BAUDRATE@ $TERM >>>> Type=idle >>>> -Restart=always >>>> +Restart=on-success >>>> UtmpIdentifier=%I >>>> TTYPath=/dev/%I >>>> TTYReset=yes >>>> >>>> Andrew >>>> >>>>> Signed-off-by: Randolph Sapp <rs@ti.com> >>>>> --- >>>>> meta-ti-bsp/conf/machine/include/am62xx.inc | 3 --- >>>>> meta-ti-bsp/conf/machine/include/k3.inc | 2 +- >>>>> 2 files changed, 1 insertion(+), 4 deletions(-) >>>>> >>>>> diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc b/meta-ti-bsp/conf/machine/include/am62xx.inc >>>>> index c98da5f5..1274d3b5 100644 >>>>> --- a/meta-ti-bsp/conf/machine/include/am62xx.inc >>>>> +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc >>>>> @@ -25,6 +25,3 @@ TFA_BOARD = "lite" >>>>> TFA_K3_SYSTEM_SUSPEND = "1" >>>>> OPTEEMACHINE = "k3-am62x" >>>>> - >>>>> -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use ttyS3, so try both >>>>> -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" >>>>> diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc >>>>> index c6b55239..97c6da94 100644 >>>>> --- a/meta-ti-bsp/conf/machine/include/k3.inc >>>>> +++ b/meta-ti-bsp/conf/machine/include/k3.inc >>>>> @@ -55,5 +55,5 @@ WKS_FILE ?= "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. >>>>> do_image_wic[depends] += "virtual/bootloader:do_deploy" >>>>> do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" >>>>> -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" >>>>> +SERIAL_CONSOLES = "115200;ttyS2" >>>>> SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}" >>> >
diff --git a/meta-ti-bsp/conf/machine/include/am62xx.inc b/meta-ti-bsp/conf/machine/include/am62xx.inc index c98da5f5..1274d3b5 100644 --- a/meta-ti-bsp/conf/machine/include/am62xx.inc +++ b/meta-ti-bsp/conf/machine/include/am62xx.inc @@ -25,6 +25,3 @@ TFA_BOARD = "lite" TFA_K3_SYSTEM_SUSPEND = "1" OPTEEMACHINE = "k3-am62x" - -# Normally AM62 boards use ttyS2, but our Jailhouse inmate may use ttyS3, so try both -SERIAL_CONSOLES = "115200;ttyS2 115200;ttyS3" diff --git a/meta-ti-bsp/conf/machine/include/k3.inc b/meta-ti-bsp/conf/machine/include/k3.inc index c6b55239..97c6da94 100644 --- a/meta-ti-bsp/conf/machine/include/k3.inc +++ b/meta-ti-bsp/conf/machine/include/k3.inc @@ -55,5 +55,5 @@ WKS_FILE ?= "${@bb.utils.contains("MACHINE_FEATURES", "efi", "sdimage-2part-efi. do_image_wic[depends] += "virtual/bootloader:do_deploy" do_image_complete[mcdepends] += "mc::k3r5:ti-sci-fw:do_deploy" -SERIAL_CONSOLES = "115200;ttyS0 115200;ttyS2" +SERIAL_CONSOLES = "115200;ttyS2" SERIAL_CONSOLES_CHECK = "${SERIAL_CONSOLES}"