bluetooth: hci_driver: Fix deadlock in MPSL workq

This commit fixes deadlock in MPSL workq caused by bt_buf_get_rx with
K_FOREVER.

Previously, calling bt_buf_get_rx with K_FOREVER could block
indefinitely if the requested pool had no available buffers. This
blocking affected the MPSL work queue, preventing it from processing
other work items, including critical timeslot events like
MPSL_TIMESLOT_SIGNAL_CANCELLED and MPSL_TIMESLOT_SIGNAL_BLOCKED.

The issue becomes more severe if a background flash operation coincides
with this scenario. Flash operations often execute on the system work
queue (sysworkq), which can delay host work items responsible for
freeing buffers in the RX pool. In such cases:

- Flash operations stall, waiting for a timeslot event from the MPSL
  work queue.
- The MPSL work queue remains blocked by bt_buf_get_rx.
- This results in a deadlock, causing the flash operation to timeout and
  triggering a warning.

This commit modifies the HCI driver to call bt_buf_get_rx with
K_NO_WAIT. If no buffer is immediately available, it relies on a
callback to notify when a buffer is freed. This change ensures the MPSL
work queue remains unblocked, allowing other work items to execute while
waiting for a buffer.

Signed-off-by: Pavel Vasilyev <[email protected]>
(cherry picked from commit 52b6395)
Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Author: PavelVPV

subsys/bluetooth/controller/hci_driver.c

@@ -270,6 +270,23 @@ static inline void receive_signal_raise(void)
 	mpsl_work_submit(&receive_work);
 }
 
+/** Storage for HCI packets from controller to host */
+static struct {
+	/* Buffer for the HCI packet. */
+	uint8_t buf[HCI_RX_BUF_SIZE];
+	/* Type of the HCI packet the buffer contains. */
+	sdc_hci_msg_type_t type;
+} rx_hci_msg;
+
+static void bt_buf_rx_freed_cb(enum bt_buf_type type_mask)
+{
+	if (((rx_hci_msg.type == SDC_HCI_MSG_TYPE_EVT && (type_mask & BT_BUF_EVT) != 0u) ||
+	     (rx_hci_msg.type == SDC_HCI_MSG_TYPE_DATA && (type_mask & BT_BUF_ACL_IN) != 0u) ||
+	     (rx_hci_msg.type == SDC_HCI_MSG_TYPE_ISO && (type_mask & BT_BUF_ISO_IN) != 0u))) {
+		receive_signal_raise();
+	}
+}
+
 static int cmd_handle(struct net_buf *cmd)
 {
 	LOG_DBG("");
@@ -371,16 +388,16 @@ static int hci_driver_send(struct net_buf *buf)
 	return err;
 }
 
-static void data_packet_process(uint8_t *hci_buf)
+static int data_packet_process(const struct device *dev, uint8_t *hci_buf)
 {
-	struct net_buf *data_buf = bt_buf_get_rx(BT_BUF_ACL_IN, K_FOREVER);
+	struct net_buf *data_buf = bt_buf_get_rx(BT_BUF_ACL_IN, K_NO_WAIT);
 	struct bt_hci_acl_hdr *hdr = (void *)hci_buf;
 	uint16_t hf, handle, len;
 	uint8_t flags, pb, bc;
 
 	if (!data_buf) {
-		LOG_ERR("No data buffer available");
-		return;
+		LOG_DBG("No data buffer available");
+		return -ENOBUFS;
 	}
 
 	len = sys_le16_to_cpu(hdr->len);
@@ -390,23 +407,51 @@ static void data_packet_process(uint8_t *hci_buf)
 	pb = bt_acl_flags_pb(flags);
 	bc = bt_acl_flags_bc(flags);
 
+	if (len + sizeof(*hdr) > HCI_RX_BUF_SIZE) {
+		LOG_ERR("Event buffer too small. %u > %u",
+			len + sizeof(*hdr),
+			HCI_RX_BUF_SIZE);
+		return -ENOMEM;
+	}
+
 	LOG_DBG("Data: handle (0x%02x), PB(%01d), BC(%01d), len(%u)", handle,
 	       pb, bc, len);
 
 	net_buf_add_mem(data_buf, &hci_buf[0], len + sizeof(*hdr));
-	bt_recv(data_buf);
+
+	struct hci_driver_data *driver_data = dev->data;
+
+	(void)driver_data->recv_func(dev, data_buf);
+
+	return 0;
 }
 
-static void iso_data_packet_process(uint8_t *hci_buf)
+static int iso_data_packet_process(const struct device *dev, uint8_t *hci_buf)
 {
-	struct net_buf *data_buf = bt_buf_get_rx(BT_BUF_ISO_IN, K_FOREVER);
+	struct net_buf *data_buf = bt_buf_get_rx(BT_BUF_ISO_IN, K_NO_WAIT);
 	struct bt_hci_iso_hdr *hdr = (void *)hci_buf;
 
 	uint16_t len = sys_le16_to_cpu(hdr->len);
 
+	if (!data_buf) {
+		LOG_DBG("No data buffer available");
+		return -ENOBUFS;
+	}
+
+	if (len + sizeof(*hdr) > HCI_RX_BUF_SIZE) {
+		LOG_ERR("Event buffer too small. %u > %u",
+			len + sizeof(*hdr),
+			HCI_RX_BUF_SIZE);
+		return -ENOMEM;
+	}
+
 	net_buf_add_mem(data_buf, &hci_buf[0], len + sizeof(*hdr));
 
-	bt_recv(data_buf);
+	struct hci_driver_data *driver_data = dev->data;
+
+	(void)driver_data->recv_func(dev, data_buf);
+
+	return 0;
 }
 
 static bool event_packet_is_discardable(const uint8_t *hci_buf)
@@ -451,12 +496,19 @@ static bool event_packet_is_discardable(const uint8_t *hci_buf)
 	}
 }
 
-static void event_packet_process(uint8_t *hci_buf)
+static int event_packet_process(const struct device *dev, uint8_t *hci_buf)
 {
 	bool discardable = event_packet_is_discardable(hci_buf);
 	struct bt_hci_evt_hdr *hdr = (void *)hci_buf;
 	struct net_buf *evt_buf;
 
+	if (hdr->len + sizeof(*hdr) > HCI_RX_BUF_SIZE) {
+		LOG_ERR("Event buffer too small. %u > %u",
+			hdr->len + sizeof(*hdr),
+			HCI_RX_BUF_SIZE);
+		return -ENOMEM;
+	}
+
 	if (hdr->evt == BT_HCI_EVT_LE_META_EVENT) {
 		struct bt_hci_evt_le_meta_event *me = (void *)&hci_buf[2];
 
@@ -480,67 +532,85 @@ static void event_packet_process(uint8_t *hci_buf)
 		LOG_DBG("Event (0x%02x) len %u", hdr->evt, hdr->len);
 	}
 
-	evt_buf = bt_buf_get_evt(hdr->evt, discardable,
-				 discardable ? K_NO_WAIT : K_FOREVER);
+	evt_buf = bt_buf_get_evt(hdr->evt, discardable, K_NO_WAIT);
 
 	if (!evt_buf) {
 		if (discardable) {
 			LOG_DBG("Discarding event");
-			return;
+			return 0;
 		}
 
-		LOG_ERR("No event buffer available");
-		return;
+		LOG_DBG("No event buffer available");
+		return -ENOBUFS;
 	}
 
 	net_buf_add_mem(evt_buf, &hci_buf[0], hdr->len + sizeof(*hdr));
-	bt_recv(evt_buf);
+
+	struct hci_driver_data *driver_data = dev->data;
+
+	(void)driver_data->recv_func(dev, evt_buf);
+
+	return 0;
 }
 
-static bool fetch_and_process_hci_msg(uint8_t *p_hci_buffer)
+static int fetch_hci_msg(uint8_t *p_hci_buffer, sdc_hci_msg_type_t *msg_type)
 {
 	int errcode;
-	sdc_hci_msg_type_t msg_type;
 
 	errcode = MULTITHREADING_LOCK_ACQUIRE();
 	if (!errcode) {
-		errcode = hci_internal_msg_get(p_hci_buffer, &msg_type);
+		errcode = hci_internal_msg_get(p_hci_buffer, msg_type);
 		MULTITHREADING_LOCK_RELEASE();
 	}
 
-	if (errcode) {
-		return false;
-	}
+	return errcode;
+}
+
+static int process_hci_msg(const struct device *dev, uint8_t *p_hci_buffer,
+			    sdc_hci_msg_type_t msg_type)
+{
+	int err;
 
 	if (msg_type == SDC_HCI_MSG_TYPE_EVT) {
-		event_packet_process(p_hci_buffer);
+		err = event_packet_process(dev, p_hci_buffer);
 	} else if (msg_type == SDC_HCI_MSG_TYPE_DATA) {
-		data_packet_process(p_hci_buffer);
+		err = data_packet_process(dev, p_hci_buffer);
 	} else if (msg_type == SDC_HCI_MSG_TYPE_ISO) {
-		iso_data_packet_process(p_hci_buffer);
+		err = iso_data_packet_process(dev, p_hci_buffer);
 	} else {
 		if (!IS_ENABLED(CONFIG_BT_CTLR_SDC_SILENCE_UNEXPECTED_MSG_TYPE)) {
 			LOG_ERR("Unexpected msg_type: %u. This if-else needs a new branch",
 				msg_type);
 		}
+		err = 0;
 	}
 
-	return true;
+	return err;
 }
 
 void hci_driver_receive_process(void)
 {
-#if defined(CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT)
-	static uint8_t hci_buf[MAX(BT_BUF_RX_SIZE,
-				   BT_BUF_EVT_SIZE(CONFIG_BT_BUF_EVT_DISCARDABLE_SIZE))];
-#else
-	static uint8_t hci_buf[BT_BUF_RX_SIZE];
-#endif
+	const struct device *dev = DEVICE_DT_GET(DT_DRV_INST(0));
+	int err;
 
-	if (fetch_and_process_hci_msg(&hci_buf[0])) {
-		/* Let other threads of same priority run in between. */
-		receive_signal_raise();
+	if (rx_hci_msg.type == SDC_HCI_MSG_TYPE_NONE &&
+	    fetch_hci_msg(&rx_hci_msg.buf[0], &rx_hci_msg.type) != 0) {
+		return;
 	}
+
+	err = process_hci_msg(dev, &rx_hci_msg.buf[0], rx_hci_msg.type);
+	if (err == -ENOBUFS) {
+		/* If we got -ENOBUFS, wait for the signal from the host. */
+		return;
+	} else if (err) {
+		LOG_ERR("Unknown error when processing hci message %d", err);
+		k_panic();
+	}
+
+	rx_hci_msg.type = SDC_HCI_MSG_TYPE_NONE;
+
+	/* Let other threads of same priority run in between. */
+	receive_signal_raise();
 }
 
 static void receive_work_handler(struct k_work *work)
@@ -1270,6 +1340,12 @@ static int hci_driver_open(void)
 
 	MULTITHREADING_LOCK_RELEASE();
 
+	struct hci_driver_data *driver_data = dev->data;
+
+	(void)driver_data->recv_func = recv_func;
+
+	bt_buf_rx_freed_cb_set(bt_buf_rx_freed_cb);
+
 	return 0;
 }
 
@@ -1302,6 +1378,8 @@ static int hci_driver_close(void)
 
 	MULTITHREADING_LOCK_RELEASE();
 
+	bt_buf_rx_freed_cb_set(NULL);
+
 	return err;
 }