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]
Date:   Tue, 28 Jun 2022 16:42:34 +0300
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     linux-serial@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: [PATCH 4/4] serial: 8250_dw: Rework ->serial_out() LCR write retry logic

Currently dw8250_verify_write() (was dw8250_check_lcr()) nullifies the
benefit from differentiated ->serial_out() by having big if tree to
select correct write type.

Rework the logic such that the LCR write can be retried within the
relevant ->serial_out() handler:
  1. Move retries counter on the caller level and pass as pointer to
     dw8250_verify_write()
  2. Make dw8250_verify_write() return bool
  3. Retry the write on caller level (if needed)

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 59 ++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index fc367d44f86d..f6846363341b 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -92,41 +92,36 @@ static void dw8250_force_idle(struct uart_port *p)
  * UART_16550_COMPATIBLE=NO or version prior to introducing that option).
  * If BUSY is set while writing to LCR register, the write is ignored and
  * needs to be retries.
+ *
+ * Returns: false if the caller should retry the write.
  */
-static void dw8250_verify_write(struct uart_port *p, int offset, int value)
+static bool dw8250_verify_write(struct uart_port *p, int offset, int value,
+				    unsigned int *retries)
 {
-	void __iomem *reg_offset = p->membase + (offset << p->regshift);
 	struct dw8250_data *d = to_dw8250_data(p->private_data);
-	int tries = 1000;
+	unsigned int lcr;
 
 	if ((offset != UART_LCR) || !d->uart_16550_compatible)
-		return;
+		return true;
 
 	/* Make sure LCR write wasn't ignored */
-	while (tries--) {
-		unsigned int lcr = p->serial_in(p, offset);
+	lcr = p->serial_in(p, offset);
 
-		if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
-			return;
+	if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
+		return true;
 
-		dw8250_force_idle(p);
+	dw8250_force_idle(p);
 
-#ifdef CONFIG_64BIT
-		if (p->type == PORT_OCTEON)
-			__raw_writeq(value & 0xff, reg_offset);
-		else
-#endif
-		if (p->iotype == UPIO_MEM32)
-			writel(value, reg_offset);
-		else if (p->iotype == UPIO_MEM32BE)
-			iowrite32be(value, reg_offset);
-		else
-			writeb(value, reg_offset);
+	if (*retries) {
+		*retries -= 1;
+		return false;
 	}
+
 	/*
 	 * FIXME: this deadlocks if port->lock is already held
 	 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
 	 */
+	return true;
 }
 
 /* Returns once the transmitter is empty or we run out of retries */
@@ -155,9 +150,13 @@ static void dw8250_tx_wait_empty(struct uart_port *p)
 
 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
 {
+	unsigned int retries = 1000;
+
+retry:
 	writeb(value, p->membase + (offset << p->regshift));
 
-	dw8250_verify_write(p, offset, value);
+	if (!dw8250_verify_write(p, offset, value, &retries))
+		goto retry;
 }
 
 static void dw8250_serial_out38x(struct uart_port *p, int offset, int value)
@@ -188,20 +187,29 @@ static unsigned int dw8250_serial_inq(struct uart_port *p, int offset)
 
 static void dw8250_serial_outq(struct uart_port *p, int offset, int value)
 {
+	unsigned int retries = 1000;
+
 	value &= 0xff;
+
+retry:
 	__raw_writeq(value, p->membase + (offset << p->regshift));
 	/* Read back to ensure register write ordering. */
 	__raw_readq(p->membase + (UART_LCR << p->regshift));
 
-	dw8250_verify_write(p, offset, value);
+	if (!dw8250_verify_write(p, offset, value, &retries))
+		goto retry;
 }
 #endif /* CONFIG_64BIT */
 
 static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
 {
+	unsigned int retries = 1000;
+
+retry:
 	writel(value, p->membase + (offset << p->regshift));
 
-	dw8250_verify_write(p, offset, value);
+	if (!dw8250_verify_write(p, offset, value, &retries))
+		goto retry;
 }
 
 static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
@@ -213,10 +221,13 @@ static unsigned int dw8250_serial_in32(struct uart_port *p, int offset)
 
 static void dw8250_serial_out32be(struct uart_port *p, int offset, int value)
 {
+	unsigned int retries = 1000;
 
+retry:
 	iowrite32be(value, p->membase + (offset << p->regshift));
 
-	dw8250_verify_write(p, offset, value);
+	if (!dw8250_verify_write(p, offset, value, &retries))
+		goto retry;
 }
 
 static unsigned int dw8250_serial_in32be(struct uart_port *p, int offset)
-- 
2.30.2

Powered by blists - more mailing lists