diff mbox series

[v6,1/6] Bluetooth: btintel: Check firmware version before download

Message ID 20210210201852.2246985-1-luiz.dentz@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v6,1/6] Bluetooth: btintel: Check firmware version before download | expand

Commit Message

Luiz Augusto von Dentz Feb. 10, 2021, 8:18 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This checks the firmware build number, week and year matches with
repective version loaded and then skip the download process.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
v2: Add patch that mover checks for operational mode after the version
checking.
v3: Fix not checking for operation mode before using btintel_read_boot_params
since some models depend on that to contruct the fw filename. Also attempt to
cleanup duplicated code.
v4: Fix forwarding -EALREADY when firmware has already been loaded.
v5: Fix not advancing fw_ptr.
v6: Fix btusb_setup_intel_new_get_fw_name error checking for ddc.

 drivers/bluetooth/btintel.c   | 94 +++++++++++++++++++++++++++--------
 drivers/bluetooth/btintel.h   |  5 +-
 drivers/bluetooth/btusb.c     | 18 ++++++-
 drivers/bluetooth/hci_intel.c |  7 ++-
 4 files changed, 98 insertions(+), 26 deletions(-)

Comments

bluez.test.bot@gmail.com Feb. 10, 2021, 9:09 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=431785

---Test result---

##############################
    Test: CheckPatch - PASS
    

    ##############################
    Test: CheckGitLint - PASS
    

    ##############################
    Test: CheckBuildK - PASS
    

    ##############################
    Test: CheckTestRunner: Setup - PASS
    

    ##############################
    Test: CheckTestRunner: l2cap-tester - PASS
    Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

    ##############################
    Test: CheckTestRunner: bnep-tester - PASS
    Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: mgmt-tester - PASS
    Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

    ##############################
    Test: CheckTestRunner: rfcomm-tester - PASS
    Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: sco-tester - PASS
    Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: smp-tester - PASS
    Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

    ##############################
    Test: CheckTestRunner: userchan-tester - PASS
    Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0

    

---
Regards,
Linux Bluetooth
Tedd Ho-Jeong An Feb. 10, 2021, 11:02 p.m. UTC | #2 
Hi Luiz,

On Wed, 2021-02-10 at 12:18 -0800, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This checks the firmware build number, week and year matches with
> repective version loaded and then skip the download process.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> v2: Add patch that mover checks for operational mode after the version
> checking.
> v3: Fix not checking for operation mode before using btintel_read_boot_params
> since some models depend on that to contruct the fw filename. Also attempt to
> cleanup duplicated code.
> v4: Fix forwarding -EALREADY when firmware has already been loaded.
> v5: Fix not advancing fw_ptr.
> v6: Fix btusb_setup_intel_new_get_fw_name error checking for ddc.
> 
>  drivers/bluetooth/btintel.c   | 94 +++++++++++++++++++++++++++--------
>  drivers/bluetooth/btintel.h   |  5 +-
>  drivers/bluetooth/btusb.c     | 18 ++++++-
>  drivers/bluetooth/hci_intel.c |  7 ++-
>  4 files changed, 98 insertions(+), 26 deletions(-)
> 

I tested with ThP and it worked great as expected. However, when I used
a legacy WsP, it failed to download, and here is the output:

[  682.597921] Bluetooth: hci1: Bootloader revision 0.0 build 26 week 38 2015
[  682.598980] Bluetooth: hci1: Device revision is 16
[  682.598981] Bluetooth: hci1: Secure boot is enabled
[  682.598982] Bluetooth: hci1: OTP lock is enabled
[  682.598983] Bluetooth: hci1: API lock is enabled
[  682.598984] Bluetooth: hci1: Debug lock is disabled
[  682.598985] Bluetooth: hci1: Minimum firmware build 1 week 10 2014
[  682.602706] Bluetooth: hci1: Found device firmware: intel/ibt-12-16.sfi
[  682.602744] Bluetooth: hci1: Boot Address: 0x40800
[  682.602745] Bluetooth: hci1: Firmware Version: 26-38.15
[  682.602745] Bluetooth: hci1: Firmware already loaded
[  682.602829] Bluetooth: hci1: Waiting for device to boot
[  682.608593] usb 1-2: USB disconnect, device number 6
[  683.618723] Bluetooth: hci1: Device boot timeout
[  683.618817] Bluetooth: hci1: sending frame failed (-19)

It looks like the parameter values of "write_boot_params" command in the firmware
file is same as the bootloader version, when it should be the actual firmware
version, and it caused the firmeare downloading to fail.

So, for those legacy device like WsP and SfP, we may need to skip for checking the
version and try not to reload the firmware.

Regards,
Tedd
cold boot
[    3.906251] Bluetooth: Core ver 2.22
[    3.906264] Bluetooth: HCI device and connection manager initialized
[    3.906266] Bluetooth: HCI socket layer initialized
[    3.906269] Bluetooth: L2CAP socket layer initialized
[    3.906270] Bluetooth: SCO socket layer initialized
[    4.060381] Bluetooth: hci0: Bootloader revision 0.1 build 42 week 52 2015
[    4.061651] Bluetooth: hci0: Device revision is 2
[    4.061655] Bluetooth: hci0: Secure boot is enabled
[    4.061656] Bluetooth: hci0: OTP lock is enabled
[    4.061656] Bluetooth: hci0: API lock is enabled
[    4.061657] Bluetooth: hci0: Debug lock is disabled
[    4.061658] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
[    4.082238] Bluetooth: hci0: Found device firmware: intel/ibt-17-16-1.sfi
[    4.082315] Bluetooth: hci0: Boot Address: 0x40800
[    4.082316] Bluetooth: hci0: Firmware Version: 26-11.20
[    5.644601] Bluetooth: hci0: Waiting for firmware download to complete
[    5.645401] Bluetooth: hci0: Firmware loaded in 1526524 usecs
[    5.645421] Bluetooth: hci0: Waiting for device to boot
[    5.658427] Bluetooth: hci0: Device booted in 12709 usecs
[    5.857453] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-17-16-1.ddc
[    5.860426] Bluetooth: hci0: Applying Intel DDC parameters completed
[    5.863385] Bluetooth: hci0: Firmware revision 0.1 build 26 week 11 2020
[    5.921369] Bluetooth: hci0: MSFT filter_enable is already on
[    6.236151] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[    6.236153] Bluetooth: BNEP filters: protocol multicast
[    6.236155] Bluetooth: BNEP socket layer initialized
[   21.400204] Bluetooth: RFCOMM TTY layer initialized
[   21.400208] Bluetooth: RFCOMM socket layer initialized
[   21.400211] Bluetooth: RFCOMM ver 1.11

Reboot
han1@han1-NUC8i7BEH:~$ dmesg | grep Bluetooth
[    3.909093] Bluetooth: Core ver 2.22
[    3.909124] Bluetooth: HCI device and connection manager initialized
[    3.909130] Bluetooth: HCI socket layer initialized
[    3.909132] Bluetooth: L2CAP socket layer initialized
[    3.909135] Bluetooth: SCO socket layer initialized
[    3.992043] Bluetooth: hci0: Firmware revision 0.1 build 26 week 11 2020
[    4.023816] Bluetooth: hci0: Found device firmware: intel/ibt-17-16-1.sfi
[    4.023953] Bluetooth: hci0: Boot Address: 0x40800
[    4.023957] Bluetooth: hci0: Firmware Version: 26-11.20
[    4.023958] Bluetooth: hci0: Firmware already loaded
[    4.082616] Bluetooth: hci0: MSFT filter_enable is already on
[    6.858276] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[    6.858278] Bluetooth: BNEP filters: protocol multicast
[    6.858281] Bluetooth: BNEP socket layer initialized
[    6.962610] Bluetooth: hci0: MSFT filter_enable is already on
[   53.815973] Bluetooth: RFCOMM TTY layer initialized
[   53.815979] Bluetooth: RFCOMM socket layer initialized
[   53.815982] Bluetooth: RFCOMM ver 1.11


New FW + Reboot
[    3.831773] Bluetooth: Core ver 2.22
[    3.831788] Bluetooth: HCI device and connection manager initialized
[    3.831790] Bluetooth: HCI socket layer initialized
[    3.831791] Bluetooth: L2CAP socket layer initialized
[    3.831794] Bluetooth: SCO socket layer initialized
[    4.120498] Bluetooth: hci0: Firmware revision 0.1 build 26 week 11 2020
[    4.128674] Bluetooth: hci0: Found device firmware: intel/ibt-17-16-1.sfi
[    4.128831] Bluetooth: hci0: Boot Address: 0x40800
[    4.128836] Bluetooth: hci0: Firmware Version: 168-48.20
[    6.146311] Bluetooth: hci0: command 0xfc01 tx timeout
[    7.027561] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
[    7.027563] Bluetooth: BNEP filters: protocol multicast
[    7.027568] Bluetooth: BNEP socket layer initialized
[   14.142710] Bluetooth: hci0: FW download error recovery failed (-110)
[   14.569654] Bluetooth: hci0: Bootloader revision 0.1 build 42 week 52 2015
[   14.570665] Bluetooth: hci0: Device revision is 2
[   14.570673] Bluetooth: hci0: Secure boot is enabled
[   14.570678] Bluetooth: hci0: OTP lock is enabled
[   14.570683] Bluetooth: hci0: API lock is enabled
[   14.570688] Bluetooth: hci0: Debug lock is disabled
[   14.570696] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
[   14.571855] Bluetooth: hci0: Found device firmware: intel/ibt-17-16-1.sfi
[   14.571892] Bluetooth: hci0: Boot Address: 0x40800
[   14.571894] Bluetooth: hci0: Firmware Version: 168-48.20
[   16.011471] Bluetooth: hci0: Waiting for firmware download to complete
[   16.011554] Bluetooth: hci0: Firmware loaded in 1405954 usecs
[   16.011576] Bluetooth: hci0: Waiting for device to boot
[   16.025717] Bluetooth: hci0: Device booted in 13807 usecs
[   16.028449] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-17-16-1.ddc
[   16.030655] Bluetooth: hci0: Applying Intel DDC parameters completed
[   16.033762] Bluetooth: hci0: Firmware revision 0.1 build 168 week 48 2020
[   16.095632] Bluetooth: hci0: MSFT filter_enable is already on
[  891.792054] Bluetooth: RFCOMM TTY layer initialized
[  891.792059] Bluetooth: RFCOMM socket layer initialized
[  891.792062] Bluetooth: RFCOMM ver 1.11
diff mbox series

Patch

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 88ce5f0ffc4b..96bca89d1b99 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -24,6 +24,14 @@ 
 #define ECDSA_OFFSET		644
 #define ECDSA_HEADER_LEN	320
 
+#define CMD_WRITE_BOOT_PARAMS	0xfc0e
+struct cmd_write_boot_params {
+	u32 boot_addr;
+	u8  fw_build_num;
+	u8  fw_build_ww;
+	u8  fw_build_yy;
+} __packed;
+
 int btintel_check_bdaddr(struct hci_dev *hdev)
 {
 	struct hci_rp_read_bd_addr *bda;
@@ -841,7 +849,7 @@  static int btintel_sfi_ecdsa_header_secure_send(struct hci_dev *hdev,
 
 static int btintel_download_firmware_payload(struct hci_dev *hdev,
 					     const struct firmware *fw,
-					     u32 *boot_param, size_t offset)
+					     size_t offset)
 {
 	int err;
 	const u8 *fw_ptr;
@@ -854,20 +862,6 @@  static int btintel_download_firmware_payload(struct hci_dev *hdev,
 	while (fw_ptr - fw->data < fw->size) {
 		struct hci_command_hdr *cmd = (void *)(fw_ptr + frag_len);
 
-		/* Each SKU has a different reset parameter to use in the
-		 * HCI_Intel_Reset command and it is embedded in the firmware
-		 * data. So, instead of using static value per SKU, check
-		 * the firmware data and save it for later use.
-		 */
-		if (le16_to_cpu(cmd->opcode) == 0xfc0e) {
-			/* The boot parameter is the first 32-bit value
-			 * and rest of 3 octets are reserved.
-			 */
-			*boot_param = get_unaligned_le32(fw_ptr + sizeof(*cmd));
-
-			bt_dev_dbg(hdev, "boot_param=0x%x", *boot_param);
-		}
-
 		frag_len += sizeof(*cmd) + cmd->plen;
 
 		/* The parameter length of the secure send command requires
@@ -896,28 +890,90 @@  static int btintel_download_firmware_payload(struct hci_dev *hdev,
 	return err;
 }
 
+static bool btintel_firmware_version(struct hci_dev *hdev,
+				     u8 num, u8 ww, u8 yy,
+				     const struct firmware *fw,
+				     u32 *boot_addr)
+{
+	const u8 *fw_ptr;
+
+	fw_ptr = fw->data;
+
+	while (fw_ptr - fw->data < fw->size) {
+		struct hci_command_hdr *cmd = (void *)(fw_ptr);
+
+		/* Each SKU has a different reset parameter to use in the
+		 * HCI_Intel_Reset command and it is embedded in the firmware
+		 * data. So, instead of using static value per SKU, check
+		 * the firmware data and save it for later use.
+		 */
+		if (le16_to_cpu(cmd->opcode) == CMD_WRITE_BOOT_PARAMS) {
+			struct cmd_write_boot_params *params;
+
+			params = (void *)(fw_ptr + sizeof(*cmd));
+
+			bt_dev_info(hdev, "Boot Address: 0x%x",
+				   le32_to_cpu(params->boot_addr));
+
+			bt_dev_info(hdev, "Firmware Version: %u-%u.%u",
+				   params->fw_build_num, params->fw_build_ww,
+				   params->fw_build_yy);
+
+			return (num == params->fw_build_num &&
+				ww == params->fw_build_ww &&
+				yy == params->fw_build_yy);
+		}
+
+		fw_ptr += sizeof(*cmd) + cmd->plen;
+	}
+
+	return false;
+}
+
 int btintel_download_firmware(struct hci_dev *hdev,
+			      struct intel_version *ver,
 			      const struct firmware *fw,
 			      u32 *boot_param)
 {
 	int err;
 
+	/* Skip download if firmware has the same version */
+	if (btintel_firmware_version(hdev, ver->fw_build_num, ver->fw_build_ww,
+				     ver->fw_build_yy, fw, boot_param)) {
+		bt_dev_info(hdev, "Firmware already loaded");
+		/* Return -EALREADY to indicate that the firmware has already
+		 * been loaded.
+		 */
+		return -EALREADY;
+	}
+
 	err = btintel_sfi_rsa_header_secure_send(hdev, fw);
 	if (err)
 		return err;
 
-	return btintel_download_firmware_payload(hdev, fw, boot_param,
-						 RSA_HEADER_LEN);
+	return btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
 }
 EXPORT_SYMBOL_GPL(btintel_download_firmware);
 
 int btintel_download_firmware_newgen(struct hci_dev *hdev,
+				     struct intel_version_tlv *ver,
 				     const struct firmware *fw, u32 *boot_param,
 				     u8 hw_variant, u8 sbe_type)
 {
 	int err;
 	u32 css_header_ver;
 
+	/* Skip download if firmware has the same version */
+	if (btintel_firmware_version(hdev, ver->min_fw_build_nn,
+				     ver->min_fw_build_cw, ver->min_fw_build_yy,
+				     fw, boot_param)) {
+		bt_dev_info(hdev, "Firmware already loaded");
+		/* Return -EALREADY to indicate that firmware has already been
+		 * loaded.
+		 */
+		return -EALREADY;
+	}
+
 	/* iBT hardware variants 0x0b, 0x0c, 0x11, 0x12, 0x13, 0x14 support
 	 * only RSA secure boot engine. Hence, the corresponding sfi file will
 	 * have RSA header of 644 bytes followed by Command Buffer.
@@ -947,7 +1003,7 @@  int btintel_download_firmware_newgen(struct hci_dev *hdev,
 		if (err)
 			return err;
 
-		err = btintel_download_firmware_payload(hdev, fw, boot_param, RSA_HEADER_LEN);
+		err = btintel_download_firmware_payload(hdev, fw, RSA_HEADER_LEN);
 		if (err)
 			return err;
 	} else if (hw_variant >= 0x17) {
@@ -968,7 +1024,6 @@  int btintel_download_firmware_newgen(struct hci_dev *hdev,
 				return err;
 
 			err = btintel_download_firmware_payload(hdev, fw,
-								boot_param,
 								RSA_HEADER_LEN + ECDSA_HEADER_LEN);
 			if (err)
 				return err;
@@ -978,7 +1033,6 @@  int btintel_download_firmware_newgen(struct hci_dev *hdev,
 				return err;
 
 			err = btintel_download_firmware_payload(hdev, fw,
-								boot_param,
 								RSA_HEADER_LEN + ECDSA_HEADER_LEN);
 			if (err)
 				return err;
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 6511b091caf5..51f1f2c883b4 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -163,9 +163,10 @@  struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
 int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param);
 int btintel_read_boot_params(struct hci_dev *hdev,
 			     struct intel_boot_params *params);
-int btintel_download_firmware(struct hci_dev *dev, const struct firmware *fw,
-			      u32 *boot_param);
+int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
+			      const struct firmware *fw, u32 *boot_param);
 int btintel_download_firmware_newgen(struct hci_dev *hdev,
+				     struct intel_version_tlv *ver,
 				     const struct firmware *fw,
 				     u32 *boot_param, u8 hw_variant,
 				     u8 sbe_type);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 66ada8217797..c92060e7472c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2623,10 +2623,17 @@  static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
 	set_bit(BTUSB_DOWNLOADING, &data->flags);
 
 	/* Start firmware downloading and get boot parameter */
-	err = btintel_download_firmware_newgen(hdev, fw, boot_param,
+	err = btintel_download_firmware_newgen(hdev, ver, fw, boot_param,
 					       INTEL_HW_VARIANT(ver->cnvi_bt),
 					       ver->sbe_type);
 	if (err < 0) {
+		if (err == -EALREADY) {
+			/* Firmware has already been loaded */
+			set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
+			err = 0;
+			goto done;
+		}
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
@@ -2817,8 +2824,15 @@  static int btusb_intel_download_firmware(struct hci_dev *hdev,
 	set_bit(BTUSB_DOWNLOADING, &data->flags);
 
 	/* Start firmware downloading and get boot parameter */
-	err = btintel_download_firmware(hdev, fw, boot_param);
+	err = btintel_download_firmware(hdev, ver, fw, boot_param);
 	if (err < 0) {
+		if (err == -EALREADY) {
+			/* Firmware has already been loaded */
+			set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
+			err = 0;
+			goto done;
+		}
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index b20a40fab83e..7249b91d9b91 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -735,7 +735,7 @@  static int intel_setup(struct hci_uart *hu)
 	set_bit(STATE_DOWNLOADING, &intel->flags);
 
 	/* Start firmware downloading and get boot parameter */
-	err = btintel_download_firmware(hdev, fw, &boot_param);
+	err = btintel_download_firmware(hdev, &ver, fw, &boot_param);
 	if (err < 0)
 		goto done;
 
@@ -784,7 +784,10 @@  static int intel_setup(struct hci_uart *hu)
 done:
 	release_firmware(fw);
 
-	if (err < 0)
+	/* Check if there was an error and if is not -EALREADY which means the
+	 * firmware has already been loaded.
+	 */
+	if (err < 0 && err != -EALREADY)
 		return err;
 
 	/* We need to restore the default speed before Intel reset */