* [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status
[not found] <20211111164601.13135-1-andrey.zhizhikin@leica-geosystems.com>
@ 2021-12-07 23:02 ` Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 1/2] crypto: caam - convert to use capabilities Andrey Zhizhikin
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Andrey Zhizhikin @ 2021-12-07 23:02 UTC (permalink / raw)
To: linux-kernel
Cc: robh+dt, shawnguo, michael, s.hauer, kernel, festevam, linux-imx,
horia.geanta, pankaj.gupta, herbert, davem, l.stach,
qiangqing.zhang, peng.fan, alice.guo, aford173, frieder.schrempf,
krzk, shengjiu.wang, gregkh, ping.bai, daniel.baluta, jun.li,
marex, thunder.leizhen, martink, leonard.crestez, hongxing.zhu,
agx, devicetree, linux-arm-kernel, linux-crypto, op-tee,
Andrey Zhizhikin
This V3 series covers points uncovered during the review of the previous
series, one major point being that register readout should not be used
for dynamic JR availability check due to its unreliability.
Instead, JR should have a proper status set in FDT which indicates the
availability of the ring in NS-World. This status is aligned with what
BootROM code configures, and can be modified by all actors in the boot
chain.
Therefore, patch in V2 series that was handling the dynamic JR
availability check is dropped in this series and replaced by the patch
which sets proper DT status for JR nodes.
Andrey Zhizhikin (2):
crypto: caam - convert to use capabilities
arm64: dts: imx8m: define proper status for caam jr
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 +
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 +
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 +
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 +
drivers/crypto/caam/caamalg_qi.c | 2 +-
drivers/crypto/caam/ctrl.c | 115 ++++++++++++++--------
drivers/crypto/caam/intern.h | 20 ++--
drivers/crypto/caam/jr.c | 19 +++-
drivers/crypto/caam/regs.h | 2 -
9 files changed, 122 insertions(+), 52 deletions(-)
base-commit: 04fe99a8d936d46a310ca61b8b63dc270962bf01
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/2] crypto: caam - convert to use capabilities
2021-12-07 23:02 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status Andrey Zhizhikin
@ 2021-12-07 23:02 ` Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr Andrey Zhizhikin
2022-01-06 10:56 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status ZHIZHIKIN Andrey
2 siblings, 0 replies; 14+ messages in thread
From: Andrey Zhizhikin @ 2021-12-07 23:02 UTC (permalink / raw)
To: linux-kernel
Cc: robh+dt, shawnguo, michael, s.hauer, kernel, festevam, linux-imx,
horia.geanta, pankaj.gupta, herbert, davem, l.stach,
qiangqing.zhang, peng.fan, alice.guo, aford173, frieder.schrempf,
krzk, shengjiu.wang, gregkh, ping.bai, daniel.baluta, jun.li,
marex, thunder.leizhen, martink, leonard.crestez, hongxing.zhu,
agx, devicetree, linux-arm-kernel, linux-crypto, op-tee,
Andrey Zhizhikin
CAAM driver contains several variables, which are used for indication
that certain capabilities are detected during initial probing of the
device. They are defined as u8, but mainly used as boolean variables to
identify capabillities. CAAM JR driver code also contains the static
variable in the probe method, which is used to derive the JR index value
and does not respect how JRs are implemented in the HW.
Clean-up all assorted variables, collect them into one bitfield value
which encodes capabilities as bit, and use them in the execution flow
instead.
Replace static indexing in JR driver with index derived from "reg"
binding, as it reflects the actual JR number in the HW.
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
---
Changes in V3:
- Address review comments from V2 series, replace inline if statements
with explicit ones
- Add more helper macros
- Merge change done in separate patch for JR indexing into here
- Change caam_ctrl_check_jr_perm() from register readout to status
binding check
- Do not export caam_ctrl_check_jr_perm() anymore, drop the declaration
from header
- Remove more local variables in probe() and replace them with
capabilities readout
drivers/crypto/caam/caamalg_qi.c | 2 +-
drivers/crypto/caam/ctrl.c | 115 ++++++++++++++++++++-----------
drivers/crypto/caam/intern.h | 20 +++---
drivers/crypto/caam/jr.c | 19 ++++-
drivers/crypto/caam/regs.h | 2 -
5 files changed, 106 insertions(+), 52 deletions(-)
diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index 189a7438b29c..1b7cdae28290 100644
--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -2610,7 +2610,7 @@ int caam_qi_algapi_init(struct device *ctrldev)
bool registered = false;
/* Make sure this runs only on (DPAA 1.x) QI */
- if (!priv->qi_present || caam_dpaa2)
+ if (!(priv->caam_caps & CAAM_CAPS_QI_PRESENT) || caam_dpaa2)
return 0;
/*
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..37c0c6af1137 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -79,6 +79,36 @@ 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.
+ * @np - pointer to JR device node
+ *
+ * Return: - 0 if Job Ring is reserved in the Secure World
+ * - 1 if Job Ring is available in NS World
+ */
+static inline int caam_ctrl_check_jr_perm(const struct device_node *np)
+{
+ struct property *p;
+
+ /* read "status" property first */
+ p = of_find_property(np, "status", NULL);
+ if (p && (!strncmp(p->value, "disabled", p->length))) {
+ /*
+ * "status" is set to "disabled", which would imply that the JR
+ * is not available for NS World. All other possible combination
+ * of "status" and "secure-status" would rather indicate that JR
+ * is either available in NS-only, or in both S and NS Worlds.
+ * In a later case, we indicate that this JR can be used by the
+ * Kernel since the resource is shared.
+ * For details, see:
+ * Documentation/devicetree/bindings/arm/secure.txt
+ */
+ return 0;
+ }
+ return 1;
+}
+
/*
* run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
* the software (no JR/QI used).
@@ -100,7 +130,7 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
int i;
- if (ctrlpriv->virt_en == 1 ||
+ if ((ctrlpriv->caam_caps & CAAM_CAPS_VIRT_ENABLED) ||
/*
* Apparently on i.MX8M{Q,M,N,P} it doesn't matter if virt_en == 1
* and the following steps should be performed regardless
@@ -169,7 +199,7 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
*status = rd_reg32(&deco->op_status_hi) &
DECO_OP_STATUS_HI_ERR_MASK;
- if (ctrlpriv->virt_en == 1)
+ if (ctrlpriv->caam_caps & CAAM_CAPS_VIRT_ENABLED)
clrsetbits_32(&ctrl->deco_rsr, DECORSR_JR0, 0);
/* Mark the DECO as free */
@@ -612,7 +642,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;
@@ -622,8 +652,6 @@ static int caam_probe(struct platform_device *pdev)
struct dentry *dfs_root;
u32 scfgr, comp_params;
u8 rng_vid;
- int pg_size;
- int BLOCK_OFFSET = 0;
bool pr_support = false;
ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL);
@@ -666,11 +694,12 @@ static int caam_probe(struct platform_device *pdev)
else
caam_ptr_sz = sizeof(u32);
caam_dpaa2 = !!(comp_params & CTPR_MS_DPAA2);
- ctrlpriv->qi_present = !!(comp_params & CTPR_MS_QI_MASK);
+ if (comp_params & CTPR_MS_QI_MASK)
+ ctrlpriv->caam_caps |= CAAM_CAPS_QI_PRESENT;
#ifdef CONFIG_CAAM_QI
/* If (DPAA 1.x) QI present, check whether dependencies are available */
- if (ctrlpriv->qi_present && !caam_dpaa2) {
+ if ((ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) && !caam_dpaa2) {
ret = qman_is_probed();
if (!ret) {
return -EPROBE_DEFER;
@@ -689,33 +718,29 @@ static int caam_probe(struct platform_device *pdev)
}
#endif
- /* Allocating the BLOCK_OFFSET based on the supported page size on
- * the platform
- */
- pg_size = (comp_params & CTPR_MS_PG_SZ_MASK) >> CTPR_MS_PG_SZ_SHIFT;
- if (pg_size == 0)
- BLOCK_OFFSET = PG_SIZE_4K;
- else
- BLOCK_OFFSET = PG_SIZE_64K;
+ /* Allocate control blocks based on the CAAM supported page size */
+ if (comp_params & CTPR_MS_PG_SZ_MASK)
+ ctrlpriv->caam_caps |= CAAM_CAPS_64K_PAGESIZE;
ctrlpriv->ctrl = (struct caam_ctrl __iomem __force *)ctrl;
ctrlpriv->assure = (struct caam_assurance __iomem __force *)
((__force uint8_t *)ctrl +
- BLOCK_OFFSET * ASSURE_BLOCK_NUMBER
+ CAAM_CAPS_PG_SZ(ctrlpriv->caam_caps) * ASSURE_BLOCK_NUMBER
);
ctrlpriv->deco = (struct caam_deco __iomem __force *)
((__force uint8_t *)ctrl +
- BLOCK_OFFSET * DECO_BLOCK_NUMBER
+ CAAM_CAPS_PG_SZ(ctrlpriv->caam_caps) * DECO_BLOCK_NUMBER
);
/* Get the IRQ of the controller (for security violations only) */
ctrlpriv->secvio_irq = irq_of_parse_and_map(nprop, 0);
np = of_find_compatible_node(NULL, NULL, "fsl,qoriq-mc");
- ctrlpriv->mc_en = !!np;
+ if (np)
+ ctrlpriv->caam_caps |= CAAM_CAPS_MC_ENABLED;
of_node_put(np);
#ifdef CONFIG_FSL_MC_BUS
- if (ctrlpriv->mc_en) {
+ if (ctrlpriv->caam_caps & CAAM_CAPS_MC_ENABLED) {
struct fsl_mc_version *mc_version;
mc_version = fsl_mc_get_version();
@@ -732,7 +757,7 @@ static int caam_probe(struct platform_device *pdev)
* In case of SoCs with Management Complex, MC f/w performs
* the configuration.
*/
- if (!ctrlpriv->mc_en)
+ if (!(ctrlpriv->caam_caps & CAAM_CAPS_MC_ENABLED))
clrsetbits_32(&ctrl->mcr, MCFGR_AWCACHE_MASK,
MCFGR_AWCACHE_CACH | MCFGR_AWCACHE_BUFF |
MCFGR_WDENABLE | MCFGR_LARGE_BURST);
@@ -745,7 +770,6 @@ static int caam_probe(struct platform_device *pdev)
*/
scfgr = rd_reg32(&ctrl->scfgr);
- ctrlpriv->virt_en = 0;
if (comp_params & CTPR_MS_VIRT_EN_INCL) {
/* VIRT_EN_INCL = 1 & VIRT_EN_POR = 1 or
* VIRT_EN_INCL = 1 & VIRT_EN_POR = 0 & SCFGR_VIRT_EN = 1
@@ -753,14 +777,14 @@ static int caam_probe(struct platform_device *pdev)
if ((comp_params & CTPR_MS_VIRT_EN_POR) ||
(!(comp_params & CTPR_MS_VIRT_EN_POR) &&
(scfgr & SCFGR_VIRT_EN)))
- ctrlpriv->virt_en = 1;
+ ctrlpriv->caam_caps |= CAAM_CAPS_VIRT_ENABLED;
} else {
/* VIRT_EN_INCL = 0 && VIRT_EN_POR_VALUE = 1 */
if (comp_params & CTPR_MS_VIRT_EN_POR)
- ctrlpriv->virt_en = 1;
+ ctrlpriv->caam_caps |= CAAM_CAPS_VIRT_ENABLED;
}
- if (ctrlpriv->virt_en == 1)
+ if (ctrlpriv->caam_caps & CAAM_CAPS_VIRT_ENABLED)
clrsetbits_32(&ctrl->jrstart, 0, JRSTART_JR0_START |
JRSTART_JR1_START | JRSTART_JR2_START |
JRSTART_JR3_START);
@@ -785,10 +809,10 @@ static int caam_probe(struct platform_device *pdev)
caam_debugfs_init(ctrlpriv, dfs_root);
/* Check to see if (DPAA 1.x) QI present. If so, enable */
- if (ctrlpriv->qi_present && !caam_dpaa2) {
+ if ((ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) && !caam_dpaa2) {
ctrlpriv->qi = (struct caam_queue_if __iomem __force *)
((__force uint8_t *)ctrl +
- BLOCK_OFFSET * QI_BLOCK_NUMBER
+ CAAM_CAPS_PG_SZ(ctrlpriv->caam_caps) * QI_BLOCK_NUMBER
);
/* This is all that's required to physically enable QI */
wr_reg32(&ctrlpriv->qi->qi_control_lo, QICTL_DQEN);
@@ -801,21 +825,32 @@ 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++;
+ 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;
+ }
+ ring_id = ring_id >> CAAM_CAPS_PG_SHIFT(ctrlpriv->caam_caps);
+
+ if (caam_ctrl_check_jr_perm(np))
+ ctrlpriv->caam_caps |= CAAM_CAPS_JR_PRESENT(ring_id);
}
- /* If no QI and no rings specified, quit and go home */
- if ((!ctrlpriv->qi_present) && (!ctrlpriv->total_jobrs)) {
+ /*
+ * 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");
return -ENOMEM;
}
@@ -832,7 +867,8 @@ static int caam_probe(struct platform_device *pdev)
* already instantiated, do RNG instantiation
* In case of SoCs with Management Complex, RNG is managed by MC f/w.
*/
- if (!(ctrlpriv->mc_en && pr_support) && rng_vid >= 4) {
+ if (!((ctrlpriv->caam_caps & CAAM_CAPS_MC_ENABLED) && pr_support) &&
+ rng_vid >= 4) {
ctrlpriv->rng4_sh_init =
rd_reg32(&ctrl->r4tst[0].rdsta);
/*
@@ -900,8 +936,9 @@ static int caam_probe(struct platform_device *pdev)
/* Report "alive" for developer to see */
dev_info(dev, "device ID = 0x%016llx (Era %d)\n", caam_id,
ctrlpriv->era);
- dev_info(dev, "job rings = %d, qi = %d\n",
- ctrlpriv->total_jobrs, ctrlpriv->qi_present);
+ dev_info(dev, "job rings = %ld, qi = %s\n",
+ hweight_long(ctrlpriv->caam_caps & CAAM_CAPS_JOBRS_MASK),
+ (ctrlpriv->caam_caps & CAAM_CAPS_QI_PRESENT) ? "yes" : "no");
ret = devm_of_platform_populate(dev);
if (ret)
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 7d45b21bd55a..37fa9db461c7 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -86,15 +86,19 @@ struct caam_drv_private {
struct iommu_domain *domain;
- /*
- * Detected geometry block. Filled in from device tree if powerpc,
- * or from register-based version detection code
- */
- u8 total_jobrs; /* Total Job Rings in device */
- u8 qi_present; /* Nonzero if QI present in device */
- u8 mc_en; /* Nonzero if MC f/w is active */
+ unsigned long caam_caps; /* CAAM Module capabilities */
+
+#define CAAM_CAPS_QI_PRESENT BIT(0) /* Queue Manager interface (QI) implemented */
+#define CAAM_CAPS_JOBRS_MASK GENMASK(15, 1) /* Job Ring is available in NS World */
+#define CAAM_CAPS_MC_ENABLED BIT(16) /* Management Complex is enabled (F/W is active) */
+#define CAAM_CAPS_VIRT_ENABLED BIT(17) /* Virtualization enabled */
+#define CAAM_CAPS_64K_PAGESIZE BIT(18) /* CAAM register page size (64KB if set, 4KB if unset) */
+
+#define CAAM_CAPS_JR_PRESENT(id) (BIT(id) & CAAM_CAPS_JOBRS_MASK)
+#define CAAM_CAPS_PG_SHIFT(caps) (((caps) & CAAM_CAPS_64K_PAGESIZE) ? 16 : 12)
+#define CAAM_CAPS_PG_SZ(caps) (1 << CAAM_CAPS_PG_SHIFT(caps))
+
int secvio_irq; /* Security violation interrupt number */
- int virt_en; /* Virtualization enabled in CAAM */
int era; /* CAAM Era (internal HW revision) */
#define RNG4_MAX_HANDLES 2
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 7f2b1101f567..e218d4ae604c 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -511,10 +511,25 @@ 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;
+ struct caam_drv_private *caamctrlpriv;
+ u32 ring_idx;
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_idx)) {
+ dev_err(&pdev->dev, "failed to get register address for jobr\n");
+ return -ENOMEM;
+ }
+ caamctrlpriv = dev_get_drvdata(pdev->dev.parent);
+ ring_idx = ring_idx >> CAAM_CAPS_PG_SHIFT(caamctrlpriv->caam_caps);
+
jrdev = &pdev->dev;
jrpriv = devm_kzalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL);
if (!jrpriv)
@@ -523,7 +538,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_idx;
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 3738625c0250..186e76e6a3e7 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -1023,6 +1023,4 @@ struct caam_deco {
#define ASSURE_BLOCK_NUMBER 6
#define QI_BLOCK_NUMBER 7
#define DECO_BLOCK_NUMBER 8
-#define PG_SIZE_4K 0x1000
-#define PG_SIZE_64K 0x10000
#endif /* REGS_H */
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2021-12-07 23:02 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 1/2] crypto: caam - convert to use capabilities Andrey Zhizhikin
@ 2021-12-07 23:02 ` Andrey Zhizhikin
2022-01-06 11:26 ` Rouven Czerwinski
2022-01-06 10:56 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status ZHIZHIKIN Andrey
2 siblings, 1 reply; 14+ messages in thread
From: Andrey Zhizhikin @ 2021-12-07 23:02 UTC (permalink / raw)
To: linux-kernel
Cc: robh+dt, shawnguo, michael, s.hauer, kernel, festevam, linux-imx,
horia.geanta, pankaj.gupta, herbert, davem, l.stach,
qiangqing.zhang, peng.fan, alice.guo, aford173, frieder.schrempf,
krzk, shengjiu.wang, gregkh, ping.bai, daniel.baluta, jun.li,
marex, thunder.leizhen, martink, leonard.crestez, hongxing.zhu,
agx, devicetree, linux-arm-kernel, linux-crypto, op-tee,
Andrey Zhizhikin
CAAM JR nodes are configured by BootROM and are used by various software
entities during the boot process before they reach the Kernel.
Default BootROM configuration have JR0 and JR1 reserved for S-only
access, while JR2 is generally available for both S and NS access. HAB
feature of i.MX8M family does require that JR0 is reserved exclusively
in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
can later reclaim the JR2 via dt_enable_secure_status() call, and modify
the DID to hold it in S-World only.
The above setup has been discovered during review of CAAM patchset
presented to U-Boot integration [1], and does not correspond to the
status on jr nodes in FDT.
This missing status settings leads to the following error message during
jr node 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
JR register readout after BootROM execution shows the following values:
JR0DID_MS = 0x8011
JR1DID_MS = 0x8011
JR2DID_MS = 0x0
This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
reserved for S-World, while JR2 remains accessible from NS-World.
Provide the correct status for JR nodes in imx8m derivatives, which have
a following meaning:
- JR0: S-only
- JR1: visible in both
- JR2: NS-only
Note, that JR2 is initially marked to be NS-only which does correspond
to DID readout when OP-TEE is not present. Once present, OP-TEE will
reclaim the JR2 and set both "status" and "secure-status" to claim JR2
for S-only access.
Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Link: [1]: https://lore.kernel.org/u-boot/AM6PR06MB4691FC905FE5658BE4B15C11A6609@AM6PR06MB4691.eurprd06.prod.outlook.com/
---
Changes in V3:
- No change, new patch introduced
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 ++++
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 ++++
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++++
4 files changed, 16 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 5b9c2cca9ac4..51465974c4ea 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -914,18 +914,22 @@ sec_jr0: jr@1000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ secure-status = "okay";
};
sec_jr1: jr@2000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x2000 0x1000>;
interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "okay";
};
sec_jr2: jr@3000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x3000 0x1000>;
interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "disabled";
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index ba23b416b5e6..e5edf14319b1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -808,18 +808,22 @@ sec_jr0: jr@1000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ secure-status = "okay";
};
sec_jr1: jr@2000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x2000 0x1000>;
interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "okay";
};
sec_jr2: jr@3000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x3000 0x1000>;
interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "disabled";
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 977783784342..3c23bf5c3910 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -661,18 +661,22 @@ sec_jr0: jr@1000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ secure-status = "okay";
};
sec_jr1: jr@2000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x2000 0x1000>;
interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "okay";
};
sec_jr2: jr@3000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x3000 0x1000>;
interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "disabled";
};
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 95d8b95d6120..16c4c9110ce7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -999,18 +999,22 @@ sec_jr0: jr@1000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ secure-status = "okay";
};
sec_jr1: jr@2000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x2000 0x1000>;
interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "okay";
};
sec_jr2: jr@3000 {
compatible = "fsl,sec-v4.0-job-ring";
reg = <0x3000 0x1000>;
interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ secure-status = "disabled";
};
};
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status
2021-12-07 23:02 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 1/2] crypto: caam - convert to use capabilities Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr Andrey Zhizhikin
@ 2022-01-06 10:56 ` ZHIZHIKIN Andrey
2022-01-07 2:36 ` Herbert Xu
2 siblings, 1 reply; 14+ messages in thread
From: ZHIZHIKIN Andrey @ 2022-01-06 10:56 UTC (permalink / raw)
To: herbert, linux-kernel
Cc: robh+dt, shawnguo, michael, s.hauer, kernel, festevam, linux-imx,
horia.geanta, pankaj.gupta, davem, l.stach, qiangqing.zhang,
peng.fan, alice.guo, aford173, frieder.schrempf, krzk,
shengjiu.wang, gregkh, ping.bai, daniel.baluta, jun.li, marex,
thunder.leizhen, martink, leonard.crestez, hongxing.zhu, agx,
devicetree, linux-arm-kernel, linux-crypto, op-tee
Hello Herbert,
Gentle ping on this V3. I see that in Patchwork this series state is set to "Deferred".
Is there anything missing here to proceed further?
> -----Original Message-----
> From: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Sent: Wednesday, December 8, 2021 12:02 AM
> To: linux-kernel@vger.kernel.org
> Cc: robh+dt@kernel.org; shawnguo@kernel.org; michael@walle.cc;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com; linux-
> imx@nxp.com; horia.geanta@nxp.com; pankaj.gupta@nxp.com;
> herbert@gondor.apana.org.au; davem@davemloft.net; l.stach@pengutronix.de;
> qiangqing.zhang@nxp.com; peng.fan@nxp.com; alice.guo@nxp.com; aford173@gmail.com;
> frieder.schrempf@kontron.de; krzk@kernel.org; shengjiu.wang@nxp.com;
> gregkh@linuxfoundation.org; ping.bai@nxp.com; daniel.baluta@nxp.com;
> jun.li@nxp.com; marex@denx.de; thunder.leizhen@huawei.com; martink@posteo.de;
> leonard.crestez@nxp.com; hongxing.zhu@nxp.com; agx@sigxcpu.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> crypto@vger.kernel.org; op-tee@lists.trustedfirmware.org; Andrey Zhizhikin
> <andrey.zhizhikin@leica-geosystems.com>
> Subject: [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status
>
> This V3 series covers points uncovered during the review of the previous
> series, one major point being that register readout should not be used
> for dynamic JR availability check due to its unreliability.
>
> Instead, JR should have a proper status set in FDT which indicates the
> availability of the ring in NS-World. This status is aligned with what
> BootROM code configures, and can be modified by all actors in the boot
> chain.
>
> Therefore, patch in V2 series that was handling the dynamic JR
> availability check is dropped in this series and replaced by the patch
> which sets proper DT status for JR nodes.
>
> Andrey Zhizhikin (2):
> crypto: caam - convert to use capabilities
> arm64: dts: imx8m: define proper status for caam jr
>
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 +
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 +
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 +
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 +
> drivers/crypto/caam/caamalg_qi.c | 2 +-
> drivers/crypto/caam/ctrl.c | 115 ++++++++++++++--------
> drivers/crypto/caam/intern.h | 20 ++--
> drivers/crypto/caam/jr.c | 19 +++-
> drivers/crypto/caam/regs.h | 2 -
> 9 files changed, 122 insertions(+), 52 deletions(-)
>
>
> base-commit: 04fe99a8d936d46a310ca61b8b63dc270962bf01
> --
> 2.25.1
>
-- andrey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2021-12-07 23:02 ` [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr Andrey Zhizhikin
@ 2022-01-06 11:26 ` Rouven Czerwinski
2022-01-06 14:08 ` ZHIZHIKIN Andrey
0 siblings, 1 reply; 14+ messages in thread
From: Rouven Czerwinski @ 2022-01-06 11:26 UTC (permalink / raw)
To: Andrey Zhizhikin, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, frieder.schrempf,
leonard.crestez, festevam, marex, herbert, horia.geanta,
aford173, krzk, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, daniel.baluta,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, jun.li, shawnguo, davem,
l.stach
Hi Andrey,
On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote:
> CAAM JR nodes are configured by BootROM and are used by various software
> entities during the boot process before they reach the Kernel.
>
> Default BootROM configuration have JR0 and JR1 reserved for S-only
> access, while JR2 is generally available for both S and NS access. HAB
> feature of i.MX8M family does require that JR0 is reserved exclusively
> in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
> can later reclaim the JR2 via dt_enable_secure_status() call, and modify
> the DID to hold it in S-World only.
>
> The above setup has been discovered during review of CAAM patchset
> presented to U-Boot integration [1], and does not correspond to the
> status on jr nodes in FDT.
>
> This missing status settings leads to the following error message during
> jr node 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
>
> JR register readout after BootROM execution shows the following values:
> JR0DID_MS = 0x8011
> JR1DID_MS = 0x8011
> JR2DID_MS = 0x0
>
> This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
> reserved for S-World, while JR2 remains accessible from NS-World.
>
> Provide the correct status for JR nodes in imx8m derivatives, which have
> a following meaning:
> - JR0: S-only
> - JR1: visible in both
> - JR2: NS-only
>
> Note, that JR2 is initially marked to be NS-only which does correspond
> to DID readout when OP-TEE is not present. Once present, OP-TEE will
> reclaim the JR2 and set both "status" and "secure-status" to claim JR2
> for S-only access.
While I can understand that you want to fix your use case for when HAB
is enabled, note that this is disabling JR0 in the none-HAB case as
well. IMO this should be handled correctly by the bootloader and/or OP-
TEE. The default upstream configuration for OP-TEE is to not use the
CAAM at runtime as well, since linux runtime PM disablement of the CAAM
will lock up OP-TEE when it tries to access the CAAM.
Kind regards,
Rouven Czerwinski
>
> Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> Link: [1]: https://lore.kernel.org/u-boot/AM6PR06MB4691FC905FE5658BE4B15C11A6609@AM6PR06MB4691.eurprd06.prod.outlook.com/
> ---
> Changes in V3:
> - No change, new patch introduced
>
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 ++++
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 ++++
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++++
> 4 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index 5b9c2cca9ac4..51465974c4ea 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -914,18 +914,22 @@ sec_jr0: jr@1000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x1000 0x1000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + secure-status = "okay";
> };
>
> sec_jr1: jr@2000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x2000 0x1000>;
> interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "okay";
> };
>
> sec_jr2: jr@3000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x3000 0x1000>;
> interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "disabled";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index ba23b416b5e6..e5edf14319b1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -808,18 +808,22 @@ sec_jr0: jr@1000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x1000 0x1000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + secure-status = "okay";
> };
>
> sec_jr1: jr@2000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x2000 0x1000>;
> interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "okay";
> };
>
> sec_jr2: jr@3000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x3000 0x1000>;
> interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "disabled";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 977783784342..3c23bf5c3910 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -661,18 +661,22 @@ sec_jr0: jr@1000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x1000 0x1000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + secure-status = "okay";
> };
>
> sec_jr1: jr@2000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x2000 0x1000>;
> interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "okay";
> };
>
> sec_jr2: jr@3000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x3000 0x1000>;
> interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "disabled";
> };
> };
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 95d8b95d6120..16c4c9110ce7 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -999,18 +999,22 @@ sec_jr0: jr@1000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x1000 0x1000>;
> interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + secure-status = "okay";
> };
>
> sec_jr1: jr@2000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x2000 0x1000>;
> interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "okay";
> };
>
> sec_jr2: jr@3000 {
> compatible = "fsl,sec-v4.0-job-ring";
> reg = <0x3000 0x1000>;
> interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
> + secure-status = "disabled";
> };
> };
>
--
Pengutronix e.K. | Rouven Czerwinski |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-06 11:26 ` Rouven Czerwinski
@ 2022-01-06 14:08 ` ZHIZHIKIN Andrey
2022-01-07 9:46 ` Rouven Czerwinski
0 siblings, 1 reply; 14+ messages in thread
From: ZHIZHIKIN Andrey @ 2022-01-06 14:08 UTC (permalink / raw)
To: Rouven Czerwinski, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, frieder.schrempf,
leonard.crestez, festevam, marex, herbert, horia.geanta,
aford173, krzk, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, daniel.baluta,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, jun.li, shawnguo, davem,
l.stach
Hello Rouven,
> -----Original Message-----
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Sent: Thursday, January 6, 2022 12:27 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> kernel@vger.kernel.org
> Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com;
> marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com;
> aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com;
> devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de;
> pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com;
> martink@posteo.de; daniel.baluta@nxp.com; linux-arm-kernel@lists.infradead.org;
> gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org;
> kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org; davem@davemloft.net;
> l.stach@pengutronix.de
> Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
>
> Hi Andrey,
>
> On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote:
> > CAAM JR nodes are configured by BootROM and are used by various software
> > entities during the boot process before they reach the Kernel.
> >
> > Default BootROM configuration have JR0 and JR1 reserved for S-only
> > access, while JR2 is generally available for both S and NS access. HAB
> > feature of i.MX8M family does require that JR0 is reserved exclusively
> > in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
> > can later reclaim the JR2 via dt_enable_secure_status() call, and modify
> > the DID to hold it in S-World only.
> >
> > The above setup has been discovered during review of CAAM patchset
> > presented to U-Boot integration [1], and does not correspond to the
> > status on jr nodes in FDT.
> >
> > This missing status settings leads to the following error message during
> > jr node 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
> >
> > JR register readout after BootROM execution shows the following values:
> > JR0DID_MS = 0x8011
> > JR1DID_MS = 0x8011
> > JR2DID_MS = 0x0
> >
> > This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
> > reserved for S-World, while JR2 remains accessible from NS-World.
> >
> > Provide the correct status for JR nodes in imx8m derivatives, which have
> > a following meaning:
> > - JR0: S-only
> > - JR1: visible in both
> > - JR2: NS-only
> >
> > Note, that JR2 is initially marked to be NS-only which does correspond
> > to DID readout when OP-TEE is not present. Once present, OP-TEE will
> > reclaim the JR2 and set both "status" and "secure-status" to claim JR2
> > for S-only access.
>
> While I can understand that you want to fix your use case for when HAB
> is enabled, note that this is disabling JR0 in the none-HAB case as
> well.
This is not totally correct, as this patch does address the reservation of
JR0 by BootROM in both HAB and non-HAB configurations. My current setup does
not include HAB functionality enabled, and I still do observe boot errors
that are listed in commit message. This is due to the fact that the BootROM
does not release JR0 to NS-World regardless of whether HAB is enabled or not.
This has been discussed in the U-Boot thread I provided the link in the patch.
This patch does rather bring the correct HW module description as seeing
from Linux.
> IMO this should be handled correctly by the bootloader and/or OP-
> TEE. The default upstream configuration for OP-TEE is to not use the
> CAAM at runtime as well, since linux runtime PM disablement of the CAAM
> will lock up OP-TEE when it tries to access the CAAM.
If by handling you mean releasing JR0 reservation - then yes, it should be
done by either SPL or TF-A as they do run in S World. In such a case, DTB
bindings need to be adapted further according to the new state. Until this
done - this patch does provide a correct state of HW to the Kernel.
>
> Kind regards,
> Rouven Czerwinski
>
> >
> > Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> > Link: [1]: https://lore.kernel.org/u-boot/AM6PR06MB4691FC905FE5658BE4B15C11A6609@AM6PR06MB4691.eurprd06.prod.outlook.com/
> > ---
> > Changes in V3:
> > - No change, new patch introduced
> >
> > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 ++++
> > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 ++++
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++
> > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++++
> > 4 files changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > index 5b9c2cca9ac4..51465974c4ea 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > @@ -914,18 +914,22 @@ sec_jr0: jr@1000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x1000 0x1000>;
> > interrupts = <GIC_SPI 105
> IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + secure-status = "okay";
> > };
> >
> > sec_jr1: jr@2000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x2000 0x1000>;
> > interrupts = <GIC_SPI 106
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "okay";
> > };
> >
> > sec_jr2: jr@3000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x3000 0x1000>;
> > interrupts = <GIC_SPI 114
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "disabled";
> > };
> > };
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > index ba23b416b5e6..e5edf14319b1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > @@ -808,18 +808,22 @@ sec_jr0: jr@1000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x1000 0x1000>;
> > interrupts = <GIC_SPI 105
> IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + secure-status = "okay";
> > };
> >
> > sec_jr1: jr@2000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x2000 0x1000>;
> > interrupts = <GIC_SPI 106
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "okay";
> > };
> >
> > sec_jr2: jr@3000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x3000 0x1000>;
> > interrupts = <GIC_SPI 114
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "disabled";
> > };
> > };
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 977783784342..3c23bf5c3910 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -661,18 +661,22 @@ sec_jr0: jr@1000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x1000 0x1000>;
> > interrupts = <GIC_SPI 105
> IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + secure-status = "okay";
> > };
> >
> > sec_jr1: jr@2000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x2000 0x1000>;
> > interrupts = <GIC_SPI 106
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "okay";
> > };
> >
> > sec_jr2: jr@3000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x3000 0x1000>;
> > interrupts = <GIC_SPI 114
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "disabled";
> > };
> > };
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > index 95d8b95d6120..16c4c9110ce7 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > @@ -999,18 +999,22 @@ sec_jr0: jr@1000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x1000 0x1000>;
> > interrupts = <GIC_SPI 105
> IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + secure-status = "okay";
> > };
> >
> > sec_jr1: jr@2000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x2000 0x1000>;
> > interrupts = <GIC_SPI 106
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "okay";
> > };
> >
> > sec_jr2: jr@3000 {
> > compatible = "fsl,sec-v4.0-job-ring";
> > reg = <0x3000 0x1000>;
> > interrupts = <GIC_SPI 114
> IRQ_TYPE_LEVEL_HIGH>;
> > + secure-status = "disabled";
> > };
> > };
> >
>
> --
> Pengutronix e.K. | Rouven Czerwinski |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
-- andrey
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status
2022-01-06 10:56 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status ZHIZHIKIN Andrey
@ 2022-01-07 2:36 ` Herbert Xu
0 siblings, 0 replies; 14+ messages in thread
From: Herbert Xu @ 2022-01-07 2:36 UTC (permalink / raw)
To: ZHIZHIKIN Andrey
Cc: linux-kernel, robh+dt, shawnguo, michael, s.hauer, kernel,
festevam, linux-imx, horia.geanta, pankaj.gupta, davem, l.stach,
qiangqing.zhang, peng.fan, alice.guo, aford173, frieder.schrempf,
krzk, shengjiu.wang, gregkh, ping.bai, daniel.baluta, jun.li,
marex, thunder.leizhen, martink, leonard.crestez, hongxing.zhu,
agx, devicetree, linux-arm-kernel, linux-crypto, op-tee
On Thu, Jan 06, 2022 at 10:56:12AM +0000, ZHIZHIKIN Andrey wrote:
> Hello Herbert,
>
> Gentle ping on this V3. I see that in Patchwork this series state is set to "Deferred".
>
> Is there anything missing here to proceed further?
Please get the caam driver maintainer to review and ack the
patch series.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-06 14:08 ` ZHIZHIKIN Andrey
@ 2022-01-07 9:46 ` Rouven Czerwinski
2022-01-07 10:40 ` ZHIZHIKIN Andrey
2022-01-07 11:47 ` Michael Walle
0 siblings, 2 replies; 14+ messages in thread
From: Rouven Czerwinski @ 2022-01-07 9:46 UTC (permalink / raw)
To: ZHIZHIKIN Andrey, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, krzk, leonard.crestez,
festevam, marex, herbert, horia.geanta, daniel.baluta,
frieder.schrempf, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, aford173,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, l.stach, shawnguo, davem,
jun.li
On Thu, 2022-01-06 at 14:08 +0000, ZHIZHIKIN Andrey wrote:
> Hello Rouven,
>
> > -----Original Message-----
> > From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > Sent: Thursday, January 6, 2022 12:27 PM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> > kernel@vger.kernel.org
> > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> > frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com;
> > marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com;
> > aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com;
> > devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de;
> > pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com;
> > martink@posteo.de; daniel.baluta@nxp.com; linux-arm-kernel@lists.infradead.org;
> > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org;
> > kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org; davem@davemloft.net;
> > l.stach@pengutronix.de
> > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
> >
> > Hi Andrey,
> >
> > On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote:
> > > CAAM JR nodes are configured by BootROM and are used by various software
> > > entities during the boot process before they reach the Kernel.
> > >
> > > Default BootROM configuration have JR0 and JR1 reserved for S-only
> > > access, while JR2 is generally available for both S and NS access. HAB
> > > feature of i.MX8M family does require that JR0 is reserved exclusively
> > > in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
> > > can later reclaim the JR2 via dt_enable_secure_status() call, and modify
> > > the DID to hold it in S-World only.
> > >
> > > The above setup has been discovered during review of CAAM patchset
> > > presented to U-Boot integration [1], and does not correspond to the
> > > status on jr nodes in FDT.
> > >
> > > This missing status settings leads to the following error message during
> > > jr node 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
> > >
> > > JR register readout after BootROM execution shows the following values:
> > > JR0DID_MS = 0x8011
> > > JR1DID_MS = 0x8011
> > > JR2DID_MS = 0x0
> > >
> > > This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
> > > reserved for S-World, while JR2 remains accessible from NS-World.
> > >
> > > Provide the correct status for JR nodes in imx8m derivatives, which have
> > > a following meaning:
> > > - JR0: S-only
> > > - JR1: visible in both
> > > - JR2: NS-only
> > >
> > > Note, that JR2 is initially marked to be NS-only which does correspond
> > > to DID readout when OP-TEE is not present. Once present, OP-TEE will
> > > reclaim the JR2 and set both "status" and "secure-status" to claim JR2
> > > for S-only access.
> >
> > While I can understand that you want to fix your use case for when HAB
> > is enabled, note that this is disabling JR0 in the none-HAB case as
> > well.
>
> This is not totally correct, as this patch does address the reservation of
> JR0 by BootROM in both HAB and non-HAB configurations. My current setup does
> not include HAB functionality enabled, and I still do observe boot errors
> that are listed in commit message. This is due to the fact that the BootROM
> does not release JR0 to NS-World regardless of whether HAB is enabled or not.
> This has been discussed in the U-Boot thread I provided the link in the patch.
>
> This patch does rather bring the correct HW module description as seeing
> from Linux.
I took a look into i.MX8MQ, the bootrom indeed sets 0x8011 for JR0 &
JR1:
JR0:0000000000008011
JR1:0000000000008011
JR2:0000000000000000
TF-A
>CAAM
JR0:0000000000000001
JR1:0000000000000001
JR2:0000000000000001
However at least the upstream TF-A reconfigures the DIDs to 1 for all
i.MX8M* derivates. So while it is technically correct to change the DT
values as you describe, the downstream TF-A and upstream TF-A seem to
differ in their configuration. I also think it's a bad move to hardcode
the JR configuration to the boot ROM config, since AFAIK i.MX8M* can
not be run without TF-A. And IMO the upstream kernel should follow what
the upstream TF-A does in this case.
>
> > IMO this should be handled correctly by the bootloader and/or OP-
> > TEE. The default upstream configuration for OP-TEE is to not use the
> > CAAM at runtime as well, since linux runtime PM disablement of the CAAM
> > will lock up OP-TEE when it tries to access the CAAM.
>
> If by handling you mean releasing JR0 reservation - then yes, it should be
> done by either SPL or TF-A as they do run in S World. In such a case, DTB
> bindings need to be adapted further according to the new state. Until this
> done - this patch does provide a correct state of HW to the Kernel.
Upstream TF-A simply releases all JRs to the normal world, matching the
current DTB description.
Kind Regards,
Rouven Czerwinski
> >
> > Kind regards,
> > Rouven Czerwinski
> >
> > >
> > > Signed-off-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
> > > Link: [1]: https://lore.kernel.org/u-boot/AM6PR06MB4691FC905FE5658BE4B15C11A6609@AM6PR06MB4691.eurprd06.prod.outlook.com/
> > > ---
> > > Changes in V3:
> > > - No change, new patch introduced
> > >
> > > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 4 ++++
> > > arch/arm64/boot/dts/freescale/imx8mn.dtsi | 4 ++++
> > > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++++
> > > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 4 ++++
> > > 4 files changed, 16 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > index 5b9c2cca9ac4..51465974c4ea 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > @@ -914,18 +914,22 @@ sec_jr0: jr@1000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x1000 0x1000>;
> > > interrupts = <GIC_SPI 105
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + status = "disabled";
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr1: jr@2000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x2000 0x1000>;
> > > interrupts = <GIC_SPI 106
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr2: jr@3000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x3000 0x1000>;
> > > interrupts = <GIC_SPI 114
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "disabled";
> > > };
> > > };
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > index ba23b416b5e6..e5edf14319b1 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> > > @@ -808,18 +808,22 @@ sec_jr0: jr@1000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x1000 0x1000>;
> > > interrupts = <GIC_SPI 105
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + status = "disabled";
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr1: jr@2000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x2000 0x1000>;
> > > interrupts = <GIC_SPI 106
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr2: jr@3000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x3000 0x1000>;
> > > interrupts = <GIC_SPI 114
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "disabled";
> > > };
> > > };
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index 977783784342..3c23bf5c3910 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -661,18 +661,22 @@ sec_jr0: jr@1000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x1000 0x1000>;
> > > interrupts = <GIC_SPI 105
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + status = "disabled";
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr1: jr@2000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x2000 0x1000>;
> > > interrupts = <GIC_SPI 106
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr2: jr@3000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x3000 0x1000>;
> > > interrupts = <GIC_SPI 114
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "disabled";
> > > };
> > > };
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > index 95d8b95d6120..16c4c9110ce7 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > @@ -999,18 +999,22 @@ sec_jr0: jr@1000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x1000 0x1000>;
> > > interrupts = <GIC_SPI 105
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + status = "disabled";
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr1: jr@2000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x2000 0x1000>;
> > > interrupts = <GIC_SPI 106
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "okay";
> > > };
> > >
> > > sec_jr2: jr@3000 {
> > > compatible = "fsl,sec-v4.0-job-ring";
> > > reg = <0x3000 0x1000>;
> > > interrupts = <GIC_SPI 114
> > IRQ_TYPE_LEVEL_HIGH>;
> > > + secure-status = "disabled";
> > > };
> > > };
> > >
> >
> > --
> > Pengutronix e.K. | Rouven Czerwinski |
> > Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>
>
> -- andrey
--
Pengutronix e.K. | Rouven Czerwinski |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 9:46 ` Rouven Czerwinski
@ 2022-01-07 10:40 ` ZHIZHIKIN Andrey
2022-01-07 11:55 ` Rouven Czerwinski
2022-01-07 11:47 ` Michael Walle
1 sibling, 1 reply; 14+ messages in thread
From: ZHIZHIKIN Andrey @ 2022-01-07 10:40 UTC (permalink / raw)
To: Rouven Czerwinski, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, krzk, leonard.crestez,
festevam, marex, herbert, horia.geanta, daniel.baluta,
frieder.schrempf, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, aford173,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, l.stach, shawnguo, davem,
jun.li
Hello Rouven,
> -----Original Message-----
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Sent: Friday, January 7, 2022 10:46 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> kernel@vger.kernel.org
> Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> krzk@kernel.org; leonard.crestez@nxp.com; festevam@gmail.com; marex@denx.de;
> herbert@gondor.apana.org.au; horia.geanta@nxp.com; daniel.baluta@nxp.com;
> frieder.schrempf@kontron.de; linux-imx@nxp.com; devicetree@vger.kernel.org;
> hongxing.zhu@nxp.com; s.hauer@pengutronix.de; pankaj.gupta@nxp.com;
> robh+dt@kernel.org; thunder.leizhen@huawei.com; martink@posteo.de;
> aford173@gmail.com; linux-arm-kernel@lists.infradead.org;
> gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org;
> kernel@pengutronix.de; l.stach@pengutronix.de; shawnguo@kernel.org;
> davem@davemloft.net; jun.li@nxp.com
> Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
>
>
> On Thu, 2022-01-06 at 14:08 +0000, ZHIZHIKIN Andrey wrote:
> > Hello Rouven,
> >
> > > -----Original Message-----
> > > From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > Sent: Thursday, January 6, 2022 12:27 PM
> > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> > > kernel@vger.kernel.org
> > > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> > > frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com;
> > > marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com;
> > > aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com;
> > > devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de;
> > > pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com;
> > > martink@posteo.de; daniel.baluta@nxp.com; linux-arm-
> kernel@lists.infradead.org;
> > > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> > > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-
> crypto@vger.kernel.org;
> > > kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org;
> davem@davemloft.net;
> > > l.stach@pengutronix.de
> > > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam
> jr
> > >
> > > Hi Andrey,
> > >
> > > On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote:
> > > > CAAM JR nodes are configured by BootROM and are used by various software
> > > > entities during the boot process before they reach the Kernel.
> > > >
> > > > Default BootROM configuration have JR0 and JR1 reserved for S-only
> > > > access, while JR2 is generally available for both S and NS access. HAB
> > > > feature of i.MX8M family does require that JR0 is reserved exclusively
> > > > in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
> > > > can later reclaim the JR2 via dt_enable_secure_status() call, and modify
> > > > the DID to hold it in S-World only.
> > > >
> > > > The above setup has been discovered during review of CAAM patchset
> > > > presented to U-Boot integration [1], and does not correspond to the
> > > > status on jr nodes in FDT.
> > > >
> > > > This missing status settings leads to the following error message during
> > > > jr node 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
> > > >
> > > > JR register readout after BootROM execution shows the following values:
> > > > JR0DID_MS = 0x8011
> > > > JR1DID_MS = 0x8011
> > > > JR2DID_MS = 0x0
> > > >
> > > > This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
> > > > reserved for S-World, while JR2 remains accessible from NS-World.
> > > >
> > > > Provide the correct status for JR nodes in imx8m derivatives, which have
> > > > a following meaning:
> > > > - JR0: S-only
> > > > - JR1: visible in both
> > > > - JR2: NS-only
> > > >
> > > > Note, that JR2 is initially marked to be NS-only which does correspond
> > > > to DID readout when OP-TEE is not present. Once present, OP-TEE will
> > > > reclaim the JR2 and set both "status" and "secure-status" to claim JR2
> > > > for S-only access.
> > >
> > > While I can understand that you want to fix your use case for when HAB
> > > is enabled, note that this is disabling JR0 in the none-HAB case as
> > > well.
> >
> > This is not totally correct, as this patch does address the reservation of
> > JR0 by BootROM in both HAB and non-HAB configurations. My current setup does
> > not include HAB functionality enabled, and I still do observe boot errors
> > that are listed in commit message. This is due to the fact that the BootROM
> > does not release JR0 to NS-World regardless of whether HAB is enabled or not.
> > This has been discussed in the U-Boot thread I provided the link in the patch.
> >
> > This patch does rather bring the correct HW module description as seeing
> > from Linux.
>
> I took a look into i.MX8MQ, the bootrom indeed sets 0x8011 for JR0 &
> JR1:
> JR0:0000000000008011
> JR1:0000000000008011
> JR2:0000000000000000
> TF-A
> >CAAM
> JR0:0000000000000001
> JR1:0000000000000001
> JR2:0000000000000001
>
> However at least the upstream TF-A reconfigures the DIDs to 1 for all
> i.MX8M* derivates. So while it is technically correct to change the DT
> values as you describe, the downstream TF-A and upstream TF-A seem to
> differ in their configuration. I also think it's a bad move to hardcode
> the JR configuration to the boot ROM config, since AFAIK i.MX8M* can
> not be run without TF-A. And IMO the upstream kernel should follow what
> the upstream TF-A does in this case.
This is indeed an interesting piece of information, thanks a lot!
From what I understood in previous discussions for this series here in
the Kernel, and on U-Boot ML: JR0 is required to be held reserved in
S-World for HAB to operate, and this is clearly communicated by NXP in [1].
If my understanding is correct, then upstream TF-A either does not support
or breaks the HAB feature.
I've been following the build documentation in U-Boot, where the downstream
TF-A is listed as build prequisites [2] without any mentioning of upstream,
hence I received a readout that matched the BootROM "1-to-1".
I believe that if the information from NXP regarding JR0 reservation for
HAB is correct (and I have no reasons to doubt it), then upstream TF-A
should be adapted in order for HAB feature to work, and in that case this
patch brings correct HW state description as seeing from Linux.
And in contrary, if the upstream TF-A is not adjusted to include HAB
support - then applying this patch would effectively just "remove" one
JR device, still keeping 2 additional available nodes for HW crypto
operations in Kernel, with added benefit that both upstream and
downstream TF-A can be used during the boot without seeing issues later
in the Kernel.
>
> >
> > > IMO this should be handled correctly by the bootloader and/or OP-
> > > TEE. The default upstream configuration for OP-TEE is to not use the
> > > CAAM at runtime as well, since linux runtime PM disablement of the CAAM
> > > will lock up OP-TEE when it tries to access the CAAM.
> >
> > If by handling you mean releasing JR0 reservation - then yes, it should be
> > done by either SPL or TF-A as they do run in S World. In such a case, DTB
> > bindings need to be adapted further according to the new state. Until this
> > done - this patch does provide a correct state of HW to the Kernel.
>
> Upstream TF-A simply releases all JRs to the normal world, matching the
> current DTB description.
>
> Kind Regards,
> Rouven Czerwinski
>
>
[snip]
Regards,
Andrey
Link: [1]: https://lore.kernel.org/u-boot/VI1PR04MB5342C8C6E651EC2CC4477EB5E79A9@VI1PR04MB5342.eurprd04.prod.outlook.com/
Link: [2]: https://source.denx.de/u-boot/u-boot/-/blob/master/doc/board/nxp/imx8mm_evk.rst
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 9:46 ` Rouven Czerwinski
2022-01-07 10:40 ` ZHIZHIKIN Andrey
@ 2022-01-07 11:47 ` Michael Walle
2022-01-07 11:58 ` Lucas Stach
1 sibling, 1 reply; 14+ messages in thread
From: Michael Walle @ 2022-01-07 11:47 UTC (permalink / raw)
To: Rouven Czerwinski
Cc: ZHIZHIKIN Andrey, linux-kernel, peng.fan, ping.bai, alice.guo,
agx, krzk, leonard.crestez, festevam, marex, herbert,
horia.geanta, daniel.baluta, frieder.schrempf, linux-imx,
devicetree, hongxing.zhu, s.hauer, pankaj.gupta, robh+dt,
thunder.leizhen, martink, aford173, linux-arm-kernel, gregkh,
shengjiu.wang, qiangqing.zhang, op-tee, linux-crypto, kernel,
l.stach, shawnguo, davem, jun.li
Hi Rouven,
Am 2022-01-07 10:46, schrieb Rouven Czerwinski:
> .. since AFAIK i.MX8M* can not be run without TF-A.
Are you sure? There probably aren't any boards out there
without TF-A, but why shouldn't it work without it?
-michael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 10:40 ` ZHIZHIKIN Andrey
@ 2022-01-07 11:55 ` Rouven Czerwinski
2022-01-08 20:48 ` ZHIZHIKIN Andrey
0 siblings, 1 reply; 14+ messages in thread
From: Rouven Czerwinski @ 2022-01-07 11:55 UTC (permalink / raw)
To: ZHIZHIKIN Andrey, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, krzk, leonard.crestez,
festevam, marex, herbert, horia.geanta, daniel.baluta,
frieder.schrempf, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, aford173,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, l.stach, shawnguo, davem,
jun.li
On Fri, 2022-01-07 at 10:40 +0000, ZHIZHIKIN Andrey wrote:
> Hello Rouven,
>
> > -----Original Message-----
> > From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > Sent: Friday, January 7, 2022 10:46 AM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> > kernel@vger.kernel.org
> > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> > krzk@kernel.org; leonard.crestez@nxp.com; festevam@gmail.com; marex@denx.de;
> > herbert@gondor.apana.org.au; horia.geanta@nxp.com; daniel.baluta@nxp.com;
> > frieder.schrempf@kontron.de; linux-imx@nxp.com; devicetree@vger.kernel.org;
> > hongxing.zhu@nxp.com; s.hauer@pengutronix.de; pankaj.gupta@nxp.com;
> > robh+dt@kernel.org; thunder.leizhen@huawei.com; martink@posteo.de;
> > aford173@gmail.com; linux-arm-kernel@lists.infradead.org;
> > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-crypto@vger.kernel.org;
> > kernel@pengutronix.de; l.stach@pengutronix.de; shawnguo@kernel.org;
> > davem@davemloft.net; jun.li@nxp.com
> > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
> >
> >
> > On Thu, 2022-01-06 at 14:08 +0000, ZHIZHIKIN Andrey wrote:
> > > Hello Rouven,
> > >
> > > > -----Original Message-----
> > > > From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > Sent: Thursday, January 6, 2022 12:27 PM
> > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; linux-
> > > > kernel@vger.kernel.org
> > > > Cc: peng.fan@nxp.com; ping.bai@nxp.com; alice.guo@nxp.com; agx@sigxcpu.org;
> > > > frieder.schrempf@kontron.de; leonard.crestez@nxp.com; festevam@gmail.com;
> > > > marex@denx.de; herbert@gondor.apana.org.au; horia.geanta@nxp.com;
> > > > aford173@gmail.com; krzk@kernel.org; linux-imx@nxp.com;
> > > > devicetree@vger.kernel.org; hongxing.zhu@nxp.com; s.hauer@pengutronix.de;
> > > > pankaj.gupta@nxp.com; robh+dt@kernel.org; thunder.leizhen@huawei.com;
> > > > martink@posteo.de; daniel.baluta@nxp.com; linux-arm-
> > kernel@lists.infradead.org;
> > > > gregkh@linuxfoundation.org; shengjiu.wang@nxp.com; qiangqing.zhang@nxp.com;
> > > > michael@walle.cc; op-tee@lists.trustedfirmware.org; linux-
> > crypto@vger.kernel.org;
> > > > kernel@pengutronix.de; jun.li@nxp.com; shawnguo@kernel.org;
> > davem@davemloft.net;
> > > > l.stach@pengutronix.de
> > > > Subject: Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam
> > jr
> > > >
> > > > Hi Andrey,
> > > >
> > > > On Wed, 2021-12-08 at 00:02 +0100, Andrey Zhizhikin wrote:
> > > > > CAAM JR nodes are configured by BootROM and are used by various software
> > > > > entities during the boot process before they reach the Kernel.
> > > > >
> > > > > Default BootROM configuration have JR0 and JR1 reserved for S-only
> > > > > access, while JR2 is generally available for both S and NS access. HAB
> > > > > feature of i.MX8M family does require that JR0 is reserved exclusively
> > > > > in S-only world, while JR1 and JR2 are both released to NS-World. OP-TEE
> > > > > can later reclaim the JR2 via dt_enable_secure_status() call, and modify
> > > > > the DID to hold it in S-World only.
> > > > >
> > > > > The above setup has been discovered during review of CAAM patchset
> > > > > presented to U-Boot integration [1], and does not correspond to the
> > > > > status on jr nodes in FDT.
> > > > >
> > > > > This missing status settings leads to the following error message during
> > > > > jr node 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
> > > > >
> > > > > JR register readout after BootROM execution shows the following values:
> > > > > JR0DID_MS = 0x8011
> > > > > JR1DID_MS = 0x8011
> > > > > JR2DID_MS = 0x0
> > > > >
> > > > > This shows that JR0 and JR1 have TZ_OWN bit set, which marks them to be
> > > > > reserved for S-World, while JR2 remains accessible from NS-World.
> > > > >
> > > > > Provide the correct status for JR nodes in imx8m derivatives, which have
> > > > > a following meaning:
> > > > > - JR0: S-only
> > > > > - JR1: visible in both
> > > > > - JR2: NS-only
> > > > >
> > > > > Note, that JR2 is initially marked to be NS-only which does correspond
> > > > > to DID readout when OP-TEE is not present. Once present, OP-TEE will
> > > > > reclaim the JR2 and set both "status" and "secure-status" to claim JR2
> > > > > for S-only access.
> > > >
> > > > While I can understand that you want to fix your use case for when HAB
> > > > is enabled, note that this is disabling JR0 in the none-HAB case as
> > > > well.
> > >
> > > This is not totally correct, as this patch does address the reservation of
> > > JR0 by BootROM in both HAB and non-HAB configurations. My current setup does
> > > not include HAB functionality enabled, and I still do observe boot errors
> > > that are listed in commit message. This is due to the fact that the BootROM
> > > does not release JR0 to NS-World regardless of whether HAB is enabled or not.
> > > This has been discussed in the U-Boot thread I provided the link in the patch.
> > >
> > > This patch does rather bring the correct HW module description as seeing
> > > from Linux.
> >
> > I took a look into i.MX8MQ, the bootrom indeed sets 0x8011 for JR0 &
> > JR1:
> > JR0:0000000000008011
> > JR1:0000000000008011
> > JR2:0000000000000000
> > TF-A
> > > CAAM
> > JR0:0000000000000001
> > JR1:0000000000000001
> > JR2:0000000000000001
> >
> > However at least the upstream TF-A reconfigures the DIDs to 1 for all
> > i.MX8M* derivates. So while it is technically correct to change the DT
> > values as you describe, the downstream TF-A and upstream TF-A seem to
> > differ in their configuration. I also think it's a bad move to hardcode
> > the JR configuration to the boot ROM config, since AFAIK i.MX8M* can
> > not be run without TF-A. And IMO the upstream kernel should follow what
> > the upstream TF-A does in this case.
>
> This is indeed an interesting piece of information, thanks a lot!
>
> From what I understood in previous discussions for this series here in
> the Kernel, and on U-Boot ML: JR0 is required to be held reserved in
> S-World for HAB to operate, and this is clearly communicated by NXP in [1].
> If my understanding is correct, then upstream TF-A either does not support
> or breaks the HAB feature.
Yes, upstream TF-A does not implement the NXP specific SIPs to
communicate with the ROM code to do further image authentication.
Thats also the reason why all JRs are released to normal world, there
is no possible HAB use after TF-A has started.
> I've been following the build documentation in U-Boot, where the downstream
> TF-A is listed as build prequisites [2] without any mentioning of upstream,
> hence I received a readout that matched the BootROM "1-to-1".
Yes, since the downstream TF-A is required to authenticate further
images.
Aside from this the bootrom HAB interface on i.MX8MQ was broken in
interesting ways, I had to implement parsing of the HAB status SRAM
area by hand within barebox.
> I believe that if the information from NXP regarding JR0 reservation for
> HAB is correct (and I have no reasons to doubt it), then upstream TF-A
> should be adapted in order for HAB feature to work, and in that case this
> patch brings correct HW state description as seeing from Linux.
JR0 for HAB in S-World makes sense to me, otherwise the bootrom will
probably refuse to work with the JR.
> And in contrary, if the upstream TF-A is not adjusted to include HAB
> support - then applying this patch would effectively just "remove" one
> JR device, still keeping 2 additional available nodes for HW crypto
> operations in Kernel, with added benefit that both upstream and
> downstream TF-A can be used during the boot without seeing issues later
> in the Kernel.
Even with the downstream TF-A there is no reason to keep JR0 asigned to
the secure world, unless you are running OP-TEE. JR0 assignement for
HAB shouldn't be required after the bootloader has run, unless you want
to HAB authenticate images from a running Linux kernel.
The reason NXP hardcodes the configuration downstream is of course to
support HAB & OP-TEE, but this is still not a reason to hardcode this
assignement into the kernel device tree. They probably also hardcode it
in their downstream kernel versions.
I can understand that it seems easier to hardcode this in the kernel,
but as I said before, if you are running OP-TEE you need to adjust the
DT anyway since nodes need to be added and you might as well adjust the
jobring configuration than.
Kind regards,
Rouven
>
> >
> > >
> > > > IMO this should be handled correctly by the bootloader and/or OP-
> > > > TEE. The default upstream configuration for OP-TEE is to not use the
> > > > CAAM at runtime as well, since linux runtime PM disablement of the CAAM
> > > > will lock up OP-TEE when it tries to access the CAAM.
> > >
> > > If by handling you mean releasing JR0 reservation - then yes, it should be
> > > done by either SPL or TF-A as they do run in S World. In such a case, DTB
> > > bindings need to be adapted further according to the new state. Until this
> > > done - this patch does provide a correct state of HW to the Kernel.
> >
> > Upstream TF-A simply releases all JRs to the normal world, matching the
> > current DTB description.
> >
> > Kind Regards,
> > Rouven Czerwinski
> >
> >
> [snip]
>
> Regards,
> Andrey
>
> Link: [1]: https://lore.kernel.org/u-boot/VI1PR04MB5342C8C6E651EC2CC4477EB5E79A9@VI1PR04MB5342.eurprd04.prod.outlook.com/
> Link: [2]: https://source.denx.de/u-boot/u-boot/-/blob/master/doc/board/nxp/imx8mm_evk.rst
--
Pengutronix e.K. | Rouven Czerwinski |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 11:47 ` Michael Walle
@ 2022-01-07 11:58 ` Lucas Stach
2022-01-07 12:05 ` Michael Walle
0 siblings, 1 reply; 14+ messages in thread
From: Lucas Stach @ 2022-01-07 11:58 UTC (permalink / raw)
To: Michael Walle, Rouven Czerwinski
Cc: ZHIZHIKIN Andrey, linux-kernel, peng.fan, ping.bai, alice.guo,
agx, krzk, leonard.crestez, festevam, marex, herbert,
horia.geanta, daniel.baluta, frieder.schrempf, linux-imx,
devicetree, hongxing.zhu, s.hauer, pankaj.gupta, robh+dt,
thunder.leizhen, martink, aford173, linux-arm-kernel, gregkh,
shengjiu.wang, qiangqing.zhang, op-tee, linux-crypto, kernel,
shawnguo, davem, jun.li
Am Freitag, dem 07.01.2022 um 12:47 +0100 schrieb Michael Walle:
> Hi Rouven,
>
> Am 2022-01-07 10:46, schrieb Rouven Czerwinski:
> > .. since AFAIK i.MX8M* can not be run without TF-A.
>
> Are you sure? There probably aren't any boards out there
> without TF-A, but why shouldn't it work without it?
PSCI, i.e. the only means to start the secondary CPUs, is implemented
in TF-A, so it's very unlikely that anyone would want to run a system
without TF-A. Also quite a bit of the lowlevel SoC initialization is
implemented in TF-A.
Regards,
Lucas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 11:58 ` Lucas Stach
@ 2022-01-07 12:05 ` Michael Walle
0 siblings, 0 replies; 14+ messages in thread
From: Michael Walle @ 2022-01-07 12:05 UTC (permalink / raw)
To: Lucas Stach
Cc: Rouven Czerwinski, ZHIZHIKIN Andrey, linux-kernel, peng.fan,
ping.bai, alice.guo, agx, krzk, leonard.crestez, festevam, marex,
herbert, horia.geanta, daniel.baluta, frieder.schrempf,
linux-imx, devicetree, hongxing.zhu, s.hauer, pankaj.gupta,
robh+dt, thunder.leizhen, martink, aford173, linux-arm-kernel,
gregkh, shengjiu.wang, qiangqing.zhang, op-tee, linux-crypto,
kernel, shawnguo, davem, jun.li
Am 2022-01-07 12:58, schrieb Lucas Stach:
> Am Freitag, dem 07.01.2022 um 12:47 +0100 schrieb Michael Walle:
>> Hi Rouven,
>>
>> Am 2022-01-07 10:46, schrieb Rouven Czerwinski:
>> > .. since AFAIK i.MX8M* can not be run without TF-A.
>>
>> Are you sure? There probably aren't any boards out there
>> without TF-A, but why shouldn't it work without it?
>
> PSCI, i.e. the only means to start the secondary CPUs, is implemented
> in TF-A, so it's very unlikely that anyone would want to run a system
> without TF-A. Also quite a bit of the lowlevel SoC initialization is
> implemented in TF-A.
Doesn't mean u-boot cannot implement PSCI; actually you doesn't need
it at all, you can still use spin tables. I just keep hearing the same
arguments for the LS1028A SoC and yet there is one board without TF-A ;)
Anyway, I admit it's rather unlikely.
-michael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr
2022-01-07 11:55 ` Rouven Czerwinski
@ 2022-01-08 20:48 ` ZHIZHIKIN Andrey
0 siblings, 0 replies; 14+ messages in thread
From: ZHIZHIKIN Andrey @ 2022-01-08 20:48 UTC (permalink / raw)
To: Rouven Czerwinski, linux-kernel
Cc: peng.fan, ping.bai, alice.guo, agx, krzk, leonard.crestez,
festevam, marex, herbert, horia.geanta, daniel.baluta,
frieder.schrempf, linux-imx, devicetree, hongxing.zhu, s.hauer,
pankaj.gupta, robh+dt, thunder.leizhen, martink, aford173,
linux-arm-kernel, gregkh, shengjiu.wang, qiangqing.zhang,
michael, op-tee, linux-crypto, kernel, l.stach, shawnguo, davem,
jun.li
Hello Rouven,
> -----Original Message-----
> From: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> Sent: Friday, January 7, 2022 12:56 PM
[snip]
>
> Yes, upstream TF-A does not implement the NXP specific SIPs to
> communicate with the ROM code to do further image authentication.
> Thats also the reason why all JRs are released to normal world, there
> is no possible HAB use after TF-A has started.
>
> > I've been following the build documentation in U-Boot, where the downstream
> > TF-A is listed as build prequisites [2] without any mentioning of upstream,
> > hence I received a readout that matched the BootROM "1-to-1".
>
> Yes, since the downstream TF-A is required to authenticate further
> images.
>
> Aside from this the bootrom HAB interface on i.MX8MQ was broken in
> interesting ways, I had to implement parsing of the HAB status SRAM
> area by hand within barebox.
>
> > I believe that if the information from NXP regarding JR0 reservation for
> > HAB is correct (and I have no reasons to doubt it), then upstream TF-A
> > should be adapted in order for HAB feature to work, and in that case this
> > patch brings correct HW state description as seeing from Linux.
>
> JR0 for HAB in S-World makes sense to me, otherwise the bootrom will
> probably refuse to work with the JR.
>
> > And in contrary, if the upstream TF-A is not adjusted to include HAB
> > support - then applying this patch would effectively just "remove" one
> > JR device, still keeping 2 additional available nodes for HW crypto
> > operations in Kernel, with added benefit that both upstream and
> > downstream TF-A can be used during the boot without seeing issues later
> > in the Kernel.
>
> Even with the downstream TF-A there is no reason to keep JR0 asigned to
> the secure world, unless you are running OP-TEE. JR0 assignement for
> HAB shouldn't be required after the bootloader has run, unless you want
> to HAB authenticate images from a running Linux kernel.
Then this probably should be communicated in U-Boot as there is a
patchset proposed in U-Boot, and one of the patches in that series [1]
disabled JR0 as well. Once merged - the JR0 is not going to be available
for Linux, regardless of the fact that TF-A would set DIDs to 0x1.
>
> The reason NXP hardcodes the configuration downstream is of course to
> support HAB & OP-TEE, but this is still not a reason to hardcode this
> assignement into the kernel device tree. They probably also hardcode it
> in their downstream kernel versions.
Actually, I've checked the downstream NXP Kernel version, and none of
the Job Ring nodes (including JR0) are disabled there.
>
> I can understand that it seems easier to hardcode this in the kernel,
> but as I said before, if you are running OP-TEE you need to adjust the
> DT anyway since nodes need to be added and you might as well adjust the
> jobring configuration than.
My first version of this patch has been targeting dynamic register
readout to check if DID is set for either S or NS Worlds, but that was
rejected due to unreliable readout in HW. Hence this attempt to
statically disable node was made.
Please note, that there are combinations out there which do have HAB,
TF-A but no OP-TEE. In that case patching DT is not an option, and
would cause the probing error at boot.
Frankly speaking, I'm not sure how to proceed with this further...
Clearly, there is an issue that JR devices are not available in certain
combination of SW entities that are setting different permissions on JR:
upstream TF-A does not do any reservation but does not support HAB (and
this is aligned with current DT nodes description); downstream TF-A does
perform reservation and support HAB, but does not release JR0 to NS-World
causing error on the boot at JR probing. Since those 2 combinations are
orthogonal - the only solution that I see (including the patch proposed
in U-Boot ML) is to reserve the JR0 for all combinations, loosing it in
Linux but covering both TF-A (HAB and non-HAB) at the same time.
If you have any other suggestions here - I'm fully opened to receive those!
>
> Kind regards,
> Rouven
>
>
Regards,
Andrey
Link: [1]: https://lore.kernel.org/u-boot/20211207074129.10955-3-gaurav.jain@nxp.com/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-01-08 20:49 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20211111164601.13135-1-andrey.zhizhikin@leica-geosystems.com>
2021-12-07 23:02 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 1/2] crypto: caam - convert to use capabilities Andrey Zhizhikin
2021-12-07 23:02 ` [PATCH v3 2/2] arm64: dts: imx8m: define proper status for caam jr Andrey Zhizhikin
2022-01-06 11:26 ` Rouven Czerwinski
2022-01-06 14:08 ` ZHIZHIKIN Andrey
2022-01-07 9:46 ` Rouven Czerwinski
2022-01-07 10:40 ` ZHIZHIKIN Andrey
2022-01-07 11:55 ` Rouven Czerwinski
2022-01-08 20:48 ` ZHIZHIKIN Andrey
2022-01-07 11:47 ` Michael Walle
2022-01-07 11:58 ` Lucas Stach
2022-01-07 12:05 ` Michael Walle
2022-01-06 10:56 ` [PATCH v3 0/2] CAAM Driver: re-factor and set proper JR status ZHIZHIKIN Andrey
2022-01-07 2:36 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).