Merge pull request #304 from smartobjectoriented/feat/rpi4-console-sigint

rpi4: make Ctrl-C work on the mini UART console

Author: daniel-rossier

so3/so3/devices/serial/bcm283x_mu.c

@@ -20,9 +20,13 @@
 #include <heap.h>
 #include <limits.h>
 #include <memory.h>
+#include <mutex.h>
+#include <process.h>
+#include <signal.h>
 
 #include <device/device.h>
 #include <device/driver.h>
+#include <device/irq.h>
 
 #include <device/serial.h>
 
@@ -31,11 +35,19 @@
 
 #include <asm/io.h> /* ioread/iowrite macros */
 
+#define SERIAL_BUFFER_SIZE 80
+
 void *__uart_vaddr = (void *) CONFIG_UART_LL_PADDR;
 
+static volatile char serial_buffer[SERIAL_BUFFER_SIZE];
+static volatile uint32_t prod = 0, cons = 0;
+
+extern mutex_t read_lock;
+
 typedef struct {
 	addr_t base;
-	bcm283x_mu_t *dev;
+	irq_def_t irq_def;
+	bool irq_enabled;
 } bcm283x_mu_dev_t;
 
 static bcm283x_mu_dev_t bcm283x_mu_dev = {
@@ -75,40 +87,162 @@ void __ll_put_byte(char c)
 static char bcm283x_mu_get_byte(bool polling)
 {
 	bcm283x_mu_t *bcm283x_mu = (bcm283x_mu_t *) bcm283x_mu_dev.base;
+	char tmp;
+
+	/* Without an RX interrupt there is no producer filling the ring, so the
+	 * only thing we can do is spin on the FIFO — the behaviour this driver
+	 * had before interrupts were wired up.
+	 */
+	if (polling || !bcm283x_mu_dev.irq_enabled) {
+		while (!(ioread32(&bcm283x_mu->lsr) & UART_LSR_RX_READY))
+			;
 
-	while (!(ioread32(&bcm283x_mu->lsr) & UART_LSR_RX_READY))
-		;
+		return (char) ioread32(&bcm283x_mu->io);
+	}
+
+	while (prod == cons) {
+		/* Ctrl-C arrived while we were blocked reading: abandon the
+		 * read and report ETX so console_getc cancels the line. */
+		if (serial_intr) {
+			serial_intr = false;
+			return 3; /* ETX */
+		}
+
+		schedule();
+
+		smp_mb();
+		wfi();
+	}
+
+	tmp = serial_buffer[cons];
+	cons = (cons + 1) % SERIAL_BUFFER_SIZE;
+
+	return tmp;
+}
+
+/*
+ * Drain the RX FIFO into the serial buffer. Reading MU_IO is what clears the
+ * interrupt, so we loop on LSR rather than decoding IIR — that also sidesteps
+ * the mini UART's muddled IIR documentation.
+ *
+ * Ctrl-C handling mirrors pl011_int(): if a thread holds read_lock it is
+ * blocked reading the console (e.g. the shell), so we only cancel its line
+ * instead of killing it; otherwise a foreground app is running and gets SIGINT.
+ */
+static irq_return_t bcm283x_mu_int(int irq, void *dummy)
+{
+	bcm283x_mu_t *bcm283x_mu = (bcm283x_mu_t *) bcm283x_mu_dev.base;
+	char c;
+
+	while (ioread32(&bcm283x_mu->lsr) & UART_LSR_RX_READY) {
+		c = (char) ioread32(&bcm283x_mu->io);
+
+		/* Check for SIGINT to be raised on Ctrl^C */
+		if (c == 3) {
+			bcm283x_mu_put_byte('^');
+			bcm283x_mu_put_byte('C');
+			bcm283x_mu_put_byte('\n');
+
+#ifdef CONFIG_IPC_SIGNAL
+			if (mutex_is_locked(&read_lock)) {
+				serial_intr = true;
+			} else {
+				/* Use fg_pcb (the shell's foreground job), NOT
+				 * current(): in IRQ context current() is just the
+				 * thread running when the key arrived (usually the
+				 * idle thread, since the foreground app is typically
+				 * asleep). Fall back to current() if no foreground is
+				 * tracked yet. */
+				pcb_t *target = fg_pcb ? fg_pcb : current()->pcb;
+				if (target != NULL)
+					sys_do_kill(target->pid, SIGINT);
+			}
+#endif
+			/* Already echoed above; keep it out of the ring. */
+			continue;
+		}
 
-	return (char) ioread32(&bcm283x_mu->io);
+		serial_buffer[prod] = c;
+		prod = (prod + 1) % SERIAL_BUFFER_SIZE;
+	}
+
+	return IRQ_COMPLETED;
+}
+
+static void bcm283x_mu_enable_irq(void)
+{
+	irq_ops.enable(bcm283x_mu_dev.irq_def.irqnr);
+}
+
+static void bcm283x_mu_disable_irq(void)
+{
+	irq_ops.disable(bcm283x_mu_dev.irq_def.irqnr);
 }
 
 static int bcm283x_mu_init(dev_t *dev, int fdt_offset)
 {
 	const struct fdt_property *prop;
 	int prop_len;
+	addr_t new_base_vaddr;
+	bcm283x_mu_t *bcm283x_mu;
 
 	/* Pins multiplexing skipped here for simplicity (done by bootloader) */
 	/* Clocks init skipped here for simplicity (done by bootloader) */
 
-	/* Initialize UART controller */
-	memset(&bcm283x_mu_dev, 0, sizeof(bcm283x_mu_dev_t));
-
 	prop = fdt_get_property(__fdt_addr, fdt_offset, "reg", &prop_len);
 	BUG_ON(!prop);
 
 	BUG_ON(prop_len != 2 * sizeof(unsigned long));
 
+	/* Keep the boot-time UART base usable until io_map() has returned, so
+	 * that io_map() itself can still print. */
 #ifdef CONFIG_ARCH_ARM32
-	bcm283x_mu_dev.base =
+	new_base_vaddr =
 		io_map(fdt32_to_cpu(((const fdt32_t *) prop->data)[0]), fdt32_to_cpu(((const fdt32_t *) prop->data)[1]));
 #else
-	bcm283x_mu_dev.base =
+	new_base_vaddr =
 		io_map(fdt64_to_cpu(((const fdt64_t *) prop->data)[0]), fdt64_to_cpu(((const fdt64_t *) prop->data)[1]));
 #endif
+	BUG_ON(!new_base_vaddr);
+
+	/* Initialize UART controller */
+	memset(&bcm283x_mu_dev, 0, sizeof(bcm283x_mu_dev_t));
+	bcm283x_mu_dev.base = new_base_vaddr;
 
 	serial_ops.put_byte = bcm283x_mu_put_byte;
 	serial_ops.get_byte = bcm283x_mu_get_byte;
 
+	/* The interrupt is optional: fdt_interrupt_node() would BUG_ON a
+	 * missing "interrupts", and a device tree that does not declare one
+	 * simply keeps the polled console this driver started out with. Ctrl-C
+	 * then cannot work, since nothing scans the RX line asynchronously.
+	 */
+	prop = fdt_get_property(__fdt_addr, fdt_offset, "interrupts", &prop_len);
+	if (!prop)
+		return 0;
+
+	fdt_interrupt_node(fdt_offset, &bcm283x_mu_dev.irq_def);
+
+#ifndef CONFIG_AVZ
+	/* Same reasoning as pl011_init(): under AVZ the RX interrupt belongs to
+	 * the agency guest, whose own console driver attaches a handler. Binding
+	 * one here would let gic_handle() consume and EOI the IRQ before the
+	 * guest ever sees it. AVZ only needs put_byte for output.
+	 */
+	irq_bind(bcm283x_mu_dev.irq_def.irqnr, bcm283x_mu_int, NULL, NULL);
+#endif
+
+	serial_ops.enable_irq = bcm283x_mu_enable_irq;
+	serial_ops.disable_irq = bcm283x_mu_disable_irq;
+
+	/* Enable RX interrupts at the controller, then at the GIC. */
+	bcm283x_mu = (bcm283x_mu_t *) bcm283x_mu_dev.base;
+	iowrite32(&bcm283x_mu->ier, UART_IER_RX_ENABLE);
+
+	serial_ops.enable_irq();
+
+	bcm283x_mu_dev.irq_enabled = true;
+
 	return 0;
 }
 

so3/so3/dts/rpi4_64.dts

@@ -55,6 +55,15 @@
 	serial@fe215040 {
 		compatible = "serial,bcm283x-mu";
 		reg = <0x0 0xfe215040 0x0 0x1000>;
+		interrupt-parent = <&gic>;
+
+		/*
+		 * AUX (mini UART) — SPI 93, as the Broadcom
+		 * bcm2711-rpi-4-b.dtb gives for /soc/serial@7e215040.
+		 * Without it the console falls back to polling and Ctrl-C
+		 * cannot raise SIGINT.
+		 */
+		interrupts = <0 93 4>;
 		status = "ok";
 	};
 

so3/so3/include/completion.h

@@ -45,6 +45,15 @@ static inline void reinit_completion(struct completion *x)
 }
 
 void wait_for_completion(completion_t *completion);
+
+/*
+ * Same as wait_for_completion(), except that a pending signal abandons the
+ * wait and returns -EINTR (0 on a normal completion). Use it whenever a
+ * user-space read may block indefinitely on an external event, so that the
+ * process stays killable while it waits.
+ */
+int wait_for_completion_interruptible(completion_t *completion);
+
 void complete(completion_t *completion);
 void init_completion(completion_t *completion);
 

so3/so3/include/device/arch/bcm283x_mu.h

@@ -30,23 +30,38 @@
 #define UART_THR 0x0
 #define UART_LSR 0x14
 
-/* Bits and regs definitions (taken from U-boot) */
+/*
+ * Bits and regs definitions.
+ *
+ * NOTE: this layout used to be copied from U-Boot's struct bcm283x_mu_regs,
+ * which lists iir before ier. That is wrong: the mini UART is 8250-compatible
+ * (Linux drives it with the generic 8250 driver, and the BCM2835 peripherals
+ * doc puts AUX_MU_IER_REG at +0x04 and AUX_MU_IIR_REG at +0x08, as the
+ * MU_IER_REG/MU_IIR_REG offsets in mach/io.h already stated). The mistake was
+ * harmless as long as neither field was touched — U-Boot never reads them and
+ * this driver only used io and lsr — but it bites the moment ier is written to
+ * enable the RX interrupt.
+ */
 typedef struct {
-	u32 io;
-	u32 iir;
-	u32 ier;
-	u32 lcr;
-	u32 mcr;
-	u32 lsr;
-	u32 msr;
-	u32 scratch;
-	u32 cntl;
-	u32 stat;
-	u32 baud;
+	u32 io; /* 0x00 */
+	u32 ier; /* 0x04 */
+	u32 iir; /* 0x08 */
+	u32 lcr; /* 0x0c */
+	u32 mcr; /* 0x10 */
+	u32 lsr; /* 0x14 */
+	u32 msr; /* 0x18 */
+	u32 scratch; /* 0x1c */
+	u32 cntl; /* 0x20 */
+	u32 stat; /* 0x24 */
+	u32 baud; /* 0x28 */
 } bcm283x_mu_t;
 
 /* LSR register bits */
 #define UART_LSR_RX_READY (1 << 0)
 #define UART_LSR_TX_READY (1 << 5)
 
+/* IER register bits (8250 semantics, see the note above) */
+#define UART_IER_RX_ENABLE (1 << 0)
+#define UART_IER_TX_ENABLE (1 << 1)
+
 #endif /* BCM28x_MU_H */

so3/so3/include/signal.h

@@ -101,6 +101,17 @@ typedef struct __sigaction {
 	sigaction_t *sa;
 } __sigaction_t;
 
+/* Forward declaration: thread.h already includes us for sigset_t. */
+struct tcb;
+
+/*
+ * True when <tcb> has at least one pending signal its mask does not block.
+ * Used by the interruptible blocking primitives to decide whether to abandon
+ * their wait; the signal itself is acted upon later, by sig_check() on the way
+ * back to user space.
+ */
+bool signal_pending(struct tcb *tcb);
+
 SYSCALL_DECLARE(rt_sigaction, int signum, const sigaction_t *action, sigaction_t *old_action, size_t sigsize);
 SYSCALL_DECLARE(rt_sigprocmask, int how, const sigset_t *set, sigset_t *old, size_t sigsize);
 SYSCALL_DECLARE(kill, int pid, int sig);

so3/so3/include/thread.h

@@ -91,6 +91,15 @@ struct tcb {
 	bool killed;
 
 #ifdef CONFIG_IPC_SIGNAL
+	/* Set while the thread sits in an interruptible blocking primitive
+	 * (see wait_for_completion_interruptible()). sys_do_kill() wakes such
+	 * threads so they can observe the signal instead of sleeping until
+	 * their completion happens to fire; the primitive then unwinds its own
+	 * wait state and returns -EINTR. Only interruptible waiters are woken:
+	 * the other primitives park a stack-allocated queue entry in a list and
+	 * must not be resumed behind their back. */
+	bool interruptible;
+
 	/* Mask for thread's disabled signals */
 	sigset_t sig_mask;
 #endif

so3/so3/ipc/completion.c

@@ -52,6 +52,60 @@ void wait_for_completion(completion_t *completion)
 	local_irq_restore(flags);
 }
 
+/*
+ * Interruptible variant of wait_for_completion(): returns 0 once completed, or
+ * -EINTR if a signal arrived first.
+ *
+ * The thread advertises itself as interruptible so that sys_do_kill() knows it
+ * may be woken (see wake_interruptible_threads()); without that, a thread
+ * parked here is off the ready list and would sleep until its completion
+ * happens to fire, no matter how many signals are raised meanwhile.
+ */
+int wait_for_completion_interruptible(completion_t *completion)
+{
+	queue_thread_t q_tcb;
+	unsigned long flags;
+	int ret = 0;
+
+	ASSERT(!__in_interrupt);
+	ASSERT(local_irq_is_enabled());
+
+	flags = local_irq_save();
+
+	q_tcb.tcb = current();
+
+	if (!completion->count) {
+		list_add_tail(&q_tcb.list, &completion->tcb_list);
+
+#ifdef CONFIG_IPC_SIGNAL
+		current()->interruptible = true;
+
+		while (!completion->count && !signal_pending(current()))
+			waiting();
+
+		current()->interruptible = false;
+
+		if (!completion->count) {
+			/* Signalled. complete() unlinks the waiter it wakes, but
+			 * nobody unlinked us — and q_tcb lives on this stack
+			 * frame, which is about to go away. Do it ourselves. */
+			list_del(&q_tcb.list);
+			local_irq_restore(flags);
+
+			return -EINTR;
+		}
+#else
+		while (!completion->count)
+			waiting();
+#endif /* CONFIG_IPC_SIGNAL */
+	}
+	completion->count--;
+
+	local_irq_restore(flags);
+
+	return ret;
+}
+
 /*
  * Wake a thread waiting on a completion.
  * IRQs are disabled; this function can be safely called from an interrupt context.