With 8250 Designware UART, if writes to the LCR failed the kernel will hung up

RFC Zhang Zhen 6 participants 13 messages Mar 6, 2015 – Mar 15, 2015 TTY layer and serial drivers

lore

Share & tools

lore

Zhang Zhen

author

2015-03-06 09:11 UTC #0 lore raw

Hi,

I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).

I found if serial is busy and writes to the LCR failed after tried 1000 times. The kernel will hung up.

The system boot success after changed from:

95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
115 }
116 }

to:

95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_info(p->dev, "Couldn't set LCR to %d\n", value); //changed here
115 }
116 }

	The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
	got port->lock.
	So i think here we should change from dev_err to dev_info ?
	Any suggestions are welcome.

Best regards!

Peter Hurley

2015-03-06 16:50 UTC #1 lore raw

Hi Zhang,

Quoted 35 lines · Zhang Zhen

Hi,
I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
I found if serial is busy and writes to the LCR failed after tried 1000 times. The kernel will hung up.
The system boot success after changed from:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
115 }
116 }
	to:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_info(p->dev, "Couldn't set LCR to %d\n", value); //changed here
115 }
116 }

Quoted 3 lines

	The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
	got port->lock.
	So i think here we should change from dev_err to dev_info ?

That's not really going to help because this will still hang if the console_loglevel is set to < KERN_INFO.

Quoted 1 line

Any suggestions are welcome.

Check that the port is not the uart_console() before logging the error, like;

	if (!uart_console(p))
		dev_err(p->dev, "Couldn't .....");

Use a global flag to note the error and check it from other contexts. Plus, find out why you can't write LCR there.

Also, consider re-designing how the 8250_dw driver implements that "feature".

Regards,
Peter Hurley

Tim Kryger

2015-03-07 03:01 UTC #2 lore raw

You only hit the silicon bug if you bombard the uart with characters and simultaneously request a baud rate or framing change.

I'm not sure why you would do either to the uart console. Is it possible your host machine is doing something weird?

If you have the leverage, remind the SoC vendor to upgrade the serial block. Synopsys fixed this a long time ago.

-Tim

Quoted 66 lines · Peter Hurley

Hi Zhang,
Quoted 35 lines · Zhang Zhen
Hi,
I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
I found if serial is busy and writes to the LCR failed after tried 1000 times. The kernel will hung up.
The system boot success after changed from:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
115 }
116 }
      to:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_info(p->dev, "Couldn't set LCR to %d\n", value); //changed here
115 }
116 }
Quoted 3 lines
      The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
      got port->lock.
      So i think here we should change from dev_err to dev_info ?
That's not really going to help because this will still hang if the console_loglevel is set to < KERN_INFO.
Quoted 1 line
Any suggestions are welcome.
Check that the port is not the uart_console() before logging the error, like;
        if (!uart_console(p))
                dev_err(p->dev, "Couldn't .....");
Use a global flag to note the error and check it from other contexts. Plus, find out why you can't write LCR there.
Also, consider re-designing how the 8250_dw driver implements that "feature".
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

long.wanglong

2015-03-09 07:10 UTC #3 lore raw

Quoted 10 lines · Tim Kryger

You only hit the silicon bug if you bombard the uart with characters and simultaneously request a baud rate or framing change.
I'm not sure why you would do either to the uart console. Is it possible your host machine is doing something weird?
If you have the leverage, remind the SoC vendor to upgrade the serial block. Synopsys fixed this a long time ago.
-Tim

Hi, Tim

thanks for you reply. if we bombard the uart with characters and simultaneously request a baud rate change, we hit the silicon bug.

Our scenario is to do stability testing（at the system booting, we bombard the uart with characters）, With this version(v3.0.6) Synopsys Serial, the expect are print out the message(Couldn't set LCR to xxxx) and the kernel will not hung.

Maybe the next release of the board we will upgrade the serial block to the new version. but the issue is that how we circumvent this problem in kernel?

do you have any ideas? thanks.

Best Regards
Wang Long

Quoted 71 lines · Peter Hurley

Quoted 59 lines
Hi Zhang,
Quoted 35 lines · Zhang Zhen
Hi,
I'm testing 4.0-rc1 kernel on my board with 8250 Designware UART.(ARM Cortex-a15 single core).
I found if serial is busy and writes to the LCR failed after tried 1000 times. The kernel will hung up.
The system boot success after changed from:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_err(p->dev, "Couldn't set LCR to %d\n", value);
115 }
116 }
      to:
95 static void dw8250_serial_out(struct uart_port *p, int offset, int value) 96 {
97 struct dw8250_data *d = p->private_data;
98
...
...
112 writeb(value, p->membase + (UART_LCR << p->regshift));
113 }
114 dev_info(p->dev, "Couldn't set LCR to %d\n", value); //changed here
115 }
116 }
Quoted 3 lines
      The reason is serial8250_console_write can't get port->lock because serial8250_do_set_termios has
      got port->lock.
      So i think here we should change from dev_err to dev_info ?
That's not really going to help because this will still hang if the console_loglevel is set to < KERN_INFO.
Quoted 1 line
Any suggestions are welcome.
Check that the port is not the uart_console() before logging the error, like;
        if (!uart_console(p))
                dev_err(p->dev, "Couldn't .....");
Use a global flag to note the error and check it from other contexts. Plus, find out why you can't write LCR there.
Also, consider re-designing how the 8250_dw driver implements that "feature".
Quoted 7 lines
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.

Alan Cox

2015-03-09 13:32 UTC #4 lore raw

Quoted 2 lines

Maybe the next release of the board we will upgrade the serial block to the new version. but the issue is that how we circumvent this problem in kernel?

What is the official vendor workaround ?

Alan

Tim Kryger

2015-03-09 14:36 UTC #5 lore raw

Quoted 4 lines · Alan Cox

Quoted 2 lines
Maybe the next release of the board we will upgrade the serial block to the new version. but the issue is that how we circumvent this problem in kernel?
What is the official vendor workaround ?

They introduced a UART_16550_COMPATIBLE option for the IP which should be selected.

If configured, writes to the LCR are allowed even if it is "busy" so there is no need to retry.

More importantly there is no possibility of failing to write the LCR after numerous retries.

-Tim

Alan Cox

2015-03-09 15:05 UTC #6 lore raw

Quoted 6 lines · Tim Kryger

Quoted 4 lines · Alan Cox
Quoted 2 lines
Maybe the next release of the board we will upgrade the serial block to the new version. but the issue is that how we circumvent this problem in kernel?
What is the official vendor workaround ?

Quoted 8 lines

They introduced a UART_16550_COMPATIBLE option for the IP which should be selected.
If configured, writes to the LCR are allowed even if it is "busy" so there is no need to retry.
More importantly there is no possibility of failing to write the LCR after numerous retries.

Ah no - I meant what is their official software workaround for existing parts with the bug ? Presumably they have an errata document that discusses this and the correct methods they recommend to avoid the hang ?

Alan

Tim Kryger

2015-03-10 02:47 UTC #7 lore raw

Quoted 4 lines · Alan Cox

Ah no - I meant what is their official software workaround for existing parts with the bug ? Presumably they have an errata document that discusses this and the correct methods they recommend to avoid the hang ?

As far as I know, the only advice they provided was rather naive.

The documentation I saw suggested stashing a copy of the LCR and then rewriting it when the special LCR write failed interrupt was raised.

That approach was not workable as the LCR might be written while the interrupt is masked causing the sequence of register writes to occur in an order other than what was desired.

Additionally, when the LCR needed to be re-written but the UART stayed busy, the interrupt would keep firing and the driver would starve out everything else on the CPU.

The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.

Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d

-Tim

Zhang Zhen

author

2015-03-10 03:15 UTC #8 lore raw

Quoted 20 lines · Tim Kryger

Quoted 4 lines · Alan Cox
Ah no - I meant what is their official software workaround for existing parts with the bug ? Presumably they have an errata document that discusses this and the correct methods they recommend to avoid the hang ?
As far as I know, the only advice they provided was rather naive.
The documentation I saw suggested stashing a copy of the LCR and then rewriting it when the special LCR write failed interrupt was raised.
That approach was not workable as the LCR might be written while the interrupt is masked causing the sequence of register writes to occur in an order other than what was desired.
Additionally, when the LCR needed to be re-written but the UART stayed busy, the interrupt would keep firing and the driver would starve out everything else on the CPU.

Quoted 6 lines

The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.
Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d

Hi,

If writes to the LCR failed and the dev_err in dw8250_serial_out lead to the system hung up.
The root cause is that serial8250_console_write try to get port->lock which has been got by serial8250_do_set_termios.

"Writes to the LCR are used to change the baud rate, parity, stop bit, or data length so the data that may be lost is likely not important." The dev_err in dw8250_serial_out must be needed? We can delete dev_err directly ?

Best regards!

Quoted 4 lines

-Tim
.

Peter Hurley

2015-03-10 13:25 UTC #9 lore raw

In reply to Tim Kryger

Quoted 3 lines · Tim Kryger

The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.

But hanging the machine is not an acceptable outcome.

Since the hang stems from the printk while already holding the port->lock, until I can fix the console write, I think it's best to comment out the dev_err() with a FIXME note.

The 8250 console write already checks port->sysrq to allow printk from within the driver so this solution just needs to be generalized. And there are debug uses of printk() from within the driver that are just as broken, so this isn't the only use case. (I want to do something similar for the au_serial-* i/o accessors too).

Regards,
Peter Hurley

Zhang Zhen

author

2015-03-11 01:20 UTC #10 lore raw

Quoted 10 lines · Peter Hurley

Quoted 3 lines · Tim Kryger
The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.
But hanging the machine is not an acceptable outcome.
Since the hang stems from the printk while already holding the port->lock, until I can fix the console write, I think it's best to comment out the dev_err() with a FIXME note.

Yeah，i agree.

Quoted 14 lines

The 8250 console write already checks port->sysrq to allow printk from within the driver so this solution just needs to be generalized. And there are debug uses of printk() from within the driver that are just as broken, so this isn't the only use case. (I want to do something similar for the au_serial-* i/o accessors too).
Regards,
Peter Hurley
.

Andy Shevchenko

2015-03-13 15:36 UTC #11 lore raw

In reply to Tim Kryger

Quoted 20 lines · Tim Kryger

Quoted 4 lines · Alan Cox
Ah no - I meant what is their official software workaround for existing parts with the bug ? Presumably they have an errata document that discusses this and the correct methods they recommend to avoid the hang ?
As far as I know, the only advice they provided was rather naive.
The documentation I saw suggested stashing a copy of the LCR and then rewriting it when the special LCR write failed interrupt was raised.
That approach was not workable as the LCR might be written while the interrupt is masked causing the sequence of register writes to occur in an order other than what was desired.
Additionally, when the LCR needed to be re-written but the UART stayed busy, the interrupt would keep firing and the driver would starve out everything else on the CPU.

Quoted 5 lines

The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.
Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d

I hit the similar problem as Zhang described quite long ago when I tried to write to the port chosen as console.
In my case seems the same (DW) IP is enumerated via PCI. Do we need to tweak PCI cases as well?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Peter Hurley

2015-03-15 14:50 UTC #12 lore raw

Hi Andy,

Quoted 25 lines · Andy Shevchenko

Quoted 24 lines · Tim Kryger
Quoted 4 lines · Alan Cox
Ah no - I meant what is their official software workaround for existing parts with the bug ? Presumably they have an errata document that discusses this and the correct methods they recommend to avoid the hang ?
As far as I know, the only advice they provided was rather naive.
The documentation I saw suggested stashing a copy of the LCR and then rewriting it when the special LCR write failed interrupt was raised.
That approach was not workable as the LCR might be written while the interrupt is masked causing the sequence of register writes to occur in an order other than what was desired.
Additionally, when the LCR needed to be re-written but the UART stayed busy, the interrupt would keep firing and the driver would starve out everything else on the CPU.
The current workaround of clearing fifos and retrying a fixed number of times isn't ideal but I'm not sure what else can be done given the way this hardware works.

Quoted 6 lines

Quoted 1 line
Additional background is in c49436b657d0a56a6ad90d14a7c3041add7cf64d
I hit the similar problem as Zhang described quite long ago when I tried to write to the port chosen as console.
In my case seems the same (DW) IP is enumerated via PCI. Do we need to tweak PCI cases as well?

Currently, 8250_dw only supports platform devices.

To support your use-case, the 8250_dw i/o accessors would need to be set for that hardware. Otherwise, even if you get this hardware past the PCI probe, it will lockup anytime the LCR is written while data is still in the RX fifo.

Regards,
Peter Hurley