Message ID | 20211111164601.13135-3-andrey.zhizhikin@leica-geosystems.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Herbert Xu |
Headers | show |
Series | CAAM Driver: re-factor and dynamic JR availability check | expand |
Am 2021-11-11 17:46, schrieb Andrey Zhizhikin: > Job Rings can be set to be exclusively used by TrustZone which makes > the > access to those rings only possible from Secure World. This access > separation is defined by setting bits in CAAM JRxDID_MS register. Once > reserved to be owned by TrustZone, this Job Ring becomes unavailable > for > the Kernel. This reservation is performed early in the boot process, > even before the Kernel starts, which leads to unavailability of the HW > at the probing stage. Moreover, the reservation can be done for any Job > Ring and is not under control of the Kernel. > > Current implementation lists Job Rings as child nodes of CAAM driver, > and tries to perform probing on those regardless of whether JR HW is > accessible or not. > > This leads to the following error while probing: > [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 > [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 > [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 > > Implement a dynamic mechanism to identify which Job Ring is actually > marked as owned by TrustZone, and fail the probing of those child nodes > with -ENODEV. For other reviewers/maintainers: I'm still not sure this is the way to go. Instead one can let u-boot fix up the device tree and remove or disable the JR node if its not available. > If the Job Ring is released from the Secure World at the later stage, > then bind can be performed, which would check the Ring availability and > proceed with probing if the Ring is released. But what if its the other way around and S world will "steal" it from NS world. > > Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com> > --- > Changes in V2: > - Create and export new method in CAAM control driver to verify JR > validity based on the address supplied. > - Re-work the logic JR driver to call CAAM control method instead of > bit > verification. This ensures the actual information retrival from CAAM > module during JR probe. > - Clean-up of internal static job indexes used during probing, new > indexing is performed based on the address supplied in DTB node. > > drivers/crypto/caam/ctrl.c | 63 ++++++++++++++++++++++++++++++------ > drivers/crypto/caam/intern.h | 2 ++ > drivers/crypto/caam/jr.c | 33 ++++++++++++++++--- > drivers/crypto/caam/regs.h | 7 ++-- > 4 files changed, 87 insertions(+), 18 deletions(-) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > index 7a14a69d89c7..ffe233f2c4ef 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c > @@ -79,6 +79,42 @@ static void build_deinstantiation_desc(u32 *desc, > int handle) > append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); > } > > +/* > + * caam_ctrl_check_jr_perm - check if the job ring can be accessed > + * from Non-Secure World. > + * @ctrldev - pointer to CAAM control device > + * @ring_addr - input address of Job Ring, which access should be > verified > + * > + * Return: - 0 if Job Ring is available in NS World > + * - 1 if Job Ring is reserved in the Secure World > + */ > +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 > ring_addr) inline? static int caam_ctrl_.. > +{ > + struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev); > + struct caam_ctrl __iomem *ctrl = ctrlpriv->ctrl; > + u32 ring_id; > + > + ring_id = ring_addr >> > + ((ctrlpriv->caam_caps & CAAM_CAPS_64K_PAGESIZE) ? > + 16 : 12); mh also here: if (ctrlpriv->caam_caps & CAAM_CAPS_64K_PAGESIZE) ring_id = ring_addr >> 16; else ring_id = ring_addr >> 12; Is there something to replace that "16" and "12" by the SZ_ macros, btw? > + /* > + * Check if the JR is not owned exclusively by TZ, > + * and update capabilities bitmap to indicate that > + * the Job Ring is available. > + * Note: Ring index is 0-based in the register map > + */ > + if (!((rd_reg32(&ctrl->jr_mid[ring_id - 1].liodn_ms)) & > + MSTRID_LOCK_TZ_OWN)) { > + ctrlpriv->caam_caps |= BIT(ring_id); See also the former patch, this should be a macro. > + return 0; > + } > + > + /* Job Ring is reserved, clear bit if is was set before */ > + ctrlpriv->caam_caps &= ~BIT(ring_id); > + return 1; > +} > +EXPORT_SYMBOL(caam_ctrl_check_jr_perm); no need for exporting this, no? > + > /* > * run_descriptor_deco0 - runs a descriptor on DECO0, under direct > control of > * the software (no JR/QI used). > @@ -612,7 +648,7 @@ static bool check_version(struct fsl_mc_version > *mc_version, u32 major, > /* Probe routine for CAAM top (controller) level */ > static int caam_probe(struct platform_device *pdev) > { > - int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; > + int ret, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; > u64 caam_id; > const struct soc_device_attribute *imx_soc_match; > struct device *dev; > @@ -803,20 +839,27 @@ 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 > - ); > - ring++; > - ctrlpriv->caam_caps |= BIT(ring); > + u32 ring_id; > + /* > + * Get register page to see the start address of CB > + * This is used to index into the bitmap of available > + * job rings and indicate if it is available in NS world. > + */ > + ret = of_property_read_u32(np, "reg", &ring_id); Not sure about this one, but I don't have any better idea. > + if (ret) { > + dev_err(dev, "failed to get register address for jobr\n"); > + continue; > + } > + caam_ctrl_check_jr_perm(dev, ring_id); What about if (!caam_ctrl_is_jr_available(dev, ring_id)) ctrlpriv->caam_caps &= ~BIT(ring_id); Again BIT() should be its own macro. > } > > - /* If no QI and no rings specified, quit and go home */ > + /* > + * If no QI, no rings specified or no rings available for NS - > + * quit and go home > + */ > if (!(ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) && > (hweight_long(ctrlpriv->caam_caps & CAAM_CAPS_JOBRS_MASK) == 0)) > { > dev_err(dev, "no queues configured, terminating\n"); > diff --git a/drivers/crypto/caam/intern.h > b/drivers/crypto/caam/intern.h > index 37f0b93c7087..8d6a0cff556a 100644 > --- a/drivers/crypto/caam/intern.h > +++ b/drivers/crypto/caam/intern.h > @@ -115,6 +115,8 @@ struct caam_drv_private { > #endif > }; > > +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 > ring_addr); > + > #ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API > > int caam_algapi_init(struct device *dev); > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > index 7f2b1101f567..e1bc1ce5515e 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -90,7 +90,7 @@ static int caam_reset_hw_jr(struct device *dev) > > if ((rd_reg32(&jrp->rregs->jrintstatus) & JRINT_ERR_HALT_MASK) != > JRINT_ERR_HALT_COMPLETE || timeout == 0) { > - dev_err(dev, "failed to flush job ring %d\n", jrp->ridx); > + dev_err(dev, "failed to flush job ring %x\n", jrp->ridx); mh? why changing this? > return -EIO; > } > > @@ -101,7 +101,7 @@ static int caam_reset_hw_jr(struct device *dev) > cpu_relax(); > > if (timeout == 0) { > - dev_err(dev, "failed to reset job ring %d\n", jrp->ridx); > + dev_err(dev, "failed to reset job ring %x\n", jrp->ridx); > return -EIO; > } > > @@ -489,7 +489,7 @@ static int caam_jr_init(struct device *dev) > error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, > IRQF_SHARED, > dev_name(dev), dev); > if (error) { > - dev_err(dev, "can't connect JobR %d interrupt (%d)\n", > + dev_err(dev, "can't connect JobR %x interrupt (%d)\n", > jrp->ridx, jrp->irq); > tasklet_kill(&jrp->irqtask); > } > @@ -511,10 +511,33 @@ static int caam_jr_probe(struct platform_device > *pdev) > struct device_node *nprop; > struct caam_job_ring __iomem *ctrl; > struct caam_drv_private_jr *jrpriv; > - static int total_jobrs; > + u32 ring_addr; > struct resource *r; > int error; > > + /* > + * Get register page to see the start address of CB. > + * Job Rings have their respective input base addresses > + * defined in the register IRBAR_JRx. This address is > + * present in the DT node and is aligned to the page > + * size defined at CAAM compile time. > + */ > + if (of_property_read_u32(pdev->dev.of_node, "reg", &ring_addr)) { > + dev_err(&pdev->dev, "failed to get register address for jobr\n"); > + return -ENOMEM; > + } > + > + if (caam_ctrl_check_jr_perm(pdev->dev.parent, ring_addr)) { With the change above, this will also have no bogus side effects on caam_caps. > + /* > + * This job ring is marked to be exclusively used by TZ, > + * do not proceed with probing as the HW block is inaccessible. > + * Defer this device probing for later, it might be released > + * into NS world. > + */ > + dev_info(&pdev->dev, "job ring is reserved in secure world\n"); > + return -ENODEV; > + } > + > jrdev = &pdev->dev; > jrpriv = devm_kzalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL); > if (!jrpriv) > @@ -523,7 +546,7 @@ static int caam_jr_probe(struct platform_device > *pdev) > dev_set_drvdata(jrdev, jrpriv); > > /* save ring identity relative to detection */ > - jrpriv->ridx = total_jobrs++; > + jrpriv->ridx = ring_addr; > > nprop = pdev->dev.of_node; > /* Get configuration properties from device tree */ > diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h > index 186e76e6a3e7..c4e8574bc570 100644 > --- a/drivers/crypto/caam/regs.h > +++ b/drivers/crypto/caam/regs.h > @@ -445,10 +445,11 @@ struct caam_perfmon { > }; > > /* LIODN programming for DMA configuration */ > -#define MSTRID_LOCK_LIODN 0x80000000 > -#define MSTRID_LOCK_MAKETRUSTED 0x00010000 /* only for JR masterid */ > +#define MSTRID_LOCK_LIODN BIT(31) > +#define MSTRID_LOCK_MAKETRUSTED BIT(16) /* only for JR: masterid */ > +#define MSTRID_LOCK_TZ_OWN BIT(15) /* only for JR: owned by TZ */ > > -#define MSTRID_LIODN_MASK 0x0fff > +#define MSTRID_LIODN_MASK GENMASK(11, 0) > struct masterid { > u32 liodn_ms; /* lock and make-trusted control bits */ > u32 liodn_ls; /* LIODN for non-sequence and seq access */
Hello Michael, > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Friday, November 12, 2021 10:18 PM > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > Cc: horia.geanta@nxp.com; pankaj.gupta@nxp.com; > herbert@gondor.apana.org.au; davem@davemloft.net; > iuliana.prodan@nxp.com; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing > > > Am 2021-11-11 17:46, schrieb Andrey Zhizhikin: > > Job Rings can be set to be exclusively used by TrustZone which makes > > the access to those rings only possible from Secure World. This access > > separation is defined by setting bits in CAAM JRxDID_MS register. Once > > reserved to be owned by TrustZone, this Job Ring becomes unavailable > > for the Kernel. This reservation is performed early in the boot > > process, even before the Kernel starts, which leads to unavailability > > of the HW at the probing stage. Moreover, the reservation can be done > > for any Job Ring and is not under control of the Kernel. > > > > Current implementation lists Job Rings as child nodes of CAAM driver, > > and tries to perform probing on those regardless of whether JR HW is > > accessible or not. > > > > This leads to the following error while probing: > > [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 > > [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 > > [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 > > > > Implement a dynamic mechanism to identify which Job Ring is actually > > marked as owned by TrustZone, and fail the probing of those child > > nodes with -ENODEV. > > For other reviewers/maintainers: I'm still not sure this is the way to go. Instead > one can let u-boot fix up the device tree and remove or disable the JR node if its > not available. Just as further clarification: this patch is intended to accommodate for cases where JR is claimed in S world at the boot and not available for Kernel. It does not account for fully dynamic cases, where JRs can be reclaimed between S <-> NS Worlds during runtime. It rather accounts for situation when any arbitrary JR can be reserved by any software entity before Kernel starts without a need to disable nodes at compile time. Full dynamic recognition is a part of much bigger task and is out of scope here. > > > If the Job Ring is released from the Secure World at the later stage, > > then bind can be performed, which would check the Ring availability > > and proceed with probing if the Ring is released. > > But what if its the other way around and S world will "steal" it from NS world. This is not accounted for in this patch, as I do not know if there is any notification mechanism exists between S <-> NS world that would allow to share the status of JR. The implementation in this patch does provide a mechanism to perform a later bind, but does not have any mechanism to indicate when it should be done... > > > > > Signed-off-by: Andrey Zhizhikin > > <andrey.zhizhikin@leica-geosystems.com> > > --- > > Changes in V2: > > - Create and export new method in CAAM control driver to verify JR > > validity based on the address supplied. > > - Re-work the logic JR driver to call CAAM control method instead of > > bit > > verification. This ensures the actual information retrival from CAAM > > module during JR probe. > > - Clean-up of internal static job indexes used during probing, new > > indexing is performed based on the address supplied in DTB node. > > > > drivers/crypto/caam/ctrl.c | 63 ++++++++++++++++++++++++++++++------ > > drivers/crypto/caam/intern.h | 2 ++ > > drivers/crypto/caam/jr.c | 33 ++++++++++++++++--- > > drivers/crypto/caam/regs.h | 7 ++-- > > 4 files changed, 87 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > > index 7a14a69d89c7..ffe233f2c4ef 100644 > > --- a/drivers/crypto/caam/ctrl.c > > +++ b/drivers/crypto/caam/ctrl.c > > @@ -79,6 +79,42 @@ static void build_deinstantiation_desc(u32 *desc, > > int handle) > > append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); } > > > > +/* > > + * caam_ctrl_check_jr_perm - check if the job ring can be accessed > > + * from Non-Secure World. > > + * @ctrldev - pointer to CAAM control device > > + * @ring_addr - input address of Job Ring, which access should be > > verified > > + * > > + * Return: - 0 if Job Ring is available in NS World > > + * - 1 if Job Ring is reserved in the Secure World > > + */ > > +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 > > ring_addr) > > inline? > static int caam_ctrl_.. > > > +{ > > + struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev); > > + struct caam_ctrl __iomem *ctrl = ctrlpriv->ctrl; > > + u32 ring_id; > > + > > + ring_id = ring_addr >> > > + ((ctrlpriv->caam_caps & CAAM_CAPS_64K_PAGESIZE) ? > > + 16 : 12); > > mh also here: > if (ctrlpriv->caam_caps & CAAM_CAPS_64K_PAGESIZE) > ring_id = ring_addr >> 16; > else > ring_id = ring_addr >> 12; > > Is there something to replace that "16" and "12" by the SZ_ macros, btw? Good point, I'd need to look into this further on with what this can be replaced. > > > + /* > > + * Check if the JR is not owned exclusively by TZ, > > + * and update capabilities bitmap to indicate that > > + * the Job Ring is available. > > + * Note: Ring index is 0-based in the register map > > + */ > > + if (!((rd_reg32(&ctrl->jr_mid[ring_id - 1].liodn_ms)) & > > + MSTRID_LOCK_TZ_OWN)) { > > + ctrlpriv->caam_caps |= BIT(ring_id); > > See also the former patch, this should be a macro. True, would be covered in V3. > > > + return 0; > > + } > > + > > + /* Job Ring is reserved, clear bit if is was set before */ > > + ctrlpriv->caam_caps &= ~BIT(ring_id); > > + return 1; > > +} > > +EXPORT_SYMBOL(caam_ctrl_check_jr_perm); > > no need for exporting this, no? Unfortunately, both CONFIG_CRYPTO_DEV_FSL_CAAM and CONFIG_CRYPTO_DEV_FSL_CAAM_JR are tristate. Setting both config options to "=m" fails to resolve caam_ctrl_check_jr_perm, therefore I had to export it. It strikes me odd however that CAAM can be compiled as module without CAAM_JR module at all. This would imply that DECO is used directly, which according to SRM is used for pure descriptor debug purposes and should never be used in production. I guess CRYPTO_DEV_FSL_CAAM _JR should be merged into CRYPTO_DEV_FSL_CAAM, so they at least comes together. In that case the export would not be necessary at all. I must admit I didn't find this a good solution, therefore any advise on a better solution here is highly appreciated. > > > + > > /* > > * run_descriptor_deco0 - runs a descriptor on DECO0, under direct > > control of > > * the software (no JR/QI used). > > @@ -612,7 +648,7 @@ static bool check_version(struct fsl_mc_version > > *mc_version, u32 major, > > /* Probe routine for CAAM top (controller) level */ static int > > caam_probe(struct platform_device *pdev) { > > - int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; > > + int ret, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; > > u64 caam_id; > > const struct soc_device_attribute *imx_soc_match; > > struct device *dev; > > @@ -803,20 +839,27 @@ 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 > > - ); > > - ring++; > > - ctrlpriv->caam_caps |= BIT(ring); > > + u32 ring_id; > > + /* > > + * Get register page to see the start address of CB > > + * This is used to index into the bitmap of available > > + * job rings and indicate if it is available in NS world. > > + */ > > + ret = of_property_read_u32(np, "reg", &ring_id); > > Not sure about this one, but I don't have any better idea. Similar approach is proposed in U-Boot series [1]. > > > > + if (ret) { > > + dev_err(dev, "failed to get register address for jobr\n"); > > + continue; > > + } > > + caam_ctrl_check_jr_perm(dev, ring_id); > > What about > if (!caam_ctrl_is_jr_available(dev, ring_id)) > ctrlpriv->caam_caps &= ~BIT(ring_id); > > Again BIT() should be its own macro. Yes, would replace it in V3. > > > } > > > > - /* If no QI and no rings specified, quit and go home */ > > + /* > > + * If no QI, no rings specified or no rings available for NS - > > + * quit and go home > > + */ > > if (!(ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) && > > (hweight_long(ctrlpriv->caam_caps & CAAM_CAPS_JOBRS_MASK) == > > 0)) { > > dev_err(dev, "no queues configured, terminating\n"); > > diff --git a/drivers/crypto/caam/intern.h > > b/drivers/crypto/caam/intern.h index 37f0b93c7087..8d6a0cff556a 100644 > > --- a/drivers/crypto/caam/intern.h > > +++ b/drivers/crypto/caam/intern.h > > @@ -115,6 +115,8 @@ struct caam_drv_private { #endif }; > > > > +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 > > ring_addr); > > + > > #ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API > > > > int caam_algapi_init(struct device *dev); diff --git > > a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index > > 7f2b1101f567..e1bc1ce5515e 100644 > > --- a/drivers/crypto/caam/jr.c > > +++ b/drivers/crypto/caam/jr.c > > @@ -90,7 +90,7 @@ static int caam_reset_hw_jr(struct device *dev) > > > > if ((rd_reg32(&jrp->rregs->jrintstatus) & JRINT_ERR_HALT_MASK) != > > JRINT_ERR_HALT_COMPLETE || timeout == 0) { > > - dev_err(dev, "failed to flush job ring %d\n", jrp->ridx); > > + dev_err(dev, "failed to flush job ring %x\n", > > + jrp->ridx); > > mh? why changing this? After the change, jrp->ridx would contain JR hex address instead of index, therefore I had to replace the debug output. > > > return -EIO; > > } > > > > @@ -101,7 +101,7 @@ static int caam_reset_hw_jr(struct device *dev) > > cpu_relax(); > > > > if (timeout == 0) { > > - dev_err(dev, "failed to reset job ring %d\n", jrp->ridx); > > + dev_err(dev, "failed to reset job ring %x\n", > > + jrp->ridx); > > return -EIO; > > } > > > > @@ -489,7 +489,7 @@ static int caam_jr_init(struct device *dev) > > error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, > > IRQF_SHARED, > > dev_name(dev), dev); > > if (error) { > > - dev_err(dev, "can't connect JobR %d interrupt (%d)\n", > > + dev_err(dev, "can't connect JobR %x interrupt (%d)\n", > > jrp->ridx, jrp->irq); > > tasklet_kill(&jrp->irqtask); > > } > > @@ -511,10 +511,33 @@ static int caam_jr_probe(struct platform_device > > *pdev) > > struct device_node *nprop; > > struct caam_job_ring __iomem *ctrl; > > struct caam_drv_private_jr *jrpriv; > > - static int total_jobrs; > > + u32 ring_addr; > > struct resource *r; > > int error; > > > > + /* > > + * Get register page to see the start address of CB. > > + * Job Rings have their respective input base addresses > > + * defined in the register IRBAR_JRx. This address is > > + * present in the DT node and is aligned to the page > > + * size defined at CAAM compile time. > > + */ > > + if (of_property_read_u32(pdev->dev.of_node, "reg", &ring_addr)) { > > + dev_err(&pdev->dev, "failed to get register address for jobr\n"); > > + return -ENOMEM; > > + } > > + > > + if (caam_ctrl_check_jr_perm(pdev->dev.parent, ring_addr)) { > > With the change above, this will also have no bogus side effects on caam_caps. > > > + /* > > + * This job ring is marked to be exclusively used by TZ, > > + * do not proceed with probing as the HW block is inaccessible. > > + * Defer this device probing for later, it might be released > > + * into NS world. > > + */ > > + dev_info(&pdev->dev, "job ring is reserved in secure world\n"); > > + return -ENODEV; > > + } > > + > > jrdev = &pdev->dev; > > jrpriv = devm_kzalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL); > > if (!jrpriv) > > @@ -523,7 +546,7 @@ static int caam_jr_probe(struct platform_device > > *pdev) > > dev_set_drvdata(jrdev, jrpriv); > > > > /* save ring identity relative to detection */ > > - jrpriv->ridx = total_jobrs++; > > + jrpriv->ridx = ring_addr; > > > > nprop = pdev->dev.of_node; > > /* Get configuration properties from device tree */ diff --git > > a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index > > 186e76e6a3e7..c4e8574bc570 100644 > > --- a/drivers/crypto/caam/regs.h > > +++ b/drivers/crypto/caam/regs.h > > @@ -445,10 +445,11 @@ struct caam_perfmon { }; > > > > /* LIODN programming for DMA configuration */ > > -#define MSTRID_LOCK_LIODN 0x80000000 > > -#define MSTRID_LOCK_MAKETRUSTED 0x00010000 /* only for JR > masterid */ > > +#define MSTRID_LOCK_LIODN BIT(31) > > +#define MSTRID_LOCK_MAKETRUSTED BIT(16) /* only for JR: masterid */ > > +#define MSTRID_LOCK_TZ_OWN BIT(15) /* only for JR: owned by TZ */ > > > > -#define MSTRID_LIODN_MASK 0x0fff > > +#define MSTRID_LIODN_MASK GENMASK(11, 0) > > struct masterid { > > u32 liodn_ms; /* lock and make-trusted control bits */ > > u32 liodn_ls; /* LIODN for non-sequence and seq access */ > > -- > -michael Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211115070014.17586-2-gaurav.jain@nxp.com/ -- andrey
>> > +EXPORT_SYMBOL(caam_ctrl_check_jr_perm); >> >> no need for exporting this, no? > > Unfortunately, both CONFIG_CRYPTO_DEV_FSL_CAAM and > CONFIG_CRYPTO_DEV_FSL_CAAM_JR are tristate. Setting both > config options to "=m" fails to resolve caam_ctrl_check_jr_perm, > therefore I had to export it. > > It strikes me odd however that CAAM can be compiled as module > without CAAM_JR module at all. This would imply that DECO is used > directly, which according to SRM is used for pure descriptor debug > purposes and should never be used in production. > > I guess CRYPTO_DEV_FSL_CAAM _JR should be merged into > CRYPTO_DEV_FSL_CAAM, so they at least comes together. In that > case the export would not be necessary at all. > > I must admit I didn't find this a good solution, therefore any advise > on a better solution here is highly appreciated. I see, and I'm too lazy at the moment to figure that out ;) but afaik new exports should be only EXPORT_SYMBOL_GPL(). >> > if ((rd_reg32(&jrp->rregs->jrintstatus) & JRINT_ERR_HALT_MASK) != >> > JRINT_ERR_HALT_COMPLETE || timeout == 0) { >> > - dev_err(dev, "failed to flush job ring %d\n", jrp->ridx); >> > + dev_err(dev, "failed to flush job ring %x\n", >> > + jrp->ridx); >> >> mh? why changing this? > > After the change, jrp->ridx would contain JR hex address instead of > index, > therefore I had to replace the debug output. ahh then, ridx should renamed accordingly, I suppose. -michael
On 11/15/2021 12:07 PM, ZHIZHIKIN Andrey wrote: > Hello Michael, > >> -----Original Message----- >> From: Michael Walle <michael@walle.cc> >> Sent: Friday, November 12, 2021 10:18 PM >> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> >> Cc: horia.geanta@nxp.com; pankaj.gupta@nxp.com; >> herbert@gondor.apana.org.au; davem@davemloft.net; >> iuliana.prodan@nxp.com; linux-crypto@vger.kernel.org; linux- >> kernel@vger.kernel.org >> Subject: Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing >> >> >> Am 2021-11-11 17:46, schrieb Andrey Zhizhikin: >>> Job Rings can be set to be exclusively used by TrustZone which makes >>> the access to those rings only possible from Secure World. This access >>> separation is defined by setting bits in CAAM JRxDID_MS register. Once >>> reserved to be owned by TrustZone, this Job Ring becomes unavailable >>> for the Kernel. This reservation is performed early in the boot >>> process, even before the Kernel starts, which leads to unavailability >>> of the HW at the probing stage. Moreover, the reservation can be done >>> for any Job Ring and is not under control of the Kernel. >>> >>> Current implementation lists Job Rings as child nodes of CAAM driver, >>> and tries to perform probing on those regardless of whether JR HW is >>> accessible or not. >>> >>> This leads to the following error while probing: >>> [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 >>> [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 >>> [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 >>> >>> Implement a dynamic mechanism to identify which Job Ring is actually >>> marked as owned by TrustZone, and fail the probing of those child >>> nodes with -ENODEV. >> >> For other reviewers/maintainers: I'm still not sure this is the way to go. Instead >> one can let u-boot fix up the device tree and remove or disable the JR node if its >> not available. > > Just as further clarification: this patch is intended to accommodate for cases where > JR is claimed in S world at the boot and not available for Kernel. It does not account > for fully dynamic cases, where JRs can be reclaimed between S <-> NS Worlds > during runtime. It rather accounts for situation when any arbitrary JR can be reserved > by any software entity before Kernel starts without a need to disable nodes at > compile time. > I prefer f/w to fix the DT before passing it to the kernel, either by adding the "secure-status" property (set explicitly to "disabled") or by removing the job ring node(s) that are reserved. OP-TEE already uses the first option. We should probably pick this up. The reason I am supporting relying on DT and consequently avoiding registers is that accessing page 0 in the caam register space from Non-secure world should be avoided when caam is managed by Secure world (e.g. OP-TEE) or a Secure Enclave (e.g. SECO). Unfortunately support for HW-enforced access control for caam register space is not that great / fine-grained, with the exception of more recent parts like i.MX8MP and i.MX8ULP. Horia
Hi Horia, >>>> Job Rings can be set to be exclusively used by TrustZone which makes >>>> the access to those rings only possible from Secure World. This >>>> access >>>> separation is defined by setting bits in CAAM JRxDID_MS register. >>>> Once >>>> reserved to be owned by TrustZone, this Job Ring becomes unavailable >>>> for the Kernel. This reservation is performed early in the boot >>>> process, even before the Kernel starts, which leads to >>>> unavailability >>>> of the HW at the probing stage. Moreover, the reservation can be >>>> done >>>> for any Job Ring and is not under control of the Kernel. >>>> >>>> Current implementation lists Job Rings as child nodes of CAAM >>>> driver, >>>> and tries to perform probing on those regardless of whether JR HW is >>>> accessible or not. >>>> >>>> This leads to the following error while probing: >>>> [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 >>>> [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 >>>> [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 >>>> >>>> Implement a dynamic mechanism to identify which Job Ring is actually >>>> marked as owned by TrustZone, and fail the probing of those child >>>> nodes with -ENODEV. >>> >>> For other reviewers/maintainers: I'm still not sure this is the way >>> to go. Instead >>> one can let u-boot fix up the device tree and remove or disable the >>> JR node if its >>> not available. >> >> Just as further clarification: this patch is intended to accommodate >> for cases where >> JR is claimed in S world at the boot and not available for Kernel. It >> does not account >> for fully dynamic cases, where JRs can be reclaimed between S <-> NS >> Worlds >> during runtime. It rather accounts for situation when any arbitrary JR >> can be reserved >> by any software entity before Kernel starts without a need to disable >> nodes at >> compile time. >> > I prefer f/w to fix the DT before passing it to the kernel, > either by adding the "secure-status" property (set explicitly to > "disabled") > or by removing the job ring node(s) that are reserved. > OP-TEE already uses the first option. We should probably pick this up. Ah, nice: Documentation/devicetree/bindings/arm/secure.txt If I understand this correctly, if optee reserves a JR it will set the secure-status to okay and status to disabled. (There is still a missing link, how u-boot will then be passed this modified device tree, I might miss something here.) But what about the HAB, if I understand Andrey correct, then JR0 will already be marked as "S world only" (or at least no EL3 program will release it again). To me it looks like then either JR0 should be (1) hardcoded to secure-status = "okay", status = "disabled", or (2) u-boot SPL (or TF-A) should return it to NS world (and optee might take it over again). > The reason I am supporting relying on DT and consequently avoiding > registers > is that accessing page 0 in the caam register space from Non-secure > world > should be avoided when caam is managed by Secure world (e.g. OP-TEE) > or a Secure Enclave (e.g. SECO). > > Unfortunately support for HW-enforced access control for caam register > space > is not that great / fine-grained, with the exception of more recent > parts > like i.MX8MP and i.MX8ULP. -michael
Hello Horia/Michael, I'd reply here to both of you since your answers are complementing each other. I've also Cc: Gaurav here as he is working on CAAM support in U-Boot so this discussion is relevant for him as well I suppose. > -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: Thursday, November 18, 2021 9:29 AM > To: Horia Geantă <horia.geanta@nxp.com> > Cc: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Pankaj Gupta > <pankaj.gupta@nxp.com>; herbert@gondor.apana.org.au; davem@davemloft.net; Iuliana > Prodan <iuliana.prodan@nxp.com>; linux-crypto@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing > > > Hi Horia, > > >>>> Job Rings can be set to be exclusively used by TrustZone which makes > >>>> the access to those rings only possible from Secure World. This > >>>> access > >>>> separation is defined by setting bits in CAAM JRxDID_MS register. > >>>> Once > >>>> reserved to be owned by TrustZone, this Job Ring becomes unavailable > >>>> for the Kernel. This reservation is performed early in the boot > >>>> process, even before the Kernel starts, which leads to > >>>> unavailability > >>>> of the HW at the probing stage. Moreover, the reservation can be > >>>> done > >>>> for any Job Ring and is not under control of the Kernel. > >>>> > >>>> Current implementation lists Job Rings as child nodes of CAAM > >>>> driver, > >>>> and tries to perform probing on those regardless of whether JR HW is > >>>> accessible or not. > >>>> > >>>> This leads to the following error while probing: > >>>> [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 > >>>> [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 > >>>> [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 > >>>> > >>>> Implement a dynamic mechanism to identify which Job Ring is actually > >>>> marked as owned by TrustZone, and fail the probing of those child > >>>> nodes with -ENODEV. > >>> > >>> For other reviewers/maintainers: I'm still not sure this is the way > >>> to go. Instead > >>> one can let u-boot fix up the device tree and remove or disable the > >>> JR node if its > >>> not available. > >> > >> Just as further clarification: this patch is intended to accommodate > >> for cases where > >> JR is claimed in S world at the boot and not available for Kernel. It > >> does not account > >> for fully dynamic cases, where JRs can be reclaimed between S <-> NS > >> Worlds > >> during runtime. It rather accounts for situation when any arbitrary JR > >> can be reserved > >> by any software entity before Kernel starts without a need to disable > >> nodes at > >> compile time. > >> > > I prefer f/w to fix the DT before passing it to the kernel, > > either by adding the "secure-status" property (set explicitly to > > "disabled") > > or by removing the job ring node(s) that are reserved. According to the DT bindings doc mentioned by Michael below, it would not be needed to remove the node. Setting status = "disabled"; secure-status = "okay" should be enough to reserve JR node in S World permanently. It would also serve the purpose to indicate that the HW do provide the correct total amount of JRs, and just some of then are not available to be used in NS World. > > OP-TEE already uses the first option. We should probably pick this up. Agree. I would drop the register access from this patch and follow-up with DT node approach. > > Ah, nice: > Documentation/devicetree/bindings/arm/secure.txt Good point, thanks for the doc guidance! This does provide a clear layout on how the DT node should be crafted! > > If I understand this correctly, if optee reserves a JR it will set the > secure-status to okay and status to disabled. (There is still a missing > link, how u-boot will then be passed this modified device tree, I might > miss something here.) I need to look at how OP-TEE does things here, but if they just set secure-status = "okay" - then the JR should be visible in both worlds. > > But what about the HAB, if I understand Andrey correct, then JR0 will > already be marked as "S world only" (or at least no EL3 program will > release it again). It's a good point, which is still unclear: can JR0 be reclaimed back after HAB is finished? Or should it stay in S-only world? > To me it looks like then either JR0 should be > (1) hardcoded to secure-status = "okay", status = "disabled", or (2) > u-boot SPL (or TF-A) should return it to NS world (and optee might > take it over again). If the answer to HAB question is: it should stay in S World, then I'd suggest to go with (1) as it presents the opportunity to define the initial state of JR0 in deterministic state, without loosing the information that HW does indeed have it implemented (node is present, but permanently disabled). Later reclamation with this combination is also possible. What I would propose in addition here as well, is to define the secure-status = "disabled" for all JR nodes on all derivatives, to have the status set consistently. If later reclamation for any arbitrary JR from NS to S world is needed - this property can be adjusted accordingly by SW entity. Same thing should be done in U-Boot I suppose. > > > The reason I am supporting relying on DT and consequently avoiding > > registers > > is that accessing page 0 in the caam register space from Non-secure > > world > > should be avoided when caam is managed by Secure world (e.g. OP-TEE) > > or a Secure Enclave (e.g. SECO). Understood, this is a valid point that I was missing. I'd re-work this to use DT bindings and push it in V3. I guess there would not be much left of this patch once I'd use DT approach, so I'd re-spin the series to include DT bindings instead. JR driver clean-up to remove static JR counter :| would go into the first one then. > > > > Unfortunately support for HW-enforced access control for caam register > > space > > is not that great / fine-grained, with the exception of more recent > > parts > > like i.MX8MP and i.MX8ULP. Are there any particular distinct differences on those derivatives that should be taken into account here with respect of JR reservation? I might include those as well in this patch series if they are not that significant, otherwise would try to address them separately. > > -michael Thanks to all of you for commenting here! -- andrey
Am 2021-11-18 11:08, schrieb ZHIZHIKIN Andrey: > I guess there would not be much left of this patch once I'd use DT > approach, > so I'd re-spin the series to include DT bindings instead. JR driver > clean-up > to remove static JR counter :| would go into the first one then. I really like the cleanup though! Esp. the removing of the static variable. -michael
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 7a14a69d89c7..ffe233f2c4ef 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -79,6 +79,42 @@ static void build_deinstantiation_desc(u32 *desc, int handle) append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT); } +/* + * caam_ctrl_check_jr_perm - check if the job ring can be accessed + * from Non-Secure World. + * @ctrldev - pointer to CAAM control device + * @ring_addr - input address of Job Ring, which access should be verified + * + * Return: - 0 if Job Ring is available in NS World + * - 1 if Job Ring is reserved in the Secure World + */ +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 ring_addr) +{ + struct caam_drv_private *ctrlpriv = dev_get_drvdata(ctrldev); + struct caam_ctrl __iomem *ctrl = ctrlpriv->ctrl; + u32 ring_id; + + ring_id = ring_addr >> + ((ctrlpriv->caam_caps & CAAM_CAPS_64K_PAGESIZE) ? + 16 : 12); + /* + * Check if the JR is not owned exclusively by TZ, + * and update capabilities bitmap to indicate that + * the Job Ring is available. + * Note: Ring index is 0-based in the register map + */ + if (!((rd_reg32(&ctrl->jr_mid[ring_id - 1].liodn_ms)) & + MSTRID_LOCK_TZ_OWN)) { + ctrlpriv->caam_caps |= BIT(ring_id); + return 0; + } + + /* Job Ring is reserved, clear bit if is was set before */ + ctrlpriv->caam_caps &= ~BIT(ring_id); + return 1; +} +EXPORT_SYMBOL(caam_ctrl_check_jr_perm); + /* * run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of * the software (no JR/QI used). @@ -612,7 +648,7 @@ static bool check_version(struct fsl_mc_version *mc_version, u32 major, /* Probe routine for CAAM top (controller) level */ static int caam_probe(struct platform_device *pdev) { - int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; + int ret, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN; u64 caam_id; const struct soc_device_attribute *imx_soc_match; struct device *dev; @@ -803,20 +839,27 @@ 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 - ); - ring++; - ctrlpriv->caam_caps |= BIT(ring); + u32 ring_id; + /* + * Get register page to see the start address of CB + * This is used to index into the bitmap of available + * job rings and indicate if it is available in NS world. + */ + ret = of_property_read_u32(np, "reg", &ring_id); + if (ret) { + dev_err(dev, "failed to get register address for jobr\n"); + continue; + } + caam_ctrl_check_jr_perm(dev, ring_id); } - /* If no QI and no rings specified, quit and go home */ + /* + * If no QI, no rings specified or no rings available for NS - + * quit and go home + */ if (!(ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) && (hweight_long(ctrlpriv->caam_caps & CAAM_CAPS_JOBRS_MASK) == 0)) { dev_err(dev, "no queues configured, terminating\n"); diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h index 37f0b93c7087..8d6a0cff556a 100644 --- a/drivers/crypto/caam/intern.h +++ b/drivers/crypto/caam/intern.h @@ -115,6 +115,8 @@ struct caam_drv_private { #endif }; +inline int caam_ctrl_check_jr_perm(struct device *ctrldev, u32 ring_addr); + #ifdef CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API int caam_algapi_init(struct device *dev); diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 7f2b1101f567..e1bc1ce5515e 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -90,7 +90,7 @@ static int caam_reset_hw_jr(struct device *dev) if ((rd_reg32(&jrp->rregs->jrintstatus) & JRINT_ERR_HALT_MASK) != JRINT_ERR_HALT_COMPLETE || timeout == 0) { - dev_err(dev, "failed to flush job ring %d\n", jrp->ridx); + dev_err(dev, "failed to flush job ring %x\n", jrp->ridx); return -EIO; } @@ -101,7 +101,7 @@ static int caam_reset_hw_jr(struct device *dev) cpu_relax(); if (timeout == 0) { - dev_err(dev, "failed to reset job ring %d\n", jrp->ridx); + dev_err(dev, "failed to reset job ring %x\n", jrp->ridx); return -EIO; } @@ -489,7 +489,7 @@ static int caam_jr_init(struct device *dev) error = devm_request_irq(dev, jrp->irq, caam_jr_interrupt, IRQF_SHARED, dev_name(dev), dev); if (error) { - dev_err(dev, "can't connect JobR %d interrupt (%d)\n", + dev_err(dev, "can't connect JobR %x interrupt (%d)\n", jrp->ridx, jrp->irq); tasklet_kill(&jrp->irqtask); } @@ -511,10 +511,33 @@ static int caam_jr_probe(struct platform_device *pdev) struct device_node *nprop; struct caam_job_ring __iomem *ctrl; struct caam_drv_private_jr *jrpriv; - static int total_jobrs; + u32 ring_addr; struct resource *r; int error; + /* + * Get register page to see the start address of CB. + * Job Rings have their respective input base addresses + * defined in the register IRBAR_JRx. This address is + * present in the DT node and is aligned to the page + * size defined at CAAM compile time. + */ + if (of_property_read_u32(pdev->dev.of_node, "reg", &ring_addr)) { + dev_err(&pdev->dev, "failed to get register address for jobr\n"); + return -ENOMEM; + } + + if (caam_ctrl_check_jr_perm(pdev->dev.parent, ring_addr)) { + /* + * This job ring is marked to be exclusively used by TZ, + * do not proceed with probing as the HW block is inaccessible. + * Defer this device probing for later, it might be released + * into NS world. + */ + dev_info(&pdev->dev, "job ring is reserved in secure world\n"); + return -ENODEV; + } + jrdev = &pdev->dev; jrpriv = devm_kzalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL); if (!jrpriv) @@ -523,7 +546,7 @@ static int caam_jr_probe(struct platform_device *pdev) dev_set_drvdata(jrdev, jrpriv); /* save ring identity relative to detection */ - jrpriv->ridx = total_jobrs++; + jrpriv->ridx = ring_addr; nprop = pdev->dev.of_node; /* Get configuration properties from device tree */ diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index 186e76e6a3e7..c4e8574bc570 100644 --- a/drivers/crypto/caam/regs.h +++ b/drivers/crypto/caam/regs.h @@ -445,10 +445,11 @@ struct caam_perfmon { }; /* LIODN programming for DMA configuration */ -#define MSTRID_LOCK_LIODN 0x80000000 -#define MSTRID_LOCK_MAKETRUSTED 0x00010000 /* only for JR masterid */ +#define MSTRID_LOCK_LIODN BIT(31) +#define MSTRID_LOCK_MAKETRUSTED BIT(16) /* only for JR: masterid */ +#define MSTRID_LOCK_TZ_OWN BIT(15) /* only for JR: owned by TZ */ -#define MSTRID_LIODN_MASK 0x0fff +#define MSTRID_LIODN_MASK GENMASK(11, 0) struct masterid { u32 liodn_ms; /* lock and make-trusted control bits */ u32 liodn_ls; /* LIODN for non-sequence and seq access */
Job Rings can be set to be exclusively used by TrustZone which makes the access to those rings only possible from Secure World. This access separation is defined by setting bits in CAAM JRxDID_MS register. Once reserved to be owned by TrustZone, this Job Ring becomes unavailable for the Kernel. This reservation is performed early in the boot process, even before the Kernel starts, which leads to unavailability of the HW at the probing stage. Moreover, the reservation can be done for any Job Ring and is not under control of the Kernel. Current implementation lists Job Rings as child nodes of CAAM driver, and tries to perform probing on those regardless of whether JR HW is accessible or not. This leads to the following error while probing: [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 Implement a dynamic mechanism to identify which Job Ring is actually marked as owned by TrustZone, and fail the probing of those child nodes with -ENODEV. If the Job Ring is released from the Secure World at the later stage, then bind can be performed, which would check the Ring availability and proceed with probing if the Ring is released. Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com> --- Changes in V2: - Create and export new method in CAAM control driver to verify JR validity based on the address supplied. - Re-work the logic JR driver to call CAAM control method instead of bit verification. This ensures the actual information retrival from CAAM module during JR probe. - Clean-up of internal static job indexes used during probing, new indexing is performed based on the address supplied in DTB node. drivers/crypto/caam/ctrl.c | 63 ++++++++++++++++++++++++++++++------ drivers/crypto/caam/intern.h | 2 ++ drivers/crypto/caam/jr.c | 33 ++++++++++++++++--- drivers/crypto/caam/regs.h | 7 ++-- 4 files changed, 87 insertions(+), 18 deletions(-)