Skip to contentSkip to conversation

intel-m10-bmc: Manage register access to control delay during sec update

PATCH Applied Ilpo Järvinen 3 participants 18 messages Apr 17, 2023 – May 15, 2023

Subsystems (5)
Hardware monitoring Multifunction devices (MFD) FPGA manager framework Intel max 10 bmc MFD driver +1 more subsystems
lore
Share & tools
lore

Ilpo Järvinen

author
#0 lore raw

v3 cover intel-m10-bmc: Manage register access to control delay during sec update

Manage handshake register access on Max 10 FPGA cards that have a major slowdown on reading handshake registers during secure update prepare and write phases. The problem does not occur with PMCI-based cards.

The first patch which moves Max M10 symbols into own namespace is otherwise independent of the other changes but it would conflict with this series if sent as separate change. Thus, it's part of this series to give the patches a well-defined order.

v3:
- Add tags properly & include series version history

v2:
- Add also m10bmc_dev_init() to SYMBOL NS
- Keep bmcfw_state only when handshake_sys_reg_nranges > 0
- Drop zero initializations for handshake_sys_reg*

Ilpo Järvinen (4):
mfd: intel-m10-bmc: Move core symbols to own namespace
mfd: intel-m10-bmc: Create m10bmc_sys_update_bits()
mfd: intel-m10-bmc: Move m10bmc_sys_read() away from header
mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

 drivers/fpga/intel-m10-bmc-sec-update.c | 47 +++++++------
 drivers/hwmon/intel-m10-bmc-hwmon.c     |  1 +
 drivers/mfd/intel-m10-bmc-core.c        | 90 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-pmci.c        |  1 +
 drivers/mfd/intel-m10-bmc-spi.c         | 15 +++++
 include/linux/mfd/intel-m10-bmc.h       | 43 ++++++++----
 6 files changed, 163 insertions(+), 34 deletions(-)
Signature · 1 line
2.30.2
author
#1 lore raw

PATCH v3 1/4 mfd: intel-m10-bmc: Move core symbols to own namespace

Create INTEL_M10_BMC_CORE namespace for symbols exported by intel-m10-bmc-core.

Reviewed-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/mfd/intel-m10-bmc-core.c | 4 ++--
 drivers/mfd/intel-m10-bmc-pmci.c | 1 +
 drivers/mfd/intel-m10-bmc-spi.c  | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
drivers/mfd/intel-m10-bmc-core.c +2 -2
index dac9cf7bcb4a..c0b8209fd842 100644
@@ -98,7 +98,7 @@ const struct attribute_group *m10bmc_dev_groups[] = {
 	&m10bmc_group,
 	NULL,
 };
-EXPORT_SYMBOL_GPL(m10bmc_dev_groups);
+EXPORT_SYMBOL_NS_GPL(m10bmc_dev_groups, INTEL_M10_BMC_CORE);
 
 int m10bmc_dev_init(struct intel_m10bmc *m10bmc, const struct intel_m10bmc_platform_info *info)
 {

@@ -115,7 +115,7 @@ int m10bmc_dev_init(struct intel_m10bmc *m10bmc, const struct intel_m10bmc_platf
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(m10bmc_dev_init);
+EXPORT_SYMBOL_NS_GPL(m10bmc_dev_init, INTEL_M10_BMC_CORE);
 
 MODULE_DESCRIPTION("Intel MAX 10 BMC core driver");
 MODULE_AUTHOR("Intel Corporation");
drivers/mfd/intel-m10-bmc-pmci.c +1 -0
index 8821f1876dd6..0392ef8b57d8 100644
@@ -453,3 +453,4 @@ module_dfl_driver(m10bmc_pmci_driver);
 MODULE_DESCRIPTION("MAX10 BMC PMCI-based interface");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(INTEL_M10_BMC_CORE);
drivers/mfd/intel-m10-bmc-spi.c +1 -0
index 957200e17fed..edd266557ab9 100644
@@ -166,3 +166,4 @@ MODULE_DESCRIPTION("Intel MAX 10 BMC SPI bus interface");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("spi:intel-m10-bmc");
+MODULE_IMPORT_NS(INTEL_M10_BMC_CORE);
Signature · 1 line
2.30.2
maintainer Reviewed-by on v3 1/4
#2 lore raw

Re: [PATCH v3 1/4] mfd: intel-m10-bmc: Move core symbols to own namespace

Quoted 5 lines · Ilpo Järvinen

Create INTEL_M10_BMC_CORE namespace for symbols exported by intel-m10-bmc-core.

Reviewed-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Reviewed-by Xu Yilun <[email protected]>
Quoted 5 lines
---
 drivers/mfd/intel-m10-bmc-core.c | 4 ++--
 drivers/mfd/intel-m10-bmc-pmci.c | 1 +
 drivers/mfd/intel-m10-bmc-spi.c  | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
maintainer Applied
#3 lore raw

Re: [PATCH v3 1/4] mfd: intel-m10-bmc: Move core symbols to own namespace

In reply to Ilpo Järvinen
Quoted 10 lines · Ilpo Järvinen

Create INTEL_M10_BMC_CORE namespace for symbols exported by intel-m10-bmc-core.

Reviewed-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/mfd/intel-m10-bmc-core.c | 4 ++--
 drivers/mfd/intel-m10-bmc-pmci.c | 1 +
 drivers/mfd/intel-m10-bmc-spi.c  | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

Applied, thanks

Signature · 1 line
Lee Jones [李琼斯]
author
#4 lore raw

PATCH v3 2/4 mfd: intel-m10-bmc: Create m10bmc_sys_update_bits()

In reply to Ilpo Järvinen

Wrap regmap_update_bits() with m10bmc_sys_update_bits() in order to be able to add additional checks into it.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 30 ++++++++++++-------------
 drivers/mfd/intel-m10-bmc-core.c        |  9 ++++++++
 include/linux/mfd/intel-m10-bmc.h       |  4 ++++
 3 files changed, 27 insertions(+), 16 deletions(-)
drivers/fpga/intel-m10-bmc-sec-update.c +14 -16
index d7e2f9f461bc..fe0127a58eff 100644
@@ -376,12 +376,11 @@ static enum fw_upload_err rsu_update_init(struct m10bmc_sec *sec)
 	u32 doorbell_reg, progress, status;
 	int ret, err;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_RSU_REQUEST | DRBL_HOST_STATUS,
-				 DRBL_RSU_REQUEST |
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_IDLE));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_RSU_REQUEST | DRBL_HOST_STATUS,
+				     DRBL_RSU_REQUEST |
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_IDLE));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -450,11 +449,10 @@ static enum fw_upload_err rsu_send_data(struct m10bmc_sec *sec)
 	u32 doorbell_reg, status;
 	int ret;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_HOST_STATUS,
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_WRITE_DONE));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_HOST_STATUS,
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_WRITE_DONE));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -517,11 +515,10 @@ static enum fw_upload_err rsu_cancel(struct m10bmc_sec *sec)
 	if (rsu_prog(doorbell) != RSU_PROG_READY)
 		return FW_UPLOAD_ERR_BUSY;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_HOST_STATUS,
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_ABORT_RSU));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_HOST_STATUS,
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_ABORT_RSU));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -764,3 +761,4 @@ module_platform_driver(intel_m10bmc_sec_driver);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Intel MAX10 BMC Secure Update");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(INTEL_M10_BMC_CORE);
drivers/mfd/intel-m10-bmc-core.c +9 -0
index c0b8209fd842..dab1bb152fd6 100644
@@ -12,6 +12,15 @@
 #include <linux/mfd/intel-m10-bmc.h>
 #include <linux/module.h>
 
+int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
+			   unsigned int msk, unsigned int val)
+{
+	const struct m10bmc_csr_map *csr_map = m10bmc->info->csr_map;
+
+	return regmap_update_bits(m10bmc->regmap, csr_map->base + offset, msk, val);
+}
+EXPORT_SYMBOL_NS_GPL(m10bmc_sys_update_bits, INTEL_M10_BMC_CORE);
+
 static ssize_t bmc_version_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
include/linux/mfd/intel-m10-bmc.h +4 -0
index 1812ebfa11a8..5418f7279ed0 100644
@@ -251,6 +251,7 @@ struct intel_m10bmc {
  *
  * m10bmc_raw_read - read m10bmc register per addr
  * m10bmc_sys_read - read m10bmc system register per offset
+ * m10bmc_sys_update_bits - update m10bmc system register per offset
  */
 static inline int
 m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,

@@ -282,6 +283,9 @@ static inline int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offs
 	return m10bmc_raw_read(m10bmc, csr_map->base + offset, val);
 }
 
+int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
+			   unsigned int msk, unsigned int val);
+
 /*
  * MAX10 BMC Core support
  */
Signature · 1 line
2.30.2
maintainer Reviewed-by on v3 2/4
#5 lore raw

Re: [PATCH v3 2/4] mfd: intel-m10-bmc: Create m10bmc_sys_update_bits()

Quoted 6 lines · Ilpo Järvinen

Wrap regmap_update_bits() with m10bmc_sys_update_bits() in order to be able to add additional checks into it.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Reviewed-by Xu Yilun <[email protected]>
Quoted 5 lines
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 30 ++++++++++++-------------
 drivers/mfd/intel-m10-bmc-core.c        |  9 ++++++++
 include/linux/mfd/intel-m10-bmc.h       |  4 ++++
 3 files changed, 27 insertions(+), 16 deletions(-)
maintainer Acked-by on v3 2/4
#6 lore raw

Re: [PATCH v3 2/4] mfd: intel-m10-bmc: Create m10bmc_sys_update_bits()

In reply to Ilpo Järvinen
Quoted 6 lines · Ilpo Järvinen

Wrap regmap_update_bits() with m10bmc_sys_update_bits() in order to be able to add additional checks into it.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Acked-by Xu Yilun <[email protected]>
Quoted 110 lines
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 30 ++++++++++++-------------
 drivers/mfd/intel-m10-bmc-core.c        |  9 ++++++++
 include/linux/mfd/intel-m10-bmc.h       |  4 ++++
 3 files changed, 27 insertions(+), 16 deletions(-)
drivers/fpga/intel-m10-bmc-sec-update.c +14 -16
index d7e2f9f461bc..fe0127a58eff 100644
@@ -376,12 +376,11 @@ static enum fw_upload_err rsu_update_init(struct m10bmc_sec *sec)
 	u32 doorbell_reg, progress, status;
 	int ret, err;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_RSU_REQUEST | DRBL_HOST_STATUS,
-				 DRBL_RSU_REQUEST |
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_IDLE));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_RSU_REQUEST | DRBL_HOST_STATUS,
+				     DRBL_RSU_REQUEST |
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_IDLE));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -450,11 +449,10 @@ static enum fw_upload_err rsu_send_data(struct m10bmc_sec *sec)
 	u32 doorbell_reg, status;
 	int ret;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_HOST_STATUS,
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_WRITE_DONE));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_HOST_STATUS,
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_WRITE_DONE));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -517,11 +515,10 @@ static enum fw_upload_err rsu_cancel(struct m10bmc_sec *sec)
 	if (rsu_prog(doorbell) != RSU_PROG_READY)
 		return FW_UPLOAD_ERR_BUSY;
 
-	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 csr_map->base + csr_map->doorbell,
-				 DRBL_HOST_STATUS,
-				 FIELD_PREP(DRBL_HOST_STATUS,
-					    HOST_STATUS_ABORT_RSU));
+	ret = m10bmc_sys_update_bits(sec->m10bmc, csr_map->doorbell,
+				     DRBL_HOST_STATUS,
+				     FIELD_PREP(DRBL_HOST_STATUS,
+						HOST_STATUS_ABORT_RSU));
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 

@@ -764,3 +761,4 @@ module_platform_driver(intel_m10bmc_sec_driver);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Intel MAX10 BMC Secure Update");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(INTEL_M10_BMC_CORE);
drivers/mfd/intel-m10-bmc-core.c +9 -0
index c0b8209fd842..dab1bb152fd6 100644
@@ -12,6 +12,15 @@
 #include <linux/mfd/intel-m10-bmc.h>
 #include <linux/module.h>
 
+int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
+			   unsigned int msk, unsigned int val)
+{
+	const struct m10bmc_csr_map *csr_map = m10bmc->info->csr_map;
+
+	return regmap_update_bits(m10bmc->regmap, csr_map->base + offset, msk, val);
+}
+EXPORT_SYMBOL_NS_GPL(m10bmc_sys_update_bits, INTEL_M10_BMC_CORE);
+
 static ssize_t bmc_version_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
include/linux/mfd/intel-m10-bmc.h +4 -0
index 1812ebfa11a8..5418f7279ed0 100644
@@ -251,6 +251,7 @@ struct intel_m10bmc {
  *
  * m10bmc_raw_read - read m10bmc register per addr
  * m10bmc_sys_read - read m10bmc system register per offset
+ * m10bmc_sys_update_bits - update m10bmc system register per offset
  */
 static inline int
 m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,

@@ -282,6 +283,9 @@ static inline int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offs
 	return m10bmc_raw_read(m10bmc, csr_map->base + offset, val);
 }
 
+int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
+			   unsigned int msk, unsigned int val);
+
 /*
  * MAX10 BMC Core support
  */
Signature · 1 line
2.30.2
maintainer Applied
#7 lore raw

Re: [PATCH v3 2/4] mfd: intel-m10-bmc: Create m10bmc_sys_update_bits()

In reply to Ilpo Järvinen
Quoted 11 lines · Ilpo Järvinen

Wrap regmap_update_bits() with m10bmc_sys_update_bits() in order to be able to add additional checks into it.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 30 ++++++++++++-------------
 drivers/mfd/intel-m10-bmc-core.c        |  9 ++++++++
 include/linux/mfd/intel-m10-bmc.h       |  4 ++++
 3 files changed, 27 insertions(+), 16 deletions(-)

Applied, thanks

Signature · 1 line
Lee Jones [李琼斯]
author
#8 lore raw

PATCH v3 3/4 mfd: intel-m10-bmc: Move m10bmc_sys_read() away from header

In reply to Ilpo Järvinen

Move m10bmc_sys_read() out from the header to prepare it for adding more code into the function which would make it too large to be a static inline any more.

While at it, replace the vague wording in function comment with more precise statements.

Reviewed-by Russ Weight <[email protected]>
Acked-by Guenter Roeck <[email protected]> # For hwmon
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/hwmon/intel-m10-bmc-hwmon.c |  1 +
 drivers/mfd/intel-m10-bmc-core.c    | 14 ++++++++++++++
 include/linux/mfd/intel-m10-bmc.h   | 17 +----------------
 3 files changed, 16 insertions(+), 16 deletions(-)
drivers/hwmon/intel-m10-bmc-hwmon.c +1 -0
index 2f0323c14bab..92900ce7986b 100644
@@ -794,3 +794,4 @@ MODULE_DEVICE_TABLE(platform, intel_m10bmc_hwmon_ids);
 MODULE_AUTHOR("Intel Corporation");
 MODULE_DESCRIPTION("Intel MAX 10 BMC hardware monitor");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(INTEL_M10_BMC_CORE);
drivers/mfd/intel-m10-bmc-core.c +14 -0
index dab1bb152fd6..51f865174796 100644
@@ -12,6 +12,20 @@
 #include <linux/mfd/intel-m10-bmc.h>
 #include <linux/module.h>
 
+/*
+ * This function helps to simplify the accessing of the system registers.
+ *
+ * The base of the system registers is configured through the struct
+ * csr_map.
+ */
+int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offset, unsigned int *val)
+{
+	const struct m10bmc_csr_map *csr_map = m10bmc->info->csr_map;
+
+	return m10bmc_raw_read(m10bmc, csr_map->base + offset, val);
+}
+EXPORT_SYMBOL_NS_GPL(m10bmc_sys_read, INTEL_M10_BMC_CORE);
+
 int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
 			   unsigned int msk, unsigned int val)
 {
include/linux/mfd/intel-m10-bmc.h +1 -16
index 5418f7279ed0..252644fa61be 100644
@@ -267,22 +267,7 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,
 	return ret;
 }
 
-/*
- * The base of the system registers could be configured by HW developers, and
- * in HW SPEC, the base is not added to the addresses of the system registers.
- *
- * This function helps to simplify the accessing of the system registers. And if
- * the base is reconfigured in HW, SW developers could simply change the
- * csr_map's base accordingly.
- */
-static inline int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offset,
-				  unsigned int *val)
-{
-	const struct m10bmc_csr_map *csr_map = m10bmc->info->csr_map;
-
-	return m10bmc_raw_read(m10bmc, csr_map->base + offset, val);
-}
-
+int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offset, unsigned int *val);
 int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
 			   unsigned int msk, unsigned int val);
Signature · 1 line
2.30.2
maintainer Reviewed-by on v3 3/4
#9 lore raw

Re: [PATCH v3 3/4] mfd: intel-m10-bmc: Move m10bmc_sys_read() away from header

Quoted 10 lines · Ilpo Järvinen

Move m10bmc_sys_read() out from the header to prepare it for adding more code into the function which would make it too large to be a static inline any more.

While at it, replace the vague wording in function comment with more precise statements.

Reviewed-by Russ Weight <[email protected]>
Acked-by Guenter Roeck <[email protected]> # For hwmon
Signed-off-by Ilpo Järvinen <[email protected]>
Reviewed-by Xu Yilun <[email protected]>
Quoted 5 lines
---
 drivers/hwmon/intel-m10-bmc-hwmon.c |  1 +
 drivers/mfd/intel-m10-bmc-core.c    | 14 ++++++++++++++
 include/linux/mfd/intel-m10-bmc.h   | 17 +----------------
 3 files changed, 16 insertions(+), 16 deletions(-)
maintainer Applied
#10 lore raw

Re: [PATCH v3 3/4] mfd: intel-m10-bmc: Move m10bmc_sys_read() away from header

In reply to Ilpo Järvinen
Quoted 7 lines · Ilpo Järvinen

Move m10bmc_sys_read() out from the header to prepare it for adding more code into the function which would make it too large to be a static inline any more.

While at it, replace the vague wording in function comment with more precise statements.

Quoted 8 lines
Reviewed-by Russ Weight <[email protected]>
Acked-by Guenter Roeck <[email protected]> # For hwmon
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/hwmon/intel-m10-bmc-hwmon.c |  1 +
 drivers/mfd/intel-m10-bmc-core.c    | 14 ++++++++++++++
 include/linux/mfd/intel-m10-bmc.h   | 17 +----------------
 3 files changed, 16 insertions(+), 16 deletions(-)

Applied, thanks

Signature · 1 line
Lee Jones [李琼斯]
author
#11 lore raw

PATCH v3 4/4 mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

In reply to Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 17 +++++--
 drivers/mfd/intel-m10-bmc-core.c        | 67 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-spi.c         | 14 ++++++
 include/linux/mfd/intel-m10-bmc.h       | 28 +++++++++++
 4 files changed, 121 insertions(+), 5 deletions(-)
drivers/fpga/intel-m10-bmc-sec-update.c +14 -3
index fe0127a58eff..31af2e08c825 100644
@@ -544,21 +544,28 @@ static enum fw_upload_err m10bmc_sec_prepare(struct fw_upload *fwl,
 	if (ret != FW_UPLOAD_ERR_NONE)
 		goto unlock_flash;
 
+	m10bmc_fw_state_set(sec->m10bmc, M10BMC_FW_STATE_SEC_UPDATE_PREPARE);
+
 	ret = rsu_update_init(sec);
 	if (ret != FW_UPLOAD_ERR_NONE)
-		goto unlock_flash;
+		goto fw_state_exit;
 
 	ret = rsu_prog_ready(sec);
 	if (ret != FW_UPLOAD_ERR_NONE)
-		goto unlock_flash;
+		goto fw_state_exit;
 
 	if (sec->cancel_request) {
 		ret = rsu_cancel(sec);
-		goto unlock_flash;
+		goto fw_state_exit;
 	}
 
+	m10bmc_fw_state_set(sec->m10bmc, M10BMC_FW_STATE_SEC_UPDATE_WRITE);
+
 	return FW_UPLOAD_ERR_NONE;
 
+fw_state_exit:
+	m10bmc_fw_state_set(sec->m10bmc, M10BMC_FW_STATE_NORMAL);
+
 unlock_flash:
 	if (sec->m10bmc->flash_bulk_ops)
 		sec->m10bmc->flash_bulk_ops->unlock_write(sec->m10bmc);

@@ -607,6 +614,8 @@ static enum fw_upload_err m10bmc_sec_poll_complete(struct fw_upload *fwl)
 	if (sec->cancel_request)
 		return rsu_cancel(sec);
 
+	m10bmc_fw_state_set(sec->m10bmc, M10BMC_FW_STATE_SEC_UPDATE_PROGRAM);
+
 	result = rsu_send_data(sec);
 	if (result != FW_UPLOAD_ERR_NONE)
 		return result;

@@ -650,6 +659,8 @@ static void m10bmc_sec_cleanup(struct fw_upload *fwl)
 
 	(void)rsu_cancel(sec);
 
+	m10bmc_fw_state_set(sec->m10bmc, M10BMC_FW_STATE_NORMAL);
+
 	if (sec->m10bmc->flash_bulk_ops)
 		sec->m10bmc->flash_bulk_ops->unlock_write(sec->m10bmc);
 }
drivers/mfd/intel-m10-bmc-core.c +65 -2

93 lines not on this page · raw message

drivers/mfd/intel-m10-bmc-spi.c +14 -0
index edd266557ab9..cbeb7de9e041 100644
@@ -116,12 +116,20 @@ static struct mfd_cell m10bmc_d5005_subdevs[] = {
 	{ .name = "d5005bmc-sec-update" },
 };
 
+static const struct regmap_range m10bmc_d5005_fw_handshake_regs[] = {
+	regmap_reg_range(M10BMC_N3000_TELEM_START, M10BMC_D5005_TELEM_END),
+};
+
 static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
 	{ .name = "n3000bmc-hwmon" },
 	{ .name = "n3000bmc-retimer" },
 	{ .name = "n3000bmc-sec-update" },
 };
 
+static const struct regmap_range m10bmc_n3000_fw_handshake_regs[] = {
+	regmap_reg_range(M10BMC_N3000_TELEM_START, M10BMC_N3000_TELEM_END),
+};
+
 static struct mfd_cell m10bmc_n5010_subdevs[] = {
 	{ .name = "n5010bmc-hwmon" },
 };

@@ -129,18 +137,24 @@ static struct mfd_cell m10bmc_n5010_subdevs[] = {
 static const struct intel_m10bmc_platform_info m10bmc_spi_n3000 = {
 	.cells = m10bmc_pacn3000_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_pacn3000_subdevs),
+	.handshake_sys_reg_ranges = m10bmc_n3000_fw_handshake_regs,
+	.handshake_sys_reg_nranges = ARRAY_SIZE(m10bmc_n3000_fw_handshake_regs),
 	.csr_map = &m10bmc_n3000_csr_map,
 };
 
 static const struct intel_m10bmc_platform_info m10bmc_spi_d5005 = {
 	.cells = m10bmc_d5005_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_d5005_subdevs),
+	.handshake_sys_reg_ranges = m10bmc_d5005_fw_handshake_regs,
+	.handshake_sys_reg_nranges = ARRAY_SIZE(m10bmc_d5005_fw_handshake_regs),
 	.csr_map = &m10bmc_n3000_csr_map,
 };
 
 static const struct intel_m10bmc_platform_info m10bmc_spi_n5010 = {
 	.cells = m10bmc_n5010_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_n5010_subdevs),
+	.handshake_sys_reg_ranges = m10bmc_n3000_fw_handshake_regs,
+	.handshake_sys_reg_nranges = ARRAY_SIZE(m10bmc_n3000_fw_handshake_regs),
 	.csr_map = &m10bmc_n3000_csr_map,
 };
include/linux/mfd/intel-m10-bmc.h +28 -0
index 252644fa61be..ee66c9751003 100644
@@ -11,6 +11,7 @@
 #include <linux/bits.h>
 #include <linux/dev_printk.h>
 #include <linux/regmap.h>
+#include <linux/rwsem.h>
 
 #define M10BMC_N3000_LEGACY_BUILD_VER	0x300468
 #define M10BMC_N3000_SYS_BASE		0x300800

@@ -39,6 +40,11 @@
 #define M10BMC_N3000_VER_PCB_INFO_MSK	GENMASK(31, 24)
 #define M10BMC_N3000_VER_LEGACY_INVALID	0xffffffff
 
+/* Telemetry registers */
+#define M10BMC_N3000_TELEM_START	0x100
+#define M10BMC_N3000_TELEM_END		0x250
+#define M10BMC_D5005_TELEM_END		0x300
+
 /* Secure update doorbell register, in system register region */
 #define M10BMC_N3000_DOORBELL		0x400
 

@@ -205,11 +211,15 @@ struct m10bmc_csr_map {
  * struct intel_m10bmc_platform_info - Intel MAX 10 BMC platform specific information
  * @cells: MFD cells
  * @n_cells: MFD cells ARRAY_SIZE()
+ * @handshake_sys_reg_ranges: array of register ranges for fw handshake regs
+ * @handshake_sys_reg_nranges: number of register ranges for fw handshake regs
  * @csr_map: the mappings for register definition of MAX10 BMC
  */
 struct intel_m10bmc_platform_info {
 	struct mfd_cell *cells;
 	int n_cells;
+	const struct regmap_range *handshake_sys_reg_ranges;
+	unsigned int handshake_sys_reg_nranges;
 	const struct m10bmc_csr_map *csr_map;
 };
 

@@ -232,18 +242,30 @@ struct intel_m10bmc_flash_bulk_ops {
 	void (*unlock_write)(struct intel_m10bmc *m10bmc);
 };
 
+enum m10bmc_fw_state {
+	M10BMC_FW_STATE_NORMAL,
+	M10BMC_FW_STATE_SEC_UPDATE_PREPARE,
+	M10BMC_FW_STATE_SEC_UPDATE_WRITE,
+	M10BMC_FW_STATE_SEC_UPDATE_PROGRAM,
+};
+
 /**
  * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure
  * @dev: this device
  * @regmap: the regmap used to access registers by m10bmc itself
  * @info: the platform information for MAX10 BMC
  * @flash_bulk_ops: optional device specific operations for flash R/W
+ * @bmcfw_lock: read/write semaphore to BMC firmware running state
+ * @bmcfw_state: BMC firmware running state. Available only when
+ *		 handshake_sys_reg_nranges > 0.
  */
 struct intel_m10bmc {
 	struct device *dev;
 	struct regmap *regmap;
 	const struct intel_m10bmc_platform_info *info;
 	const struct intel_m10bmc_flash_bulk_ops *flash_bulk_ops;
+	struct rw_semaphore bmcfw_lock;		/* Protects bmcfw_state */
+	enum m10bmc_fw_state bmcfw_state;
 };
 
 /*

@@ -271,6 +293,12 @@ int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offset, unsigned i
 int m10bmc_sys_update_bits(struct intel_m10bmc *m10bmc, unsigned int offset,
 			   unsigned int msk, unsigned int val);
 
+/*
+ * Track the state of the firmware, as it is not available for register
+ * handshakes during secure updates on some MAX 10 cards.
+ */
+void m10bmc_fw_state_set(struct intel_m10bmc *m10bmc, enum m10bmc_fw_state new_state);
+
 /*
  * MAX10 BMC Core support
  */
Signature · 1 line
2.30.2
maintainer Reviewed-by on v3 4/4
#12 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

Quoted 11 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

Quoted 8 lines

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Reviewed-by Xu Yilun <[email protected]>

Hi Lee:

Could the fpga part also been applied to mfd tree when everyone is good?

Thanks,
Yilun

Quoted 6 lines
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 17 +++++--
 drivers/mfd/intel-m10-bmc-core.c        | 67 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-spi.c         | 14 ++++++
 include/linux/mfd/intel-m10-bmc.h       | 28 +++++++++++
 4 files changed, 121 insertions(+), 5 deletions(-)
maintainer
#13 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

Quoted 21 lines · Xu Yilun
Quoted 11 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

Quoted 8 lines

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Quoted 5 lines
Reviewed-by Xu Yilun <[email protected]>

Hi Lee:

Could the fpga part also been applied to mfd tree when everyone is good?

Yes, with an Acked-by it can.

Signature · 1 line
Lee Jones [李琼斯]
maintainer Acked-by
#14 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

Quoted 23 lines · Lee Jones
Quoted 21 lines · Xu Yilun
Quoted 19 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Quoted 7 lines
Quoted 5 lines
Reviewed-by Xu Yilun <[email protected]>

Hi Lee:

Could the fpga part also been applied to mfd tree when everyone is good?

Yes, with an Acked-by it can.

Acked-by Xu Yilun <[email protected]>
Quoted 3 lines
Signature · 2 lines
-- 
Lee Jones [李琼斯]
maintainer Applied
#15 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

In reply to Ilpo Järvinen
Quoted 19 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Quoted 6 lines
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 17 +++++--
 drivers/mfd/intel-m10-bmc-core.c        | 67 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-spi.c         | 14 ++++++
 include/linux/mfd/intel-m10-bmc.h       | 28 +++++++++++
 4 files changed, 121 insertions(+), 5 deletions(-)

Applied, thanks

Signature · 1 line
Lee Jones [李琼斯]
author
#16 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

Quoted 27 lines · Lee Jones
Quoted 19 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
Quoted 6 lines
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 17 +++++--
 drivers/mfd/intel-m10-bmc-core.c        | 67 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-spi.c         | 14 ++++++
 include/linux/mfd/intel-m10-bmc.h       | 28 +++++++++++
 4 files changed, 121 insertions(+), 5 deletions(-)
Quoted 1 line

Applied, thanks

Did these end up falling throught the cracks as I've not been able to locate where they were applied?

Signature · 1 line
 i.
maintainer
#17 lore raw

Re: [PATCH v3 4/4] mfd: intel-m10-bmc: Manage access to MAX 10 fw handshake registers

Quoted 28 lines · Ilpo Järvinen
Quoted 27 lines · Lee Jones
Quoted 25 lines · Ilpo Järvinen

On some MAX 10 cards, the BMC firmware is not available to service handshake registers during secure update erase and write phases at normal speeds. This problem affects at least hwmon driver. When the MAX 10 hwmon driver tries to read the sensor values during a secure update, the reads are slowed down (e.g., reading all D5005 sensors takes ~24s which is magnitudes worse than the normal <0.02s).

Manage access to the handshake registers using a rw semaphore and a FW state variable to prevent accesses during those secure update phases and return -EBUSY instead.

If handshake_sys_reg_nranges == 0, don't update bwcfw_state as it is not used. This avoids the locking cost.

Co-developed-by Russ Weight <[email protected]>
Signed-off-by Russ Weight <[email protected]>
Co-developed-by Xu Yilun <[email protected]>
Signed-off-by Xu Yilun <[email protected]>
Signed-off-by Ilpo Järvinen <[email protected]>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 17 +++++--
 drivers/mfd/intel-m10-bmc-core.c        | 67 ++++++++++++++++++++++++-
 drivers/mfd/intel-m10-bmc-spi.c         | 14 ++++++
 include/linux/mfd/intel-m10-bmc.h       | 28 +++++++++++
 4 files changed, 121 insertions(+), 5 deletions(-)
Quoted 4 lines
Quoted 1 line

Applied, thanks

Did these end up falling throught the cracks as I've not been able to locate where they were applied?

They've been in -next for a couple of weeks.

Signature · 1 line
Lee Jones [李琼斯]