The i.MX8M Mini uses the same crypto engine as the i.MX8MQ, but
the driver is restricting the check to just the i.MX8MQ.
This patch lets the driver support all i.MX8M Variants if enabled.
Signed-off-by: Adam Ford <[email protected]>
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index db22777d59b4..1ce03f8961b6 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -527,7 +527,7 @@ static const struct soc_device_attribute caam_imx_soc_table[] = {
{ .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
{ .soc_id = "i.MX6*", .data = &caam_imx6_data },
{ .soc_id = "i.MX7*", .data = &caam_imx7_data },
- { .soc_id = "i.MX8MQ", .data = &caam_imx7_data },
+ { .soc_id = "i.MX8M*", .data = &caam_imx7_data },
{ .family = "Freescale i.MX" },
{ /* sentinel */ }
};
--
2.20.1
The i.MX8M Mini supports the same crypto engine as what is in
the i.MX8MQ, but it is not currently present in the device tree,
because it may be resricted by security features.
This patch places in into the device tree and marks it as disabled,
but anyone not restricting the CAAM with secure mode functions
can mark it as enabled.
Signed-off-by: Adam Ford <[email protected]>
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 2ed1a3537f05..68c7c1adeb60 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -723,6 +723,37 @@
status = "disabled";
};
+ crypto: crypto@30900000 {
+ compatible = "fsl,sec-v4.0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x30900000 0x40000>;
+ ranges = <0 0x30900000 0x40000>;
+ interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_AHB>,
+ <&clk IMX8MM_CLK_IPG_ROOT>;
+ clock-names = "aclk", "ipg";
+ status = "disabled";
+
+ sec_jr0: jr@1000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x1000 0x1000>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr1: jr@2000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x2000 0x1000>;
+ interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr2: jr@3000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x3000 0x1000>;
+ interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
i2c1: i2c@30a20000 {
compatible = "fsl,imx8mm-i2c", "fsl,imx21-i2c";
#address-cells = <1>;
--
2.20.1
Hi Adam,
On 30.11.19 23:51, Adam Ford wrote:
> The i.MX8M Mini uses the same crypto engine as the i.MX8MQ, but
> the driver is restricting the check to just the i.MX8MQ.
>
> This patch lets the driver support all i.MX8M Variants if enabled.
>
> Signed-off-by: Adam Ford <[email protected]>
What about the following lines in run_descriptor_deco0()? Does this
condition also apply to i.MX8MM?
drivers/crypto/caam/ctrl.c:
if (ctrlpriv->virt_en == 1 ||
/*
* Apparently on i.MX8MQ it doesn't matter if virt_en == 1
* and the following steps should be performed regardless
*/
of_machine_is_compatible("fsl,imx8mq")) {
clrsetbits_32(&ctrl->deco_rsr, 0, DECORSR_JR0);
while (!(rd_reg32(&ctrl->deco_rsr) & DECORSR_VALID) &&
--timeout)
cpu_relax();
timeout = 100000;
}
Regards,
Frieder
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index db22777d59b4..1ce03f8961b6 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -527,7 +527,7 @@ static const struct soc_device_attribute caam_imx_soc_table[] = {
> { .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
> { .soc_id = "i.MX6*", .data = &caam_imx6_data },
> { .soc_id = "i.MX7*", .data = &caam_imx7_data },
> - { .soc_id = "i.MX8MQ", .data = &caam_imx7_data },
> + { .soc_id = "i.MX8M*", .data = &caam_imx7_data },
> { .family = "Freescale i.MX" },
> { /* sentinel */ }
> };
>
On Wed, Dec 4, 2019 at 5:38 AM Schrempf Frieder
<[email protected]> wrote:
>
> Hi Adam,
>
> On 30.11.19 23:51, Adam Ford wrote:
> > The i.MX8M Mini uses the same crypto engine as the i.MX8MQ, but
> > the driver is restricting the check to just the i.MX8MQ.
> >
> > This patch lets the driver support all i.MX8M Variants if enabled.
> >
> > Signed-off-by: Adam Ford <[email protected]>
>
> What about the following lines in run_descriptor_deco0()? Does this
> condition also apply to i.MX8MM?
I think that's a question for NXP. I am not seeing that in the NXP
Linux Release, and I don't have an 8MQ to compare.
I was able to get the driver working on the i.MXMM with the patch.
NXP Team,
Do you have any opinions on this?
adam
>
> drivers/crypto/caam/ctrl.c:
>
> if (ctrlpriv->virt_en == 1 ||
> /*
> * Apparently on i.MX8MQ it doesn't matter if virt_en == 1
> * and the following steps should be performed regardless
> */
> of_machine_is_compatible("fsl,imx8mq")) {
> clrsetbits_32(&ctrl->deco_rsr, 0, DECORSR_JR0);
>
> while (!(rd_reg32(&ctrl->deco_rsr) & DECORSR_VALID) &&
> --timeout)
> cpu_relax();
>
> timeout = 100000;
> }
>
> Regards,
> Frieder
>
> >
> > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> > index db22777d59b4..1ce03f8961b6 100644
> > --- a/drivers/crypto/caam/ctrl.c
> > +++ b/drivers/crypto/caam/ctrl.c
> > @@ -527,7 +527,7 @@ static const struct soc_device_attribute caam_imx_soc_table[] = {
> > { .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
> > { .soc_id = "i.MX6*", .data = &caam_imx6_data },
> > { .soc_id = "i.MX7*", .data = &caam_imx7_data },
> > - { .soc_id = "i.MX8MQ", .data = &caam_imx7_data },
> > + { .soc_id = "i.MX8M*", .data = &caam_imx7_data },
> > { .family = "Freescale i.MX" },
> > { /* sentinel */ }
> > };
> >
On 12/1/2019 12:52 AM, Adam Ford wrote:
> The i.MX8M Mini supports the same crypto engine as what is in
> the i.MX8MQ, but it is not currently present in the device tree,
> because it may be resricted by security features.
>
What exactly are you referring to?
> This patch places in into the device tree and marks it as disabled,
> but anyone not restricting the CAAM with secure mode functions
> can mark it as enabled.
>
Even if - due to export control regulations - CAAM is "trimmed down",
it loses only the encryption capabilities (hashing etc. still working).
Again, please clarify what you mean by "secure mode functions",
"security features" etc.
Horia
On 10.12.19 08:56, Horia Geanta wrote:
> On 12/6/2019 9:55 PM, Adam Ford wrote:
>> On Wed, Dec 4, 2019 at 5:38 AM Schrempf Frieder
>> <[email protected]> wrote:
>>>
>>> Hi Adam,
>>>
>>> On 30.11.19 23:51, Adam Ford wrote:
>>>> The i.MX8M Mini uses the same crypto engine as the i.MX8MQ, but
>>>> the driver is restricting the check to just the i.MX8MQ.
>>>>
>>>> This patch lets the driver support all i.MX8M Variants if enabled.
>>>>
>>>> Signed-off-by: Adam Ford <[email protected]>
>>>
>>> What about the following lines in run_descriptor_deco0()? Does this
>>> condition also apply to i.MX8MM?
>>
>> I think that's a question for NXP. I am not seeing that in the NXP
>> Linux Release, and I don't have an 8MQ to compare.
>>
> IIRC the i.MX BSP releases use the JRI for initializing the RNG,
> and not the DECO register interface.
>
>> I was able to get the driver working on the i.MXMM with the patch.
>>
> You are probably using a newer U-boot, which includes
> commit dfaec76029f2 ("crypto/fsl: instantiate all rng state handles")
>
>> NXP Team,
>>
>> Do you have any opinions on this?
>>
> Since current U-boot initializes both RNG state handles, practically
> instantiate_rng() is a no-op.
>
> A simple experiment is to "lie" about the state_handle_mask, to exercise
> the DECO acquire code (or, as mentioned above, to run with an older U-boot):
>
> @@ -268,12 +272,19 @@ static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
> struct caam_ctrl __iomem *ctrl;
> u32 *desc, status = 0, rdsta_val;
> int ret = 0, sh_idx;
> + static int force_init = 1;
>
> ctrl = (struct caam_ctrl __iomem *)ctrlpriv->ctrl;
> desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL);
> if (!desc)
> return -ENOMEM;
>
> + if (force_init && (state_handle_mask == 0x3)) {
> + dev_err(ctrldev, "Forcing reinit of RNG state handle 0!\n");
> + force_init = 0;
> + state_handle_mask = 0x2;
> + }
> +
> for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
> /*
> * If the corresponding bit is set, this state handle
>
> In this case boot log confirms the DECO cannot be acquired:
> [ 2.137101] caam 30900000.crypto: Forcing reinit of RNG state handle 0!
> [ 2.172293] caam 30900000.crypto: failed to acquire DECO 0
> [ 2.177786] caam 30900000.crypto: failed to instantiate RNG
>
> To sum up, writing to DECORSR is mandatory.
Thanks Horia for providing the details.
Adam, can you update your patch to enable the code in
run_descriptor_deco0() for i.MX8MM?
If I understand this correctly, this is necessary to have the RNG
initialize correctly no matter what version of U-Boot is used.
Thanks,
Frieder
On Wed, Dec 11, 2019 at 8:23 AM Schrempf Frieder
<[email protected]> wrote:
>
> On 10.12.19 08:56, Horia Geanta wrote:
> > On 12/6/2019 9:55 PM, Adam Ford wrote:
> >> On Wed, Dec 4, 2019 at 5:38 AM Schrempf Frieder
> >> <[email protected]> wrote:
> >>>
> >>> Hi Adam,
> >>>
> >>> On 30.11.19 23:51, Adam Ford wrote:
> >>>> The i.MX8M Mini uses the same crypto engine as the i.MX8MQ, but
> >>>> the driver is restricting the check to just the i.MX8MQ.
> >>>>
> >>>> This patch lets the driver support all i.MX8M Variants if enabled.
> >>>>
> >>>> Signed-off-by: Adam Ford <[email protected]>
> >>>
> >>> What about the following lines in run_descriptor_deco0()? Does this
> >>> condition also apply to i.MX8MM?
> >>
> >> I think that's a question for NXP. I am not seeing that in the NXP
> >> Linux Release, and I don't have an 8MQ to compare.
> >>
> > IIRC the i.MX BSP releases use the JRI for initializing the RNG,
> > and not the DECO register interface.
> >
> >> I was able to get the driver working on the i.MXMM with the patch.
> >>
> > You are probably using a newer U-boot, which includes
> > commit dfaec76029f2 ("crypto/fsl: instantiate all rng state handles")
> >
> >> NXP Team,
> >>
> >> Do you have any opinions on this?
> >>
> > Since current U-boot initializes both RNG state handles, practically
> > instantiate_rng() is a no-op.
> >
> > A simple experiment is to "lie" about the state_handle_mask, to exercise
> > the DECO acquire code (or, as mentioned above, to run with an older U-boot):
> >
> > @@ -268,12 +272,19 @@ static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
> > struct caam_ctrl __iomem *ctrl;
> > u32 *desc, status = 0, rdsta_val;
> > int ret = 0, sh_idx;
> > + static int force_init = 1;
> >
> > ctrl = (struct caam_ctrl __iomem *)ctrlpriv->ctrl;
> > desc = kmalloc(CAAM_CMD_SZ * 7, GFP_KERNEL);
> > if (!desc)
> > return -ENOMEM;
> >
> > + if (force_init && (state_handle_mask == 0x3)) {
> > + dev_err(ctrldev, "Forcing reinit of RNG state handle 0!\n");
> > + force_init = 0;
> > + state_handle_mask = 0x2;
> > + }
> > +
> > for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
> > /*
> > * If the corresponding bit is set, this state handle
> >
> > In this case boot log confirms the DECO cannot be acquired:
> > [ 2.137101] caam 30900000.crypto: Forcing reinit of RNG state handle 0!
> > [ 2.172293] caam 30900000.crypto: failed to acquire DECO 0
> > [ 2.177786] caam 30900000.crypto: failed to instantiate RNG
> >
> > To sum up, writing to DECORSR is mandatory.
>
> Thanks Horia for providing the details.
I appreciate it too.
>
> Adam, can you update your patch to enable the code in
> run_descriptor_deco0() for i.MX8MM?
I will work on that. I have been trying to get the mainline U-Boot to
start correctly, because I wanted to see if/how this interacted. I'll
try to get a V2 pushed today.
>
> If I understand this correctly, this is necessary to have the RNG
> initialize correctly no matter what version of U-Boot is used.
That makes sense based on Horia's feedback. With the holidays this
month, my spare time and weekends have been full.
adam
>
> Thanks,
> Frieder