LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Cc: "Linux Crypto Mailing List" <linux-crypto@vger.kernel.org>,
	"Ash Logan" <ash@heyquark.com>,
	"Jonathan Neuschäfer" <j.ne@posteo.net>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
	"Paul Mackerras" <paulus@samba.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)"
	<linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH 1/4] crypto: nintendo-aes - add a new AES driver
Date: Wed, 22 Sep 2021 12:10:41 +0200	[thread overview]
Message-ID: <CAMj1kXF6RpaAsN2zUgkO0NW7gMwwhXMHEEM-wpQXxeNJbGJ79A@mail.gmail.com> (raw)
In-Reply-To: <20210921213930.10366-2-linkmauve@linkmauve.fr>

On Tue, 21 Sept 2021 at 23:49, Emmanuel Gil Peyrot
<linkmauve@linkmauve.fr> wrote:
>
> This engine implements AES in CBC mode, using 128-bit keys only.  It is
> present on both the Wii and the Wii U, and is apparently identical in
> both consoles.
>
> The hardware is capable of firing an interrupt when the operation is
> done, but this driver currently uses a busy loop, I’m not too sure
> whether it would be preferable to switch, nor how to achieve that.
>
> It also supports a mode where no operation is done, and thus could be
> used as a DMA copy engine, but I don’t know how to expose that to the
> kernel or whether it would even be useful.
>
> In my testing, on a Wii U, this driver reaches 80.7 MiB/s, while the
> aes-generic driver only reaches 30.9 MiB/s, so it is a quite welcome
> speedup.
>
> This driver was written based on reversed documentation, see:
> https://wiibrew.org/wiki/Hardware/AES
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> Tested-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>  # on Wii U

This is redundant - everybody should test the code they submit.

...
> +       /* TODO: figure out how to use interrupts here, this will probably
> +        * lower throughput but let the CPU do other things while the AES
> +        * engine is doing its work. */

So is it worthwhile like this? How much faster is it to use this
accelerator rather than the CPU?

> +       do {
> +               status = ioread32be(base + AES_CTRL);
> +               cpu_relax();
> +       } while ((status & AES_CTRL_EXEC) && --counter);
> +
> +       /* Do we ever get called with dst ≠ src?  If so we have to invalidate
> +        * dst in addition to the earlier flush of src. */
> +       if (unlikely(dst != src)) {
> +               for (i = 0; i < len; i += 32)
> +                       __asm__("dcbi 0, %0" : : "r" (dst + i));
> +               __asm__("sync" : : : "memory");
> +       }
> +
> +       return counter ? 0 : 1;
> +}
> +
> +static void
> +nintendo_aes_crypt(const void *src, void *dst, u32 len, u8 *iv, int dir,
> +                  bool firstchunk)
> +{
> +       u32 flags = 0;
> +       unsigned long iflags;
> +       int ret;
> +
> +       flags |= AES_CTRL_EXEC_INIT /* | AES_CTRL_IRQ */ | AES_CTRL_ENA;
> +
> +       if (dir == AES_DIR_DECRYPT)
> +               flags |= AES_CTRL_DEC;
> +
> +       if (!firstchunk)
> +               flags |= AES_CTRL_IV;
> +
> +       /* Start the critical section */
> +       spin_lock_irqsave(&lock, iflags);
> +
> +       if (firstchunk)
> +               writefield(AES_IV, iv);
> +
> +       ret = do_crypt(src, dst, len, flags);
> +       BUG_ON(ret);
> +
> +       spin_unlock_irqrestore(&lock, iflags);
> +}
> +
> +static int nintendo_setkey_skcipher(struct crypto_skcipher *tfm, const u8 *key,
> +                                   unsigned int len)
> +{
> +       /* The hardware only supports AES-128 */
> +       if (len != AES_KEYSIZE_128)
> +               return -EINVAL;
> +
> +       writefield(AES_KEY, key);
> +       return 0;
> +}
> +
> +static int nintendo_skcipher_crypt(struct skcipher_request *req, int dir)
> +{
> +       struct skcipher_walk walk;
> +       unsigned int nbytes;
> +       int err;
> +       char ivbuf[AES_BLOCK_SIZE];
> +       unsigned int ivsize;
> +
> +       bool firstchunk = true;
> +
> +       /* Reset the engine */
> +       iowrite32be(0, base + AES_CTRL);
> +
> +       err = skcipher_walk_virt(&walk, req, false);
> +       ivsize = min(sizeof(ivbuf), walk.ivsize);
> +
> +       while ((nbytes = walk.nbytes) != 0) {
> +               unsigned int chunkbytes = round_down(nbytes, AES_BLOCK_SIZE);
> +               unsigned int ret = nbytes % AES_BLOCK_SIZE;
> +
> +               if (walk.total == chunkbytes && dir == AES_DIR_DECRYPT) {
> +                       /* If this is the last chunk and we're decrypting, take
> +                        * note of the IV (which is the last ciphertext block)
> +                        */
> +                       memcpy(ivbuf, walk.src.virt.addr + walk.total - ivsize,
> +                              ivsize);
> +               }
> +
> +               nintendo_aes_crypt(walk.src.virt.addr, walk.dst.virt.addr,
> +                                  chunkbytes, walk.iv, dir, firstchunk);
> +
> +               if (walk.total == chunkbytes && dir == AES_DIR_ENCRYPT) {
> +                       /* If this is the last chunk and we're encrypting, take
> +                        * note of the IV (which is the last ciphertext block)
> +                        */
> +                       memcpy(walk.iv,
> +                              walk.dst.virt.addr + walk.total - ivsize,
> +                              ivsize);
> +               } else if (walk.total == chunkbytes && dir == AES_DIR_DECRYPT) {
> +                       memcpy(walk.iv, ivbuf, ivsize);
> +               }
> +
> +               err = skcipher_walk_done(&walk, ret);
> +               firstchunk = false;
> +       }
> +
> +       return err;
> +}
> +
> +static int nintendo_cbc_encrypt(struct skcipher_request *req)
> +{
> +       return nintendo_skcipher_crypt(req, AES_DIR_ENCRYPT);
> +}
> +
> +static int nintendo_cbc_decrypt(struct skcipher_request *req)
> +{
> +       return nintendo_skcipher_crypt(req, AES_DIR_DECRYPT);
> +}
> +
> +static struct skcipher_alg nintendo_alg = {
> +       .base.cra_name          = "cbc(aes)",
> +       .base.cra_driver_name   = "cbc-aes-nintendo",
> +       .base.cra_priority      = 400,
> +       .base.cra_flags         = CRYPTO_ALG_KERN_DRIVER_ONLY,
> +       .base.cra_blocksize     = AES_BLOCK_SIZE,
> +       .base.cra_alignmask     = 15,
> +       .base.cra_module        = THIS_MODULE,
> +       .setkey                 = nintendo_setkey_skcipher,
> +       .encrypt                = nintendo_cbc_encrypt,
> +       .decrypt                = nintendo_cbc_decrypt,
> +       .min_keysize            = AES_KEYSIZE_128,
> +       .max_keysize            = AES_KEYSIZE_128,
> +       .ivsize                 = AES_BLOCK_SIZE,
> +};
> +
> +static int nintendo_aes_remove(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +
> +       crypto_unregister_skcipher(&nintendo_alg);
> +       devm_iounmap(dev, base);
> +       base = NULL;
> +
> +       return 0;
> +}
> +
> +static int nintendo_aes_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct resource *res;
> +       int ret;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(base))
> +               return PTR_ERR(base);
> +
> +       spin_lock_init(&lock);
> +
> +       ret = crypto_register_skcipher(&nintendo_alg);
> +       if (ret)
> +               goto eiomap;
> +
> +       dev_notice(dev, "Nintendo Wii and Wii U AES engine enabled\n");
> +       return 0;
> +
> + eiomap:
> +       devm_iounmap(dev, base);
> +
> +       dev_err(dev, "Nintendo Wii and Wii U AES initialization failed\n");
> +       return ret;
> +}
> +
> +static const struct of_device_id nintendo_aes_of_match[] = {
> +       { .compatible = "nintendo,hollywood-aes", },
> +       { .compatible = "nintendo,latte-aes", },
> +       {/* sentinel */},
> +};
> +MODULE_DEVICE_TABLE(of, nintendo_aes_of_match);
> +
> +static struct platform_driver nintendo_aes_driver = {
> +       .driver = {
> +               .name = "nintendo-aes",
> +               .of_match_table = nintendo_aes_of_match,
> +       },
> +       .probe = nintendo_aes_probe,
> +       .remove = nintendo_aes_remove,
> +};
> +
> +module_platform_driver(nintendo_aes_driver);
> +
> +MODULE_AUTHOR("Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>");
> +MODULE_DESCRIPTION("Nintendo Wii and Wii U Hardware AES driver");
> +MODULE_LICENSE("GPL");
> --
> 2.33.0
>

  parent reply	other threads:[~2021-09-22 10:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 21:39 [PATCH 0/4] crypto: nintendo-aes - add a new AES driver Emmanuel Gil Peyrot
2021-09-21 21:39 ` [PATCH 1/4] " Emmanuel Gil Peyrot
2021-09-22  2:02   ` Joel Stanley
2021-09-28  9:00     ` Geert Uytterhoeven
2021-09-22  6:04   ` Corentin Labbe
2021-09-22 10:10   ` Ard Biesheuvel [this message]
2021-09-22 10:43     ` Emmanuel Gil Peyrot
2021-09-22 10:55       ` Ard Biesheuvel
2021-09-21 21:39 ` [PATCH 2/4] dt-bindings: nintendo-aes: Document the Wii and Wii U AES support Emmanuel Gil Peyrot
2021-09-27 18:01   ` Rob Herring
2021-09-21 21:39 ` [PATCH 3/4] powerpc: wii.dts: Expose the AES engine on this platform Emmanuel Gil Peyrot
2021-09-21 21:39 ` [PATCH 4/4] powerpc: wii_defconfig: Enable AES by default Emmanuel Gil Peyrot
2021-09-21 21:59 ` [PATCH 0/4] crypto: nintendo-aes - add a new AES driver Eric Biggers
2021-09-21 22:37   ` Emmanuel Gil Peyrot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMj1kXF6RpaAsN2zUgkO0NW7gMwwhXMHEEM-wpQXxeNJbGJ79A@mail.gmail.com \
    --to=ardb@kernel.org \
    --cc=ash@heyquark.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=j.ne@posteo.net \
    --cc=linkmauve@linkmauve.fr \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).