From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 65AF6CEDD8F for ; Wed, 9 Oct 2024 12:49:54 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1syW84-0005ZL-HA; Wed, 09 Oct 2024 08:49:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1syW7x-0005Xo-S9 for qemu-devel@nongnu.org; Wed, 09 Oct 2024 08:49:18 -0400 Received: from mail-vk1-xa2e.google.com ([2607:f8b0:4864:20::a2e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1syW7t-0006er-G4 for qemu-devel@nongnu.org; Wed, 09 Oct 2024 08:49:17 -0400 Received: by mail-vk1-xa2e.google.com with SMTP id 71dfb90a1353d-50ac27e3530so487720e0c.1 for ; Wed, 09 Oct 2024 05:49:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philjordan-eu.20230601.gappssmtp.com; s=20230601; t=1728478151; x=1729082951; darn=nongnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=oPm1he9/FDOXRLEfi7epLWeNRiGUJWX/4NhDJcBQcFE=; b=XXN7oXXW413q6tvRJz+Gz+6G7JPaWP1WmFvWQdur2pCvBWtoCrF6MuZ8DFLusvMqxQ zlUyEBH/V+cC+HXaoZTujsKiWjHG0IumuIUiCwmoIWDdMzTkeJchMqBKmIYxPIRP3XQq CM9cOCarY1zLd3ryBvFzaClMxatrlxmxy6G3e7rYmrhp+lLczKwIHI3TuPjHM/4BD73O QNgaYdqaUWjmpixbkUOmRD5UMMRGFNYBsXeGauAUY2yw9t3eK/ngbyRfYYWgglQPn575 hxeJ6kTZlL7LQWs3r4TE4jPq3xDYek30v/wDaYsuGRPpovWZGtk2AkLXMOL+84ofr//r bJyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728478151; x=1729082951; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=oPm1he9/FDOXRLEfi7epLWeNRiGUJWX/4NhDJcBQcFE=; b=psZIHUA6npGjSYoJPrCMJroMIJ2LFGR1O/EZfkxo+vNhg+C0y5mDkVVp0Wn9gU0hWR KOS+OjBBkg/RE8OKQSYW1l79Uhd+uxlQ6AXFmoI8LxvzluY8RvsyP1aBOC5wNDSwMzDE E6QblJ47ldGWcefd8G7APJOyw6A8T9FaGnFiJGRZsIfhpZ0PVLMYUcZo5Tx5qgYFfjK+ +Ro5Sbuj92bNLx8O5W0+jx1eNU2NOiUpV8J3W8zie3VFmWwZG1zM3rt75y4uX+q+Ai0+ sIX8UWiQAqI7jauZC+YSc3mDRGtiQo2WamhIfLWxFc2AjR292IsJoDO0rOYmb1iDwPSU 2HMw== X-Gm-Message-State: AOJu0YyAFlQpY4fRZXXmxkjT1jloJ6VmoaRGzzsDC1h2S17Mhht2qpAP +oyHfVjy7xn5tv7aK6BIDLg9g90QSax+gwV2dDIURQKfS7JX0KweIr6xZHbYCluFAw4aEI5U3Kb vZrwzcP45mX1rSGBrAcH5MPRA09t9Hwy1KtcM X-Google-Smtp-Source: AGHT+IEFfE/vFjby47wgj7j6Hc8/kqJMVdzHVUV77g/PAGUayZPypBy9S0GRTosJhLjxThdIUkk3aNvWrezBYwwAwrs= X-Received: by 2002:a05:6122:1daa:b0:50c:d988:a567 with SMTP id 71dfb90a1353d-50cd988ac5amr1427574e0c.4.1728478151002; Wed, 09 Oct 2024 05:49:11 -0700 (PDT) MIME-Version: 1.0 References: <20240928085727.56883-1-phil@philjordan.eu> <20240928085727.56883-11-phil@philjordan.eu> <6f0e5a6a-ea1b-48ae-b3d5-6a68cc0366d2@daynix.com> In-Reply-To: <6f0e5a6a-ea1b-48ae-b3d5-6a68cc0366d2@daynix.com> From: Phil Dennis-Jordan Date: Wed, 9 Oct 2024 14:48:59 +0200 Message-ID: Subject: Re: [PATCH v3 10/14] hw/vmapple/aes: Introduce aes engine To: Akihiko Odaki Cc: qemu-devel@nongnu.org, agraf@csgraf.de, peter.maydell@linaro.org, pbonzini@redhat.com, rad@semihalf.com, quic_llindhol@quicinc.com, marcin.juszkiewicz@linaro.org, stefanha@redhat.com, mst@redhat.com, slp@redhat.com, richard.henderson@linaro.org, eduardo@habkost.net, marcel.apfelbaum@gmail.com, gaosong@loongson.cn, jiaxun.yang@flygoat.com, chenhuacai@kernel.org, kwolf@redhat.com, hreitz@redhat.com, philmd@linaro.org, shorne@gmail.com, palmer@dabbelt.com, alistair.francis@wdc.com, bmeng.cn@gmail.com, liwei1518@gmail.com, dbarboza@ventanamicro.com, zhiwei_liu@linux.alibaba.com, jcmvbkbc@gmail.com, marcandre.lureau@redhat.com, berrange@redhat.com, qemu-arm@nongnu.org, qemu-block@nongnu.org, qemu-riscv@nongnu.org, Alexander Graf Content-Type: multipart/alternative; boundary="000000000000316da506240aae7a" Received-SPF: neutral client-ip=2607:f8b0:4864:20::a2e; envelope-from=phil@philjordan.eu; helo=mail-vk1-xa2e.google.com X-Spam_score_int: -10 X-Spam_score: -1.1 X-Spam_bar: - X-Spam_report: (-1.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NEUTRAL=0.779 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --000000000000316da506240aae7a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 4 Oct 2024 at 07:32, Akihiko Odaki wrote= : > On 2024/09/28 17:57, Phil Dennis-Jordan wrote: > > From: Alexander Graf > > > > VMApple contains an "aes" engine device that it uses to encrypt and > > decrypt its nvram. It has trivial hard coded keys it uses for that > > purpose. > > > > Add device emulation for this device model. > > > > Signed-off-by: Alexander Graf > > Co-authored-by: Phil Dennis-Jordan > > Signed-off-by: Phil Dennis-Jordan > > > > --- > > v3: > > > > * Rebased on latest upstream and fixed minor breakages. > > * Replaced legacy device reset method with Resettable method > > > > hw/vmapple/Kconfig | 2 + > > hw/vmapple/aes.c | 584 +++++++++++++++++++++++++++++++++++++++= + > > hw/vmapple/meson.build | 1 + > > hw/vmapple/trace-events | 19 ++ > > 4 files changed, 606 insertions(+) > > create mode 100644 hw/vmapple/aes.c > > > > diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig > > index 8b137891791..a73504d5999 100644 > > --- a/hw/vmapple/Kconfig > > +++ b/hw/vmapple/Kconfig > > @@ -1 +1,3 @@ > > +config VMAPPLE_AES > > + bool > > > > diff --git a/hw/vmapple/aes.c b/hw/vmapple/aes.c > > new file mode 100644 > > index 00000000000..074fbdd9c36 > > --- /dev/null > > +++ b/hw/vmapple/aes.c > > @@ -0,0 +1,584 @@ > > +/* > > + * QEMU Apple AES device emulation > > + * > > + * Copyright =C2=A9 2023 Amazon.com, Inc. or its affiliates. All Right= s > Reserved. > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > > + * See the COPYING file in the top-level directory. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "hw/irq.h" > > +#include "migration/vmstate.h" > > +#include "qemu/log.h" > > +#include "qemu/module.h" > > +#include "trace.h" > > +#include "hw/sysbus.h" > > +#include "crypto/hash.h" > > +#include "crypto/aes.h" > > +#include "crypto/cipher.h" > > + > > +#define TYPE_AES "apple-aes" > > +#define MAX_FIFO_SIZE 9 > > + > > +#define CMD_KEY 0x1 > > +#define CMD_KEY_CONTEXT_SHIFT 27 > > +#define CMD_KEY_CONTEXT_MASK (0x1 << CMD_KEY_CONTEXT_SHIFT) > > +#define CMD_KEY_SELECT_SHIFT 24 > > +#define CMD_KEY_SELECT_MASK (0x7 << CMD_KEY_SELECT_SHIFT) > > +#define CMD_KEY_KEY_LEN_SHIFT 22 > > +#define CMD_KEY_KEY_LEN_MASK (0x3 << CMD_KEY_KEY_LEN_SHIFT) > > +#define CMD_KEY_ENCRYPT_SHIFT 20 > > +#define CMD_KEY_ENCRYPT_MASK (0x1 << CMD_KEY_ENCRYPT_SHIFT) > > +#define CMD_KEY_BLOCK_MODE_SHIFT 16 > > +#define CMD_KEY_BLOCK_MODE_MASK (0x3 << CMD_KEY_BLOCK_MODE_SHIFT) > > +#define CMD_IV 0x2 > > +#define CMD_IV_CONTEXT_SHIFT 26 > > +#define CMD_IV_CONTEXT_MASK (0x3 << CMD_KEY_CONTEXT_SHIFT) > > +#define CMD_DSB 0x3 > > +#define CMD_SKG 0x4 > > +#define CMD_DATA 0x5 > > +#define CMD_DATA_KEY_CTX_SHIFT 27 > > +#define CMD_DATA_KEY_CTX_MASK (0x1 << CMD_DATA_KEY_CTX_SHIFT) > > +#define CMD_DATA_IV_CTX_SHIFT 25 > > +#define CMD_DATA_IV_CTX_MASK (0x3 << CMD_DATA_IV_CTX_SHIFT) > > +#define CMD_DATA_LEN_MASK 0xffffff > > +#define CMD_STORE_IV 0x6 > > +#define CMD_STORE_IV_ADDR_MASK 0xffffff > > +#define CMD_WRITE_REG 0x7 > > +#define CMD_FLAG 0x8 > > +#define CMD_FLAG_STOP_MASK BIT(26) > > +#define CMD_FLAG_RAISE_IRQ_MASK BIT(27) > > +#define CMD_FLAG_INFO_MASK 0xff > > +#define CMD_MAX 0x10 > > + > > +#define CMD_SHIFT 28 > > + > > +#define REG_STATUS 0xc > > +#define REG_STATUS_DMA_READ_RUNNING BIT(0) > > +#define REG_STATUS_DMA_READ_PENDING BIT(1) > > +#define REG_STATUS_DMA_WRITE_RUNNING BIT(2) > > +#define REG_STATUS_DMA_WRITE_PENDING BIT(3) > > +#define REG_STATUS_BUSY BIT(4) > > +#define REG_STATUS_EXECUTING BIT(5) > > +#define REG_STATUS_READY BIT(6) > > +#define REG_STATUS_TEXT_DPA_SEEDED BIT(7) > > +#define REG_STATUS_UNWRAP_DPA_SEEDED BIT(8) > > + > > +#define REG_IRQ_STATUS 0x18 > > +#define REG_IRQ_STATUS_INVALID_CMD BIT(2) > > +#define REG_IRQ_STATUS_FLAG BIT(5) > > +#define REG_IRQ_ENABLE 0x1c > > +#define REG_WATERMARK 0x20 > > +#define REG_Q_STATUS 0x24 > > +#define REG_FLAG_INFO 0x30 > > +#define REG_FIFO 0x200 > > + > > +static const uint32_t key_lens[4] =3D { > > I suggest deriving the size from CMD_KEY_KEY_LEN_MASK. > > > + [0] =3D 16, > > + [1] =3D 24, > > + [2] =3D 32, > > + [3] =3D 64, > > +}; > > + > > +struct key { > > Please add typedef. > > > + uint32_t key_len; > > + uint32_t key[8]; > > +}; > > + > > +struct iv { > > + uint32_t iv[4]; > > +}; > > + > > +struct context { > > + struct key key; > > + struct iv iv; > > +}; > > + > > +static struct key builtin_keys[7] =3D { > > + [1] =3D {> + .key_len =3D 32, > > + .key =3D { 0x1 }, > > + }, > > + [2] =3D { > > + .key_len =3D 32, > > + .key =3D { 0x2 }, > > + }, > > + [3] =3D { > > + .key_len =3D 32, > > + .key =3D { 0x3 }, > > + } > > +}; > > + > > +typedef struct AESState { > > + /* Private */ > > This private/public comments are unnecessary; this struct can be > referenced only from this file and all members are automatically private. > > > + SysBusDevice parent_obj; > > + > > + /* Public */ > > + qemu_irq irq; > > + MemoryRegion iomem1; > > + MemoryRegion iomem2; > > + > > + uint32_t status; > > + uint32_t q_status; > > + uint32_t irq_status; > > + uint32_t irq_enable; > > + uint32_t watermark; > > + uint32_t flag_info; > > + uint32_t fifo[MAX_FIFO_SIZE]; > > + uint32_t fifo_idx; > > + struct key key[2]; > > + struct iv iv[4]; > > + bool is_encrypt; > > + QCryptoCipherMode block_mode; > > +} AESState; > > + > > +OBJECT_DECLARE_SIMPLE_TYPE(AESState, AES) > > + > > +static void aes_update_irq(AESState *s) > > +{ > > + qemu_set_irq(s->irq, !!(s->irq_status & s->irq_enable)); > > +} > > + > > +static uint64_t aes1_read(void *opaque, hwaddr offset, unsigned size) > > +{ > > + AESState *s =3D opaque; > > + uint64_t res =3D 0; > > + > > + switch (offset) { > > + case REG_STATUS: > > + res =3D s->status; > > + break; > > + case REG_IRQ_STATUS: > > + res =3D s->irq_status; > > + break; > > + case REG_IRQ_ENABLE: > > + res =3D s->irq_enable; > > + break; > > + case REG_WATERMARK: > > + res =3D s->watermark; > > + break; > > + case REG_Q_STATUS: > > + res =3D s->q_status; > > + break; > > + case REG_FLAG_INFO: > > + res =3D s->flag_info; > > + break; > > + > > + default: > > + trace_aes_read_unknown(offset); > > Use: LOG_UNIMP > > > + break; > > + } > > + > > + trace_aes_read(offset, res); > > + > > + return res; > > +} > > + > > +static void fifo_append(AESState *s, uint64_t val) > > +{ > > + if (s->fifo_idx =3D=3D MAX_FIFO_SIZE) { > > + /* Exceeded the FIFO. Bail out */ > > + return; > > + } > > + > > + s->fifo[s->fifo_idx++] =3D val; > > +} > > + > > +static bool has_payload(AESState *s, uint32_t elems) > > +{ > > + return s->fifo_idx >=3D (elems + 1); > > +} > > + > > +static bool cmd_key(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0]; > > + uint32_t key_select =3D (cmd & CMD_KEY_SELECT_MASK) >> > CMD_KEY_SELECT_SHIFT; > > + uint32_t ctxt =3D (cmd & CMD_KEY_CONTEXT_MASK) >> > CMD_KEY_CONTEXT_SHIFT; > > + uint32_t key_len; > > + > > + switch ((cmd & CMD_KEY_BLOCK_MODE_MASK) >> > CMD_KEY_BLOCK_MODE_SHIFT) { > > + case 0: > > + s->block_mode =3D QCRYPTO_CIPHER_MODE_ECB; > > + break; > > + case 1: > > + s->block_mode =3D QCRYPTO_CIPHER_MODE_CBC; > > + break; > > + default: > > + return false; > > + } > > + > > + s->is_encrypt =3D !!((cmd & CMD_KEY_ENCRYPT_MASK) >> > CMD_KEY_ENCRYPT_SHIFT); > > CMD_KEY_ENCRYPT_SHIFT is unnecessary as in case of > CMD_FLAG_RAISE_IRQ_MASK. "!!" is also unnecessary; it will be implicitly > casted into bool. > > > + key_len =3D key_lens[((cmd & CMD_KEY_KEY_LEN_MASK) >> > CMD_KEY_KEY_LEN_SHIFT)]; > > + > > + if (key_select) { > > + trace_aes_cmd_key_select_builtin(ctxt, key_select, > > + s->is_encrypt ? "en" : "de", > > + > QCryptoCipherMode_str(s->block_mode)); > > + s->key[ctxt] =3D builtin_keys[key_select]; > > I guess this should be: builtin_keys[key_select - 1] > I don't think it should: the builtin keys start at index 1. This seems fine, and I'd rather leave the indices alone to avoid needing to do arithmetic on them. However in that case, builtin_keys needs to have 8 elements not 7 as it currently does to avoid overflow. I'll fix this and use symbolic constants. (Thanks for the other review comments, I've reworked those parts for v4.) > > + } else { > > + trace_aes_cmd_key_select_new(ctxt, key_len, > > + s->is_encrypt ? "en" : "de", > > + > QCryptoCipherMode_str(s->block_mode)); > > + if (key_len > sizeof(s->key[ctxt].key)) { > > + return false; > > + } > > + if (!has_payload(s, key_len / sizeof(uint32_t))) { > > + /* wait for payload */ > > + return false; > > + } > > + memcpy(&s->key[ctxt].key, &s->fifo[1], key_len); > > + s->key[ctxt].key_len =3D key_len; > > + } > > + > > + return true; > > +} > > + > > +static bool cmd_iv(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0]; > > + uint32_t ctxt =3D (cmd & CMD_IV_CONTEXT_MASK) >> CMD_IV_CONTEXT_SH= IFT; > > + > > + if (!has_payload(s, 4)) { > > + /* wait for payload */ > > + return false; > > + } > > + memcpy(&s->iv[ctxt].iv, &s->fifo[1], sizeof(s->iv[ctxt].iv)); > > + trace_aes_cmd_iv(ctxt, s->fifo[1], s->fifo[2], s->fifo[3], > s->fifo[4]); > > + > > + return true; > > +} > > + > > +static char hexdigit2str(uint8_t val) > > +{ > > + g_assert(val < 0x10); > > + if (val >=3D 0xa) { > > + return 'a' + (val - 0xa); > > + } else { > > + return '0' + val; > > + } > > +} > > + > > +static void dump_data(const char *desc, const void *p, size_t len) > > +{ > > + char *hex =3D alloca((len * 2) + 1); > > Don't use alloca(); use a fixed-length array instead. > > > + const uint8_t *data =3D p; > > + char *hexp =3D hex; > > + size_t i; > > + > > + if (len > 0x1000) { > > + /* Too large buffer, let's bail out */ > > + return; > > + } > > + > > + for (i =3D 0; i < len; i++) { > > + uint8_t val =3D data[i]; > > + *(hexp++) =3D hexdigit2str(val >> 4); > > + *(hexp++) =3D hexdigit2str(val & 0xf); > > + } > > + *hexp =3D '\0'; > > Let's move this to: util/hexdump.c > > > + > > + trace_aes_dump_data(desc, hex); > > +} > > + > > +static bool cmd_data(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0]; > > + uint32_t ctxt_iv =3D 0; > > + uint32_t ctxt_key =3D (cmd & CMD_DATA_KEY_CTX_MASK) >> > CMD_DATA_KEY_CTX_SHIFT; > > + uint32_t len =3D cmd & CMD_DATA_LEN_MASK; > > + uint64_t src_addr =3D s->fifo[2]; > > + uint64_t dst_addr =3D s->fifo[3]; > > + QCryptoCipherAlgo alg; > > + QCryptoCipher *cipher; > > + char *src; > > + char *dst; > > + > > + src_addr |=3D ((uint64_t)s->fifo[1] << 16) & 0xffff00000000ULL; > > + dst_addr |=3D ((uint64_t)s->fifo[1] << 32) & 0xffff00000000ULL; > > + > > + trace_aes_cmd_data(ctxt_key, ctxt_iv, src_addr, dst_addr, len); > > + > > + if (!has_payload(s, 3)) { > > + /* wait for payload */ > > + trace_aes_cmd_data_error("No payload"); > > Use: LOG_GUEST_ERROR > > > + return false; > > + } > > + > > + if (ctxt_key >=3D ARRAY_SIZE(s->key) || > > + ctxt_iv >=3D ARRAY_SIZE(s->iv)) { > > + /* Invalid input */ > > + trace_aes_cmd_data_error("Invalid key or iv"); > > + return false; > > + } > > + > > + src =3D g_malloc0(len); > > + dst =3D g_malloc0(len); > > + > > + cpu_physical_memory_read(src_addr, src, len); > > Use: dma_memory_read() > > > + > > + dump_data("cmd_data(): src_data=3D", src, len); > > + > > + switch (s->key[ctxt_key].key_len) { > > + case 128 / 8: > > + alg =3D QCRYPTO_CIPHER_ALGO_AES_128; > > + break; > > + case 192 / 8: > > + alg =3D QCRYPTO_CIPHER_ALGO_AES_192; > > + break; > > + case 256 / 8: > > + alg =3D QCRYPTO_CIPHER_ALGO_AES_256; > > + break; > > + default: > > + trace_aes_cmd_data_error("Invalid key len"); > > + goto err_free; > > Use g_autoptr instead of goto and g_free(). > > > + } > > + cipher =3D qcrypto_cipher_new(alg, s->block_mode, > > + (void *)s->key[ctxt_key].key, > > + s->key[ctxt_key].key_len, NULL); > > + g_assert(cipher !=3D NULL); > Handle this error as you do for qcrypto_cipher_setiv(), > qcrypto_cipher_encrypt() and qcrypto_cipher_decrypt(). > > > + if (s->block_mode !=3D QCRYPTO_CIPHER_MODE_ECB) { > > + if (qcrypto_cipher_setiv(cipher, (void *)s->iv[ctxt_iv].iv, > > + sizeof(s->iv[ctxt_iv].iv), NULL) !=3D= 0) > { > > + trace_aes_cmd_data_error("Failed to set IV"); > > + goto err_free_cipher; > > + } > > + } > > + if (s->is_encrypt) { > > + if (qcrypto_cipher_encrypt(cipher, src, dst, len, NULL) !=3D 0= ) { > > + trace_aes_cmd_data_error("Encrypt failed"); > > + goto err_free_cipher; > > + } > > + } else { > > + if (qcrypto_cipher_decrypt(cipher, src, dst, len, NULL) !=3D 0= ) { > > + trace_aes_cmd_data_error("Decrypt failed"); > > + goto err_free_cipher; > > + } > > + } > > + qcrypto_cipher_free(cipher); > > + > > + dump_data("cmd_data(): dst_data=3D", dst, len); > > + cpu_physical_memory_write(dst_addr, dst, len); > > + g_free(src); > > + g_free(dst); > > + > > + return true; > > + > > +err_free_cipher: > > + qcrypto_cipher_free(cipher); > > +err_free: > > + g_free(src); > > + g_free(dst); > > + return false; > > +} > > + > > +static bool cmd_store_iv(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0]; > > + uint32_t ctxt =3D (cmd & CMD_IV_CONTEXT_MASK) >> CMD_IV_CONTEXT_SH= IFT; > > + uint64_t addr =3D s->fifo[1]; > > + > > + if (!has_payload(s, 1)) { > > + /* wait for payload */ > > Add a log as it's done for cmd_data(). > > > + return false; > > + } > > + > > + if (ctxt >=3D ARRAY_SIZE(s->iv)) { > > + /* Invalid context selected */ > > + return false; > > + } > > + > > + addr |=3D ((uint64_t)cmd << 32) & 0xff00000000ULL; > > + cpu_physical_memory_write(addr, &s->iv[ctxt].iv, > sizeof(s->iv[ctxt].iv)); > > + > > + trace_aes_cmd_store_iv(ctxt, addr, s->iv[ctxt].iv[0], > s->iv[ctxt].iv[1], > > + s->iv[ctxt].iv[2], s->iv[ctxt].iv[3]); > > + > > + return true; > > +} > > + > > +static bool cmd_flag(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0]; > > + uint32_t raise_irq =3D cmd & CMD_FLAG_RAISE_IRQ_MASK; > > + > > + /* We always process data when it's coming in, so fire an IRQ > immediately */ > > + if (raise_irq) { > > + s->irq_status |=3D REG_IRQ_STATUS_FLAG; > > + } > > + > > + s->flag_info =3D cmd & CMD_FLAG_INFO_MASK; > > + > > + trace_aes_cmd_flag(!!raise_irq, s->flag_info); > > + > > + return true; > > +} > > + > > +static void fifo_process(AESState *s) > > +{ > > + uint32_t cmd =3D s->fifo[0] >> CMD_SHIFT; > > + bool success =3D false; > > + > > + if (!s->fifo_idx) { > > + return; > > + } > > + > > + switch (cmd) { > > + case CMD_KEY: > > + success =3D cmd_key(s); > > + break; > > + case CMD_IV: > > + success =3D cmd_iv(s); > > + break; > > + case CMD_DATA: > > + success =3D cmd_data(s); > > + break; > > + case CMD_STORE_IV: > > + success =3D cmd_store_iv(s); > > + break; > > + case CMD_FLAG: > > + success =3D cmd_flag(s); > > + break; > > + default: > > + s->irq_status |=3D REG_IRQ_STATUS_INVALID_CMD; > > + break; > > + } > > + > > + if (success) { > > + s->fifo_idx =3D 0; > > + } > > + > > + trace_aes_fifo_process(cmd, success ? 1 : 0); > > +} > > + > > +static void aes1_write(void *opaque, hwaddr offset, uint64_t val, > unsigned size) > > +{ > > + AESState *s =3D opaque; > > + > > + trace_aes_write(offset, val); > > + > > + switch (offset) { > > + case REG_IRQ_STATUS: > > + s->irq_status &=3D ~val; > > + break; > > + case REG_IRQ_ENABLE: > > + s->irq_enable =3D val; > > + break; > > + case REG_FIFO: > > + fifo_append(s, val); > > + fifo_process(s); > > + break; > > + default: > > + trace_aes_write_unknown(offset); > > + return; > > + } > > + > > + aes_update_irq(s); > > +} > > + > > +static const MemoryRegionOps aes1_ops =3D { > > + .read =3D aes1_read, > > + .write =3D aes1_write, > > + .endianness =3D DEVICE_NATIVE_ENDIAN, > > + .valid =3D { > > + .min_access_size =3D 4, > > + .max_access_size =3D 8, > > + }, > > + .impl =3D { > > + .min_access_size =3D 4, > > + .max_access_size =3D 4, > > + }, > > +}; > > + > > +static uint64_t aes2_read(void *opaque, hwaddr offset, unsigned size) > > +{ > > + uint64_t res =3D 0; > > + > > + switch (offset) { > > + case 0: > > + res =3D 0; > > + break; > > + default: > > + trace_aes_2_read_unknown(offset); > > + break; > > + } > > + > > + trace_aes_2_read(offset, res); > > + > > + return res; > > +} > > + > > +static void aes2_write(void *opaque, hwaddr offset, uint64_t val, > unsigned size) > > +{ > > + trace_aes_2_write(offset, val); > > + > > + switch (offset) { > > + default: > > + trace_aes_2_write_unknown(offset); > > + return; > > + } > > +} > > + > > +static const MemoryRegionOps aes2_ops =3D { > > + .read =3D aes2_read, > > + .write =3D aes2_write, > > + .endianness =3D DEVICE_NATIVE_ENDIAN, > > + .valid =3D { > > + .min_access_size =3D 4, > > + .max_access_size =3D 8, > > + }, > > + .impl =3D { > > + .min_access_size =3D 4, > > + .max_access_size =3D 4, > > + }, > > +}; > > + > > +static void aes_reset(Object *obj, ResetType type) > > +{ > > + AESState *s =3D AES(obj); > > + > > + s->status =3D 0x3f80; > > + s->q_status =3D 2; > > + s->irq_status =3D 0; > > + s->irq_enable =3D 0; > > + s->watermark =3D 0; > > +} > > + > > +static void aes_init(Object *obj) > > +{ > > + AESState *s =3D AES(obj); > > + > > + memory_region_init_io(&s->iomem1, obj, &aes1_ops, s, TYPE_AES, > 0x4000); > > + memory_region_init_io(&s->iomem2, obj, &aes2_ops, s, TYPE_AES, > 0x4000); > > + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem1); > > + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem2); > > + sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq); > > +} > > + > > +static void aes_realize(DeviceState *dev, Error **errp) > > +{ > > +} > > This shouldn't be necessary. > > > + > > +static void aes_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc =3D DEVICE_CLASS(klass); > > + ResettableClass *rc =3D RESETTABLE_CLASS(klass); > > + > > + rc->phases.hold =3D aes_reset; > > + dc->realize =3D aes_realize; > > +} > > + > > +static const TypeInfo aes_info =3D { > > + .name =3D TYPE_AES, > > + .parent =3D TYPE_SYS_BUS_DEVICE, > > + .instance_size =3D sizeof(AESState), > > + .class_init =3D aes_class_init, > > + .instance_init =3D aes_init, > > +}; > > + > > +static void aes_register_types(void) > > +{ > > + type_register_static(&aes_info); > > +} > > + > > +type_init(aes_register_types) > > diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build > > index e69de29bb2d..bcd4dcb28d2 100644 > > --- a/hw/vmapple/meson.build > > +++ b/hw/vmapple/meson.build > > @@ -0,0 +1 @@ > > +system_ss.add(when: 'CONFIG_VMAPPLE_AES', if_true: files('aes.c')) > > diff --git a/hw/vmapple/trace-events b/hw/vmapple/trace-events > > index 9ccc5790487..1c9a3326eb4 100644 > > --- a/hw/vmapple/trace-events > > +++ b/hw/vmapple/trace-events > > @@ -1,2 +1,21 @@ > > # See docs/devel/tracing.rst for syntax documentation. > > > > +# aes.c > > +aes_read_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > > +aes_read(uint64_t offset, uint64_t res) "offset=3D0x%"PRIx64" > res=3D0x%"PRIx64 > > +aes_cmd_key_select_builtin(uint32_t ctx, uint32_t key_id, const char > *direction, const char *cipher) "[%d] Selecting builtin key %d to %scrypt > with %s" > > +aes_cmd_key_select_new(uint32_t ctx, uint32_t key_len, const char > *direction, const char *cipher) "[%d] Selecting new key size=3D%d to %scr= ypt > with %s" > > +aes_cmd_iv(uint32_t ctx, uint32_t iv0, uint32_t iv1, uint32_t iv2, > uint32_t iv3) "[%d] 0x%08x 0x%08x 0x%08x 0x%08x" > > +aes_cmd_data(uint32_t key, uint32_t iv, uint64_t src, uint64_t dst, > uint32_t len) "[key=3D%d iv=3D%d] src=3D0x%"PRIx64" dst=3D0x%"PRIx64" len= =3D0x%x" > > +aes_cmd_data_error(const char *reason) "reason=3D%s" > > +aes_cmd_store_iv(uint32_t ctx, uint64_t addr, uint32_t iv0, uint32_t > iv1, uint32_t iv2, uint32_t iv3) "[%d] addr=3D0x%"PRIx64"x -> 0x%08x 0x%0= 8x > 0x%08x 0x%08x" > > +aes_cmd_flag(uint32_t raise, uint32_t flag_info) "raise=3D%d > flag_info=3D0x%x" > > +aes_fifo_process(uint32_t cmd, uint32_t success) "cmd=3D%d success=3D%= d" > > +aes_write_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > > +aes_write(uint64_t offset, uint64_t val) "offset=3D0x%"PRIx64" > val=3D0x%"PRIx64 > > +aes_2_read_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > > +aes_2_read(uint64_t offset, uint64_t res) "offset=3D0x%"PRIx64" > res=3D0x%"PRIx64 > > +aes_2_write_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > > +aes_2_write(uint64_t offset, uint64_t val) "offset=3D0x%"PRIx64" > val=3D0x%"PRIx64 > > +aes_dump_data(const char *desc, const char *hex) "%s%s" > > + > > --000000000000316da506240aae7a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, 4 Oct 2024 at 07:32, Akihiko = Odaki <akihiko.odaki@daynix.= com> wrote:
On 2024/09/28 17:57, Phil Dennis-Jordan wrote:
> From: Alexander Graf <graf@amazon.com>
>
> VMApple contains an "aes" engine device that it uses to encr= ypt and
> decrypt its nvram. It has trivial hard coded keys it uses for that
> purpose.
>
> Add device emulation for this device model.
>
> Signed-off-by: Alexander Graf <graf@amazon.com>
> Co-authored-by: Phil Dennis-Jordan=C2=A0<phil@philjordan.eu>
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
>
> ---
> v3:
>
>=C2=A0 =C2=A0* Rebased on latest upstream and fixed minor breakages. >=C2=A0 =C2=A0* Replaced legacy device reset method with Resettable meth= od
>
>=C2=A0 =C2=A0hw/vmapple/Kconfig=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A02 + >=C2=A0 =C2=A0hw/vmapple/aes.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 | 584 ++++++++= ++++++++++++++++++++++++++++++++
>=C2=A0 =C2=A0hw/vmapple/meson.build=C2=A0 |=C2=A0 =C2=A01 +
>=C2=A0 =C2=A0hw/vmapple/trace-events |=C2=A0 19 ++
>=C2=A0 =C2=A04 files changed, 606 insertions(+)
>=C2=A0 =C2=A0create mode 100644 hw/vmapple/aes.c
>
> diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig
> index 8b137891791..a73504d5999 100644
> --- a/hw/vmapple/Kconfig
> +++ b/hw/vmapple/Kconfig
> @@ -1 +1,3 @@
> +config VMAPPLE_AES
> +=C2=A0 =C2=A0 bool
>=C2=A0 =C2=A0
> diff --git a/hw/vmapple/aes.c b/hw/vmapple/aes.c
> new file mode 100644
> index 00000000000..074fbdd9c36
> --- /dev/null
> +++ b/hw/vmapple/aes.c
> @@ -0,0 +1,584 @@
> +/*
> + * QEMU Apple AES device emulation
> + *
> + * Copyright =C2=A9 2023 Amazon.com, Inc. or its affiliates. All Righ= ts Reserved.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or= later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/irq.h"
> +#include "migration/vmstate.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "trace.h"
> +#include "hw/sysbus.h"
> +#include "crypto/hash.h"
> +#include "crypto/aes.h"
> +#include "crypto/cipher.h"
> +
> +#define TYPE_AES=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "apple-aes&qu= ot;
> +#define MAX_FIFO_SIZE=C2=A0 =C2=A0 =C2=A09
> +
> +#define CMD_KEY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x1
> +#define CMD_KEY_CONTEXT_SHIFT=C2=A0 =C2=A0 27
> +#define CMD_KEY_CONTEXT_MASK=C2=A0 =C2=A0 =C2=A0(0x1 << CMD_KEY= _CONTEXT_SHIFT)
> +#define CMD_KEY_SELECT_SHIFT=C2=A0 =C2=A0 =C2=A024
> +#define CMD_KEY_SELECT_MASK=C2=A0 =C2=A0 =C2=A0 (0x7 << CMD_KEY= _SELECT_SHIFT)
> +#define CMD_KEY_KEY_LEN_SHIFT=C2=A0 =C2=A0 22
> +#define CMD_KEY_KEY_LEN_MASK=C2=A0 =C2=A0 =C2=A0(0x3 << CMD_KEY= _KEY_LEN_SHIFT)
> +#define CMD_KEY_ENCRYPT_SHIFT=C2=A0 =C2=A0 20
> +#define CMD_KEY_ENCRYPT_MASK=C2=A0 =C2=A0 =C2=A0(0x1 << CMD_KEY= _ENCRYPT_SHIFT)
> +#define CMD_KEY_BLOCK_MODE_SHIFT 16
> +#define CMD_KEY_BLOCK_MODE_MASK=C2=A0 (0x3 << CMD_KEY_BLOCK_MOD= E_SHIFT)
> +#define CMD_IV=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0x2
> +#define CMD_IV_CONTEXT_SHIFT=C2=A0 =C2=A0 =C2=A026
> +#define CMD_IV_CONTEXT_MASK=C2=A0 =C2=A0 =C2=A0 (0x3 << CMD_KEY= _CONTEXT_SHIFT)
> +#define CMD_DSB=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x3
> +#define CMD_SKG=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x4
> +#define CMD_DATA=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0x5
> +#define CMD_DATA_KEY_CTX_SHIFT=C2=A0 =C2=A027
> +#define CMD_DATA_KEY_CTX_MASK=C2=A0 =C2=A0 (0x1 << CMD_DATA_KEY= _CTX_SHIFT)
> +#define CMD_DATA_IV_CTX_SHIFT=C2=A0 =C2=A0 25
> +#define CMD_DATA_IV_CTX_MASK=C2=A0 =C2=A0 =C2=A0(0x3 << CMD_DAT= A_IV_CTX_SHIFT)
> +#define CMD_DATA_LEN_MASK=C2=A0 =C2=A0 =C2=A0 =C2=A0 0xffffff
> +#define CMD_STORE_IV=C2=A0 =C2=A0 =C2=A0 0x6
> +#define CMD_STORE_IV_ADDR_MASK=C2=A0 =C2=A00xffffff
> +#define CMD_WRITE_REG=C2=A0 =C2=A0 =C2=A00x7
> +#define CMD_FLAG=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0x8
> +#define CMD_FLAG_STOP_MASK=C2=A0 =C2=A0 =C2=A0 =C2=A0BIT(26)
> +#define CMD_FLAG_RAISE_IRQ_MASK=C2=A0 BIT(27)
> +#define CMD_FLAG_INFO_MASK=C2=A0 =C2=A0 =C2=A0 =C2=A00xff
> +#define CMD_MAX=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x10
> +
> +#define CMD_SHIFT=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A028
> +
> +#define REG_STATUS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0xc
> +#define REG_STATUS_DMA_READ_RUNNING=C2=A0 =C2=A0 =C2=A0BIT(0)
> +#define REG_STATUS_DMA_READ_PENDING=C2=A0 =C2=A0 =C2=A0BIT(1)
> +#define REG_STATUS_DMA_WRITE_RUNNING=C2=A0 =C2=A0 BIT(2)
> +#define REG_STATUS_DMA_WRITE_PENDING=C2=A0 =C2=A0 BIT(3)
> +#define REG_STATUS_BUSY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0BIT(4)
> +#define REG_STATUS_EXECUTING=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= BIT(5)
> +#define REG_STATUS_READY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 BIT(6)
> +#define REG_STATUS_TEXT_DPA_SEEDED=C2=A0 =C2=A0 =C2=A0 BIT(7)
> +#define REG_STATUS_UNWRAP_DPA_SEEDED=C2=A0 =C2=A0 BIT(8)
> +
> +#define REG_IRQ_STATUS=C2=A0 =C2=A0 =C2=A0 =C2=A0 0x18
> +#define REG_IRQ_STATUS_INVALID_CMD=C2=A0 =C2=A0 =C2=A0 BIT(2)
> +#define REG_IRQ_STATUS_FLAG=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0BIT(5)
> +#define REG_IRQ_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 0x1c
> +#define REG_WATERMARK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x20
> +#define REG_Q_STATUS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0x24
> +#define REG_FLAG_INFO=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00x30
> +#define REG_FIFO=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0x20= 0
> +
> +static const uint32_t key_lens[4] =3D {

I suggest deriving the size from CMD_KEY_KEY_LEN_MASK.

> +=C2=A0 =C2=A0 [0] =3D 16,
> +=C2=A0 =C2=A0 [1] =3D 24,
> +=C2=A0 =C2=A0 [2] =3D 32,
> +=C2=A0 =C2=A0 [3] =3D 64,
> +};
> +
> +struct key {

Please add typedef.

> +=C2=A0 =C2=A0 uint32_t key_len;
> +=C2=A0 =C2=A0 uint32_t key[8];
> +};
> +
> +struct iv {
> +=C2=A0 =C2=A0 uint32_t iv[4];
> +};
> +
> +struct context {
> +=C2=A0 =C2=A0 struct key key;
> +=C2=A0 =C2=A0 struct iv iv;
> +};
> +
> +static struct key builtin_keys[7] =3D {
=C2=A0> +=C2=A0 =C2=A0 [1] =3D {> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key_l= en =3D 32,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key =3D { 0x1 },
> +=C2=A0 =C2=A0 },
> +=C2=A0 =C2=A0 [2] =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key_len =3D 32,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key =3D { 0x2 },
> +=C2=A0 =C2=A0 },
> +=C2=A0 =C2=A0 [3] =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key_len =3D 32,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .key =3D { 0x3 },
> +=C2=A0 =C2=A0 }
> +};
> +
> +typedef struct AESState {
> +=C2=A0 =C2=A0 /* Private */

This private/public comments are unnecessary; this struct can be
referenced only from this file and all members are automatically private.
> +=C2=A0 =C2=A0 SysBusDevice parent_obj;
> +
> +=C2=A0 =C2=A0 /* Public */
> +=C2=A0 =C2=A0 qemu_irq irq;
> +=C2=A0 =C2=A0 MemoryRegion iomem1;
> +=C2=A0 =C2=A0 MemoryRegion iomem2;
> +
> +=C2=A0 =C2=A0 uint32_t status;
> +=C2=A0 =C2=A0 uint32_t q_status;
> +=C2=A0 =C2=A0 uint32_t irq_status;
> +=C2=A0 =C2=A0 uint32_t irq_enable;
> +=C2=A0 =C2=A0 uint32_t watermark;
> +=C2=A0 =C2=A0 uint32_t flag_info;
> +=C2=A0 =C2=A0 uint32_t fifo[MAX_FIFO_SIZE];
> +=C2=A0 =C2=A0 uint32_t fifo_idx;
> +=C2=A0 =C2=A0 struct key key[2];
> +=C2=A0 =C2=A0 struct iv iv[4];
> +=C2=A0 =C2=A0 bool is_encrypt;
> +=C2=A0 =C2=A0 QCryptoCipherMode block_mode;
> +} AESState;
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(AESState, AES)
> +
> +static void aes_update_irq(AESState *s)
> +{
> +=C2=A0 =C2=A0 qemu_set_irq(s->irq, !!(s->irq_status & s->= ;irq_enable));
> +}
> +
> +static uint64_t aes1_read(void *opaque, hwaddr offset, unsigned size)=
> +{
> +=C2=A0 =C2=A0 AESState *s =3D opaque;
> +=C2=A0 =C2=A0 uint64_t res =3D 0;
> +
> +=C2=A0 =C2=A0 switch (offset) {
> +=C2=A0 =C2=A0 case REG_STATUS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->status;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_IRQ_STATUS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->irq_status;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_IRQ_ENABLE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->irq_enable;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_WATERMARK:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->watermark;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_Q_STATUS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->q_status;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_FLAG_INFO:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D s->flag_info;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_read_unknown(offset);

Use: LOG_UNIMP

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 trace_aes_read(offset, res);
> +
> +=C2=A0 =C2=A0 return res;
> +}
> +
> +static void fifo_append(AESState *s, uint64_t val)
> +{
> +=C2=A0 =C2=A0 if (s->fifo_idx =3D=3D MAX_FIFO_SIZE) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Exceeded the FIFO. Bail out */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 s->fifo[s->fifo_idx++] =3D val;
> +}
> +
> +static bool has_payload(AESState *s, uint32_t elems)
> +{
> +=C2=A0 =C2=A0 return s->fifo_idx >=3D (elems + 1);
> +}
> +
> +static bool cmd_key(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0];
> +=C2=A0 =C2=A0 uint32_t key_select =3D (cmd & CMD_KEY_SELECT_MASK)= >> CMD_KEY_SELECT_SHIFT;
> +=C2=A0 =C2=A0 uint32_t ctxt =3D (cmd & CMD_KEY_CONTEXT_MASK) >= > CMD_KEY_CONTEXT_SHIFT;
> +=C2=A0 =C2=A0 uint32_t key_len;
> +
> +=C2=A0 =C2=A0 switch ((cmd & CMD_KEY_BLOCK_MODE_MASK) >> CM= D_KEY_BLOCK_MODE_SHIFT) {
> +=C2=A0 =C2=A0 case 0:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->block_mode =3D QCRYPTO_CIPHER_MODE_= ECB;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 1:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->block_mode =3D QCRYPTO_CIPHER_MODE_= CBC;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 s->is_encrypt =3D !!((cmd & CMD_KEY_ENCRYPT_MASK= ) >> CMD_KEY_ENCRYPT_SHIFT);

CMD_KEY_ENCRYPT_SHIFT is unnecessary as in case of
CMD_FLAG_RAISE_IRQ_MASK. "!!" is also unnecessary; it will be imp= licitly
casted into bool.

> +=C2=A0 =C2=A0 key_len =3D key_lens[((cmd & CMD_KEY_KEY_LEN_MASK) = >> CMD_KEY_KEY_LEN_SHIFT)];
> +
> +=C2=A0 =C2=A0 if (key_select) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_key_select_builtin(ctxt, ke= y_select,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0s->is_encrypt ? "en" : "de",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0QCryptoCipherMode_str(s->block_mode));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->key[ctxt] =3D builtin_keys[key_sele= ct];

I guess this should be: builtin_keys[key_select - 1]
<= br>
I don't think it should: the builtin keys start at index = 1. This seems fine, and I'd rather leave the indices alone to avoid nee= ding to do arithmetic on them. However in that case, builtin_keys needs to = have 8 elements not 7 as it currently does to avoid overflow. I'll fix = this and use symbolic constants.

(Thanks for the = other review comments, I've reworked those parts for v4.)
=C2=A0
> +=C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_key_select_new(ctxt, key_le= n,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0s->is_enc= rypt ? "en" : "de",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QCryptoCiphe= rMode_str(s->block_mode));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (key_len > sizeof(s->key[ctxt].k= ey)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!has_payload(s, key_len / sizeof(uint= 32_t))) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* wait for payload */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 memcpy(&s->key[ctxt].key, &s-&= gt;fifo[1], key_len);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->key[ctxt].key_len =3D key_len;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static bool cmd_iv(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0];
> +=C2=A0 =C2=A0 uint32_t ctxt =3D (cmd & CMD_IV_CONTEXT_MASK) >&= gt; CMD_IV_CONTEXT_SHIFT;
> +
> +=C2=A0 =C2=A0 if (!has_payload(s, 4)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* wait for payload */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 memcpy(&s->iv[ctxt].iv, &s->fifo[1], size= of(s->iv[ctxt].iv));
> +=C2=A0 =C2=A0 trace_aes_cmd_iv(ctxt, s->fifo[1], s->fifo[2], s-= >fifo[3], s->fifo[4]);
> +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static char hexdigit2str(uint8_t val)
> +{
> +=C2=A0 =C2=A0 g_assert(val < 0x10);
> +=C2=A0 =C2=A0 if (val >=3D 0xa) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return 'a' + (val - 0xa);
> +=C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return '0' + val;
> +=C2=A0 =C2=A0 }
> +}
> +
> +static void dump_data(const char *desc, const void *p, size_t len) > +{
> +=C2=A0 =C2=A0 char *hex =3D alloca((len * 2) + 1);

Don't use alloca(); use a fixed-length array instead.

> +=C2=A0 =C2=A0 const uint8_t *data =3D p;
> +=C2=A0 =C2=A0 char *hexp =3D hex;
> +=C2=A0 =C2=A0 size_t i;
> +
> +=C2=A0 =C2=A0 if (len > 0x1000) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Too large buffer, let's bail out *= /
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 for (i =3D 0; i < len; i++) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint8_t val =3D data[i];
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *(hexp++) =3D hexdigit2str(val >> 4= );
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *(hexp++) =3D hexdigit2str(val & 0xf)= ;
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 *hexp =3D '\0';

Let's move this to: util/hexdump.c

> +
> +=C2=A0 =C2=A0 trace_aes_dump_data(desc, hex);
> +}
> +
> +static bool cmd_data(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0];
> +=C2=A0 =C2=A0 uint32_t ctxt_iv =3D 0;
> +=C2=A0 =C2=A0 uint32_t ctxt_key =3D (cmd & CMD_DATA_KEY_CTX_MASK)= >> CMD_DATA_KEY_CTX_SHIFT;
> +=C2=A0 =C2=A0 uint32_t len =3D cmd & CMD_DATA_LEN_MASK;
> +=C2=A0 =C2=A0 uint64_t src_addr =3D s->fifo[2];
> +=C2=A0 =C2=A0 uint64_t dst_addr =3D s->fifo[3];
> +=C2=A0 =C2=A0 QCryptoCipherAlgo alg;
> +=C2=A0 =C2=A0 QCryptoCipher *cipher;
> +=C2=A0 =C2=A0 char *src;
> +=C2=A0 =C2=A0 char *dst;
> +
> +=C2=A0 =C2=A0 src_addr |=3D ((uint64_t)s->fifo[1] << 16) &am= p; 0xffff00000000ULL;
> +=C2=A0 =C2=A0 dst_addr |=3D ((uint64_t)s->fifo[1] << 32) &am= p; 0xffff00000000ULL;
> +
> +=C2=A0 =C2=A0 trace_aes_cmd_data(ctxt_key, ctxt_iv, src_addr, dst_add= r, len);
> +
> +=C2=A0 =C2=A0 if (!has_payload(s, 3)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* wait for payload */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error("No payload= ");

Use: LOG_GUEST_ERROR

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (ctxt_key >=3D ARRAY_SIZE(s->key) ||
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ctxt_iv >=3D ARRAY_SIZE(s->iv)) { > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Invalid input */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error("Invalid ke= y or iv");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 src =3D g_malloc0(len);
> +=C2=A0 =C2=A0 dst =3D g_malloc0(len);
> +
> +=C2=A0 =C2=A0 cpu_physical_memory_read(src_addr, src, len);

Use: dma_memory_read()

> +
> +=C2=A0 =C2=A0 dump_data("cmd_data(): src_data=3D", src, len= );
> +
> +=C2=A0 =C2=A0 switch (s->key[ctxt_key].key_len) {
> +=C2=A0 =C2=A0 case 128 / 8:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 alg =3D QCRYPTO_CIPHER_ALGO_AES_128;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 192 / 8:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 alg =3D QCRYPTO_CIPHER_ALGO_AES_192;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case 256 / 8:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 alg =3D QCRYPTO_CIPHER_ALGO_AES_256;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error("Invalid ke= y len");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err_free;

Use g_autoptr instead of goto and g_free().

> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 cipher =3D qcrypto_cipher_new(alg, s->block_mode, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (void *)s->key[ctxt_key].key,=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 s->key[ctxt_key].key_len, NUL= L);
> +=C2=A0 =C2=A0 g_assert(cipher !=3D NULL);
Handle this error as you do for qcrypto_cipher_setiv(),
qcrypto_cipher_encrypt() and qcrypto_cipher_decrypt().

> +=C2=A0 =C2=A0 if (s->block_mode !=3D QCRYPTO_CIPHER_MODE_ECB) { > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (qcrypto_cipher_setiv(cipher, (void *)= s->iv[ctxt_iv].iv,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sizeof(s->iv[ctxt_iv].i= v), NULL) !=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error(&q= uot;Failed to set IV");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err_free_cipher;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 if (s->is_encrypt) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (qcrypto_cipher_encrypt(cipher, src, d= st, len, NULL) !=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error(&q= uot;Encrypt failed");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err_free_cipher;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (qcrypto_cipher_decrypt(cipher, src, d= st, len, NULL) !=3D 0) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_cmd_data_error(&q= uot;Decrypt failed");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 goto err_free_cipher;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 qcrypto_cipher_free(cipher);
> +
> +=C2=A0 =C2=A0 dump_data("cmd_data(): dst_data=3D", dst, len= );
> +=C2=A0 =C2=A0 cpu_physical_memory_write(dst_addr, dst, len);
> +=C2=A0 =C2=A0 g_free(src);
> +=C2=A0 =C2=A0 g_free(dst);
> +
> +=C2=A0 =C2=A0 return true;
> +
> +err_free_cipher:
> +=C2=A0 =C2=A0 qcrypto_cipher_free(cipher);
> +err_free:
> +=C2=A0 =C2=A0 g_free(src);
> +=C2=A0 =C2=A0 g_free(dst);
> +=C2=A0 =C2=A0 return false;
> +}
> +
> +static bool cmd_store_iv(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0];
> +=C2=A0 =C2=A0 uint32_t ctxt =3D (cmd & CMD_IV_CONTEXT_MASK) >&= gt; CMD_IV_CONTEXT_SHIFT;
> +=C2=A0 =C2=A0 uint64_t addr =3D s->fifo[1];
> +
> +=C2=A0 =C2=A0 if (!has_payload(s, 1)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* wait for payload */

Add a log as it's done for cmd_data().

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (ctxt >=3D ARRAY_SIZE(s->iv)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Invalid context selected */
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return false;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 addr |=3D ((uint64_t)cmd << 32) & 0xff0000000= 0ULL;
> +=C2=A0 =C2=A0 cpu_physical_memory_write(addr, &s->iv[ctxt].iv,= sizeof(s->iv[ctxt].iv));
> +
> +=C2=A0 =C2=A0 trace_aes_cmd_store_iv(ctxt, addr, s->iv[ctxt].iv[0]= , s->iv[ctxt].iv[1],
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0s->iv[ctxt].iv[2], s->iv[ctxt].iv[3]); > +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static bool cmd_flag(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0];
> +=C2=A0 =C2=A0 uint32_t raise_irq =3D cmd & CMD_FLAG_RAISE_IRQ_MAS= K;
> +
> +=C2=A0 =C2=A0 /* We always process data when it's coming in, so f= ire an IRQ immediately */
> +=C2=A0 =C2=A0 if (raise_irq) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->irq_status |=3D REG_IRQ_STATUS_FLAG= ;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 s->flag_info =3D cmd & CMD_FLAG_INFO_MASK;
> +
> +=C2=A0 =C2=A0 trace_aes_cmd_flag(!!raise_irq, s->flag_info);
> +
> +=C2=A0 =C2=A0 return true;
> +}
> +
> +static void fifo_process(AESState *s)
> +{
> +=C2=A0 =C2=A0 uint32_t cmd =3D s->fifo[0] >> CMD_SHIFT;
> +=C2=A0 =C2=A0 bool success =3D false;
> +
> +=C2=A0 =C2=A0 if (!s->fifo_idx) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 switch (cmd) {
> +=C2=A0 =C2=A0 case CMD_KEY:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 success =3D cmd_key(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case CMD_IV:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 success =3D cmd_iv(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case CMD_DATA:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 success =3D cmd_data(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case CMD_STORE_IV:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 success =3D cmd_store_iv(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case CMD_FLAG:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 success =3D cmd_flag(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->irq_status |=3D REG_IRQ_STATUS_INVA= LID_CMD;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 if (success) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->fifo_idx =3D 0;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 trace_aes_fifo_process(cmd, success ? 1 : 0);
> +}
> +
> +static void aes1_write(void *opaque, hwaddr offset, uint64_t val, uns= igned size)
> +{
> +=C2=A0 =C2=A0 AESState *s =3D opaque;
> +
> +=C2=A0 =C2=A0 trace_aes_write(offset, val);
> +
> +=C2=A0 =C2=A0 switch (offset) {
> +=C2=A0 =C2=A0 case REG_IRQ_STATUS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->irq_status &=3D ~val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_IRQ_ENABLE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->irq_enable =3D val;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case REG_FIFO:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 fifo_append(s, val);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 fifo_process(s);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_write_unknown(offset);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 aes_update_irq(s);
> +}
> +
> +static const MemoryRegionOps aes1_ops =3D {
> +=C2=A0 =C2=A0 .read =3D aes1_read,
> +=C2=A0 =C2=A0 .write =3D aes1_write,
> +=C2=A0 =C2=A0 .endianness =3D DEVICE_NATIVE_ENDIAN,
> +=C2=A0 =C2=A0 .valid =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .min_access_size =3D 4,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .max_access_size =3D 8,
> +=C2=A0 =C2=A0 },
> +=C2=A0 =C2=A0 .impl =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .min_access_size =3D 4,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .max_access_size =3D 4,
> +=C2=A0 =C2=A0 },
> +};
> +
> +static uint64_t aes2_read(void *opaque, hwaddr offset, unsigned size)=
> +{
> +=C2=A0 =C2=A0 uint64_t res =3D 0;
> +
> +=C2=A0 =C2=A0 switch (offset) {
> +=C2=A0 =C2=A0 case 0:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 res =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_2_read_unknown(offset);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 trace_aes_2_read(offset, res);
> +
> +=C2=A0 =C2=A0 return res;
> +}
> +
> +static void aes2_write(void *opaque, hwaddr offset, uint64_t val, uns= igned size)
> +{
> +=C2=A0 =C2=A0 trace_aes_2_write(offset, val);
> +
> +=C2=A0 =C2=A0 switch (offset) {
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 trace_aes_2_write_unknown(offset);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
> +=C2=A0 =C2=A0 }
> +}
> +
> +static const MemoryRegionOps aes2_ops =3D {
> +=C2=A0 =C2=A0 .read =3D aes2_read,
> +=C2=A0 =C2=A0 .write =3D aes2_write,
> +=C2=A0 =C2=A0 .endianness =3D DEVICE_NATIVE_ENDIAN,
> +=C2=A0 =C2=A0 .valid =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .min_access_size =3D 4,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .max_access_size =3D 8,
> +=C2=A0 =C2=A0 },
> +=C2=A0 =C2=A0 .impl =3D {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .min_access_size =3D 4,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 .max_access_size =3D 4,
> +=C2=A0 =C2=A0 },
> +};
> +
> +static void aes_reset(Object *obj, ResetType type)
> +{
> +=C2=A0 =C2=A0 AESState *s =3D AES(obj);
> +
> +=C2=A0 =C2=A0 s->status =3D 0x3f80;
> +=C2=A0 =C2=A0 s->q_status =3D 2;
> +=C2=A0 =C2=A0 s->irq_status =3D 0;
> +=C2=A0 =C2=A0 s->irq_enable =3D 0;
> +=C2=A0 =C2=A0 s->watermark =3D 0;
> +}
> +
> +static void aes_init(Object *obj)
> +{
> +=C2=A0 =C2=A0 AESState *s =3D AES(obj);
> +
> +=C2=A0 =C2=A0 memory_region_init_io(&s->iomem1, obj, &aes1= _ops, s, TYPE_AES, 0x4000);
> +=C2=A0 =C2=A0 memory_region_init_io(&s->iomem2, obj, &aes2= _ops, s, TYPE_AES, 0x4000);
> +=C2=A0 =C2=A0 sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem1);=
> +=C2=A0 =C2=A0 sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem2);=
> +=C2=A0 =C2=A0 sysbus_init_irq(SYS_BUS_DEVICE(s), &s->irq);
> +}
> +
> +static void aes_realize(DeviceState *dev, Error **errp)
> +{
> +}

This shouldn't be necessary.

> +
> +static void aes_class_init(ObjectClass *klass, void *data)
> +{
> +=C2=A0 =C2=A0 DeviceClass *dc =3D DEVICE_CLASS(klass);
> +=C2=A0 =C2=A0 ResettableClass *rc =3D RESETTABLE_CLASS(klass);
> +
> +=C2=A0 =C2=A0 rc->phases.hold =3D aes_reset;
> +=C2=A0 =C2=A0 dc->realize =3D aes_realize;
> +}
> +
> +static const TypeInfo aes_info =3D {
> +=C2=A0 =C2=A0 .name=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D TYPE_AES, > +=C2=A0 =C2=A0 .parent=C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D TYPE_SYS_BUS_DEV= ICE,
> +=C2=A0 =C2=A0 .instance_size =3D sizeof(AESState),
> +=C2=A0 =C2=A0 .class_init=C2=A0 =C2=A0 =3D aes_class_init,
> +=C2=A0 =C2=A0 .instance_init =3D aes_init,
> +};
> +
> +static void aes_register_types(void)
> +{
> +=C2=A0 =C2=A0 type_register_static(&aes_info);
> +}
> +
> +type_init(aes_register_types)
> diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build
> index e69de29bb2d..bcd4dcb28d2 100644
> --- a/hw/vmapple/meson.build
> +++ b/hw/vmapple/meson.build
> @@ -0,0 +1 @@
> +system_ss.add(when: 'CONFIG_VMAPPLE_AES',=C2=A0 if_true: file= s('aes.c'))
> diff --git a/hw/vmapple/trace-events b/hw/vmapple/trace-events
> index 9ccc5790487..1c9a3326eb4 100644
> --- a/hw/vmapple/trace-events
> +++ b/hw/vmapple/trace-events
> @@ -1,2 +1,21 @@
>=C2=A0 =C2=A0# See docs/devel/tracing.rst for syntax documentation.
>=C2=A0 =C2=A0
> +# aes.c
> +aes_read_unknown(uint64_t offset) "offset=3D0x%"PRIx64
> +aes_read(uint64_t offset, uint64_t res) "offset=3D0x%"PRIx6= 4" res=3D0x%"PRIx64
> +aes_cmd_key_select_builtin(uint32_t ctx, uint32_t key_id, const char = *direction, const char *cipher) "[%d] Selecting builtin key %d to %scr= ypt with %s"
> +aes_cmd_key_select_new(uint32_t ctx, uint32_t key_len, const char *di= rection, const char *cipher) "[%d] Selecting new key size=3D%d to %scr= ypt with %s"
> +aes_cmd_iv(uint32_t ctx, uint32_t iv0, uint32_t iv1, uint32_t iv2, ui= nt32_t iv3) "[%d] 0x%08x 0x%08x 0x%08x 0x%08x"
> +aes_cmd_data(uint32_t key, uint32_t iv, uint64_t src, uint64_t dst, u= int32_t len) "[key=3D%d iv=3D%d] src=3D0x%"PRIx64" dst=3D0x%= "PRIx64" len=3D0x%x"
> +aes_cmd_data_error(const char *reason) "reason=3D%s"
> +aes_cmd_store_iv(uint32_t ctx, uint64_t addr, uint32_t iv0, uint32_t = iv1, uint32_t iv2, uint32_t iv3) "[%d] addr=3D0x%"PRIx64"x -= > 0x%08x 0x%08x 0x%08x 0x%08x"
> +aes_cmd_flag(uint32_t raise, uint32_t flag_info) "raise=3D%d fla= g_info=3D0x%x"
> +aes_fifo_process(uint32_t cmd, uint32_t success) "cmd=3D%d succe= ss=3D%d"
> +aes_write_unknown(uint64_t offset) "offset=3D0x%"PRIx64
> +aes_write(uint64_t offset, uint64_t val) "offset=3D0x%"PRIx= 64" val=3D0x%"PRIx64
> +aes_2_read_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > +aes_2_read(uint64_t offset, uint64_t res) "offset=3D0x%"PRI= x64" res=3D0x%"PRIx64
> +aes_2_write_unknown(uint64_t offset) "offset=3D0x%"PRIx64 > +aes_2_write(uint64_t offset, uint64_t val) "offset=3D0x%"PR= Ix64" val=3D0x%"PRIx64
> +aes_dump_data(const char *desc, const char *hex) "%s%s"
> +

--000000000000316da506240aae7a--