linux-kernel - Re: arm64: csdlock at early boot due to slow serial (?)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250711095023.GB1093654@e132581.arm.com>
Date: Fri, 11 Jul 2025 10:50:23 +0100
From: Leo Yan <leo.yan@....com>
To: Breno Leitao <leitao@...ian.org>
Cc: Mark Rutland <mark.rutland@....com>, ankita@...dia.com,
	bwicaksono@...dia.com, rmk+kernel@...linux.org.uk,
	catalin.marinas@....com, linux-serial@...r.kernel.org,
	rmikey@...a.com, linux-arm-kernel@...ts.infradead.org,
	usamaarif642@...il.com, linux-kernel@...r.kernel.org,
	paulmck@...nel.org
Subject: Re: arm64: csdlock at early boot due to slow serial (?)

Hi Breno,

On Thu, Jul 10, 2025 at 10:30:45AM -0700, Breno Leitao wrote:

[...]

> > The atomic path is introduced recently by the commit:
> > 
> >   2eb2608618ce ("serial: amba-pl011: Implement nbcon console")
> > 
> > My conclusion is that changing the initcall will not disable the atomic
> > path, changing to console_initcall() will cause AMBA device init
> > failure, and as a result, the clock operations will not be invoked.
> > Thus, I am curious if you have ruled out the issue is caused by the UART
> > clock (as I mentioned in another reply).
> > 
> > BTW, since the atomic path is enabled in the commit 2eb2608618ce, what
> > is the result after reverting the commit?
> 
> I've reverted commit 2eb2608618ce ("serial: amba-pl011: Implement nbcon
> console"), and I don't see the CSD locks anymoer. The serial speed is
> the same and continue to be slow, but, the CSD lock is not there. Here
> is the time spent on the serial flush when reverting the commit above
> 
> 	[    0.309561] printk: legacy console [ttyAMA0] enabled
> 	[    8.657938] ACPI: PCI Root Bridge [PCI2] (domain 0002 [bus 00-ff])

>From this result, we can know both the atomic path and the thread path
take a long time polling.

Since both paths configure the UART clock, I'm curious about the
behaviour if the UART clock is untouched. The relevant code is shown
below.

I may seem a bit stubborn in suspecting a clock issue :) But if you
have confirmed that a standard pl011 UART IP is being used, then the
only external factor I am aware of is the clock.

Thanks,
Leo

---8<---

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 71482d639a6d..b04773ba2602 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -64,6 +64,8 @@ static int amba_get_enable_pclk(struct amba_device *pcdev)
 {
 	int ret;
 
+	return 0;
+
 	pcdev->pclk = clk_get(&pcdev->dev, "apb_pclk");
 	if (IS_ERR(pcdev->pclk))
 		return PTR_ERR(pcdev->pclk);
@@ -77,8 +79,8 @@ static int amba_get_enable_pclk(struct amba_device *pcdev)
 
 static void amba_put_disable_pclk(struct amba_device *pcdev)
 {
-	clk_disable_unprepare(pcdev->pclk);
-	clk_put(pcdev->pclk);
+	//clk_disable_unprepare(pcdev->pclk);
+	//clk_put(pcdev->pclk);
 }
 
 
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 22939841b1de..9ba24afb6e4f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1777,7 +1777,7 @@ static int pl011_hwinit(struct uart_port *port)
 	/*
 	 * Try to enable the clock producer.
 	 */
-	retval = clk_prepare_enable(uap->clk);
+	retval = clk_prepare(uap->clk);
 	if (retval)
 		return retval;
 
@@ -1934,7 +1934,7 @@ static int pl011_startup(struct uart_port *port)
 	return 0;
 
  clk_dis:
-	clk_disable_unprepare(uap->clk);
+	//clk_disable_unprepare(uap->clk);
 	return retval;
 }
 
@@ -2025,7 +2025,7 @@ static void pl011_shutdown(struct uart_port *port)
 	/*
 	 * Shut down the clock producer
 	 */
-	clk_disable_unprepare(uap->clk);
+	//clk_disable_unprepare(uap->clk);
 	/* Optionally let pins go into sleep states */
 	pinctrl_pm_select_sleep_state(port->dev);
 
@@ -2524,7 +2524,7 @@ pl011_console_write_atomic(struct console *co, struct nbcon_write_context *wctxt
 	if (!nbcon_enter_unsafe(wctxt))
 		return;
 
-	clk_enable(uap->clk);
+	//clk_enable(uap->clk);
 
 	if (!uap->vendor->always_enabled) {
 		old_cr = pl011_read(uap, REG_CR);
@@ -2542,7 +2542,7 @@ pl011_console_write_atomic(struct console *co, struct nbcon_write_context *wctxt
 	if (!uap->vendor->always_enabled)
 		pl011_write(old_cr, uap, REG_CR);
 
-	clk_disable(uap->clk);
+	//clk_disable(uap->clk);
 
 	nbcon_exit_unsafe(wctxt);
 }
@@ -2556,7 +2556,7 @@ pl011_console_write_thread(struct console *co, struct nbcon_write_context *wctxt
 	if (!nbcon_enter_unsafe(wctxt))
 		return;
 
-	clk_enable(uap->clk);
+	//clk_enable(uap->clk);
 
 	if (!uap->vendor->always_enabled) {
 		old_cr = pl011_read(uap, REG_CR);
@@ -2586,7 +2586,7 @@ pl011_console_write_thread(struct console *co, struct nbcon_write_context *wctxt
 	if (!uap->vendor->always_enabled)
 		pl011_write(old_cr, uap, REG_CR);
 
-	clk_disable(uap->clk);
+	//clk_disable(uap->clk);
 
 	nbcon_exit_unsafe(wctxt);
 }