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

[Eric Biggers <ebiggers@kernel.org>]

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