Bluetooth: hci: Use extended VS fatal error in hci and hci_rpmsg sample

Provide common helper functions to create extended extended Zephyr
Fatal Error functionality in HCI common code.

Use the implementation in hci_rpmsg sample.

The sample didn't provide an information about Controllers assert
or system fatal error to an application code while run with nRF5340
SoC. The goal for hci_rpmsg sample change is to enhance user experience
for conformance testing of the Bluetooth Controller while executed with
nRF5340.

Signed-off-by: Piotr Pryga <[email protected]>

Author: ppryga-nordic

samples/bluetooth/hci_rpmsg/src/main.c

@@ -22,6 +22,11 @@
 #include <zephyr/bluetooth/hci.h>
 #include <zephyr/bluetooth/buf.h>
 #include <zephyr/bluetooth/hci_raw.h>
+#include <zephyr/bluetooth/hci_vs.h>
+
+#if defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+#include <zephyr/logging/log_ctrl.h>
+#endif /* CONFIG_BT_HCI_VS_FATAL_ERROR */
 
 #define BT_DBG_ENABLED 0
 #define LOG_MODULE_NAME hci_rpmsg
@@ -33,13 +38,22 @@ static K_THREAD_STACK_DEFINE(tx_thread_stack, CONFIG_BT_HCI_TX_STACK_SIZE);
 static struct k_thread tx_thread_data;
 static K_FIFO_DEFINE(tx_queue);
 static K_SEM_DEFINE(ipc_bound_sem, 0, 1);
+#if defined(CONFIG_BT_CTLR_ASSERT_HANDLER) || defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+/* A flag used to store information if the IPC endpoint has already been bound. The end point can't
+ * be used before that happens.
+ */
+static bool ipc_ept_ready;
+#endif /* CONFIG_BT_CTLR_ASSERT_HANDLER || CONFIG_BT_HCI_VS_FATAL_ERROR */
 
 #define HCI_RPMSG_CMD 0x01
 #define HCI_RPMSG_ACL 0x02
 #define HCI_RPMSG_SCO 0x03
 #define HCI_RPMSG_EVT 0x04
 #define HCI_RPMSG_ISO 0x05
 
+#define HCI_FATAL_ERR_MSG true
+#define HCI_REGULAR_MSG false
+
 static struct net_buf *hci_rpmsg_cmd_recv(uint8_t *data, size_t remaining)
 {
 	struct bt_hci_cmd_hdr *hdr = (void *)data;
@@ -191,7 +205,7 @@ static void tx_thread(void *p1, void *p2, void *p3)
 	}
 }
 
-static void hci_rpmsg_send(struct net_buf *buf)
+static void hci_rpmsg_send(struct net_buf *buf, bool is_fatal_err)
 {
 	uint8_t pkt_indicator;
 	uint8_t retries = 0;
@@ -230,7 +244,21 @@ static void hci_rpmsg_send(struct net_buf *buf)
 				LOG_WRN("IPC send has been blocked for 1.5 seconds.");
 				retries = 0;
 			}
-			k_yield();
+
+			/* The function can be called by the application main thread,
+			 * bt_ctlr_assert_handle and k_sys_fatal_error_handler. In case of a call by
+			 * Bluetooth Controller assert handler or system fatal error handler the
+			 * call can be from ISR context, hence there is no thread to yield. Besides
+			 * that both handlers implement a policy to provide error information and
+			 * stop the system in an infinite loop. The goal is to prevent any other
+			 * damage to the system if one of such exeptional situations occur, hence
+			 * call to k_yield is against it.
+			 */
+			if (is_fatal_err) {
+				LOG_ERR("IPC service send error: %d", ret);
+			} else {
+				k_yield();
+			}
 		}
 	} while (ret < 0);
 
@@ -242,13 +270,74 @@ static void hci_rpmsg_send(struct net_buf *buf)
 #if defined(CONFIG_BT_CTLR_ASSERT_HANDLER)
 void bt_ctlr_assert_handle(char *file, uint32_t line)
 {
-	BT_ASSERT_MSG(false, "Controller assert in: %s at %d", file, line);
+#if defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+	/* Disable interrupts, this is unrecoverable */
+	(void)irq_lock();
+
+	/* Generate an error event only when IPC service endpoint is already bound. */
+	if (ipc_ept_ready) {
+		/* Prepare vendor specific HCI debug event */
+		struct net_buf *buf;
+
+		buf = hci_vs_err_assert(file, line);
+		if (buf == NULL) {
+			/* Send the event over rpmsg */
+			hci_rpmsg_send(buf, HCI_FATAL_ERR_MSG);
+		} else {
+			LOG_ERR("Can't create Fatal Error HCI event: %s at %d", __FILE__, __LINE__);
+		}
+	} else {
+		LOG_ERR("IPC endpoint is not redy yet: %s at %d", __FILE__, __LINE__);
+	}
+
+	LOG_ERR("Halting system");
+
+	while (true) {
+	};
+#else
+	LOG_ERR("Controller assert in: %s at %d", file, line);
+#endif /* CONFIG_BT_HCI_VS_FATAL_ERROR */
 }
 #endif /* CONFIG_BT_CTLR_ASSERT_HANDLER */
 
+#if defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+void k_sys_fatal_error_handler(unsigned int reason, const z_arch_esf_t *esf)
+{
+	LOG_PANIC();
+
+	/* Disable interrupts, this is unrecoverable */
+	(void)irq_lock();
+
+	/* Generate an error event only when there is a stack frame and IPC service endpoint is
+	 * already bound.
+	 */
+	if (esf != NULL && ipc_ept_ready) {
+		/* Prepare vendor specific HCI debug event */
+		struct net_buf *buf;
+
+		buf = hci_vs_err_stack_frame(reason, esf);
+		if (buf != NULL) {
+			hci_rpmsg_send(buf, HCI_FATAL_ERR_MSG);
+		} else {
+			LOG_ERR("Can't create Fatal Error HCI event.\n");
+		}
+	}
+
+	LOG_ERR("Halting system");
+
+	while (true) {
+	};
+
+	CODE_UNREACHABLE;
+}
+#endif /* CONFIG_BT_HCI_VS_FATAL_ERROR */
+
 static void hci_ept_bound(void *priv)
 {
 	k_sem_give(&ipc_bound_sem);
+#if defined(CONFIG_BT_CTLR_ASSERT_HANDLER) || defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+	ipc_ept_ready = true;
+#endif /* CONFIG_BT_CTLR_ASSERT_HANDLER || CONFIG_BT_HCI_VS_FATAL_ERROR */
 }
 
 static void hci_ept_recv(const void *data, size_t len, void *priv)
@@ -304,6 +393,6 @@ void main(void)
 		struct net_buf *buf;
 
 		buf = net_buf_get(&rx_queue, K_FOREVER);
-		hci_rpmsg_send(buf);
+		hci_rpmsg_send(buf, HCI_REGULAR_MSG);
 	}
 }

subsys/bluetooth/common/Kconfig

@@ -214,6 +214,14 @@ config BT_HCI_VS_EXT
 	  Host and/or Controller. This enables  Write BD_ADDR, Read Build Info,
 	  Read Static Addresses and Read Key Hierarchy Roots vendor commands.
 
+config BT_HCI_VS_FATAL_ERROR
+	bool "Enable vendor specific HCI event Zephyr Fatal Error"
+	depends on BT_HCI_VS_EXT
+	default n
+	help
+	  Enable emiting HCI Vendor-Specific events for system and Controller error that are
+	  unrecoverable.
+
 config BT_HCI_VS_EXT_DETECT
 	bool "Use heuristics to guess HCI vendor extensions support in advance"
 	depends on BT_HCI_VS_EXT && !BT_CTLR

subsys/bluetooth/controller/hci/hci.c

@@ -94,6 +94,8 @@
 #include "common/log.h"
 #include "hal/debug.h"
 
+#define STR_NULL_TERMINATOR 0x00
+
 /* opcode of the HCI command currently being processed. The opcode is stored
  * by hci_cmd_handle() and then used during the creation of cmd complete and
  * cmd status events to avoid passing it up the call chain.
@@ -4801,6 +4803,155 @@ static void vs_read_tx_power_level(struct net_buf *buf, struct net_buf **evt)
 	rp->handle = sys_cpu_to_le16(handle);
 }
 #endif /* CONFIG_BT_CTLR_TX_PWR_DYNAMIC_CONTROL */
+
+#if defined(CONFIG_BT_HCI_VS_FATAL_ERROR)
+/* A memory pool for vandor specific events for fatal error reporting purposes. */
+NET_BUF_POOL_FIXED_DEFINE(vs_err_tx_pool, 1, BT_BUF_EVT_RX_SIZE, 8, NULL);
+
+/* The alias for convenience of Controller HCI implementation. Controller is build for
+ * a particular architecture hence the alias will allow to avoid conditional compilation.
+ * Host may be not aware of hardware architecture the Controller is working on, hence
+ * all CPU data types for supported architectures should be available during build, hence
+ * the alias is defined here.
+ */
+#if defined(CONFIG_CPU_CORTEX_M)
+typedef struct bt_hci_vs_fata_error_cpu_data_cortex_m bt_hci_vs_fatal_error_cpu_data;
+
+static void vs_err_fatal_cpu_data_fill(bt_hci_vs_fatal_error_cpu_data *cpu_data,
+				       const z_arch_esf_t *esf)
+{
+	cpu_data->a1 = sys_cpu_to_le32(esf->basic.a1);
+	cpu_data->a2 = sys_cpu_to_le32(esf->basic.a2);
+	cpu_data->a3 = sys_cpu_to_le32(esf->basic.a3);
+	cpu_data->a4 = sys_cpu_to_le32(esf->basic.a4);
+	cpu_data->ip = sys_cpu_to_le32(esf->basic.ip);
+	cpu_data->lr = sys_cpu_to_le32(esf->basic.lr);
+	cpu_data->xpsr = sys_cpu_to_le32(esf->basic.xpsr);
+}
+#endif /* CONFIG_CPU_CORTEX_M */
+
+static struct net_buf *vs_err_evt_create(uint8_t subevt, uint8_t len)
+{
+	struct net_buf *buf;
+
+	buf = net_buf_alloc(&vs_err_tx_pool, K_FOREVER);
+	if (buf) {
+		struct bt_hci_evt_le_meta_event *me;
+		struct bt_hci_evt_hdr *hdr;
+
+		net_buf_reserve(buf, BT_BUF_RESERVE);
+		bt_buf_set_type(buf, BT_BUF_EVT);
+
+		hdr = net_buf_add(buf, sizeof(*hdr));
+		hdr->evt = BT_HCI_EVT_VENDOR;
+		hdr->len = len + sizeof(*me);
+
+		me = net_buf_add(buf, sizeof(*me));
+		me->subevent = subevt;
+	}
+
+	return buf;
+}
+
+struct net_buf *hci_vs_err_stack_frame(unsigned int reason, const z_arch_esf_t *esf)
+{
+	/* Prepare vendor specific HCI Fatal Error event */
+	struct bt_hci_vs_fatal_error_stack_frame *sf;
+	bt_hci_vs_fatal_error_cpu_data *cpu_data;
+	struct net_buf *buf;
+
+	buf = vs_err_evt_create(BT_HCI_EVT_VS_ERROR_DATA_TYPE_STACK_FRAME,
+				sizeof(*sf) + sizeof(*cpu_data));
+	if (buf != NULL) {
+		sf = net_buf_add(buf, (sizeof(*sf) + sizeof(*cpu_data)));
+		sf->reason = sys_cpu_to_le32(reason);
+		sf->cpu_type = BT_HCI_EVT_VS_ERROR_CPU_TYPE_CORTEX_M;
+
+		vs_err_fatal_cpu_data_fill(
+			(bt_hci_vs_fatal_error_cpu_data *)sf->cpu_data, esf);
+	} else {
+		BT_ERR("Can't create HCI Fatal Error event");
+	}
+
+	return buf;
+}
+
+static struct net_buf *hci_vs_err_trace_create(uint8_t data_type,
+					       const char *file_path,
+					       uint32_t line, uint64_t pc)
+{
+	uint32_t file_name_len = 0U, pos = 0U;
+	struct net_buf *buf = NULL;
+
+	if (file_path) {
+		/* Extract file name from a path */
+		while (file_path[file_name_len] != '\0') {
+			if (file_path[file_name_len] == '/') {
+				pos = file_name_len + 1;
+			}
+			file_name_len++;
+		}
+		file_path += pos;
+		file_name_len -= pos;
+
+		/* If file name was found in file_path, in other words: file_path is not empty
+		 * string and is not `foo/bar/`.
+		 */
+		if (file_name_len) {
+			/* Total data length: len = file name strlen + \0 + sizeof(line number)
+			 * Maximum length of an HCI event data is BT_BUF_EVT_RX_SIZE. If total data
+			 * length exceeds this maximum, truncate file name.
+			 */
+			uint32_t data_len = 1 + sizeof(line);
+
+			/* If a buffer is created for a TRACE data, include sizeof(pc) in total
+			 * length.
+			 */
+			if (data_type == BT_HCI_EVT_VS_ERROR_DATA_TYPE_TRACE) {
+				data_len += sizeof(pc);
+			}
+
+			if (data_len + file_name_len > BT_BUF_EVT_RX_SIZE) {
+				uint32_t overflow_len =
+					file_name_len + data_len - BT_BUF_EVT_RX_SIZE;
+
+				/* Truncate the file name length by number of overflow bytes */
+				file_name_len -= overflow_len;
+			}
+
+			/* Get total event data length including file name length */
+			data_len += file_name_len;
+
+			/* Prepare vendor specific HCI Fatal Error event */
+			buf = vs_err_evt_create(data_type, data_len);
+			if (buf != NULL) {
+				if (data_type == BT_HCI_EVT_VS_ERROR_DATA_TYPE_TRACE) {
+					net_buf_add_le64(buf, pc);
+				}
+				net_buf_add_mem(buf, file_path, file_name_len);
+				net_buf_add_u8(buf, STR_NULL_TERMINATOR);
+				net_buf_add_le32(buf, line);
+			} else {
+				BT_ERR("Can't create HCI Fatal Error event");
+			}
+		}
+	}
+
+	return buf;
+}
+
+struct net_buf *hci_vs_err_trace(const char *file, uint32_t line, uint64_t pc)
+{
+	return hci_vs_err_trace_create(BT_HCI_EVT_VS_ERROR_DATA_TYPE_TRACE, file, line, pc);
+}
+
+struct net_buf *hci_vs_err_assert(const char *file, uint32_t line)
+{
+	/* ASSERT data does not contain PC counter, because of that zero constant is used */
+	return hci_vs_err_trace_create(BT_HCI_EVT_VS_ERROR_DATA_TYPE_CTRL_ASSERT, file, line, 0U);
+}
+#endif /* CONFIG_BT_HCI_VS_FATAL_ERROR */
+
 #endif /* CONFIG_BT_HCI_VS_EXT */
 
 #if defined(CONFIG_BT_HCI_MESH_EXT)