Re: [PATCH v2 2/4] mmc: Mediatek: enable crypto hardware engine

Eric Biggers
2021-03-15T23:02:58Z

On Mon, Mar 15, 2021 at 02:41:58PM +0100, Linus Walleij wrote:
> Hi Eric,
>
> thanks for stepping in and clarifying! I get it better now, I though
> this was some other encryption scheme "on the side".
>
> There is one worrying thing in the patch still:
>
> On Thu, Mar 11, 2021 at 8:08 PM Eric Biggers <ebiggers@kernel.org> wrote:
> > On Thu, Mar 11, 2021 at 02:48:23PM +0100, Linus Walleij wrote:
> > > On Tue, Mar 9, 2021 at 3:06 AM Peng Zhou <peng.zhou@mediatek.com> wrote:
>
> > > > + /*
> > > > + * 1: MSDC_AES_CTL_INIT
> > > > + * 4: cap_id, no-meaning now
> > > > + * 1: cfg_id, we choose the second cfg group
> > > > + */
> > > > + if (mmc->caps2 & MMC_CAP2_CRYPTO)
> > > > + arm_smccc_smc(MTK_SIP_MMC_CONTROL,
> > > > + 1, 4, 1, 0, 0, 0, 0, &smccc_res);
>
> So MSDC_AES_CTL_INIT. Assumes we are using AES and AES
> only I suppose?
>
> > It happens in the same place, cqhci-crypto.c. Mediatek's eMMC inline encryption
> > hardware follows the eMMC standard fairly closely, so Peng's patch series just
> > sets MMC_CAP2_CRYPTO to make it use the standard cqhci crypto code, and does a
> > couple extra things to actually enable the hardware's crypto support on Mediatek
> > platforms since it isn't enabled by default. (*Why* it requires an SMC call to
> > enable instead of just working as expected, I don't know though.)
>
> Now I don't know the limitations of cqhci crypto. Clearly it only supports
> AES today.
>
> However would the cqhci crypto grow support for any other crypto
> like 2Fish or DES and the user request this, then I suppose there is
> no way for the MTK driver to announce "uh no I don't do that"?
>
> Or will this cqhci hardware only ever support AES?

The standard specifies the encryption algorithms that may be supported, and it
specifies that host controllers have a set of crypto capability registers that
list the subset of those algorithms that the hardware actually supports. See
cqhci_crypto_init() which reads these registers.

If new algorithms get added, the hardware won't declare support for them.
So what you describe won't be a problem.

If, nevertheless, there is broken hardware that declares support for algorithms
it doesn't support, we could work around it using a method in cqhci_host_ops.
That isn't necessary now though.

- Eric