Page MenuHomeFreeBSD

[POWERPC] opal_console: fix serial console output corruption
ClosedPublic

Authored by alfredo on Mar 4 2021, 4:29 PM.

Details

Summary

On machines using OPAL the serial output can be corrupted if OS writes too fast since driver doesn't retry if OPAL is BUSY. This is well visible reading the output of "dmesg" or cat a long file, user will find missing chars and mixed parts of two different lines.

This patch adds OPAL_CONSOLE_WRITE error handling and implements a call to OPAL_CONSOLE_WRITE_BUFFER_SPACE to verify if there's enough space on opal console before writing to it.

The patch was tested on Raptor Blackbird using a serial cable connected to another machine. Tested target is powerpc64 (BE)

Test Plan

boot
run dmesg
cat message files

Check if output is ok

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Your changes look good to me overall.

But what about uart_opal_cnputc? It still blindly calls OPAL_POLL_EVENTS 20 times. It would be nice if it could use your new code too.

sys/powerpc/powernv/opal_console.c
182

It's better to cast the address to the same type of variable you're using: vm_paddr_t

185

style: this line should be indented with 4 extra spaces from where the above 'if' column begins.

509–511

Wouldn't it be better to break instead of returning, to keep the handling of everything available in ttydisc?

alfredo marked 3 inline comments as done.

Update with reviewer comments
retested

sys/powerpc/powernv/opal_console.c
509–511

Currently it should return an error only if terminal is invalid, and in this case doesn't make sense to process pending data. But since I made uart_opal_console_write_buffer_space() generic in terms of error handling and to keep consistent with previous code, I agree with your suggestion, thanks!

sys/powerpc/powernv/opal_console.c
520

For the same reasons above and for consistency, it would be better to change this return to a break too.

Break instead of return, keeping it consistent with previous code

This revision is now accepted and ready to land.Apr 16 2021, 2:32 PM