| Message ID | 20240112150307.1.I7dc0993c1e758a1efedd651e7e1670deb1b430fb@changeid |
|---|---|
| State | Accepted |
| Commit | 486676116f4852d4198690c2c98af060cd96ab83 |
| Headers | show |
| Series | [1/2] soc: qcom: geni-se: Add M_TX_FIFO_NOT_EMPTY bit definition | expand |
As of commit d7402513c935 ("arm64: smp: IPI_CPU_STOP and
IPI_CPU_CRASH_STOP should try for NMI"), if we've got pseudo-NMI
enabled then we'll use it to stop CPUs at panic time. This is nice,
but it does mean that there's a pretty good chance that we'll end up
stopping a CPU while it holds the port lock for the console
UART. Specifically, I see a CPU get stopped while holding the port
lock nearly 100% of the time on my sc7180-trogdor based Chromebook by
enabling the "buddy" hardlockup detector and then doing:
sysctl -w kernel.hardlockup_all_cpu_backtrace=1
sysctl -w kernel.hardlockup_panic=1
echo HARDLOCKUP > /sys/kernel/debug/provoke-crash/DIRECT
UART drivers are _supposed_ to handle this case OK and this is why
UART drivers check "oops_in_progress" and only do a "trylock" in that
case. However, before we enabled pseudo-NMI to stop CPUs it wasn't a
very well-tested situation.
Now that we're testing the situation a lot, it can be seen that the
Qualcomm GENI UART driver is pretty broken. Specifically, when I run
my test case and look at the console output I just see a bunch of
garbled output like:
[ 201.069084] NMI backtrace[ 201.069084] NM[ 201.069087] CPU: 6
PID: 10296 Comm: dnsproxyd Not tainted 6.7.0-06265-gb13e8c0ede12
#1 01112b9f14923cbd0b[ 201.069090] Hardware name: Google Lazor
([ 201.069092] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DI[
201.069095] pc : smp_call_function_man[ 201.069099]
That's obviously not so great. This happens because each call to the
console driver exits after the data has been written to the FIFO but
before it's actually been flushed out of the serial port. When we have
multiple calls into the console one after the other then (if we can't
get the lock) each call tells the UART to throw away any data in the
FIFO that hadn't been transferred yet.
I've posted up a patch to change the arm64 core to avoid this
situation most of the time [1] much like x86 seems to do, but even if
that patch lands the GENI driver should still be fixed.
On 13.01.2024 00:03, Douglas Anderson wrote: > According to the docs I have, bit 21 of the status register is > asserted when the FIFO is _not_ empty. Add the definition. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Looks like. Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On Fri, Jan 12, 2024 at 03:03:07PM -0800, Douglas Anderson wrote: > According to the docs I have, bit 21 of the status register is > asserted when the FIFO is _not_ empty. Add the definition. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> Acked-by: Bjorn Andersson <andersson@kernel.org> Regards, Bjorn > --- > > include/linux/soc/qcom/geni-se.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h > index 29e06905bc1f..0f038a1a0330 100644 > --- a/include/linux/soc/qcom/geni-se.h > +++ b/include/linux/soc/qcom/geni-se.h > @@ -178,6 +178,7 @@ struct geni_se { > #define M_GP_IRQ_3_EN BIT(12) > #define M_GP_IRQ_4_EN BIT(13) > #define M_GP_IRQ_5_EN BIT(14) > +#define M_TX_FIFO_NOT_EMPTY_EN BIT(21) > #define M_IO_DATA_DEASSERT_EN BIT(22) > #define M_IO_DATA_ASSERT_EN BIT(23) > #define M_RX_FIFO_RD_ERR_EN BIT(24) > -- > 2.43.0.275.g3460e3d667-goog >
diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h index 29e06905bc1f..0f038a1a0330 100644 --- a/include/linux/soc/qcom/geni-se.h +++ b/include/linux/soc/qcom/geni-se.h @@ -178,6 +178,7 @@ struct geni_se { #define M_GP_IRQ_3_EN BIT(12) #define M_GP_IRQ_4_EN BIT(13) #define M_GP_IRQ_5_EN BIT(14) +#define M_TX_FIFO_NOT_EMPTY_EN BIT(21) #define M_IO_DATA_DEASSERT_EN BIT(22) #define M_IO_DATA_ASSERT_EN BIT(23) #define M_RX_FIFO_RD_ERR_EN BIT(24)
According to the docs I have, bit 21 of the status register is asserted when the FIFO is _not_ empty. Add the definition. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- include/linux/soc/qcom/geni-se.h | 1 + 1 file changed, 1 insertion(+)