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

[Linus Walleij <linus.walleij@linaro.org>]

Hi Peng,

thanks for your patch!

On Tue, Mar 9, 2021 at 3:06 AM Peng Zhou <peng.zhou@mediatek.com> wrote:

> Use SMC call enable hardware crypto engine
> due to it only be changed in ATF(EL3).
>
> Signed-off-by: Peng Zhou <peng.zhou@mediatek.com>

Unfortunately this commit message is way to short to
understand what is going on, and has a lot of assumed
previous knowledge.

Can you expand the commit message so that anyone
who just know MMC and some SoC basics can understand
what an SMC call and and what ATF(EL3) means?

I assume this some kind of inline encryption?

I think maybe linux-block mailing list need to be involved
because there is certain a Linux standard way of setting
up inline encryption for the block layer.

For example: how is the key to be used derived?
How is the device unlocked in the first place?

If I insert a LUKS encrypted harddrive in a Linux machine
the whole system is pretty much aware of how this should
be handled and everything "just works", I enter a pass
phrase and off it goes. I can use symmetric keys as well.
How is this stuff done for this hardware?

> +       /*
> +        * 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);

The same as above: these comments assume that everyone
already knows what is going on.

AES encryption requires a key and I don't see the driver
setting up any key. How is the code in this file:
drivers/mmc/core/crypto.c
interacting with your driver?
drivers/mmc/host/cqhci-crypto.c
is used by SDHCI and is quite readable and I see what is going on.
For example it contains functions like:
cqhci_crypto_program_key()
cqhci_crypto_keyslot_program()
cqhci_crypto_clear_keyslot()
cqhci_crypto_keyslot_evict()
cqhci_find_blk_crypto_mode()

MMC_CAP2_CRYPTO is used as a sign that the driver
can do inline encryption, then devm_blk_ksm_init() is called
to initialize a block encryption abstraction with the block layer.
Ops are registered using
struct blk_ksm_ll_ops cqhci_ksm_ops.

This is very straight forward.

But where does all the above happen for this driver?

I get the feeling that some magic is happening in outoftree
patches or in the secure world, and that is not how we do
these things, you have to use the frameworks.

Yours,
Linus Walleij