2022-05-09 12:46:44

by Pankaj Gupta

[permalink] [raw]
Subject: RE: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports blob encap/decap

Hi Ahmad,

Check for AES CHAs is done only for Era >= 10.

Please find the comments in-line.

Regards
Pankaj

> -----Original Message-----
> From: Ahmad Fatoum <[email protected]>
> Sent: Friday, May 6, 2022 11:56 AM
> To: Horia Geanta <[email protected]>; Pankaj Gupta
> <[email protected]>; Herbert Xu <[email protected]>; David
> S. Miller <[email protected]>
> Cc: [email protected]; Michael Walle <[email protected]>; Ahmad Fatoum
> <[email protected]>; James Bottomley <[email protected]>; Jarkko
> Sakkinen <[email protected]>; Mimi Zohar <[email protected]>; David
> Howells <[email protected]>; James Morris <[email protected]>; Eric
> Biggers <[email protected]>; Serge E. Hallyn <[email protected]>; Jan
> Luebbe <[email protected]>; David Gstir <[email protected]>; Richard
> Weinberger <[email protected]>; Franck Lenormand
> <[email protected]>; Matthias Schiffer <[email protected]
> group.com>; Sumit Garg <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-security-
> [email protected]
> Subject: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports
> blob encap/decap
>
> Caution: EXT Email
>
> Depending on SoC variant, a CAAM may be available, but with some futures
> fused out. The LS1028A (non-E) SoC is one such SoC and while it indicates BLOB
> support, BLOB operations will ultimately fail, because there is no AES support.
> Add a new blob_present member to reflect whether both BLOB support and the
> AES support it depends on is available.
>
> These will be used in a follow-up commit to allow blob driver initialization to
> error out on SoCs without the necessary hardware support instead of failing at
> runtime with a cryptic
>
> caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.
>
> Co-developed-by: Michael Walle <[email protected]>
> Signed-off-by: Michael Walle <[email protected]>
> Signed-off-by: Ahmad Fatoum <[email protected]>
>
> ---
> v8 -> v9:
> - New patch
>
> To: "Horia Geantă" <[email protected]>
> To: Pankaj Gupta <[email protected]>
> To: Herbert Xu <[email protected]>
> To: "David S. Miller" <[email protected]>
> Cc: James Bottomley <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Cc: Mimi Zohar <[email protected]>
> Cc: David Howells <[email protected]>
> Cc: James Morris <[email protected]>
> Cc: Eric Biggers <[email protected]>
> Cc: "Serge E. Hallyn" <[email protected]>
> Cc: Jan Luebbe <[email protected]>
> Cc: David Gstir <[email protected]>
> Cc: Richard Weinberger <[email protected]>
> Cc: Franck LENORMAND <[email protected]>
> Cc: Matthias Schiffer <[email protected]>
> Cc: Sumit Garg <[email protected]>
> Cc: Michael Walle <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/crypto/caam/ctrl.c | 10 ++++++++--
> drivers/crypto/caam/intern.h | 1 +
> drivers/crypto/caam/regs.h | 4 +++-
> 3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..6426ffec5980 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -820,12 +820,18 @@ static int caam_probe(struct platform_device *pdev)
> return -ENOMEM;
> }
>
> - if (ctrlpriv->era < 10)
> + comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
> + ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
> +
> + if (ctrlpriv->era < 10) {
> rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
> CHA_ID_LS_RNG_MASK) >> CHA_ID_LS_RNG_SHIFT;

Check for AES CHAs for Era < 10, should be added.

> - else
> + } else {
> rng_vid = (rd_reg32(&ctrl->vreg.rng) & CHA_VER_VID_MASK) >>
> CHA_VER_VID_SHIFT;
> + ctrlpriv->blob_present = ctrlpriv->blob_present &&
> + (rd_reg32(&ctrl->vreg.aesa) & CHA_VER_MISC_AES_NUM_MASK);
> + }
>
> /*
> * If SEC has RNG version >= 4 and RNG state handle has not been diff --git
> a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h index
> 7d45b21bd55a..e92210e2ab76 100644
> --- a/drivers/crypto/caam/intern.h
> +++ b/drivers/crypto/caam/intern.h
> @@ -92,6 +92,7 @@ struct caam_drv_private {
> */
> u8 total_jobrs; /* Total Job Rings in device */
> u8 qi_present; /* Nonzero if QI present in device */
> + u8 blob_present; /* Nonzero if BLOB support present in device */
> u8 mc_en; /* Nonzero if MC f/w is active */
> int secvio_irq; /* Security violation interrupt number */
> int virt_en; /* Virtualization enabled in CAAM */
> diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h index
> 3738625c0250..66d6dad841bb 100644
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -320,7 +320,8 @@ struct version_regs {
> #define CHA_VER_VID_MASK (0xffull << CHA_VER_VID_SHIFT)
>
> /* CHA Miscellaneous Information - AESA_MISC specific */
> -#define CHA_VER_MISC_AES_GCM BIT(1 + CHA_VER_MISC_SHIFT)
> +#define CHA_VER_MISC_AES_NUM_MASK GENMASK(7, 0)
> +#define CHA_VER_MISC_AES_GCM BIT(1 + CHA_VER_MISC_SHIFT)
>
> /* CHA Miscellaneous Information - PKHA_MISC specific */
> #define CHA_VER_MISC_PKHA_NO_CRYPT BIT(7 + CHA_VER_MISC_SHIFT)
> @@ -414,6 +415,7 @@ struct caam_perfmon {
> #define CTPR_MS_PG_SZ_MASK 0x10
> #define CTPR_MS_PG_SZ_SHIFT 4
> u32 comp_parms_ms; /* CTPR - Compile Parameters Register */
> +#define CTPR_LS_BLOB BIT(1)
> u32 comp_parms_ls; /* CTPR - Compile Parameters Register */
> u64 rsvd1[2];
>
> --
> 2.30.2


2022-05-09 13:05:10

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports blob encap/decap

Hello Pankaj,

On Mon, 2022-05-09 at 12:39 +0000, Pankaj Gupta wrote:
> > -       if (ctrlpriv->era < 10)
> > +       comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
> > +       ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
> > +
> > +       if (ctrlpriv->era < 10) {
> >                 rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
> >                            CHA_ID_LS_RNG_MASK) >>
> > CHA_ID_LS_RNG_SHIFT;
>
> Check for AES CHAs for Era < 10, should be added.

Do I need this? I only do this check for Era >= 10, because apparently
there are Layerscape non-E processors that indicate BLOB support via
CTPR_LS_BLOB, but fail at runtime. Are there any Era < 10 SoCs
that are similarly broken?

Cheers,
Ahmad


2022-05-11 10:08:32

by Pankaj Gupta

[permalink] [raw]
Subject: RE: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports blob encap/decap

Hi Ahmad,

Comments in-line.

Regards
Pankaj

> -----Original Message-----
> From: Ahmad Fatoum <[email protected]>
> Sent: Monday, May 9, 2022 6:34 PM
> To: Pankaj Gupta <[email protected]>; Horia Geanta
> <[email protected]>; Herbert Xu <[email protected]>; David S.
> Miller <[email protected]>
> Cc: [email protected]; Michael Walle <[email protected]>; James
> Bottomley <[email protected]>; Jarkko Sakkinen <[email protected]>; Mimi
> Zohar <[email protected]>; David Howells <[email protected]>; James
> Morris <[email protected]>; Eric Biggers <[email protected]>; Serge E.
> Hallyn <[email protected]>; Jan Luebbe <[email protected]>; David Gstir
> <[email protected]>; Richard Weinberger <[email protected]>; Franck
> Lenormand <[email protected]>; Matthias Schiffer
> <[email protected]>; Sumit Garg <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-security-
> [email protected]
> Subject: Re: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM
> supports blob encap/decap
>
> Caution: EXT Email
>
> Hello Pankaj,
>
> On Mon, 2022-05-09 at 12:39 +0000, Pankaj Gupta wrote:
> > > - if (ctrlpriv->era < 10)
> > > + comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
> > > + ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
> > > +
> > > + if (ctrlpriv->era < 10) {
> > > rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
> > > CHA_ID_LS_RNG_MASK) >>
> > > CHA_ID_LS_RNG_SHIFT;
> >
> > Check for AES CHAs for Era < 10, should be added.
>
> Do I need this? I only do this check for Era >= 10, because apparently there are
> Layerscape non-E processors that indicate BLOB support via CTPR_LS_BLOB, but
> fail at runtime. Are there any Era < 10 SoCs that are similarly broken?
>

For non-E variants, it might happen that Blob protocol is enabled, but number of AES CHA are zero.
If the output of below expression is > 0, then only blob_present should be marked present or true.
For era > 10, you handled. But for era < 10, please add the below code.

(rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
CHA_ID_LS_AES_MASK) >> CHA_ID_LS_AES_SHIFT;

> Cheers,
> Ahmad

2022-05-11 10:10:08

by Michael Walle

[permalink] [raw]
Subject: Re: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports blob encap/decap

Hi,

Am 2022-05-11 11:16, schrieb Pankaj Gupta:
>> -----Original Message-----
>> From: Ahmad Fatoum <[email protected]>
>> Sent: Monday, May 9, 2022 6:34 PM
>> To: Pankaj Gupta <[email protected]>; Horia Geanta
>> <[email protected]>; Herbert Xu <[email protected]>;
>> David S.
>> Miller <[email protected]>
>> Cc: [email protected]; Michael Walle <[email protected]>; James
>> Bottomley <[email protected]>; Jarkko Sakkinen <[email protected]>;
>> Mimi
>> Zohar <[email protected]>; David Howells <[email protected]>;
>> James
>> Morris <[email protected]>; Eric Biggers <[email protected]>; Serge
>> E.
>> Hallyn <[email protected]>; Jan Luebbe <[email protected]>; David
>> Gstir
>> <[email protected]>; Richard Weinberger <[email protected]>; Franck
>> Lenormand <[email protected]>; Matthias Schiffer
>> <[email protected]>; Sumit Garg
>> <[email protected]>;
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]; linux-security-
>> [email protected]
>> Subject: Re: [EXT] [PATCH v9 3/7] crypto: caam - determine whether
>> CAAM
>> supports blob encap/decap
>>
>> Caution: EXT Email
>>
>> Hello Pankaj,
>>
>> On Mon, 2022-05-09 at 12:39 +0000, Pankaj Gupta wrote:
>> > > - if (ctrlpriv->era < 10)
>> > > + comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
>> > > + ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
>> > > +
>> > > + if (ctrlpriv->era < 10) {
>> > > rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
>> > > CHA_ID_LS_RNG_MASK) >>
>> > > CHA_ID_LS_RNG_SHIFT;
>> >
>> > Check for AES CHAs for Era < 10, should be added.
>>
>> Do I need this? I only do this check for Era >= 10, because apparently
>> there are
>> Layerscape non-E processors that indicate BLOB support via
>> CTPR_LS_BLOB, but
>> fail at runtime. Are there any Era < 10 SoCs that are similarly
>> broken?
>>
>
> For non-E variants, it might happen that Blob protocol is enabled, but
> number of AES CHA are zero.
> If the output of below expression is > 0, then only blob_present
> should be marked present or true.
> For era > 10, you handled. But for era < 10, please add the below code.

Are there any CAAMs which can be just enabled partially for era < 10?
I didn't found anything. To me it looks like the non-export controlled
CAAM is only available for era >= 10. For era < 10, the CAAM is either
fully featured there or it is not available at all and thus the node
is removed in the bootloader (at least that is the case for layerscape).

-michael

2022-05-11 10:10:40

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [EXT] [PATCH v9 3/7] crypto: caam - determine whether CAAM supports blob encap/decap

Hello Pankaj,

On 11.05.22 11:16, Pankaj Gupta wrote:
>> -----Original Message-----
>> From: Ahmad Fatoum <[email protected]>
>>>> + if (ctrlpriv->era < 10) {
>>>> rng_vid = (rd_reg32(&ctrl->perfmon.cha_id_ls) &
>>>> CHA_ID_LS_RNG_MASK) >>
>>>> CHA_ID_LS_RNG_SHIFT;
>>>
>>> Check for AES CHAs for Era < 10, should be added.
>>
>> Do I need this? I only do this check for Era >= 10, because apparently there are
>> Layerscape non-E processors that indicate BLOB support via CTPR_LS_BLOB, but
>> fail at runtime. Are there any Era < 10 SoCs that are similarly broken?
>>
>
> For non-E variants, it might happen that Blob protocol is enabled, but number of AES CHA are zero.

Do you know any SoC where this is the case?
(i.e. era < 10 && CTPR_LS_BLOB && AES_CHA == 0)

> If the output of below expression is > 0, then only blob_present should be marked present or true.
> For era > 10, you handled. But for era < 10, please add the below code.
>
> (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
> CHA_ID_LS_AES_MASK) >> CHA_ID_LS_AES_SHIFT;

Sorry, I am not fond of adding quirk handling for Hardware that might
not even exist.

Cheers,
Ahmad

>
>> Cheers,
>> Ahmad
>


--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |