| Message ID | 20241005064307.18972-4-quic_rdwivedi@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for multiple ICE algorithms | expand |
Le 05/10/2024 à 08:43, Ram Kumar Dwivedi a écrit : > Add support for ICE algorithms for Qualcomm UFS V5.0 and above which > uses a pool of crypto cores for TX stream (UFS Write – Encryption) > and RX stream (UFS Read – Decryption). > > Using these algorithms, crypto cores can be dynamically allocated > to either RX stream or TX stream based on algorithm selected. > Qualcomm UFS controller supports three ICE algorithms: > Floor based algorithm, Static Algorithm and Instantaneous algorithm > to share crypto cores between TX and RX stream. > > Floor Based allocation is selected by default after power On or Reset. > > Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> > Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> > Co-developed-by: Nitin Rawat <quic_nitirawa@quicinc.com> > Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> > Co-developed-by: Can Guo <quic_cang@quicinc.com> > Signed-off-by: Can Guo <quic_cang@quicinc.com> > Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com> > --- > drivers/ufs/host/ufs-qcom.c | 232 ++++++++++++++++++++++++++++++++++++ > drivers/ufs/host/ufs-qcom.h | 38 +++++- > 2 files changed, 269 insertions(+), 1 deletion(-) Hi, a few nitpicks below. > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 810e637047d0..c0ca835f13f3 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) > } > > #ifdef CONFIG_SCSI_UFS_CRYPTO > +/* > + * Default overrides: > + * There're 10 sets of settings for floor-based algorithm > + */ > +static struct ice_alg2_config alg2_config[] = { I think that this could easily be a const struct. > + {"G0", {5, 12, 0, 0, 32, 0}}, > + {"G1", {12, 5, 32, 0, 0, 0}}, > + {"G2", {6, 11, 4, 1, 32, 1}}, > + {"G3", {6, 11, 7, 1, 32, 1}}, > + {"G4", {7, 10, 11, 1, 32, 1}}, > + {"G5", {7, 10, 14, 1, 32, 1}}, > + {"G6", {8, 9, 18, 1, 32, 1}}, > + {"G7", {9, 8, 21, 1, 32, 1}}, > + {"G8", {10, 7, 24, 1, 32, 1}}, > + {"G9", {10, 7, 32, 1, 32, 1}}, > +}; > + > +/** This does nor look like a kernel-doc. Just /* ? > + * Refer struct ice_alg2_config > + */ > +static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t) > +{ > + *c = ((val[0] << 8) | val[1] | (1 << 31)); > + *t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]); > +} ... > +/** > + * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm > + * > + * @hba: host controller instance > + * Return: zero for success and non-zero in case of a failure. > + */ > +static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba) > +{ > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > + unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0; > + /* 6 values for each group, refer struct ice_alg2_config */ > + unsigned int override_val[ICE_ALG2_NUM_PARAMS]; > + char name[8] = {0}; > + int i, ret; > + > + ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG); > + for (i = 0; i < ARRAY_SIZE(alg2_config); i++) { > + int core = 0, task = 0; > + > + if (host->ice_conf) { > + snprintf(name, sizeof(name), "%s%d", "g", i); Why not just "g%d"? > + ret = of_property_read_variable_u32_array(host->ice_conf, > + name, > + override_val, > + ICE_ALG2_NUM_PARAMS, > + ICE_ALG2_NUM_PARAMS); ... CJ
Hi Ram, kernel test robot noticed the following build warnings: [auto build test WARNING on robh/for-next] [also build test WARNING on mkp-scsi/for-next jejb-scsi/for-next linus/master v6.12-rc1 next-20241004] [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/Ram-Kumar-Dwivedi/dt-bindings-ufs-qcom-Document-ice-configuration-table/20241005-144559 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20241005064307.18972-4-quic_rdwivedi%40quicinc.com patch subject: [PATCH V1 3/3] scsi: ufs: qcom: Add support for multiple ICE algorithms config: arm-randconfig-001-20241006 (https://download.01.org/0day-ci/archive/20241006/202410061425.FamovVLB-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241006/202410061425.FamovVLB-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/202410061425.FamovVLB-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/ufs/host/ufs-qcom.c:126: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst * Refer struct ice_alg2_config vim +126 drivers/ufs/host/ufs-qcom.c 124 125 /** > 126 * Refer struct ice_alg2_config 127 */ 128 static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t) 129 { 130 *c = ((val[0] << 8) | val[1] | (1 << 31)); 131 *t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]); 132 } 133
On 06-Oct-24 1:03 AM, Christophe JAILLET wrote: > Le 05/10/2024 à 08:43, Ram Kumar Dwivedi a écrit : >> Add support for ICE algorithms for Qualcomm UFS V5.0 and above which >> uses a pool of crypto cores for TX stream (UFS Write – Encryption) >> and RX stream (UFS Read – Decryption). >> >> Using these algorithms, crypto cores can be dynamically allocated >> to either RX stream or TX stream based on algorithm selected. >> Qualcomm UFS controller supports three ICE algorithms: >> Floor based algorithm, Static Algorithm and Instantaneous algorithm >> to share crypto cores between TX and RX stream. >> >> Floor Based allocation is selected by default after power On or Reset. >> >> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> >> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> >> Co-developed-by: Nitin Rawat <quic_nitirawa@quicinc.com> >> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> >> Co-developed-by: Can Guo <quic_cang@quicinc.com> >> Signed-off-by: Can Guo <quic_cang@quicinc.com> >> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com> >> --- >> drivers/ufs/host/ufs-qcom.c | 232 ++++++++++++++++++++++++++++++++++++ >> drivers/ufs/host/ufs-qcom.h | 38 +++++- >> 2 files changed, 269 insertions(+), 1 deletion(-) > > Hi, > > a few nitpicks below. > >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index 810e637047d0..c0ca835f13f3 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) >> } >> #ifdef CONFIG_SCSI_UFS_CRYPTO >> +/* >> + * Default overrides: >> + * There're 10 sets of settings for floor-based algorithm >> + */ >> +static struct ice_alg2_config alg2_config[] = { > > I think that this could easily be a const struct. > >> + {"G0", {5, 12, 0, 0, 32, 0}}, >> + {"G1", {12, 5, 32, 0, 0, 0}}, >> + {"G2", {6, 11, 4, 1, 32, 1}}, >> + {"G3", {6, 11, 7, 1, 32, 1}}, >> + {"G4", {7, 10, 11, 1, 32, 1}}, >> + {"G5", {7, 10, 14, 1, 32, 1}}, >> + {"G6", {8, 9, 18, 1, 32, 1}}, >> + {"G7", {9, 8, 21, 1, 32, 1}}, >> + {"G8", {10, 7, 24, 1, 32, 1}}, >> + {"G9", {10, 7, 32, 1, 32, 1}}, >> +}; >> + >> +/** > > This does nor look like a kernel-doc. Just /* ? > >> + * Refer struct ice_alg2_config >> + */ >> +static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t) >> +{ >> + *c = ((val[0] << 8) | val[1] | (1 << 31)); >> + *t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]); >> +} > > ... > >> +/** >> + * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm >> + * >> + * @hba: host controller instance >> + * Return: zero for success and non-zero in case of a failure. >> + */ >> +static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba) >> +{ >> + struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> + unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0; >> + /* 6 values for each group, refer struct ice_alg2_config */ >> + unsigned int override_val[ICE_ALG2_NUM_PARAMS]; >> + char name[8] = {0}; >> + int i, ret; >> + >> + ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG); >> + for (i = 0; i < ARRAY_SIZE(alg2_config); i++) { >> + int core = 0, task = 0; >> + >> + if (host->ice_conf) { >> + snprintf(name, sizeof(name), "%s%d", "g", i); > > Why not just "g%d"? > >> + ret = of_property_read_variable_u32_array(host->ice_conf, >> + name, >> + override_val, >> + ICE_ALG2_NUM_PARAMS, >> + ICE_ALG2_NUM_PARAMS); > > ... > > CJ > Hi Christophe, I have addressed your comments in latest patchset. Thanks, Ram.
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 810e637047d0..c0ca835f13f3 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -105,6 +105,217 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) } #ifdef CONFIG_SCSI_UFS_CRYPTO +/* + * Default overrides: + * There're 10 sets of settings for floor-based algorithm + */ +static struct ice_alg2_config alg2_config[] = { + {"G0", {5, 12, 0, 0, 32, 0}}, + {"G1", {12, 5, 32, 0, 0, 0}}, + {"G2", {6, 11, 4, 1, 32, 1}}, + {"G3", {6, 11, 7, 1, 32, 1}}, + {"G4", {7, 10, 11, 1, 32, 1}}, + {"G5", {7, 10, 14, 1, 32, 1}}, + {"G6", {8, 9, 18, 1, 32, 1}}, + {"G7", {9, 8, 21, 1, 32, 1}}, + {"G8", {10, 7, 24, 1, 32, 1}}, + {"G9", {10, 7, 32, 1, 32, 1}}, +}; + +/** + * Refer struct ice_alg2_config + */ +static inline void __get_alg2_grp_params(unsigned int *val, int *c, int *t) +{ + *c = ((val[0] << 8) | val[1] | (1 << 31)); + *t = ((val[2] << 24) | (val[3] << 16) | (val[4] << 8) | val[5]); +} + +static inline void get_alg2_grp_params(unsigned int group, int *core, int *task) +{ + struct ice_alg2_config *p = &alg2_config[group]; + + __get_alg2_grp_params(p->val, core, task); +} + +/** + * ufs_qcom_ice_config_alg1 - Static ICE Algorithm + * + * @hba: host controller instance + * Return: zero for success and non-zero in case of a failure. + */ +static int ufs_qcom_ice_config_alg1(struct ufs_hba *hba) +{ + struct ufs_qcom_host *host = ufshcd_get_variant(hba); + unsigned int val, rx_aes; + unsigned int num_aes_cores; + int ret; + + ret = of_property_read_u32(host->ice_conf, "rx-alloc-percent", &val); + if (ret) + return ret; + + num_aes_cores = ufshcd_readl(hba, REG_UFS_MEM_ICE_NUM_AES_CORES); + ufshcd_writel(hba, STATIC_ALLOC_ALG1, REG_UFS_MEM_ICE_CONFIG); + + /* + * DTS specifies the percent allocation to rx stream + * Calculation - + * Num Tx stream = N_TOT - (N_TOT * percent of rx stream allocation) + */ + rx_aes = DIV_ROUND_CLOSEST(num_aes_cores * val, 100); + val = rx_aes | ((num_aes_cores - rx_aes) << 8); + ufshcd_writel(hba, val, REG_UFS_MEM_ICE_ALG1_NUM_CORE); + + return 0; +} + +/** + * ufs_qcom_ice_config_alg2 - Floor based ICE algorithm + * + * @hba: host controller instance + * Return: zero for success and non-zero in case of a failure. + */ +static int ufs_qcom_ice_config_alg2(struct ufs_hba *hba) +{ + struct ufs_qcom_host *host = ufshcd_get_variant(hba); + unsigned int reg = REG_UFS_MEM_ICE_ALG2_NUM_CORE_0; + /* 6 values for each group, refer struct ice_alg2_config */ + unsigned int override_val[ICE_ALG2_NUM_PARAMS]; + char name[8] = {0}; + int i, ret; + + ufshcd_writel(hba, FLOOR_BASED_ALG2, REG_UFS_MEM_ICE_CONFIG); + for (i = 0; i < ARRAY_SIZE(alg2_config); i++) { + int core = 0, task = 0; + + if (host->ice_conf) { + snprintf(name, sizeof(name), "%s%d", "g", i); + ret = of_property_read_variable_u32_array(host->ice_conf, + name, + override_val, + ICE_ALG2_NUM_PARAMS, + ICE_ALG2_NUM_PARAMS); + /* Some/All parameters may be overwritten */ + if (ret > 0) + __get_alg2_grp_params(override_val, &core, + &task); + else + get_alg2_grp_params(i, &core, &task); + } else { + get_alg2_grp_params(i, &core, &task); + } + + /* Num Core and Num task are contiguous & configured for a group together */ + ufshcd_writel(hba, core, reg); + reg += 4; + ufshcd_writel(hba, task, reg); + reg += 4; + } + + return 0; +} + +/** + * ufs_qcom_ice_config_alg3 - Instantaneous ICE Algorithm + * + * @hba: host controller instance + * Return: zero for success and non-zero in case of a failure. + */ +static int ufs_qcom_ice_config_alg3(struct ufs_hba *hba) +{ + unsigned int val[4]; + struct ufs_qcom_host *host = ufshcd_get_variant(hba); + unsigned int config; + int ret; + + ret = of_property_read_variable_u32_array(host->ice_conf, "num-core", val, + 4, 4); + if (ret < 0) + return ret; + + config = val[0] | (val[1] << 8) | (val[2] << 16) | (val[3] << 24); + + ufshcd_writel(hba, INSTANTANEOUS_ALG3, REG_UFS_MEM_ICE_CONFIG); + ufshcd_writel(hba, config, REG_UFS_MEM_ICE_ALG3_NUM_CORE); + + return 0; +} + +static int ufs_qcom_parse_ice_config(struct ufs_hba *hba) +{ + struct ufs_qcom_host *host = ufshcd_get_variant(hba); + struct device_node *np; + struct device_node *ice_np; + const char *alg_name; + int ret; + + np = hba->dev->of_node; + if (!np) + return -ENOENT; + + ice_np = of_get_next_available_child(np, NULL); + if (!ice_np) + return -ENOENT; + + /* Only 1 algo can be enabled, pick the first */ + host->ice_conf = of_get_next_available_child(ice_np, NULL); + if (!host->ice_conf) { + /* No overrides, use floor based as default */ + host->chosen_algo = FLOOR_BASED_ALG2; + dev_info(hba->dev, "Resort to default ice alg2\n"); + return 0; + } + + ret = of_property_read_string(host->ice_conf, "alg-name", &alg_name); + if (ret < 0) + return ret; + + if (!strcmp(alg_name, "alg1")) + host->chosen_algo = STATIC_ALLOC_ALG1; + else if (!strcmp(alg_name, "alg2")) + host->chosen_algo = FLOOR_BASED_ALG2; + else if (!strcmp(alg_name, "alg3")) + host->chosen_algo = INSTANTANEOUS_ALG3; + else { + dev_err(hba->dev, "Failed to find a valid ice alg name\n"); + ret = -ENODATA; + } + + return ret; +} + +static int ufs_qcom_config_ice(struct ufs_qcom_host *host) +{ + if (!is_ice_config_supported(host)) + return 0; + + switch (host->chosen_algo) { + case STATIC_ALLOC_ALG1: + return ufs_qcom_ice_config_alg1(host->hba); + case FLOOR_BASED_ALG2: + return ufs_qcom_ice_config_alg2(host->hba); + case INSTANTANEOUS_ALG3: + return ufs_qcom_ice_config_alg3(host->hba); + } + + return -EINVAL; +} + +static int ufs_qcom_ice_config_init(struct ufs_qcom_host *host) +{ + struct ufs_hba *hba = host->hba; + int ret; + + if (!is_ice_config_supported(host)) + return 0; + + ret = ufs_qcom_parse_ice_config(hba); + if (!ret) + dev_dbg(hba->dev, "ice config initialization success!!"); + + return ret; +} static inline void ufs_qcom_ice_enable(struct ufs_qcom_host *host) { @@ -117,6 +328,7 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host) struct ufs_hba *hba = host->hba; struct device *dev = hba->dev; struct qcom_ice *ice; + int ret; ice = of_qcom_ice_get(dev); if (ice == ERR_PTR(-EOPNOTSUPP)) { @@ -130,6 +342,10 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host) host->ice = ice; hba->caps |= UFSHCD_CAP_CRYPTO; + ret = ufs_qcom_ice_config_init(host); + if (ret) + dev_info(dev, "Continue with default ice configuration\n"); + return 0; } @@ -196,6 +412,12 @@ static inline int ufs_qcom_ice_suspend(struct ufs_qcom_host *host) { return 0; } + +static int ufs_qcom_config_ice(struct ufs_qcom_host *host) +{ + return 0; +} + #endif static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host) @@ -435,6 +657,11 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, err = ufs_qcom_enable_lane_clks(host); break; case POST_CHANGE: + err = ufs_qcom_config_ice(host); + if (err) { + dev_err(hba->dev, "failed to configure ice, ret=%d\n", err); + break; + } /* check if UFS PHY moved from DISABLED to HIBERN8 */ err = ufs_qcom_check_hibern8(hba); ufs_qcom_enable_hw_clk_gating(hba); @@ -914,12 +1141,17 @@ static void ufs_qcom_set_host_params(struct ufs_hba *hba) static void ufs_qcom_set_caps(struct ufs_hba *hba) { + struct ufs_qcom_host *host = ufshcd_get_variant(hba); + hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING; hba->caps |= UFSHCD_CAP_CLK_SCALING | UFSHCD_CAP_WB_WITH_CLK_SCALING; hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND; hba->caps |= UFSHCD_CAP_WB_EN; hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE; hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND; + + if (host->hw_ver.major >= 0x5) + host->caps |= UFS_QCOM_CAP_ICE_CONFIG; } /** diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h index b9de170983c9..6884408eb807 100644 --- a/drivers/ufs/host/ufs-qcom.h +++ b/drivers/ufs/host/ufs-qcom.h @@ -195,8 +195,11 @@ struct ufs_qcom_host { #ifdef CONFIG_SCSI_UFS_CRYPTO struct qcom_ice *ice; + struct device_node *ice_conf; + int chosen_algo; #endif - + #define UFS_QCOM_CAP_ICE_CONFIG BIT(4) + u32 caps; void __iomem *dev_ref_clk_ctrl_mmio; bool is_dev_ref_clk_enabled; struct ufs_hw_version hw_ver; @@ -226,6 +229,39 @@ ufs_qcom_get_debug_reg_offset(struct ufs_qcom_host *host, u32 reg) return UFS_CNTLR_3_x_x_VEN_REGS_OFFSET(reg); }; +#ifdef CONFIG_SCSI_UFS_CRYPTO +static inline bool is_ice_config_supported(struct ufs_qcom_host *host) +{ + return (host->caps & UFS_QCOM_CAP_ICE_CONFIG); +} + +/* Algorithm Selection */ +#define STATIC_ALLOC_ALG1 0x0 +#define FLOOR_BASED_ALG2 BIT(0) +#define INSTANTANEOUS_ALG3 BIT(1) + +enum { + REG_UFS_MEM_ICE_NUM_AES_CORES = 0x2608, + REG_UFS_MEM_ICE_CONFIG = 0x260C, + REG_UFS_MEM_ICE_ALG1_NUM_CORE = 0x2610, + REG_UFS_MEM_ICE_ALG2_NUM_CORE_0 = 0x2614, + REG_UFS_MEM_ICE_ALG3_NUM_CORE = 0x2664, +}; + +#define ICE_ALG2_NAME_LEN 3 +#define ICE_ALG2_NUM_PARAMS 6 + +struct ice_alg2_config { + /* group names */ + char name[ICE_ALG2_NAME_LEN]; + /* + * num_core_tx_stream, num_core_rx_stream, num_wr_task_max, + * num_wr_task_min, num_rd_task_max, num_rd_task_min + */ + unsigned int val[ICE_ALG2_NUM_PARAMS]; +}; +#endif + #define ufs_qcom_is_link_off(hba) ufshcd_is_link_off(hba) #define ufs_qcom_is_link_active(hba) ufshcd_is_link_active(hba) #define ufs_qcom_is_link_hibern8(hba) ufshcd_is_link_hibern8(hba)