linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot
@ 2024-02-03  0:32 Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 1/4] wifi: rtw89: fw: consider checksum length of security data Ping-Ke Shih
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-03  0:32 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

Firmware secure boot is to ensure firmware running on chip is the one
which is signed and released officially. Without this, it will be failed
to download firmware result from wrong security key data.

The main difference between secure and non-secure boot is content of
secure section, which is one type of many firmware sections, and is to
provide key data.

The basic steps for key data are:
 1. read cryptography method and key_index from efuse (patch 2/4)
 2. parse firmware file to select secure section by the information of
    step 1 (patch 3/4)
 3. download firmware with selected secure section and key data (patch 4/4)

Ping-Ke Shih (4):
  wifi: rtw89: fw: consider checksum length of security data
  wifi: rtw89: fw: read firmware secure information from efuse
  wifi: rtw89: fw: parse secure section from firmware file
  wifi: rtw89: fw: download firmware with key data for secure boot

 drivers/net/wireless/realtek/rtw89/core.h     |  15 +
 drivers/net/wireless/realtek/rtw89/efuse.h    |   1 +
 drivers/net/wireless/realtek/rtw89/efuse_be.c | 142 +++++++++
 drivers/net/wireless/realtek/rtw89/fw.c       | 296 ++++++++++++++++--
 drivers/net/wireless/realtek/rtw89/fw.h       |  47 ++-
 drivers/net/wireless/realtek/rtw89/pci.c      |   2 +
 drivers/net/wireless/realtek/rtw89/rtw8922a.c |   3 +
 7 files changed, 484 insertions(+), 22 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] wifi: rtw89: fw: consider checksum length of security data
  2024-02-03  0:32 [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot Ping-Ke Shih
@ 2024-02-03  0:32 ` Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 2/4] wifi: rtw89: fw: read firmware secure information from efuse Ping-Ke Shih
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-03  0:32 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

The newer firmware file provides security data with checksum, so we need to
consider the length. Otherwise it will fail to validate total firmware
length resulting in failed to probe.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/fw.c | 3 +++
 drivers/net/wireless/realtek/rtw89/fw.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 2f3f2b503507..00417364ab22 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -177,6 +177,7 @@ static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 	u32 i;
 
 	info->section_num = le32_get_bits(fw_hdr->w6, FW_HDR_V1_W6_SEC_NUM);
+	info->dsp_checksum = le32_get_bits(fw_hdr->w6, FW_HDR_V1_W6_DSP_CHKSUM);
 	base_hdr_len = struct_size(fw_hdr, sections, info->section_num);
 	info->dynamic_hdr_en = le32_get_bits(fw_hdr->w7, FW_HDR_V1_W7_DYN_HDR);
 
@@ -205,6 +206,8 @@ static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 			section_info->mssc =
 				le32_get_bits(section->w2, FWSECTION_HDR_V1_W2_MSSC);
 			mssc_len += section_info->mssc * FWDL_SECURITY_SIGLEN;
+			if (info->dsp_checksum)
+				mssc_len += section_info->mssc * FWDL_SECURITY_CHKSUM_LEN;
 		} else {
 			section_info->mssc = 0;
 		}
diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
index ae69e455cd64..5b536c2e365d 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.h
+++ b/drivers/net/wireless/realtek/rtw89/fw.h
@@ -237,6 +237,7 @@ struct rtw89_fw_bin_info {
 	u32 hdr_len;
 	bool dynamic_hdr_en;
 	u32 dynamic_hdr_len;
+	bool dsp_checksum;
 	struct rtw89_fw_hdr_section_info section_info[FWDL_SECTION_MAX_NUM];
 };
 
@@ -466,6 +467,7 @@ static inline void RTW89_SET_EDCA_PARAM(void *cmd, u32 val)
 
 #define FWDL_SECURITY_SECTION_TYPE 9
 #define FWDL_SECURITY_SIGLEN 512
+#define FWDL_SECURITY_CHKSUM_LEN 8
 
 struct rtw89_fw_dynhdr_sec {
 	__le32 w0;
@@ -568,6 +570,7 @@ struct rtw89_fw_hdr_v1 {
 #define FW_HDR_V1_W5_YEAR GENMASK(15, 0)
 #define FW_HDR_V1_W5_HDR_SIZE GENMASK(31, 16)
 #define FW_HDR_V1_W6_SEC_NUM GENMASK(15, 8)
+#define FW_HDR_V1_W6_DSP_CHKSUM BIT(24)
 #define FW_HDR_V1_W7_DYN_HDR BIT(16)
 
 static inline void SET_FW_HDR_PART_SIZE(void *fwhdr, u32 val)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] wifi: rtw89: fw: read firmware secure information from efuse
  2024-02-03  0:32 [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 1/4] wifi: rtw89: fw: consider checksum length of security data Ping-Ke Shih
@ 2024-02-03  0:32 ` Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 4/4] wifi: rtw89: fw: download firmware with key data for secure boot Ping-Ke Shih
  3 siblings, 0 replies; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-03  0:32 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

To support firmware secure boot, read secure information from efuse to
know if current hardware module can support secure boot with certain
cryptography method.

This information should be prepared before downloading firmware, so read
efuse right after power on at probe stage. The secure information includes
secure cryptography method and secure key index that are used to choose
proper key material when downloading firmware.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.h     |  15 ++
 drivers/net/wireless/realtek/rtw89/efuse.h    |   1 +
 drivers/net/wireless/realtek/rtw89/efuse_be.c | 142 ++++++++++++++++++
 drivers/net/wireless/realtek/rtw89/pci.c      |   2 +
 drivers/net/wireless/realtek/rtw89/rtw8922a.c |   3 +
 5 files changed, 163 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index 30cc77ac78c5..8bbea97c3b1c 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -4030,6 +4030,19 @@ struct rtw89_fw_elm_info {
 	struct rtw89_phy_rfk_log_fmt *rfk_log_fmt;
 };
 
+enum rtw89_fw_mss_dev_type {
+	RTW89_FW_MSS_DEV_TYPE_FWSEC_DEF = 0xF,
+	RTW89_FW_MSS_DEV_TYPE_FWSEC_INV = 0xFF,
+};
+
+struct rtw89_fw_secure {
+	bool secure_boot;
+	u32 sb_sel_mgn;
+	u8 mss_dev_type;
+	u8 mss_cust_idx;
+	u8 mss_key_num;
+};
+
 struct rtw89_fw_info {
 	struct rtw89_fw_req_info req;
 	int fw_format;
@@ -4044,6 +4057,7 @@ struct rtw89_fw_info {
 	struct rtw89_fw_log log;
 	u32 feature_map;
 	struct rtw89_fw_elm_info elm_info;
+	struct rtw89_fw_secure sec;
 };
 
 #define RTW89_CHK_FW_FEATURE(_feat, _fw) \
@@ -4196,6 +4210,7 @@ enum rtw89_flags {
 	RTW89_FLAG_CMAC1_FUNC,
 	RTW89_FLAG_FW_RDY,
 	RTW89_FLAG_RUNNING,
+	RTW89_FLAG_PROBE_DONE,
 	RTW89_FLAG_BFEE_MON,
 	RTW89_FLAG_BFEE_EN,
 	RTW89_FLAG_BFEE_TIMER_KEEP,
diff --git a/drivers/net/wireless/realtek/rtw89/efuse.h b/drivers/net/wireless/realtek/rtw89/efuse.h
index 5c6787179bad..72416f56a071 100644
--- a/drivers/net/wireless/realtek/rtw89/efuse.h
+++ b/drivers/net/wireless/realtek/rtw89/efuse.h
@@ -23,5 +23,6 @@ int rtw89_parse_efuse_map_be(struct rtw89_dev *rtwdev);
 int rtw89_parse_phycap_map_be(struct rtw89_dev *rtwdev);
 int rtw89_cnv_efuse_state_be(struct rtw89_dev *rtwdev, bool idle);
 int rtw89_read_efuse_ver(struct rtw89_dev *rtwdev, u8 *efv);
+int rtw89_efuse_read_fw_secure_be(struct rtw89_dev *rtwdev);
 
 #endif
diff --git a/drivers/net/wireless/realtek/rtw89/efuse_be.c b/drivers/net/wireless/realtek/rtw89/efuse_be.c
index 8e8b7cd315f7..0be26d5fdf7c 100644
--- a/drivers/net/wireless/realtek/rtw89/efuse_be.c
+++ b/drivers/net/wireless/realtek/rtw89/efuse_be.c
@@ -7,6 +7,31 @@
 #include "mac.h"
 #include "reg.h"
 
+#define EFUSE_EXTERNALPN_ADDR_BE 0x1580
+#define EFUSE_B1_MSSDEVTYPE_MASK GENMASK(3, 0)
+#define EFUSE_B1_MSSCUSTIDX0_MASK GENMASK(7, 4)
+#define EFUSE_SERIALNUM_ADDR_BE 0x1581
+#define EFUSE_B2_MSSKEYNUM_MASK GENMASK(3, 0)
+#define EFUSE_B2_MSSCUSTIDX1_MASK BIT(6)
+#define EFUSE_SB_CRYP_SEL_ADDR 0x1582
+#define EFUSE_SB_CRYP_SEL_SIZE 2
+#define EFUSE_SB_CRYP_SEL_DEFAULT 0xFFFF
+#define SB_SEL_MGN_MAX_SIZE 2
+#define EFUSE_SEC_BE_START 0x1580
+#define EFUSE_SEC_BE_SIZE 4
+
+enum rtw89_efuse_mss_dev_type {
+	MSS_DEV_TYPE_FWSEC_DEF = 0xF,
+	MSS_DEV_TYPE_FWSEC_WINLIN_INBOX = 0xC,
+	MSS_DEV_TYPE_FWSEC_NONLIN_INBOX_NON_COB = 0xA,
+	MSS_DEV_TYPE_FWSEC_NONLIN_INBOX_COB = 0x9,
+	MSS_DEV_TYPE_FWSEC_NONWIN_INBOX = 0x6,
+};
+
+static const u32 sb_sel_mgn[SB_SEL_MGN_MAX_SIZE] = {
+	0x8000100, 0xC000180
+};
+
 static void rtw89_enable_efuse_pwr_cut_ddv_be(struct rtw89_dev *rtwdev)
 {
 	const struct rtw89_chip_info *chip = rtwdev->chip;
@@ -418,3 +443,120 @@ int rtw89_parse_phycap_map_be(struct rtw89_dev *rtwdev)
 
 	return ret;
 }
+
+static u16 get_sb_cryp_sel_idx(u16 sb_cryp_sel)
+{
+	u8 low_bit, high_bit, cnt_zero = 0;
+	u8 idx, sel_form_v, sel_idx_v;
+	u16 sb_cryp_sel_v = 0x0;
+
+	sel_form_v = u16_get_bits(sb_cryp_sel, MASKBYTE0);
+	sel_idx_v = u16_get_bits(sb_cryp_sel, MASKBYTE1);
+
+	for (idx = 0; idx < 4; idx++) {
+		low_bit = !!(sel_form_v & BIT(idx));
+		high_bit = !!(sel_form_v & BIT(7 - idx));
+		if (low_bit != high_bit)
+			return U16_MAX;
+		if (low_bit)
+			continue;
+
+		cnt_zero++;
+		if (cnt_zero == 1)
+			sb_cryp_sel_v = idx * 16;
+		else if (cnt_zero > 1)
+			return U16_MAX;
+	}
+
+	low_bit = u8_get_bits(sel_idx_v, 0x0F);
+	high_bit = u8_get_bits(sel_idx_v, 0xF0);
+
+	if ((low_bit ^ high_bit) != 0xF)
+		return U16_MAX;
+
+	return sb_cryp_sel_v + low_bit;
+}
+
+static u8 get_mss_dev_type_idx(struct rtw89_dev *rtwdev, u8 mss_dev_type)
+{
+	switch (mss_dev_type) {
+	case MSS_DEV_TYPE_FWSEC_WINLIN_INBOX:
+		mss_dev_type = 0x0;
+		break;
+	case MSS_DEV_TYPE_FWSEC_NONLIN_INBOX_NON_COB:
+		mss_dev_type = 0x1;
+		break;
+	case MSS_DEV_TYPE_FWSEC_NONLIN_INBOX_COB:
+		mss_dev_type = 0x2;
+		break;
+	case MSS_DEV_TYPE_FWSEC_NONWIN_INBOX:
+		mss_dev_type = 0x3;
+		break;
+	case MSS_DEV_TYPE_FWSEC_DEF:
+		mss_dev_type = RTW89_FW_MSS_DEV_TYPE_FWSEC_DEF;
+		break;
+	default:
+		rtw89_warn(rtwdev, "unknown mss_dev_type %d", mss_dev_type);
+		mss_dev_type = RTW89_FW_MSS_DEV_TYPE_FWSEC_INV;
+		break;
+	}
+
+	return mss_dev_type;
+}
+
+int rtw89_efuse_read_fw_secure_be(struct rtw89_dev *rtwdev)
+{
+	struct rtw89_fw_secure *sec = &rtwdev->fw.sec;
+	u32 sec_addr = EFUSE_SEC_BE_START;
+	u32 sec_size = EFUSE_SEC_BE_SIZE;
+	u16 sb_cryp_sel, sb_cryp_sel_idx;
+	u8 sec_map[EFUSE_SEC_BE_SIZE];
+	u8 mss_dev_type;
+	u8 b1, b2;
+	int ret;
+
+	ret = rtw89_dump_physical_efuse_map_be(rtwdev, sec_map,
+					       sec_addr, sec_size, false);
+	if (ret) {
+		rtw89_warn(rtwdev, "failed to dump secsel map\n");
+		return ret;
+	}
+
+	sb_cryp_sel = sec_map[EFUSE_SB_CRYP_SEL_ADDR - sec_addr] |
+		      sec_map[EFUSE_SB_CRYP_SEL_ADDR - sec_addr + 1] << 8;
+	if (sb_cryp_sel == EFUSE_SB_CRYP_SEL_DEFAULT)
+		goto out;
+
+	sb_cryp_sel_idx = get_sb_cryp_sel_idx(sb_cryp_sel);
+	if (sb_cryp_sel_idx >= SB_SEL_MGN_MAX_SIZE) {
+		rtw89_warn(rtwdev, "invalid SB cryp sel idx %d\n", sb_cryp_sel_idx);
+		goto out;
+	}
+
+	sec->sb_sel_mgn = sb_sel_mgn[sb_cryp_sel_idx];
+
+	b1 = sec_map[EFUSE_EXTERNALPN_ADDR_BE - sec_addr];
+	b2 = sec_map[EFUSE_SERIALNUM_ADDR_BE - sec_addr];
+
+	mss_dev_type = u8_get_bits(b1, EFUSE_B1_MSSDEVTYPE_MASK);
+	sec->mss_cust_idx = 0x1F - (u8_get_bits(b1, EFUSE_B1_MSSCUSTIDX0_MASK) |
+				    u8_get_bits(b2, EFUSE_B2_MSSCUSTIDX1_MASK) << 4);
+	sec->mss_key_num = 0xF - u8_get_bits(b2, EFUSE_B2_MSSKEYNUM_MASK);
+
+	sec->mss_dev_type = get_mss_dev_type_idx(rtwdev, mss_dev_type);
+	if (sec->mss_dev_type == RTW89_FW_MSS_DEV_TYPE_FWSEC_INV) {
+		rtw89_warn(rtwdev, "invalid mss_dev_type %d\n", mss_dev_type);
+		goto out;
+	}
+
+	sec->secure_boot = true;
+
+out:
+	rtw89_debug(rtwdev, RTW89_DBG_FW,
+		    "MSS secure_boot=%d dev_type=%d cust_idx=%d key_num=%d\n",
+		    sec->secure_boot, sec->mss_dev_type, sec->mss_cust_idx,
+		    sec->mss_key_num);
+
+	return 0;
+}
+EXPORT_SYMBOL(rtw89_efuse_read_fw_secure_be);
diff --git a/drivers/net/wireless/realtek/rtw89/pci.c b/drivers/net/wireless/realtek/rtw89/pci.c
index b51ec3cbc715..67d7294e488a 100644
--- a/drivers/net/wireless/realtek/rtw89/pci.c
+++ b/drivers/net/wireless/realtek/rtw89/pci.c
@@ -4180,6 +4180,8 @@ int rtw89_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_free_irq;
 	}
 
+	set_bit(RTW89_FLAG_PROBE_DONE, rtwdev->flags);
+
 	return 0;
 
 err_free_irq:
diff --git a/drivers/net/wireless/realtek/rtw89/rtw8922a.c b/drivers/net/wireless/realtek/rtw89/rtw8922a.c
index aefad3f2e612..c9e50fc38083 100644
--- a/drivers/net/wireless/realtek/rtw89/rtw8922a.c
+++ b/drivers/net/wireless/realtek/rtw89/rtw8922a.c
@@ -376,6 +376,9 @@ static int rtw8922a_pwr_on_func(struct rtw89_dev *rtwdev)
 	rtw89_write32_set(rtwdev, R_BE_FEN_RST_ENABLE, B_BE_FEN_BB_IP_RSTN |
 						       B_BE_FEN_BBPLAT_RSTB);
 
+	if (!test_bit(RTW89_FLAG_PROBE_DONE, rtwdev->flags))
+		rtw89_efuse_read_fw_secure_be(rtwdev);
+
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file
  2024-02-03  0:32 [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 1/4] wifi: rtw89: fw: consider checksum length of security data Ping-Ke Shih
  2024-02-03  0:32 ` [PATCH 2/4] wifi: rtw89: fw: read firmware secure information from efuse Ping-Ke Shih
@ 2024-02-03  0:32 ` Ping-Ke Shih
  2024-02-03 19:22   ` kernel test robot
  2024-02-03  0:32 ` [PATCH 4/4] wifi: rtw89: fw: download firmware with key data for secure boot Ping-Ke Shih
  3 siblings, 1 reply; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-03  0:32 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

A firmware file can contains more than one section with secure type, so
use secure information from efuse to choose the one with matched
cryptography method. Then choose key data from MSS poll (multiple security
section pool; see below picture) according to key_index from efuse.

Note that the size of MSS pool isn't included in section size defined
in firmware header, so we also need to parse header of MSS pool to get
its size as shift to parse next section.

If secure boot isn't supported by current hardware, the first secure
section will be adopted, and no need additional process to key data.

  +---------------------------+
  |      firmware header      |
  |                           |
  | +-----------------------+ |
  | | section type/size * N-|-|-------+
  | | ...                   | |       |
  | +-----------------------+ |       |
  +---------------------------+       |
  :                           :       |
  +---------------------------+ -\    |
  | secure section type (ID:9)|  |    |
  |                           |  | <--+
  |                           |  |
  +---------------------------+ -/
  |MSS Pool for above section |
  |                           |
  |                           |
  +---------------------------+

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/fw.c | 202 ++++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw89/fw.h |  39 +++++
 2 files changed, 227 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 00417364ab22..4f648071a5be 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -13,6 +13,8 @@
 #include "reg.h"
 #include "util.h"
 
+static const u8 mss_signature[] = {0x4D, 0x53, 0x53, 0x4B, 0x50, 0x4F, 0x4F, 0x4C};
+
 union rtw89_fw_element_arg {
 	size_t offset;
 	enum rtw89_rf_path rf_path;
@@ -163,6 +165,161 @@ static int rtw89_fw_hdr_parser_v0(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 	return 0;
 }
 
+static int __get_mssc_key_idx(struct rtw89_dev *rtwdev,
+			      const struct rtw89_fw_mss_pool_hdr *mss_hdr,
+			      u32 rmp_tbl_size, u32 *key_idx)
+{
+	struct rtw89_fw_secure *sec = &rtwdev->fw.sec;
+	u32 sel_byte_idx;
+	u32 mss_sel_idx;
+	u8 sel_bit_idx;
+	int i;
+
+	if (sec->mss_dev_type == RTW89_FW_MSS_DEV_TYPE_FWSEC_DEF) {
+		if (!mss_hdr->defen)
+			return -ENOENT;
+
+		mss_sel_idx = sec->mss_cust_idx * le16_to_cpu(mss_hdr->msskey_num_max) +
+			      sec->mss_key_num;
+	} else {
+		if (mss_hdr->defen)
+			mss_sel_idx = FWDL_MSS_POOL_DEFKEYSETS_SIZE << 3;
+		else
+			mss_sel_idx = 0;
+		mss_sel_idx += sec->mss_dev_type * le16_to_cpu(mss_hdr->msskey_num_max) *
+						   le16_to_cpu(mss_hdr->msscust_max) +
+			       sec->mss_cust_idx * le16_to_cpu(mss_hdr->msskey_num_max) +
+			       sec->mss_key_num;
+	}
+
+	sel_byte_idx = mss_sel_idx >> 3;
+	sel_bit_idx = mss_sel_idx & 0x7;
+
+	if (sel_byte_idx >= rmp_tbl_size)
+		return -EFAULT;
+
+	if (!(mss_hdr->rmp_tbl[sel_byte_idx] & BIT(sel_bit_idx)))
+		return -ENOENT;
+
+	*key_idx = hweight8(mss_hdr->rmp_tbl[sel_byte_idx] & (BIT(sel_bit_idx) - 1));
+
+	for (i = 0; i < sel_byte_idx; i++)
+		*key_idx += hweight8(mss_hdr->rmp_tbl[i]);
+
+	return 0;
+}
+
+static int __parse_formatted_mssc(struct rtw89_dev *rtwdev,
+				  struct rtw89_fw_bin_info *info,
+				  struct rtw89_fw_hdr_section_info *section_info,
+				  const struct rtw89_fw_hdr_section_v1 *section,
+				  const void *content,
+				  u32 *mssc_len)
+{
+	const struct rtw89_fw_mss_pool_hdr *mss_hdr = content + section_info->len;
+	const union rtw89_fw_section_mssc_content *section_content = content;
+	struct rtw89_fw_secure *sec = &rtwdev->fw.sec;
+	u32 rmp_tbl_size;
+	u32 key_sign_len;
+	u32 real_key_idx;
+	u32 sb_sel_ver;
+	int ret;
+
+	if (memcmp(mss_signature, mss_hdr->signature, sizeof(mss_signature)) != 0) {
+		rtw89_err(rtwdev, "[ERR] wrong MSS signature\n");
+		return -ENOENT;
+	}
+
+	if (mss_hdr->rmpfmt == MSS_POOL_RMP_TBL_BITMASK) {
+		rmp_tbl_size = (le16_to_cpu(mss_hdr->msskey_num_max) *
+				le16_to_cpu(mss_hdr->msscust_max) *
+				mss_hdr->mssdev_max) >> 3;
+		if (mss_hdr->defen)
+			rmp_tbl_size += FWDL_MSS_POOL_DEFKEYSETS_SIZE;
+	} else {
+		rtw89_err(rtwdev, "[ERR] MSS Key Pool Remap Table Format Unsupport:%X\n",
+			  mss_hdr->rmpfmt);
+		return -EINVAL;
+	}
+
+	if (rmp_tbl_size + sizeof(*mss_hdr) != le32_to_cpu(mss_hdr->key_raw_offset)) {
+		rtw89_err(rtwdev, "[ERR] MSS Key Pool Format Error:0x%X + 0x%X != 0x%X\n",
+			  rmp_tbl_size, (int)sizeof(*mss_hdr),
+			  le32_to_cpu(mss_hdr->key_raw_offset));
+		return -EINVAL;
+	}
+
+	key_sign_len = le16_to_cpu(section_content->key_sign_len.v) >> 2;
+	if (!key_sign_len)
+		key_sign_len = 512;
+
+	if (info->dsp_checksum)
+		key_sign_len += FWDL_SECURITY_CHKSUM_LEN;
+
+	*mssc_len = sizeof(*mss_hdr) + rmp_tbl_size +
+		    le16_to_cpu(mss_hdr->keypair_num) * key_sign_len;
+
+	if (!sec->secure_boot)
+		goto out;
+
+	sb_sel_ver = le32_to_cpu(section_content->sb_sel_ver.v);
+	if (sb_sel_ver && sb_sel_ver != sec->sb_sel_mgn)
+		goto ignore;
+
+	ret = __get_mssc_key_idx(rtwdev, mss_hdr, rmp_tbl_size, &real_key_idx);
+	if (ret)
+		goto ignore;
+
+	section_info->key_addr = content + section_info->len +
+				le32_to_cpu(mss_hdr->key_raw_offset) +
+				key_sign_len * real_key_idx;
+	section_info->key_len = key_sign_len;
+	section_info->key_idx = real_key_idx;
+
+out:
+	if (info->secure_section_exist) {
+		section_info->ignore = true;
+		return 0;
+	}
+
+	info->secure_section_exist = true;
+
+	return 0;
+
+ignore:
+	section_info->ignore = true;
+
+	return 0;
+}
+
+static int __parse_security_section(struct rtw89_dev *rtwdev,
+				    struct rtw89_fw_bin_info *info,
+				    struct rtw89_fw_hdr_section_info *section_info,
+				    const struct rtw89_fw_hdr_section_v1 *section,
+				    const void *content,
+				    u32 *mssc_len)
+{
+	int ret;
+
+	section_info->mssc =
+		le32_get_bits(section->w2, FWSECTION_HDR_V1_W2_MSSC);
+
+	if (section_info->mssc == FORMATTED_MSSC) {
+		ret = __parse_formatted_mssc(rtwdev, info, section_info,
+					     section, content, mssc_len);
+		if (ret)
+			return -EINVAL;
+	} else {
+		*mssc_len = section_info->mssc * FWDL_SECURITY_SIGLEN;
+		if (info->dsp_checksum)
+			*mssc_len += section_info->mssc * FWDL_SECURITY_CHKSUM_LEN;
+
+		info->secure_section_exist = true;
+	}
+
+	return 0;
+}
+
 static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 len,
 				  struct rtw89_fw_bin_info *info)
 {
@@ -173,7 +330,8 @@ static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 	const u8 *fw_end = fw + len;
 	const u8 *bin;
 	u32 base_hdr_len;
-	u32 mssc_len = 0;
+	u32 mssc_len;
+	int ret;
 	u32 i;
 
 	info->section_num = le32_get_bits(fw_hdr->w6, FW_HDR_V1_W6_SEC_NUM);
@@ -200,18 +358,9 @@ static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 	section_info = info->section_info;
 	for (i = 0; i < info->section_num; i++) {
 		section = &fw_hdr->sections[i];
+
 		section_info->type =
 			le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_SECTIONTYPE);
-		if (section_info->type == FWDL_SECURITY_SECTION_TYPE) {
-			section_info->mssc =
-				le32_get_bits(section->w2, FWSECTION_HDR_V1_W2_MSSC);
-			mssc_len += section_info->mssc * FWDL_SECURITY_SIGLEN;
-			if (info->dsp_checksum)
-				mssc_len += section_info->mssc * FWDL_SECURITY_CHKSUM_LEN;
-		} else {
-			section_info->mssc = 0;
-		}
-
 		section_info->len =
 			le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_SEC_SIZE);
 		if (le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_CHECKSUM))
@@ -220,15 +369,40 @@ static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 le
 		section_info->dladdr =
 			le32_get_bits(section->w0, FWSECTION_HDR_V1_W0_DL_ADDR);
 		section_info->addr = bin;
-		bin += section_info->len;
+
+		if (section_info->type == FWDL_SECURITY_SECTION_TYPE) {
+			ret = __parse_security_section(rtwdev, info, section_info,
+						       section, bin, &mssc_len);
+			if (ret)
+				return ret;
+		} else {
+			section_info->mssc = 0;
+			mssc_len = 0;
+		}
+
+		rtw89_debug(rtwdev, RTW89_DBG_FW,
+			    "section[%d] type=%d len=0x%-6x mssc=%d mssc_len=%d addr=%lx\n",
+			    i, section_info->type, section_info->len,
+			    section_info->mssc, mssc_len, bin - fw);
+		rtw89_debug(rtwdev, RTW89_DBG_FW,
+			    "           ignore=%d key_addr=%p (0x%lx) key_len=%d key_idx=%d\n",
+			    section_info->ignore, section_info->key_addr,
+			    section_info->key_addr ?
+			    section_info->key_addr - section_info->addr : 0,
+			    section_info->key_len, section_info->key_idx);
+
+		bin += section_info->len + mssc_len;
 		section_info++;
 	}
 
-	if (fw_end != bin + mssc_len) {
+	if (fw_end != bin) {
 		rtw89_err(rtwdev, "[ERR]fw bin size\n");
 		return -EINVAL;
 	}
 
+	if (!info->secure_section_exist)
+		rtw89_warn(rtwdev, "no firmware secure section\n");
+
 	return 0;
 }
 
@@ -1106,7 +1280,7 @@ static int rtw89_fw_download_suit(struct rtw89_dev *rtwdev,
 				  struct rtw89_fw_suit *fw_suit)
 {
 	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
-	struct rtw89_fw_bin_info info;
+	struct rtw89_fw_bin_info info = {};
 	int ret;
 
 	ret = rtw89_fw_hdr_parser(rtwdev, fw_suit, &info);
diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
index 5b536c2e365d..c05ddb0d5900 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.h
+++ b/drivers/net/wireless/realtek/rtw89/fw.h
@@ -230,6 +230,10 @@ struct rtw89_fw_hdr_section_info {
 	u32 dladdr;
 	u32 mssc;
 	u8 type;
+	bool ignore;
+	const u8 *key_addr;
+	u32 key_len;
+	u32 key_idx;
 };
 
 struct rtw89_fw_bin_info {
@@ -238,6 +242,7 @@ struct rtw89_fw_bin_info {
 	bool dynamic_hdr_en;
 	u32 dynamic_hdr_len;
 	bool dsp_checksum;
+	bool secure_section_exist;
 	struct rtw89_fw_hdr_section_info section_info[FWDL_SECTION_MAX_NUM];
 };
 
@@ -538,6 +543,7 @@ struct rtw89_fw_hdr_section_v1 {
 #define FWSECTION_HDR_V1_W1_CHECKSUM BIT(28)
 #define FWSECTION_HDR_V1_W1_REDL BIT(29)
 #define FWSECTION_HDR_V1_W2_MSSC GENMASK(7, 0)
+#define FORMATTED_MSSC 0xFF
 #define FWSECTION_HDR_V1_W2_BBMCU_IDX GENMASK(27, 24)
 
 struct rtw89_fw_hdr_v1 {
@@ -578,6 +584,39 @@ static inline void SET_FW_HDR_PART_SIZE(void *fwhdr, u32 val)
 	le32p_replace_bits((__le32 *)fwhdr + 7, val, GENMASK(15, 0));
 }
 
+enum rtw89_fw_mss_pool_rmp_tbl_type {
+	MSS_POOL_RMP_TBL_BITMASK = 0x0,
+	MSS_POOL_RMP_TBL_RECORD = 0x1,
+};
+
+#define FWDL_MSS_POOL_DEFKEYSETS_SIZE 8
+
+struct rtw89_fw_mss_pool_hdr {
+	u8 signature[8]; /* equal to mss_signature[] */
+	__le32 rmp_tbl_offset;
+	__le32 key_raw_offset;
+	u8 defen;
+	u8 rsvd[3];
+	u8 rmpfmt; /* enum rtw89_fw_mss_pool_rmp_tbl_type */
+	u8 mssdev_max;
+	__le16 keypair_num;
+	__le16 msscust_max;
+	__le16 msskey_num_max;
+	__le32 rsvd3;
+	u8 rmp_tbl[];
+} __packed;
+
+union rtw89_fw_section_mssc_content {
+	struct {
+		u8 pad[58];
+		__le32 v;
+	} __packed sb_sel_ver;
+	struct {
+		u8 pad[60];
+		__le16 v;
+	} __packed key_sign_len;
+} __packed;
+
 static inline void SET_CTRL_INFO_MACID(void *table, u32 val)
 {
 	le32p_replace_bits((__le32 *)(table) + 0, val, GENMASK(6, 0));
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] wifi: rtw89: fw: download firmware with key data for secure boot
  2024-02-03  0:32 [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot Ping-Ke Shih
                   ` (2 preceding siblings ...)
  2024-02-03  0:32 ` [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file Ping-Ke Shih
@ 2024-02-03  0:32 ` Ping-Ke Shih
  3 siblings, 0 replies; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-03  0:32 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless

Since firmware header contains multiple secure sections, we need to trim
ignored sections, and then download firmware header with single one secure
section.

For secure boot, when downloading secure section, copy security key data
from MSS poll by key_idx read from efuse. If non-secure boot, no need this
extra copy.

           +---------------------------+ -\
           |      firmware header      |  |
           |                           |  |
           | +-----------------------+ |  | only preserve single one secure
           | | section type/size * N | |  | section
           | | ...                   | |  |
           | +-----------------------+ |  |
           +---------------------------+ -/
           :                           :
           +---------------------------+ -\
           | secure section type (ID:9)|  |
           |                           |  |
      +----|-> [ security key data ]   |  |
      |    +---------------------------+ -/
      |    |MSS Pool for above section |
      |    |  [ security key data 0 ]  |
      +----|- [ security key data 1 ]  |
by key_idx |  [ security key data 2 ]  |
           |  ...                      |
           +---------------------------+

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/fw.c | 95 +++++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw89/fw.h |  7 +-
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 4f648071a5be..61d6297475e5 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -1098,9 +1098,56 @@ static void rtw89_h2c_pkt_set_hdr_fwdl(struct rtw89_dev *rtwdev,
 					   len + H2C_HEADER_LEN));
 }
 
-static int __rtw89_fw_download_hdr(struct rtw89_dev *rtwdev, const u8 *fw, u32 len)
+static u32 __rtw89_fw_download_tweak_hdr_v0(struct rtw89_dev *rtwdev,
+					    struct rtw89_fw_bin_info *info,
+					    struct rtw89_fw_hdr *fw_hdr)
 {
+	le32p_replace_bits(&fw_hdr->w7, FWDL_SECTION_PER_PKT_LEN,
+			   FW_HDR_W7_PART_SIZE);
+
+	return 0;
+}
+
+static u32 __rtw89_fw_download_tweak_hdr_v1(struct rtw89_dev *rtwdev,
+					    struct rtw89_fw_bin_info *info,
+					    struct rtw89_fw_hdr_v1 *fw_hdr)
+{
+	struct rtw89_fw_hdr_section_info *section_info;
+	struct rtw89_fw_hdr_section_v1 *section;
+	u8 dst_sec_idx = 0;
+	u8 sec_idx;
+
+	le32p_replace_bits(&fw_hdr->w7, FWDL_SECTION_PER_PKT_LEN,
+			   FW_HDR_V1_W7_PART_SIZE);
+
+	for (sec_idx = 0; sec_idx < info->section_num; sec_idx++) {
+		section_info = &info->section_info[sec_idx];
+		section = &fw_hdr->sections[sec_idx];
+
+		if (section_info->ignore)
+			continue;
+
+		if (dst_sec_idx != sec_idx)
+			fw_hdr->sections[dst_sec_idx] = *section;
+
+		dst_sec_idx++;
+	}
+
+	le32p_replace_bits(&fw_hdr->w6, dst_sec_idx, FW_HDR_V1_W6_SEC_NUM);
+
+	return (info->section_num - dst_sec_idx) * sizeof(*section);
+}
+
+static int __rtw89_fw_download_hdr(struct rtw89_dev *rtwdev,
+				   const struct rtw89_fw_suit *fw_suit,
+				   struct rtw89_fw_bin_info *info)
+{
+	u32 len = info->hdr_len - info->dynamic_hdr_len;
+	struct rtw89_fw_hdr_v1 *fw_hdr_v1;
+	const u8 *fw = fw_suit->data;
+	struct rtw89_fw_hdr *fw_hdr;
 	struct sk_buff *skb;
+	u32 truncated;
 	u32 ret = 0;
 
 	skb = rtw89_fw_h2c_alloc_skb_with_hdr(rtwdev, len);
@@ -1110,7 +1157,26 @@ static int __rtw89_fw_download_hdr(struct rtw89_dev *rtwdev, const u8 *fw, u32 l
 	}
 
 	skb_put_data(skb, fw, len);
-	SET_FW_HDR_PART_SIZE(skb->data, FWDL_SECTION_PER_PKT_LEN);
+
+	switch (fw_suit->hdr_ver) {
+	case 0:
+		fw_hdr = (struct rtw89_fw_hdr *)skb->data;
+		truncated = __rtw89_fw_download_tweak_hdr_v0(rtwdev, info, fw_hdr);
+		break;
+	case 1:
+		fw_hdr_v1 = (struct rtw89_fw_hdr_v1 *)skb->data;
+		truncated = __rtw89_fw_download_tweak_hdr_v1(rtwdev, info, fw_hdr_v1);
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		goto fail;
+	}
+
+	if (truncated) {
+		len -= truncated;
+		skb_trim(skb, len);
+	}
+
 	rtw89_h2c_pkt_set_hdr_fwdl(rtwdev, skb, FWCMD_TYPE_H2C,
 				   H2C_CAT_MAC, H2C_CL_MAC_FWDL,
 				   H2C_FUNC_MAC_FWHDR_DL, len);
@@ -1129,12 +1195,14 @@ static int __rtw89_fw_download_hdr(struct rtw89_dev *rtwdev, const u8 *fw, u32 l
 	return ret;
 }
 
-static int rtw89_fw_download_hdr(struct rtw89_dev *rtwdev, const u8 *fw, u32 len)
+static int rtw89_fw_download_hdr(struct rtw89_dev *rtwdev,
+				 const struct rtw89_fw_suit *fw_suit,
+				 struct rtw89_fw_bin_info *info)
 {
 	const struct rtw89_mac_gen_def *mac = rtwdev->chip->mac_def;
 	int ret;
 
-	ret = __rtw89_fw_download_hdr(rtwdev, fw, len);
+	ret = __rtw89_fw_download_hdr(rtwdev, fw_suit, info);
 	if (ret) {
 		rtw89_err(rtwdev, "[ERR]FW header download\n");
 		return ret;
@@ -1158,9 +1226,21 @@ static int __rtw89_fw_download_main(struct rtw89_dev *rtwdev,
 	struct sk_buff *skb;
 	const u8 *section = info->addr;
 	u32 residue_len = info->len;
+	bool copy_key = false;
 	u32 pkt_len;
 	int ret;
 
+	if (info->ignore)
+		return 0;
+
+	if (info->key_addr && info->key_len) {
+		if (info->len > FWDL_SECTION_PER_PKT_LEN || info->len < info->key_len)
+			rtw89_warn(rtwdev, "ignore to copy key data because of len %d, %d, %d\n",
+				   info->len, FWDL_SECTION_PER_PKT_LEN, info->key_len);
+		else
+			copy_key = true;
+	}
+
 	while (residue_len) {
 		if (residue_len >= FWDL_SECTION_PER_PKT_LEN)
 			pkt_len = FWDL_SECTION_PER_PKT_LEN;
@@ -1174,6 +1254,10 @@ static int __rtw89_fw_download_main(struct rtw89_dev *rtwdev,
 		}
 		skb_put_data(skb, section, pkt_len);
 
+		if (copy_key)
+			memcpy(skb->data + pkt_len - info->key_len,
+			       info->key_addr, info->key_len);
+
 		ret = rtw89_h2c_tx(rtwdev, skb, true);
 		if (ret) {
 			rtw89_err(rtwdev, "failed to send h2c\n");
@@ -1299,8 +1383,7 @@ static int rtw89_fw_download_suit(struct rtw89_dev *rtwdev,
 		return ret;
 	}
 
-	ret = rtw89_fw_download_hdr(rtwdev, fw_suit->data, info.hdr_len -
-							   info.dynamic_hdr_len);
+	ret = rtw89_fw_download_hdr(rtwdev, fw_suit, &info);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
index c05ddb0d5900..58e4802cb766 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.h
+++ b/drivers/net/wireless/realtek/rtw89/fw.h
@@ -526,6 +526,7 @@ struct rtw89_fw_hdr {
 #define FW_HDR_W4_MIN GENMASK(31, 24)
 #define FW_HDR_W5_YEAR GENMASK(31, 0)
 #define FW_HDR_W6_SEC_NUM GENMASK(15, 8)
+#define FW_HDR_W7_PART_SIZE GENMASK(15, 0)
 #define FW_HDR_W7_DYN_HDR BIT(16)
 #define FW_HDR_W7_CMD_VERSERION GENMASK(31, 24)
 
@@ -577,13 +578,9 @@ struct rtw89_fw_hdr_v1 {
 #define FW_HDR_V1_W5_HDR_SIZE GENMASK(31, 16)
 #define FW_HDR_V1_W6_SEC_NUM GENMASK(15, 8)
 #define FW_HDR_V1_W6_DSP_CHKSUM BIT(24)
+#define FW_HDR_V1_W7_PART_SIZE GENMASK(15, 0)
 #define FW_HDR_V1_W7_DYN_HDR BIT(16)
 
-static inline void SET_FW_HDR_PART_SIZE(void *fwhdr, u32 val)
-{
-	le32p_replace_bits((__le32 *)fwhdr + 7, val, GENMASK(15, 0));
-}
-
 enum rtw89_fw_mss_pool_rmp_tbl_type {
 	MSS_POOL_RMP_TBL_BITMASK = 0x0,
 	MSS_POOL_RMP_TBL_RECORD = 0x1,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file
  2024-02-03  0:32 ` [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file Ping-Ke Shih
@ 2024-02-03 19:22   ` kernel test robot
  2024-02-04  1:31     ` Ping-Ke Shih
  0 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2024-02-03 19:22 UTC (permalink / raw)
  To: Ping-Ke Shih, kvalo; +Cc: oe-kbuild-all, linux-wireless

Hi Ping-Ke,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.8-rc2 next-20240202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ping-Ke-Shih/wifi-rtw89-fw-consider-checksum-length-of-security-data/20240203-085038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20240203003251.10641-4-pkshih%40realtek.com
patch subject: [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file
config: parisc-allmodconfig (https://download.01.org/0day-ci/archive/20240204/202402040350.rRpOepoU-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240204/202402040350.rRpOepoU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402040350.rRpOepoU-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/wireless/realtek/rtw89/fw.c: In function 'rtw89_fw_hdr_parser_v1':
>> drivers/net/wireless/realtek/rtw89/fw.c:384:88: warning: format '%lx' expects argument of type 'long unsigned int', but argument 9 has type 'int' [-Wformat=]
     384 |                             "section[%d] type=%d len=0x%-6x mssc=%d mssc_len=%d addr=%lx\n",
         |                                                                                      ~~^
         |                                                                                        |
         |                                                                                        long unsigned int
         |                                                                                      %x
     385 |                             i, section_info->type, section_info->len,
     386 |                             section_info->mssc, mssc_len, bin - fw);
         |                                                           ~~~~~~~~                      
         |                                                               |
         |                                                               int
   drivers/net/wireless/realtek/rtw89/fw.c:388:68: warning: format '%lx' expects argument of type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
     388 |                             "           ignore=%d key_addr=%p (0x%lx) key_len=%d key_idx=%d\n",
         |                                                                  ~~^
         |                                                                    |
         |                                                                    long unsigned int
         |                                                                  %x
     389 |                             section_info->ignore, section_info->key_addr,
     390 |                             section_info->key_addr ?
         |                             ~~~~~~~~~~~~~~~~~~~~~~~~                
     391 |                             section_info->key_addr - section_info->addr : 0,
         |                             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                                                         |
         |                                                                         int


vim +384 drivers/net/wireless/realtek/rtw89/fw.c

   322	
   323	static int rtw89_fw_hdr_parser_v1(struct rtw89_dev *rtwdev, const u8 *fw, u32 len,
   324					  struct rtw89_fw_bin_info *info)
   325	{
   326		const struct rtw89_fw_hdr_v1 *fw_hdr = (const struct rtw89_fw_hdr_v1 *)fw;
   327		struct rtw89_fw_hdr_section_info *section_info;
   328		const struct rtw89_fw_dynhdr_hdr *fwdynhdr;
   329		const struct rtw89_fw_hdr_section_v1 *section;
   330		const u8 *fw_end = fw + len;
   331		const u8 *bin;
   332		u32 base_hdr_len;
   333		u32 mssc_len;
   334		int ret;
   335		u32 i;
   336	
   337		info->section_num = le32_get_bits(fw_hdr->w6, FW_HDR_V1_W6_SEC_NUM);
   338		info->dsp_checksum = le32_get_bits(fw_hdr->w6, FW_HDR_V1_W6_DSP_CHKSUM);
   339		base_hdr_len = struct_size(fw_hdr, sections, info->section_num);
   340		info->dynamic_hdr_en = le32_get_bits(fw_hdr->w7, FW_HDR_V1_W7_DYN_HDR);
   341	
   342		if (info->dynamic_hdr_en) {
   343			info->hdr_len = le32_get_bits(fw_hdr->w5, FW_HDR_V1_W5_HDR_SIZE);
   344			info->dynamic_hdr_len = info->hdr_len - base_hdr_len;
   345			fwdynhdr = (const struct rtw89_fw_dynhdr_hdr *)(fw + base_hdr_len);
   346			if (le32_to_cpu(fwdynhdr->hdr_len) != info->dynamic_hdr_len) {
   347				rtw89_err(rtwdev, "[ERR]invalid fw dynamic header len\n");
   348				return -EINVAL;
   349			}
   350		} else {
   351			info->hdr_len = base_hdr_len;
   352			info->dynamic_hdr_len = 0;
   353		}
   354	
   355		bin = fw + info->hdr_len;
   356	
   357		/* jump to section header */
   358		section_info = info->section_info;
   359		for (i = 0; i < info->section_num; i++) {
   360			section = &fw_hdr->sections[i];
   361	
   362			section_info->type =
   363				le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_SECTIONTYPE);
   364			section_info->len =
   365				le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_SEC_SIZE);
   366			if (le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_CHECKSUM))
   367				section_info->len += FWDL_SECTION_CHKSUM_LEN;
   368			section_info->redl = le32_get_bits(section->w1, FWSECTION_HDR_V1_W1_REDL);
   369			section_info->dladdr =
   370				le32_get_bits(section->w0, FWSECTION_HDR_V1_W0_DL_ADDR);
   371			section_info->addr = bin;
   372	
   373			if (section_info->type == FWDL_SECURITY_SECTION_TYPE) {
   374				ret = __parse_security_section(rtwdev, info, section_info,
   375							       section, bin, &mssc_len);
   376				if (ret)
   377					return ret;
   378			} else {
   379				section_info->mssc = 0;
   380				mssc_len = 0;
   381			}
   382	
   383			rtw89_debug(rtwdev, RTW89_DBG_FW,
 > 384				    "section[%d] type=%d len=0x%-6x mssc=%d mssc_len=%d addr=%lx\n",
   385				    i, section_info->type, section_info->len,
   386				    section_info->mssc, mssc_len, bin - fw);
   387			rtw89_debug(rtwdev, RTW89_DBG_FW,
   388				    "           ignore=%d key_addr=%p (0x%lx) key_len=%d key_idx=%d\n",
   389				    section_info->ignore, section_info->key_addr,
   390				    section_info->key_addr ?
   391				    section_info->key_addr - section_info->addr : 0,
   392				    section_info->key_len, section_info->key_idx);
   393	
   394			bin += section_info->len + mssc_len;
   395			section_info++;
   396		}
   397	
   398		if (fw_end != bin) {
   399			rtw89_err(rtwdev, "[ERR]fw bin size\n");
   400			return -EINVAL;
   401		}
   402	
   403		if (!info->secure_section_exist)
   404			rtw89_warn(rtwdev, "no firmware secure section\n");
   405	
   406		return 0;
   407	}
   408	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file
  2024-02-03 19:22   ` kernel test robot
@ 2024-02-04  1:31     ` Ping-Ke Shih
  2024-02-05  6:47       ` Kalle Valo
  0 siblings, 1 reply; 8+ messages in thread
From: Ping-Ke Shih @ 2024-02-04  1:31 UTC (permalink / raw)
  To: kvalo@kernel.org, lkp@intel.com
  Cc: linux-wireless@vger.kernel.org, oe-kbuild-all@lists.linux.dev

On Sun, 2024-02-04 at 03:22 +0800, kernel test robot wrote:
> 
>    drivers/net/wireless/realtek/rtw89/fw.c: In function 'rtw89_fw_hdr_parser_v1':
> > > drivers/net/wireless/realtek/rtw89/fw.c:384:88: warning: format '%lx' expects argument of type
> > > 'long unsigned int', but argument 9 has type 'int' [-Wformat=]
>      384 |                             "section[%d] type=%d len=0x%-6x mssc=%d mssc_len=%d
> addr=%lx\n",
>          |                                                                                      ~~
> ^
>          |                                                                                        
> |
>          |                                                                                        
> long unsigned int
>          |                                                                                      %x
>      385 |                             i, section_info->type, section_info->len,
>      386 |                             section_info->mssc, mssc_len, bin - fw);
>          |                                                           ~~~~~~~~
>          |                                                               |
>          |                                                               int

I looked for how to print out differences (subtraction) of points, and
"%tx" is the desired format [1]. I corrected this by v2.

[1] https://docs.kernel.org/core-api/printk-formats.html#pointer-differences

Ping-Ke 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file
  2024-02-04  1:31     ` Ping-Ke Shih
@ 2024-02-05  6:47       ` Kalle Valo
  0 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2024-02-05  6:47 UTC (permalink / raw)
  To: Ping-Ke Shih
  Cc: lkp@intel.com, linux-wireless@vger.kernel.org,
	oe-kbuild-all@lists.linux.dev

Ping-Ke Shih <pkshih@realtek.com> writes:

> On Sun, 2024-02-04 at 03:22 +0800, kernel test robot wrote:
>> 
>>    drivers/net/wireless/realtek/rtw89/fw.c: In function 'rtw89_fw_hdr_parser_v1':
>> > > drivers/net/wireless/realtek/rtw89/fw.c:384:88: warning: format
>> > > '%lx' expects argument of type
>> > > 'long unsigned int', but argument 9 has type 'int' [-Wformat=]
>>      384 | "section[%d] type=%d len=0x%-6x mssc=%d mssc_len=%d
>> addr=%lx\n",
>>          |                                                                                      ~~
>> ^
>>          |
>> |
>>          |
>> long unsigned int
>>          |                                                                                      %x
>>      385 |                             i, section_info->type, section_info->len,
>>      386 |                             section_info->mssc, mssc_len, bin - fw);
>>          |                                                           ~~~~~~~~
>>          |                                                               |
>>          |                                                               int
>
> I looked for how to print out differences (subtraction) of points, and
> "%tx" is the desired format [1]. I corrected this by v2.
>
> [1] https://docs.kernel.org/core-api/printk-formats.html#pointer-differences

Heh, never heard of %td and %tx before. Thanks for teaching us :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-02-05  6:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-03  0:32 [PATCH 0/4] wifi: rtw89: fw: support firmware secure boot Ping-Ke Shih
2024-02-03  0:32 ` [PATCH 1/4] wifi: rtw89: fw: consider checksum length of security data Ping-Ke Shih
2024-02-03  0:32 ` [PATCH 2/4] wifi: rtw89: fw: read firmware secure information from efuse Ping-Ke Shih
2024-02-03  0:32 ` [PATCH 3/4] wifi: rtw89: fw: parse secure section from firmware file Ping-Ke Shih
2024-02-03 19:22   ` kernel test robot
2024-02-04  1:31     ` Ping-Ke Shih
2024-02-05  6:47       ` Kalle Valo
2024-02-03  0:32 ` [PATCH 4/4] wifi: rtw89: fw: download firmware with key data for secure boot Ping-Ke Shih

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).