| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AM9PR04MB8211E61777776A4AE60D135EE8809@AM9PR04MB8211.eurprd04.prod.outlook.com>
Date: Mon, 20 Mar 2023 09:12:49 +0000
From: Varun Sethi <V.Sethi@....com>
To: Meenakshi Aggarwal <meenakshi.aggarwal@....com>,
Horia Geanta <horia.geanta@....com>,
Pankaj Gupta <pankaj.gupta@....com>,
Gaurav Jain <gaurav.jain@....com>,
"herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
"davem@...emloft.net" <davem@...emloft.net>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] crypto: caam - reduce page 0 regs access to minimum
Reviewed-by: Varun Sethi <v.sethi@....com>
Regards
Varun
> -----Original Message-----
> From: Meenakshi Aggarwal <meenakshi.aggarwal@....com>
> Sent: Thursday, March 2, 2023 11:51 AM
> To: Horia Geanta <horia.geanta@....com>; Varun Sethi <V.Sethi@....com>;
> Pankaj Gupta <pankaj.gupta@....com>; Gaurav Jain <gaurav.jain@....com>;
> herbert@...dor.apana.org.au; davem@...emloft.net; linux-
> crypto@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: [PATCH 1/2] crypto: caam - reduce page 0 regs access to minimum
>
> From: Horia GeantA <horia.geanta@....com>
>
> Use job ring register map, in place of controller register map to access page 0
> registers, as access to the controller register map is not permitted.
>
> Signed-off-by: Horia GeantA <horia.geanta@....com>
> ---
> drivers/crypto/caam/caamalg.c | 21 +++++-----
> drivers/crypto/caam/caamhash.c | 10 +++-- drivers/crypto/caam/caampkc.c |
> 6 +-- drivers/crypto/caam/caamrng.c | 6 +--
> drivers/crypto/caam/ctrl.c | 74 +++++++++++++++++++++-------------
> drivers/crypto/caam/debugfs.c | 8 ++-- drivers/crypto/caam/debugfs.h | 7
> +++-
> 7 files changed, 78 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index 4a9b998a8d26..29838b80a0bb 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -3,7 +3,7 @@
> * caam - Freescale FSL CAAM support for crypto API
> *
> * Copyright 2008-2011 Freescale Semiconductor, Inc.
> - * Copyright 2016-2019 NXP
> + * Copyright 2016-2019, 2023 NXP
> *
> * Based on talitos crypto API driver.
> *
> @@ -3530,13 +3530,14 @@ int caam_algapi_init(struct device *ctrldev)
> * First, detect presence and attributes of DES, AES, and MD blocks.
> */
> if (priv->era < 10) {
> + struct caam_perfmon __iomem *perfmon = &priv->jr[0]-
> >perfmon;
> u32 cha_vid, cha_inst, aes_rn;
>
> - cha_vid = rd_reg32(&priv->ctrl->perfmon.cha_id_ls);
> + cha_vid = rd_reg32(&perfmon->cha_id_ls);
> aes_vid = cha_vid & CHA_ID_LS_AES_MASK;
> md_vid = (cha_vid & CHA_ID_LS_MD_MASK) >>
> CHA_ID_LS_MD_SHIFT;
>
> - cha_inst = rd_reg32(&priv->ctrl->perfmon.cha_num_ls);
> + cha_inst = rd_reg32(&perfmon->cha_num_ls);
> des_inst = (cha_inst & CHA_ID_LS_DES_MASK) >>
> CHA_ID_LS_DES_SHIFT;
> aes_inst = cha_inst & CHA_ID_LS_AES_MASK; @@ -3544,23
> +3545,23 @@ int caam_algapi_init(struct device *ctrldev)
> ccha_inst = 0;
> ptha_inst = 0;
>
> - aes_rn = rd_reg32(&priv->ctrl->perfmon.cha_rev_ls) &
> - CHA_ID_LS_AES_MASK;
> + aes_rn = rd_reg32(&perfmon->cha_rev_ls) &
> CHA_ID_LS_AES_MASK;
> gcm_support = !(aes_vid == CHA_VER_VID_AES_LP && aes_rn <
> 8);
> } else {
> + struct version_regs __iomem *vreg = &priv->jr[0]->vreg;
> u32 aesa, mdha;
>
> - aesa = rd_reg32(&priv->ctrl->vreg.aesa);
> - mdha = rd_reg32(&priv->ctrl->vreg.mdha);
> + aesa = rd_reg32(&vreg->aesa);
> + mdha = rd_reg32(&vreg->mdha);
>
> aes_vid = (aesa & CHA_VER_VID_MASK) >>
> CHA_VER_VID_SHIFT;
> md_vid = (mdha & CHA_VER_VID_MASK) >>
> CHA_VER_VID_SHIFT;
>
> - des_inst = rd_reg32(&priv->ctrl->vreg.desa) &
> CHA_VER_NUM_MASK;
> + des_inst = rd_reg32(&vreg->desa) & CHA_VER_NUM_MASK;
> aes_inst = aesa & CHA_VER_NUM_MASK;
> md_inst = mdha & CHA_VER_NUM_MASK;
> - ccha_inst = rd_reg32(&priv->ctrl->vreg.ccha) &
> CHA_VER_NUM_MASK;
> - ptha_inst = rd_reg32(&priv->ctrl->vreg.ptha) &
> CHA_VER_NUM_MASK;
> + ccha_inst = rd_reg32(&vreg->ccha) & CHA_VER_NUM_MASK;
> + ptha_inst = rd_reg32(&vreg->ptha) & CHA_VER_NUM_MASK;
>
> gcm_support = aesa & CHA_VER_MISC_AES_GCM;
> }
> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
> index 82d3c730a502..80deb003f0a5 100644
> --- a/drivers/crypto/caam/caamhash.c
> +++ b/drivers/crypto/caam/caamhash.c
> @@ -3,7 +3,7 @@
> * caam - Freescale FSL CAAM support for ahash functions of crypto API
> *
> * Copyright 2011 Freescale Semiconductor, Inc.
> - * Copyright 2018-2019 NXP
> + * Copyright 2018-2019, 2023 NXP
> *
> * Based on caamalg.c crypto API driver.
> *
> @@ -1956,12 +1956,14 @@ int caam_algapi_hash_init(struct device *ctrldev)
> * presence and attributes of MD block.
> */
> if (priv->era < 10) {
> - md_vid = (rd_reg32(&priv->ctrl->perfmon.cha_id_ls) &
> + struct caam_perfmon __iomem *perfmon = &priv->jr[0]-
> >perfmon;
> +
> + md_vid = (rd_reg32(&perfmon->cha_id_ls) &
> CHA_ID_LS_MD_MASK) >> CHA_ID_LS_MD_SHIFT;
> - md_inst = (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
> + md_inst = (rd_reg32(&perfmon->cha_num_ls) &
> CHA_ID_LS_MD_MASK) >> CHA_ID_LS_MD_SHIFT;
> } else {
> - u32 mdha = rd_reg32(&priv->ctrl->vreg.mdha);
> + u32 mdha = rd_reg32(&priv->jr[0]->vreg.mdha);
>
> md_vid = (mdha & CHA_VER_VID_MASK) >>
> CHA_VER_VID_SHIFT;
> md_inst = mdha & CHA_VER_NUM_MASK;
> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
> index e40614fef39d..72afc249d42f 100644
> --- a/drivers/crypto/caam/caampkc.c
> +++ b/drivers/crypto/caam/caampkc.c
> @@ -3,7 +3,7 @@
> * caam - Freescale FSL CAAM support for Public Key Cryptography
> *
> * Copyright 2016 Freescale Semiconductor, Inc.
> - * Copyright 2018-2019 NXP
> + * Copyright 2018-2019, 2023 NXP
> *
> * There is no Shared Descriptor for PKC so that the Job Descriptor must carry
> * all the desired key parameters, input and output pointers.
> @@ -1168,10 +1168,10 @@ int caam_pkc_init(struct device *ctrldev)
>
> /* Determine public key hardware accelerator presence. */
> if (priv->era < 10) {
> - pk_inst = (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
> + pk_inst = (rd_reg32(&priv->jr[0]->perfmon.cha_num_ls) &
> CHA_ID_LS_PK_MASK) >> CHA_ID_LS_PK_SHIFT;
> } else {
> - pkha = rd_reg32(&priv->ctrl->vreg.pkha);
> + pkha = rd_reg32(&priv->jr[0]->vreg.pkha);
> pk_inst = pkha & CHA_VER_NUM_MASK;
>
> /*
> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
> index 1fd8ff965006..50eb55da45c2 100644
> --- a/drivers/crypto/caam/caamrng.c
> +++ b/drivers/crypto/caam/caamrng.c
> @@ -3,7 +3,7 @@
> * caam - Freescale FSL CAAM support for hw_random
> *
> * Copyright 2011 Freescale Semiconductor, Inc.
> - * Copyright 2018-2019 NXP
> + * Copyright 2018-2019, 2023 NXP
> *
> * Based on caamalg.c crypto API driver.
> *
> @@ -227,10 +227,10 @@ int caam_rng_init(struct device *ctrldev)
>
> /* Check for an instantiated RNG before registration */
> if (priv->era < 10)
> - rng_inst = (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
> + rng_inst = (rd_reg32(&priv->jr[0]->perfmon.cha_num_ls) &
> CHA_ID_LS_RNG_MASK) >> CHA_ID_LS_RNG_SHIFT;
> else
> - rng_inst = rd_reg32(&priv->ctrl->vreg.rng) &
> CHA_VER_NUM_MASK;
> + rng_inst = rd_reg32(&priv->jr[0]->vreg.rng) &
> CHA_VER_NUM_MASK;
>
> if (!rng_inst)
> return 0;
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> 6278afb951c3..be9724d05d01 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -3,7 +3,7 @@
> * Controller-level driver, kernel property detection, initialization
> *
> * Copyright 2008-2012 Freescale Semiconductor, Inc.
> - * Copyright 2018-2019 NXP
> + * Copyright 2018-2019, 2023 NXP
> */
>
> #include <linux/device.h>
> @@ -395,7 +395,7 @@ static void kick_trng(struct platform_device *pdev, int
> ent_delay)
> RTMCTL_SAMP_MODE_RAW_ES_SC);
> }
>
> -static int caam_get_era_from_hw(struct caam_ctrl __iomem *ctrl)
> +static int caam_get_era_from_hw(struct caam_perfmon __iomem *perfmon)
> {
> static const struct {
> u16 ip_id;
> @@ -421,12 +421,12 @@ static int caam_get_era_from_hw(struct caam_ctrl
> __iomem *ctrl)
> u16 ip_id;
> int i;
>
> - ccbvid = rd_reg32(&ctrl->perfmon.ccb_id);
> + ccbvid = rd_reg32(&perfmon->ccb_id);
> era = (ccbvid & CCBVID_ERA_MASK) >> CCBVID_ERA_SHIFT;
> if (era) /* This is '0' prior to CAAM ERA-6 */
> return era;
>
> - id_ms = rd_reg32(&ctrl->perfmon.caam_id_ms);
> + id_ms = rd_reg32(&perfmon->caam_id_ms);
> ip_id = (id_ms & SECVID_MS_IPID_MASK) >> SECVID_MS_IPID_SHIFT;
> maj_rev = (id_ms & SECVID_MS_MAJ_REV_MASK) >>
> SECVID_MS_MAJ_REV_SHIFT;
>
> @@ -446,7 +446,7 @@ static int caam_get_era_from_hw(struct caam_ctrl
> __iomem *ctrl)
> *
> * @ctrl: controller region
> */
> -static int caam_get_era(struct caam_ctrl __iomem *ctrl)
> +static int caam_get_era(struct caam_perfmon __iomem *perfmon)
> {
> struct device_node *caam_node;
> int ret;
> @@ -459,7 +459,7 @@ static int caam_get_era(struct caam_ctrl __iomem *ctrl)
> if (!ret)
> return prop;
> else
> - return caam_get_era_from_hw(ctrl);
> + return caam_get_era_from_hw(perfmon);
> }
>
> /*
> @@ -626,6 +626,7 @@ static int caam_probe(struct platform_device *pdev)
> struct device_node *nprop, *np;
> struct caam_ctrl __iomem *ctrl;
> struct caam_drv_private *ctrlpriv;
> + struct caam_perfmon __iomem *perfmon;
> struct dentry *dfs_root;
> u32 scfgr, comp_params;
> u8 rng_vid;
> @@ -665,9 +666,36 @@ static int caam_probe(struct platform_device *pdev)
> return ret;
> }
>
> - caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
> + ring = 0;
> + for_each_available_child_of_node(nprop, np)
> + if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
> + of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
> + u32 reg;
> +
> + if (of_property_read_u32_index(np, "reg", 0, ®)) {
> + dev_err(dev, "%s read reg property error\n",
> + np->full_name);
> + continue;
> + }
> +
> + ctrlpriv->jr[ring] = (struct caam_job_ring __iomem
> __force *)
> + ((__force uint8_t *)ctrl + reg);
> +
> + ctrlpriv->total_jobrs++;
> + ring++;
> + }
> +
> + /*
> + * Wherever possible, instead of accessing registers from the global
> page,
> + * use the alias registers in the first (cf. DT nodes order)
> + * job ring's page.
> + */
> + perfmon = ring ? (struct caam_perfmon *)&ctrlpriv->jr[0]->perfmon :
> + (struct caam_perfmon *)&ctrl->perfmon;
> +
> + caam_little_end = !(bool)(rd_reg32(&perfmon->status) &
> (CSTA_PLEND | CSTA_ALT_PLEND));
> - comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ms);
> + comp_params = rd_reg32(&perfmon->comp_parms_ms);
> if (comp_params & CTPR_MS_PS && rd_reg32(&ctrl->mcr) &
> MCFGR_LONG_PTR)
> caam_ptr_sz = sizeof(u64);
> else
> @@ -778,7 +806,7 @@ static int caam_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ctrlpriv->era = caam_get_era(ctrl);
> + ctrlpriv->era = caam_get_era(perfmon);
> ctrlpriv->domain = iommu_get_domain_for_dev(dev);
>
> dfs_root = debugfs_create_dir(dev_name(dev), NULL); @@ -789,7
> +817,7 @@ static int caam_probe(struct platform_device *pdev)
> return ret;
> }
>
> - caam_debugfs_init(ctrlpriv, dfs_root);
> + caam_debugfs_init(ctrlpriv, perfmon, dfs_root);
>
> /* Check to see if (DPAA 1.x) QI present. If so, enable */
> if (ctrlpriv->qi_present && !caam_dpaa2) { @@ -808,19 +836,6 @@
> static int caam_probe(struct platform_device *pdev) #endif
> }
>
> - ring = 0;
> - for_each_available_child_of_node(nprop, np)
> - if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
> - of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
> - ctrlpriv->jr[ring] = (struct caam_job_ring __iomem
> __force *)
> - ((__force uint8_t *)ctrl +
> - (ring + JR_BLOCK_NUMBER) *
> - BLOCK_OFFSET
> - );
> - ctrlpriv->total_jobrs++;
> - ring++;
> - }
> -
> /* If no QI and no rings specified, quit and go home */
> if ((!ctrlpriv->qi_present) && (!ctrlpriv->total_jobrs)) {
> dev_err(dev, "no queues configured, terminating\n"); @@ -
> 836,12 +851,17 @@ static int caam_probe(struct platform_device *pdev)
> * check both here.
> */
> if (ctrlpriv->era < 10) {
> - rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
> + rng_vid = (rd_reg32(&perfmon->cha_id_ls) &
> CHA_ID_LS_RNG_MASK) >> CHA_ID_LS_RNG_SHIFT;
> ctrlpriv->blob_present = ctrlpriv->blob_present &&
> (rd_reg32(&ctrl->perfmon.cha_num_ls) &
> CHA_ID_LS_AES_MASK);
> } else {
> - rng_vid = (rd_reg32(&ctrl->vreg.rng) & CHA_VER_VID_MASK) >>
> + struct version_regs __iomem *vreg;
> +
> + vreg = ring ? (struct version_regs *)&ctrlpriv->jr[0]->vreg :
> + (struct version_regs *)&ctrl->vreg;
> +
> + rng_vid = (rd_reg32(&vreg->rng) & CHA_VER_VID_MASK) >>
> CHA_VER_VID_SHIFT;
> ctrlpriv->blob_present = ctrlpriv->blob_present &&
> (rd_reg32(&ctrl->vreg.aesa) &
> CHA_VER_MISC_AES_NUM_MASK); @@ -925,8 +945,8 @@ static int
> caam_probe(struct platform_device *pdev)
>
> /* NOTE: RTIC detection ought to go here, around Si time */
>
> - caam_id = (u64)rd_reg32(&ctrl->perfmon.caam_id_ms) << 32 |
> - (u64)rd_reg32(&ctrl->perfmon.caam_id_ls);
> + caam_id = (u64)rd_reg32(&perfmon->caam_id_ms) << 32 |
> + (u64)rd_reg32(&perfmon->caam_id_ls);
>
> /* Report "alive" for developer to see */
> dev_info(dev, "device ID = 0x%016llx (Era %d)\n", caam_id, diff --git
> a/drivers/crypto/caam/debugfs.c b/drivers/crypto/caam/debugfs.c index
> 806bb20d2aa1..73fe8530c0d6 100644
> --- a/drivers/crypto/caam/debugfs.c
> +++ b/drivers/crypto/caam/debugfs.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> -/* Copyright 2019 NXP */
> +/* Copyright 2019, 2023 NXP */
>
> #include <linux/debugfs.h>
> #include "compat.h"
> @@ -42,16 +42,14 @@ void caam_debugfs_qi_init(struct caam_drv_private
> *ctrlpriv) } #endif
>
> -void caam_debugfs_init(struct caam_drv_private *ctrlpriv, struct dentry *root)
> +void caam_debugfs_init(struct caam_drv_private *ctrlpriv,
> + struct caam_perfmon *perfmon, 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);
>
> diff --git a/drivers/crypto/caam/debugfs.h b/drivers/crypto/caam/debugfs.h
> index 661d768acdbf..fd2f0fc46d53 100644
> --- a/drivers/crypto/caam/debugfs.h
> +++ b/drivers/crypto/caam/debugfs.h
> @@ -1,16 +1,19 @@
> /* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
> -/* Copyright 2019 NXP */
> +/* Copyright 2019, 2023 NXP */
>
> #ifndef CAAM_DEBUGFS_H
> #define CAAM_DEBUGFS_H
>
> struct dentry;
> struct caam_drv_private;
> +struct caam_perfmon;
>
> #ifdef CONFIG_DEBUG_FS
> -void caam_debugfs_init(struct caam_drv_private *ctrlpriv, struct dentry *root);
> +void caam_debugfs_init(struct caam_drv_private *ctrlpriv,
> + struct caam_perfmon *perfmon, struct dentry *root);
> #else
> static inline void caam_debugfs_init(struct caam_drv_private *ctrlpriv,
> + struct caam_perfmon *perfmon,
> struct dentry *root)
> {}
> #endif
> --
> 2.25.1
Powered by blists - more mailing lists