| Message ID | 20250410004324.89383-4-abhishek@linux.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fixed UART handling | expand |
Hi Abhishek, On 25/04/10 06:13AM, ABHISHEK SINGH TOMAR wrote: > From: Abhishek Singh Tomar <abhishek@linux.ibm.com> > > The __length variable is not initialized in the Linux > kernel source code but is compared against a value in > uart_opal_write_buffer_space. This results in invalid behavior. > > Changes include: > > Removing the invalid comparison of __length. > Adding a temporary log statement for testing purposes: > Inside uart_opal_write_buffer_space: __length=0xf03baaaa050000c0. This line might not be required now ^^, as you seem to have removed the log statement. > > opal_console_write_buffer_space is called twice in the > kernel code, but __length remains uninitialized in both > cases. This fix ensures correct behavior by eliminating > the problematic comparison. > > This can finally lead to hang/softlock issue in older hardware. > > Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com> > --- > hw/lpc-uart.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/lpc-uart.c b/hw/lpc-uart.c > index c0f9c52ff..a5ffc5f22 100644 > --- a/hw/lpc-uart.c > +++ b/hw/lpc-uart.c > @@ -337,7 +337,7 @@ static int64_t uart_opal_write_buffer_space(int64_t term_number, > lock(&uart_lock); > tx_buf_len = uart_tx_buf_space(); > > - if ((tx_buf_len < be64_to_cpu(*__length)) && uart_timed_out(1000)) > + if (uart_timed_out(1000)) > ret = OPAL_TIMEOUT; > > *__length = cpu_to_be64(tx_buf_len); > -- > 2.48.1 Looks good to me. QEMU powerNV boots fine for me. Reviewed-by: Aditya Gupta <adityag@linux.ibm.com> Thanks, - Aditya G
On Fri, Apr 11, 2025 at 03:09:52PM +0530, Aditya Gupta wrote: >Hi Abhishek, > >On 25/04/10 06:13AM, ABHISHEK SINGH TOMAR wrote: >> From: Abhishek Singh Tomar <abhishek@linux.ibm.com> >> >> The __length variable is not initialized in the Linux >> kernel source code but is compared against a value in >> uart_opal_write_buffer_space. This results in invalid behavior. >> >> Changes include: >> >> Removing the invalid comparison of __length. >> Adding a temporary log statement for testing purposes: >> Inside uart_opal_write_buffer_space: __length=0xf03baaaa050000c0. > >This line might not be required now ^^, as you seem to have removed the >log statement. > >> >> opal_console_write_buffer_space is called twice in the >> kernel code, but __length remains uninitialized in both >> cases. This fix ensures correct behavior by eliminating >> the problematic comparison. >> >> This can finally lead to hang/softlock issue in older hardware. >> >> Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com> I fixed up the commit log and applied this to master.
diff --git a/hw/lpc-uart.c b/hw/lpc-uart.c index c0f9c52ff..a5ffc5f22 100644 --- a/hw/lpc-uart.c +++ b/hw/lpc-uart.c @@ -337,7 +337,7 @@ static int64_t uart_opal_write_buffer_space(int64_t term_number, lock(&uart_lock); tx_buf_len = uart_tx_buf_space(); - if ((tx_buf_len < be64_to_cpu(*__length)) && uart_timed_out(1000)) + if (uart_timed_out(1000)) ret = OPAL_TIMEOUT; *__length = cpu_to_be64(tx_buf_len);