On Tue, Aug 03, 2021 at 03:19:01PM +0206, John Ogness wrote:
Implement an NMI-safe write_atomic() console function in order to
support synchronous console printing.
Since interrupts need to be disabled during transmit, all usage of
the IER register is wrapped with access functions that use the
printk cpulock to synchronize register access while tracking the
state of the interrupts. This is necessary because write_atomic()
can be called from an NMI context that has preempted write_atomic().
...
quoted hunk ↗ jump to hunk
+static inline void serial8250_set_IER(struct uart_8250_port *up,
+ unsigned char ier)
+{
+ struct uart_port *port = &up->port;
+ unsigned long flags;
+ bool is_console;
quoted hunk ↗ jump to hunk
+ is_console = uart_console(port);
+
+ if (is_console)
+ console_atomic_cpu_lock(flags);
+
+ serial_out(up, UART_IER, ier);
+
+ if (is_console)
+ console_atomic_cpu_unlock(flags);
I would rewrite it as
if (uart_console()) {
console_atomic_cpu_lock(flags);
serial_out(up, UART_IER, ier);
console_atomic_cpu_unlock(flags);
} else {
serial_out(up, UART_IER, ier);
}
No additional variable, easier to get the algorithm on the first glance, less
error prone.
+}
quoted hunk ↗ jump to hunk
+static inline unsigned char serial8250_clear_IER(struct uart_8250_port *up)
+{
+ struct uart_port *port = &up->port;
+ unsigned int clearval = 0;
+ unsigned long flags;
+ unsigned int prior;
+ bool is_console;
+
+ is_console = uart_console(port);
+
+ if (up->capabilities & UART_CAP_UUE)
+ clearval = UART_IER_UUE;
+
+ if (is_console)
+ console_atomic_cpu_lock(flags);
+
+ prior = serial_port_in(port, UART_IER);
+ serial_port_out(port, UART_IER, clearval);
+
+ if (is_console)
+ console_atomic_cpu_unlock(flags);
Ditto.
+ return prior;
+}
...
quoted hunk ↗ jump to hunk
+ is_console = uart_console(port);
+
+ if (is_console)
+ console_atomic_cpu_lock(flags);
up->ier = port->serial_in(port, UART_IER);
+ if (is_console)
+ console_atomic_cpu_unlock(flags);
+
I'm wondering why you can't call above function here?
...
quoted hunk ↗ jump to hunk
+ is_console = uart_console(p);
+ if (is_console)
+ console_atomic_cpu_lock(flags);
ier = p->serial_in(p, UART_IER);
+ if (is_console)
+ console_atomic_cpu_unlock(flags);
Ditto.
...
quoted hunk ↗ jump to hunk
+ is_console = uart_console(port);
+
+ if (is_console)
+ console_atomic_cpu_lock(flags);
+
+ ier = serial_in(up, UART_IER);
+ serial_out(up, UART_IER, ier & (~mask));
+
+ if (is_console)
+ console_atomic_cpu_unlock(flags);
Ditto.
...
quoted hunk ↗ jump to hunk
+ if (uart_console(port))
+ console_atomic_cpu_lock(flags);
+
+ ier = serial_in(up, UART_IER);
+ serial_out(up, UART_IER, ier | mask);
+
+ if (uart_console(port))
+ console_atomic_cpu_unlock(flags);
Ditto.
Looking into above note, that uart_console(port) can give different results
here, AFAIR.
--
With Best Regards,
Andy Shevchenko