On 8/1/2020 3:42 PM, Herbert Xu wrote: > On Fri, Jul 31, 2020 at 07:46:07PM +0300, Horia Geantă wrote: >> >> Below hunk is needed for fixing the compilation when CONFIG_DEBUG_FS=y: > > Thanks for catching this. The NULL pointer assignments are also > a bit iffy. So here is an updated version: > Thanks. > @@ -912,56 +916,54 @@ static int caam_probe(struct platform_device *pdev) > dev_info(dev, "job rings = %d, qi = %d\n", > ctrlpriv->total_jobrs, ctrlpriv->qi_present); > > -#ifdef CONFIG_DEBUG_FS > - debugfs_create_file("rq_dequeued", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->req_dequeued, > - &caam_fops_u64_ro); > - debugfs_create_file("ob_rq_encrypted", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ob_enc_req, > - &caam_fops_u64_ro); > - debugfs_create_file("ib_rq_decrypted", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ib_dec_req, > - &caam_fops_u64_ro); > - debugfs_create_file("ob_bytes_encrypted", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ob_enc_bytes, > - &caam_fops_u64_ro); > - debugfs_create_file("ob_bytes_protected", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ob_prot_bytes, > - &caam_fops_u64_ro); > - debugfs_create_file("ib_bytes_decrypted", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ib_dec_bytes, > - &caam_fops_u64_ro); > - debugfs_create_file("ib_bytes_validated", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->ib_valid_bytes, > - &caam_fops_u64_ro); > + caam_debugfs_create_file_u64("rq_dequeued", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->req_dequeued); > + caam_debugfs_create_file_u64("ob_rq_encrypted", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ob_enc_req); > + caam_debugfs_create_file_u64("ib_rq_decrypted", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ib_dec_req); > + caam_debugfs_create_file_u64("ob_bytes_encrypted", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ob_enc_bytes); > + caam_debugfs_create_file_u64("ob_bytes_protected", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ob_prot_bytes); > + caam_debugfs_create_file_u64("ib_bytes_decrypted", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ib_dec_bytes); > + caam_debugfs_create_file_u64("ib_bytes_validated", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->ib_valid_bytes); > > /* Controller level - global status values */ > - debugfs_create_file("fault_addr", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->faultaddr, > - &caam_fops_u32_ro); > - debugfs_create_file("fault_detail", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->faultdetail, > - &caam_fops_u32_ro); > - debugfs_create_file("fault_status", S_IRUSR | S_IRGRP | S_IROTH, > - ctrlpriv->ctl, &perfmon->status, > - &caam_fops_u32_ro); > + caam_debugfs_create_file_u32("fault_addr", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->faultaddr); > + caam_debugfs_create_file_u32("fault_detail", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->faultdetail); > + caam_debugfs_create_file_u32("fault_status", > + S_IRUSR | S_IRGRP | S_IROTH, > + ctl, &perfmon->status); > > /* Internal covering keys (useful in non-secure mode only) */ > - ctrlpriv->ctl_kek_wrap.data = (__force void *)&ctrlpriv->ctrl->kek[0]; > - ctrlpriv->ctl_kek_wrap.size = KEK_KEY_SIZE * sizeof(u32); > - debugfs_create_blob("kek", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, > - &ctrlpriv->ctl_kek_wrap); > - > - ctrlpriv->ctl_tkek_wrap.data = (__force void *)&ctrlpriv->ctrl->tkek[0]; > - ctrlpriv->ctl_tkek_wrap.size = KEK_KEY_SIZE * sizeof(u32); > - debugfs_create_blob("tkek", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, > - &ctrlpriv->ctl_tkek_wrap); > - > - ctrlpriv->ctl_tdsk_wrap.data = (__force void *)&ctrlpriv->ctrl->tdsk[0]; > - ctrlpriv->ctl_tdsk_wrap.size = KEK_KEY_SIZE * sizeof(u32); > - debugfs_create_blob("tdsk", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, > - &ctrlpriv->ctl_tdsk_wrap); > -#endif > + blob = caam_debugfs_ptr(&ctrlpriv->ctl_kek_wrap, &blob0); > + blob->data = (__force void *)&ctrlpriv->ctrl->kek[0]; > + blob->size = KEK_KEY_SIZE * sizeof(u32); > + debugfs_create_blob("kek", S_IRUSR | S_IRGRP | S_IROTH, ctl, blob); > + > + blob = caam_debugfs_ptr(&ctrlpriv->ctl_tkek_wrap, &blob0); > + blob->data = (__force void *)&ctrlpriv->ctrl->tkek[0]; > + blob->size = KEK_KEY_SIZE * sizeof(u32); > + debugfs_create_blob("tkek", S_IRUSR | S_IRGRP | S_IROTH, ctl, blob); > + > + blob = caam_debugfs_ptr(&ctrlpriv->ctl_tdsk_wrap, &blob0); > + blob->data = (__force void *)&ctrlpriv->ctrl->tdsk[0]; > + blob->size = KEK_KEY_SIZE * sizeof(u32); > + debugfs_create_blob("tdsk", S_IRUSR | S_IRGRP | S_IROTH, ctl, blob); > I'd rather move these in the newly-created debugfs.c, see below v3. > --- a/drivers/crypto/caam/qi.c > +++ b/drivers/crypto/caam/qi.c > @@ -11,6 +11,7 @@ > #include <linux/kthread.h> > #include <soc/fsl/qman.h> > > +#include "debugfs.h" > #include "regs.h" > #include "qi.h" > #include "desc.h" > @@ -776,8 +777,8 @@ int caam_qi_init(struct platform_device *caam_pdev) > } > > #ifdef CONFIG_DEBUG_FS > - debugfs_create_file("qi_congested", 0444, ctrlpriv->ctl, > - ×_congested, &caam_fops_u64_ro); > + caam_debugfs_create_file_u64("qi_congested", 0444, ctrlpriv->ctl, > + ×_congested); > #endif ifdef should go away. I've done some changes on top of v2, mainly moving the debugfs-related operations from ctrl.c, qi.c into debugfs.c. I've kept you Sign-off, but if you consider v3 is too different from v2 feel free to change it into Suggested-by / Reported-by. ---8<--- Currently the debugfs fops are defined in caam/intern.h. This causes problems because it creates identical static functions and variables in multiple files. It also creates warnings when those files don't use the fops. This patch moves them into a standalone file, debugfs.c. It also removes unnecessary uses of ifdefs on CONFIG_DEBUG_FS. Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> [Moved most of debugfs-related operations into debugfs.c.] Signed-off-by: Horia Geantă <horia.geanta@xxxxxxx> --- drivers/crypto/caam/Makefile | 2 + drivers/crypto/caam/ctrl.c | 77 +++------------------------ drivers/crypto/caam/debugfs.c | 96 +++++++++++++++++++++++++++++++++++ drivers/crypto/caam/debugfs.h | 26 ++++++++++ drivers/crypto/caam/intern.h | 17 ------ drivers/crypto/caam/qi.c | 20 ++------ 6 files changed, 137 insertions(+), 102 deletions(-) create mode 100644 drivers/crypto/caam/debugfs.c create mode 100644 drivers/crypto/caam/debugfs.h diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile index 68d5cc0f28e2..3570286eb9ce 100644 --- a/drivers/crypto/caam/Makefile +++ b/drivers/crypto/caam/Makefile @@ -27,6 +27,8 @@ ifneq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),) ccflags-y += -DCONFIG_CAAM_QI endif +caam-$(CONFIG_DEBUG_FS) += debugfs.o + obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o dpaa2_caam-y := caamalg_qi2.o dpseci.o diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 94502f1d4b48..65de57f169d9 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -13,6 +13,7 @@ #include <linux/fsl/mc.h> #include "compat.h" +#include "debugfs.h" #include "regs.h" #include "intern.h" #include "jr.h" @@ -582,12 +583,10 @@ static int init_clocks(struct device *dev, const struct caam_imx_data *data) return devm_add_action_or_reset(dev, disable_clocks, ctrlpriv); } -#ifdef CONFIG_DEBUG_FS static void caam_remove_debugfs(void *root) { debugfs_remove_recursive(root); } -#endif #ifdef CONFIG_FSL_MC_BUS static bool check_version(struct fsl_mc_version *mc_version, u32 major, @@ -619,10 +618,7 @@ static int caam_probe(struct platform_device *pdev) struct device_node *nprop, *np; struct caam_ctrl __iomem *ctrl; struct caam_drv_private *ctrlpriv; -#ifdef CONFIG_DEBUG_FS - struct caam_perfmon *perfmon; struct dentry *dfs_root; -#endif u32 scfgr, comp_params; u8 rng_vid; int pg_size; @@ -777,21 +773,15 @@ static int caam_probe(struct platform_device *pdev) ctrlpriv->era = caam_get_era(ctrl); ctrlpriv->domain = iommu_get_domain_for_dev(dev); -#ifdef CONFIG_DEBUG_FS - /* - * FIXME: needs better naming distinction, as some amalgamation of - * "caam" and nprop->full_name. The OF name isn't distinctive, - * but does separate instances - */ - perfmon = (struct caam_perfmon __force *)&ctrl->perfmon; - dfs_root = debugfs_create_dir(dev_name(dev), NULL); - ret = devm_add_action_or_reset(dev, caam_remove_debugfs, dfs_root); - if (ret) - return ret; + if (IS_ENABLED(CONFIG_DEBUG_FS)) { + ret = devm_add_action_or_reset(dev, caam_remove_debugfs, + dfs_root); + if (ret) + return ret; + } - ctrlpriv->ctl = debugfs_create_dir("ctl", dfs_root); -#endif + caam_debugfs_init(ctrlpriv, dfs_root); /* Check to see if (DPAA 1.x) QI present. If so, enable */ if (ctrlpriv->qi_present && !caam_dpaa2) { @@ -912,57 +902,6 @@ static int caam_probe(struct platform_device *pdev) dev_info(dev, "job rings = %d, qi = %d\n", ctrlpriv->total_jobrs, ctrlpriv->qi_present); -#ifdef CONFIG_DEBUG_FS - debugfs_create_file("rq_dequeued", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->req_dequeued, - &caam_fops_u64_ro); - debugfs_create_file("ob_rq_encrypted", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ob_enc_req, - &caam_fops_u64_ro); - debugfs_create_file("ib_rq_decrypted", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ib_dec_req, - &caam_fops_u64_ro); - debugfs_create_file("ob_bytes_encrypted", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ob_enc_bytes, - &caam_fops_u64_ro); - debugfs_create_file("ob_bytes_protected", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ob_prot_bytes, - &caam_fops_u64_ro); - debugfs_create_file("ib_bytes_decrypted", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ib_dec_bytes, - &caam_fops_u64_ro); - debugfs_create_file("ib_bytes_validated", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->ib_valid_bytes, - &caam_fops_u64_ro); - - /* Controller level - global status values */ - debugfs_create_file("fault_addr", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->faultaddr, - &caam_fops_u32_ro); - debugfs_create_file("fault_detail", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->faultdetail, - &caam_fops_u32_ro); - debugfs_create_file("fault_status", S_IRUSR | S_IRGRP | S_IROTH, - ctrlpriv->ctl, &perfmon->status, - &caam_fops_u32_ro); - - /* Internal covering keys (useful in non-secure mode only) */ - ctrlpriv->ctl_kek_wrap.data = (__force void *)&ctrlpriv->ctrl->kek[0]; - ctrlpriv->ctl_kek_wrap.size = KEK_KEY_SIZE * sizeof(u32); - debugfs_create_blob("kek", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, - &ctrlpriv->ctl_kek_wrap); - - ctrlpriv->ctl_tkek_wrap.data = (__force void *)&ctrlpriv->ctrl->tkek[0]; - ctrlpriv->ctl_tkek_wrap.size = KEK_KEY_SIZE * sizeof(u32); - debugfs_create_blob("tkek", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, - &ctrlpriv->ctl_tkek_wrap); - - ctrlpriv->ctl_tdsk_wrap.data = (__force void *)&ctrlpriv->ctrl->tdsk[0]; - ctrlpriv->ctl_tdsk_wrap.size = KEK_KEY_SIZE * sizeof(u32); - debugfs_create_blob("tdsk", S_IRUSR | S_IRGRP | S_IROTH, ctrlpriv->ctl, - &ctrlpriv->ctl_tdsk_wrap); -#endif - ret = devm_of_platform_populate(dev); if (ret) dev_err(dev, "JR platform devices creation error\n"); diff --git a/drivers/crypto/caam/debugfs.c b/drivers/crypto/caam/debugfs.c new file mode 100644 index 000000000000..b6137df155e0 --- /dev/null +++ b/drivers/crypto/caam/debugfs.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) +/* Copyright 2019 NXP */ + +#include <linux/debugfs.h> +#include "compat.h" +#include "debugfs.h" +#include "regs.h" +#include "intern.h" + +static int caam_debugfs_u64_get(void *data, u64 *val) +{ + *val = caam64_to_cpu(*(u64 *)data); + return 0; +} + +static int caam_debugfs_u32_get(void *data, u64 *val) +{ + *val = caam32_to_cpu(*(u32 *)data); + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u32_ro, caam_debugfs_u32_get, NULL, "%llu\n"); +DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u64_ro, caam_debugfs_u64_get, NULL, "%llu\n"); + +#ifdef CONFIG_CAAM_QI +/* + * This is a counter for the number of times the congestion group (where all + * the request and response queueus are) reached congestion. Incremented + * each time the congestion callback is called with congested == true. + */ +static u64 times_congested; + +void caam_debugfs_qi_congested(void) +{ + times_congested++; +} + +void caam_debugfs_qi_init(struct caam_drv_private *ctrlpriv) +{ + debugfs_create_file("qi_congested", 0444, ctrlpriv->ctl, + ×_congested, &caam_fops_u64_ro); +} +#endif + +void caam_debugfs_init(struct caam_drv_private *ctrlpriv, struct dentry *root) +{ + struct caam_perfmon *perfmon; + + /* + * FIXME: needs better naming distinction, as some amalgamation of + * "caam" and nprop->full_name. The OF name isn't distinctive, + * but does separate instances + */ + perfmon = (struct caam_perfmon __force *)&ctrlpriv->ctrl->perfmon; + + ctrlpriv->ctl = debugfs_create_dir("ctl", root); + + debugfs_create_file("rq_dequeued", 0444, ctrlpriv->ctl, + &perfmon->req_dequeued, &caam_fops_u64_ro); + debugfs_create_file("ob_rq_encrypted", 0444, ctrlpriv->ctl, + &perfmon->ob_enc_req, &caam_fops_u64_ro); + debugfs_create_file("ib_rq_decrypted", 0444, ctrlpriv->ctl, + &perfmon->ib_dec_req, &caam_fops_u64_ro); + debugfs_create_file("ob_bytes_encrypted", 0444, ctrlpriv->ctl, + &perfmon->ob_enc_bytes, &caam_fops_u64_ro); + debugfs_create_file("ob_bytes_protected", 0444, ctrlpriv->ctl, + &perfmon->ob_prot_bytes, &caam_fops_u64_ro); + debugfs_create_file("ib_bytes_decrypted", 0444, ctrlpriv->ctl, + &perfmon->ib_dec_bytes, &caam_fops_u64_ro); + debugfs_create_file("ib_bytes_validated", 0444, ctrlpriv->ctl, + &perfmon->ib_valid_bytes, &caam_fops_u64_ro); + + /* Controller level - global status values */ + debugfs_create_file("fault_addr", 0444, ctrlpriv->ctl, + &perfmon->faultaddr, &caam_fops_u32_ro); + debugfs_create_file("fault_detail", 0444, ctrlpriv->ctl, + &perfmon->faultdetail, &caam_fops_u32_ro); + debugfs_create_file("fault_status", 0444, ctrlpriv->ctl, + &perfmon->status, &caam_fops_u32_ro); + + /* Internal covering keys (useful in non-secure mode only) */ + ctrlpriv->ctl_kek_wrap.data = (__force void *)&ctrlpriv->ctrl->kek[0]; + ctrlpriv->ctl_kek_wrap.size = KEK_KEY_SIZE * sizeof(u32); + debugfs_create_blob("kek", 0444, ctrlpriv->ctl, + &ctrlpriv->ctl_kek_wrap); + + ctrlpriv->ctl_tkek_wrap.data = (__force void *)&ctrlpriv->ctrl->tkek[0]; + ctrlpriv->ctl_tkek_wrap.size = KEK_KEY_SIZE * sizeof(u32); + debugfs_create_blob("tkek", 0444, ctrlpriv->ctl, + &ctrlpriv->ctl_tkek_wrap); + + ctrlpriv->ctl_tdsk_wrap.data = (__force void *)&ctrlpriv->ctrl->tdsk[0]; + ctrlpriv->ctl_tdsk_wrap.size = KEK_KEY_SIZE * sizeof(u32); + debugfs_create_blob("tdsk", 0444, ctrlpriv->ctl, + &ctrlpriv->ctl_tdsk_wrap); +} diff --git a/drivers/crypto/caam/debugfs.h b/drivers/crypto/caam/debugfs.h new file mode 100644 index 000000000000..661d768acdbf --- /dev/null +++ b/drivers/crypto/caam/debugfs.h @@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */ +/* Copyright 2019 NXP */ + +#ifndef CAAM_DEBUGFS_H +#define CAAM_DEBUGFS_H + +struct dentry; +struct caam_drv_private; + +#ifdef CONFIG_DEBUG_FS +void caam_debugfs_init(struct caam_drv_private *ctrlpriv, struct dentry *root); +#else +static inline void caam_debugfs_init(struct caam_drv_private *ctrlpriv, + struct dentry *root) +{} +#endif + +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_CAAM_QI) +void caam_debugfs_qi_congested(void); +void caam_debugfs_qi_init(struct caam_drv_private *ctrlpriv); +#else +static inline void caam_debugfs_qi_congested(void) {} +static inline void caam_debugfs_qi_init(struct caam_drv_private *ctrlpriv) {} +#endif + +#endif /* CAAM_DEBUGFS_H */ diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h index 402d6a362e8c..9112279a4de0 100644 --- a/drivers/crypto/caam/intern.h +++ b/drivers/crypto/caam/intern.h @@ -195,23 +195,6 @@ static inline void caam_qi_algapi_exit(void) #endif /* CONFIG_CAAM_QI */ -#ifdef CONFIG_DEBUG_FS -static int caam_debugfs_u64_get(void *data, u64 *val) -{ - *val = caam64_to_cpu(*(u64 *)data); - return 0; -} - -static int caam_debugfs_u32_get(void *data, u64 *val) -{ - *val = caam32_to_cpu(*(u32 *)data); - return 0; -} - -DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u32_ro, caam_debugfs_u32_get, NULL, "%llu\n"); -DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u64_ro, caam_debugfs_u64_get, NULL, "%llu\n"); -#endif - static inline u64 caam_get_dma_mask(struct device *dev) { struct device_node *nprop = dev->of_node; diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c index b390b935db6d..ec53528d8205 100644 --- a/drivers/crypto/caam/qi.c +++ b/drivers/crypto/caam/qi.c @@ -11,6 +11,7 @@ #include <linux/kthread.h> #include <soc/fsl/qman.h> +#include "debugfs.h" #include "regs.h" #include "qi.h" #include "desc.h" @@ -73,15 +74,6 @@ static struct caam_qi_priv qipriv ____cacheline_aligned; bool caam_congested __read_mostly; EXPORT_SYMBOL(caam_congested); -#ifdef CONFIG_DEBUG_FS -/* - * This is a counter for the number of times the congestion group (where all - * the request and response queueus are) reached congestion. Incremented - * each time the congestion callback is called with congested == true. - */ -static u64 times_congested; -#endif - /* * This is a a cache of buffers, from which the users of CAAM QI driver * can allocate short (CAAM_QI_MEMCACHE_SIZE) buffers. It's faster than @@ -544,9 +536,8 @@ static void cgr_cb(struct qman_portal *qm, struct qman_cgr *cgr, int congested) caam_congested = congested; if (congested) { -#ifdef CONFIG_DEBUG_FS - times_congested++; -#endif + caam_debugfs_qi_congested(); + pr_debug_ratelimited("CAAM entered congestion\n"); } else { @@ -775,10 +766,7 @@ int caam_qi_init(struct platform_device *caam_pdev) return -ENOMEM; } -#ifdef CONFIG_DEBUG_FS - debugfs_create_file("qi_congested", 0444, ctrlpriv->ctl, - ×_congested, &caam_fops_u64_ro); -#endif + caam_debugfs_qi_init(ctrlpriv); err = devm_add_action_or_reset(qidev, caam_qi_shutdown, ctrlpriv); if (err) -- 2.17.1