RE: [PATCH v2 01/17] hw/misc/aspeed_hace: Support the crypto command in direct access mode

> -----Original Message-----
> From: Jamin Lin <[email protected]>
> Sent: Wednesday, July 15, 2026 11:33 AM
> To: Daniel P. Berrangé <[email protected]>; Cédric Le Goater
> <[email protected]>; Peter Maydell <[email protected]>; Steven Lee
> <[email protected]>; Troy Lee <[email protected]>; Kane Chen
> <[email protected]>; Andrew Jeffery
> <[email protected]>; Joel Stanley <[email protected]>; Eric Blake
> <[email protected]>; Markus Armbruster <[email protected]>; Fabiano
> Rosas <[email protected]>; Laurent Vivier <[email protected]>; Paolo Bonzini
> <[email protected]>; open list:All patches CC here
> <[email protected]>; open list:ASPEED BMCs
> <[email protected]>
> Cc: Jamin Lin <[email protected]>; Troy Lee
> <[email protected]>
> Subject: [PATCH v2 01/17] hw/misc/aspeed_hace: Support the crypto
> command in direct access mode
> 
> The crypt command register was previously stubbed out. Implement it for the
> direct access mode, where HACE00/HACE04 point directly at contiguous source
> and destination buffers. AES-128/192/256, DES and 3DES are supported in ECB
> and CBC modes via the qcrypto cipher API; the IV and key are read from the
> context buffer (HACE08) and, for CBC, the resulting chaining IV is written back
> to the context.
> 
> The completion interrupt is now raised for every HACE variant as the hardware
> does, which fixes the crypt command hang on the AST2500, AST2600 and
> AST1030. The AST2700 crypto engine still needs 64-bit DMA and AES-GCM,
> which are added later, so it keeps its temporary interrupt-only workaround
> until then.
> 
> For debugging, the context, source and destination buffers are dumped through
> the existing aspeed_hace_hexdump trace event (disabled by default). CTR
> mode, scatter-gather mode and AES-GCM are added separately.
> 
> Signed-off-by: Jamin Lin <[email protected]>
> ---
>  hw/misc/aspeed_hace.c | 283
> +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 269 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> c61efe50c4..de4399f322 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -18,11 +18,44 @@
>  #include "qapi/error.h"
>  #include "migration/vmstate.h"
>  #include "crypto/hash.h"
> +#include "crypto/cipher.h"
>  #include "hw/core/qdev-properties.h"
>  #include "hw/core/irq.h"
>  #include "trace.h"
> 
> -#define R_CRYPT_CMD     (0x10 / 4)
> +/* Crypto engine registers */
> +#define R_CRYPT_SRC         (0x00 / 4)
> +#define R_CRYPT_DEST        (0x04 / 4)
> +#define R_CRYPT_CONTEXT     (0x08 / 4)
> +#define R_CRYPT_DATA_LEN    (0x0c / 4)
> +/* HACE0C[27:0] holds the crypto data length */
> +#define  CRYPT_DATA_LEN_MASK    0x0FFFFFFF
> +#define R_CRYPT_CMD         (0x10 / 4)
> +/* Crypto engine command register (HACE10) bits */
> +#define  CRYPT_CMD_ENCRYPT          BIT(7)
> +#define  CRYPT_CMD_ISR_EN           BIT(12)

Regarding CRYPT_CMD_ISR_EN, the current code already has similar definitions, such as CRYPT_IRQ and CRYPT_IRQ_EN. Would it make sense to migrate these definitions together for consistency?

> +#define  CRYPT_CMD_DES_SELECT       BIT(16)
> +#define  CRYPT_CMD_TRIPLE_DES       BIT(17)
> +#define  CRYPT_CMD_SRC_SG_CTRL      BIT(18)
> +#define  CRYPT_CMD_DST_SG_CTRL      BIT(19)

CRYPT_CMD_DST_SG_CTRL is currently defined but not used anywhere. Could we defer adding this definition until it is actually needed?

> +/* Operation mode HACE10[6:4] */
> +#define  CRYPT_CMD_OP_MODE_MASK     (0x7 << 4)
> +#define  CRYPT_CMD_ECB              (0x0 << 4)
> +#define  CRYPT_CMD_CBC              (0x1 << 4)
> +/* AES key length HACE10[3:2] */
> +#define  CRYPT_CMD_AES_KEY_LEN_MASK (0x3 << 2)
> +#define  CRYPT_CMD_AES256           (0x2 << 2)
> +#define  CRYPT_CMD_AES192           (0x1 << 2)
> +#define  CRYPT_CMD_AES128           (0x0 << 2)
> +
> +/*
> + * Crypto context buffer layout (HACE08). The IV is at the start of the
> +buffer
> + * (DES places its 8 byte IV at offset 8) and the cipher key at offset 0x10.
> + */
> +#define CRYPT_CTX_IV_OFFSET         0x00
> +#define CRYPT_CTX_DES_IV_OFFSET     0x08
> +#define CRYPT_CTX_KEY_OFFSET        0x10
> +#define CRYPT_CTX_SIZE              0x30
> 
>  #define R_STATUS        (0x1c / 4)
>  #define HASH_IRQ        BIT(9)
> @@ -501,6 +534,216 @@ static void do_hash_operation(AspeedHACEState *s,
> int algo, bool sg_mode,
>      }
>  }
> 
> +static bool crypt_aes_alg(uint32_t cmd, QCryptoCipherAlgo *alg, size_t
> +*keylen) {
> +    switch (cmd & CRYPT_CMD_AES_KEY_LEN_MASK) {
> +    case CRYPT_CMD_AES128:
> +        *alg = QCRYPTO_CIPHER_ALGO_AES_128;
> +        *keylen = 16;
> +        break;
> +    case CRYPT_CMD_AES192:
> +        *alg = QCRYPTO_CIPHER_ALGO_AES_192;
> +        *keylen = 24;
> +        break;
> +    case CRYPT_CMD_AES256:
> +        *alg = QCRYPTO_CIPHER_ALGO_AES_256;
> +        *keylen = 32;
> +        break;
> +    default:
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +/*
> + * Decode the crypto command register into a libqcrypto algorithm/mode
> +pair
> + * and the block/IV geometry. Returns false for unsupported selections.
> + */
> +static bool crypt_decode_cmd(uint32_t cmd, QCryptoCipherAlgo *alg,
> +                             QCryptoCipherMode *mode, size_t
> *keylen,
> +                             size_t *blocklen, size_t *iv_offset) {
> +    if (cmd & CRYPT_CMD_DES_SELECT) {
> +        *blocklen = 8;
> +        *iv_offset = CRYPT_CTX_DES_IV_OFFSET;
> +        if (cmd & CRYPT_CMD_TRIPLE_DES) {
> +            *alg = QCRYPTO_CIPHER_ALGO_3DES;
> +            *keylen = 24;
> +        } else {
> +            *alg = QCRYPTO_CIPHER_ALGO_DES;
> +            *keylen = 8;
> +        }
> +    } else {
> +        *blocklen = 16;
> +        *iv_offset = CRYPT_CTX_IV_OFFSET;
> +        if (!crypt_aes_alg(cmd, alg, keylen)) {
> +            return false;
> +        }
> +    }
> +
> +    switch (cmd & CRYPT_CMD_OP_MODE_MASK) {
> +    case CRYPT_CMD_ECB:
> +        *mode = QCRYPTO_CIPHER_MODE_ECB;
> +        break;
> +    case CRYPT_CMD_CBC:
> +        *mode = QCRYPTO_CIPHER_MODE_CBC;
> +        break;
> +    default:
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +/*
> + * Direct access mode: the source/destination register (HACE00/HACE04)
> +points
> + * at a single contiguous buffer in DRAM. Copy @len bytes between it
> +and the
> + * bounce buffer @buf; when @to_dram is true @buf is written out,
> +otherwise it
> + * is read in. Returns true on success.
> + */
> +static bool crypt_prepare_direct(AspeedHACEState *s, uint64_t addr,
> +                                 uint8_t *buf, uint32_t len, bool
> +to_dram) {
> +    return !address_space_rw(&s->dram_as, addr,
> MEMTXATTRS_UNSPECIFIED,
> +                             buf, len, to_dram); }
> +
> +/*
> + * Perform an AES/DES/3DES ECB/CBC operation in direct access mode: the
> +source
> + * and destination are single contiguous buffers (HACE00/HACE04) and
> +the IV/key
> + * come from the context buffer (HACE08). For CBC the resulting
> +chaining IV is
> + * written back to the context buffer so the driver can continue the chain.
> + */
> +static void do_crypt_operation(AspeedHACEState *s, uint32_t cmd) {
> +    uint32_t len = s->regs[R_CRYPT_DATA_LEN];
> +    bool encrypt = cmd & CRYPT_CMD_ENCRYPT;
> +    g_autoptr(QCryptoCipher) cipher = NULL;
> +    g_autofree uint8_t *src_buf = NULL;
> +    g_autofree uint8_t *dst_buf = NULL;
> +    uint8_t ctx[CRYPT_CTX_SIZE];
> +    Error *local_err = NULL;
> +    QCryptoCipherMode mode;
> +    QCryptoCipherAlgo alg;
> +    const uint8_t *next_iv;
> +    uint64_t ctx_addr;
> +    uint64_t src_addr;
> +    uint64_t dst_addr;
> +    size_t iv_offset;
> +    size_t blocklen;
> +    size_t keylen;
> +
> +    if (len == 0) {
> +        return;
> +    }
> +
> +    if (!crypt_decode_cmd(cmd, &alg, &mode, &keylen, &blocklen,
> &iv_offset)) {
> +        qemu_log_mask(LOG_UNIMP,
> +                      "%s: Unsupported crypt command 0x%x\n",
> __func__, cmd);
> +        return;
> +    }
> +
> +    if (!qcrypto_cipher_supports(alg, mode)) {
> +        qemu_log_mask(LOG_UNIMP,
> +                      "%s: cipher mode not supported by the crypto
> backend\n",
> +                      __func__);
> +        return;
> +    }
> +
> +    /* Fetch the IV and key from the context buffer in DRAM. */
> +    ctx_addr = s->regs[R_CRYPT_CONTEXT];
> +    if (address_space_read(&s->dram_as, ctx_addr,
> MEMTXATTRS_UNSPECIFIED,
> +                           ctx, sizeof(ctx))) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Failed to read context, addr=0x%"
> HWADDR_PRIx "\n",
> +                      __func__, ctx_addr);
> +        return;
> +    }
> +
> +    if (trace_event_get_state_backends(TRACE_ASPEED_HACE_HEXDUMP))
> {
> +        hace_hexdump("context", (char *)ctx, sizeof(ctx));
> +    }
> +
> +    cipher = qcrypto_cipher_new(alg, mode, ctx + CRYPT_CTX_KEY_OFFSET,
> keylen,
> +                                &local_err);
> +    if (cipher == NULL) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto cipher new
> failed: %s\n",
> +                      __func__, error_get_pretty(local_err));
> +        error_free(local_err);
> +        return;
> +    }
> +
> +    if (mode != QCRYPTO_CIPHER_MODE_ECB &&
> +        qcrypto_cipher_setiv(cipher, ctx + iv_offset, blocklen,
> +                             &local_err) < 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: qcrypto cipher setiv
> failed: %s\n",
> +                      __func__, error_get_pretty(local_err));
> +        error_free(local_err);
> +        return;
> +    }
> +
> +    src_buf = g_malloc0(len);
> +    dst_buf = g_malloc0(len);
> +
> +    src_addr = s->regs[R_CRYPT_SRC];
> +    if (!crypt_prepare_direct(s, src_addr, src_buf, len, false)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Failed to read src, addr=0x%" HWADDR_PRIx
> "\n",
> +                      __func__, src_addr);
> +        return;
> +    }
> +
> +    if (trace_event_get_state_backends(TRACE_ASPEED_HACE_HEXDUMP))
> {
> +        hace_hexdump("src", (char *)src_buf, len);
> +    }
> +
> +    if (encrypt) {
> +        if (qcrypto_cipher_encrypt(cipher, src_buf, dst_buf, len,
> +                                   &local_err) < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: encrypt failed:
> %s\n",
> +                          __func__, error_get_pretty(local_err));
> +            error_free(local_err);
> +            return;
> +        }
> +    } else {
> +        if (qcrypto_cipher_decrypt(cipher, src_buf, dst_buf, len,
> +                                   &local_err) < 0) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: decrypt failed:
> %s\n",
> +                          __func__, error_get_pretty(local_err));
> +            error_free(local_err);
> +            return;
> +        }
> +    }
> +
> +    dst_addr = s->regs[R_CRYPT_DEST];
> +    if (!crypt_prepare_direct(s, dst_addr, dst_buf, len, true)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Failed to write dst, addr=0x%" HWADDR_PRIx
> "\n",
> +                      __func__, dst_addr);
> +        return;
> +    }
> +
> +    if (trace_event_get_state_backends(TRACE_ASPEED_HACE_HEXDUMP))
> {
> +        hace_hexdump("dst", (char *)dst_buf, len);
> +    }
> +
> +    if (mode == QCRYPTO_CIPHER_MODE_CBC) {
> +        /*
> +         * CBC chains on the last ciphertext block: the final block of the
> +         * output when encrypting, or of the input when decrypting. Write
> it
> +         * back as the IV for the next request.
> +         */
> +        next_iv = (encrypt ? dst_buf : src_buf) + len - blocklen;
> +        if (address_space_write(&s->dram_as, ctx_addr + iv_offset,
> +                                MEMTXATTRS_UNSPECIFIED, next_iv,
> blocklen)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "%s: Failed to write IV, addr=0x%"
> HWADDR_PRIx "\n",
> +                          __func__, ctx_addr + iv_offset);
> +        }
> +    }
> +}
> +
>  static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int
> size)  {
>      AspeedHACEState *s = ASPEED_HACE(opaque); @@ -531,16 +774,22
> @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>                  qemu_irq_lower(s->irq);
>              }
>          }
> -        if (ahc->raise_crypt_interrupt_workaround) {
> -            if (data & CRYPT_IRQ) {
> -                data &= ~CRYPT_IRQ;
> +        if (data & CRYPT_IRQ) {
> +            data &= ~CRYPT_IRQ;
> 
> -                if (s->regs[addr] & CRYPT_IRQ) {
> -                    qemu_irq_lower(s->irq);
> -                }
> +            if (s->regs[addr] & CRYPT_IRQ) {
> +                qemu_irq_lower(s->irq);
>              }
>          }
>          break;
> +    case R_CRYPT_SRC:
> +    case R_CRYPT_DEST:
> +    case R_CRYPT_CONTEXT:
> +        data &= ahc->src_mask;
> +        break;
> +    case R_CRYPT_DATA_LEN:
> +        data &= CRYPT_DATA_LEN_MASK;
> +        break;
>      case R_HASH_SRC:
>          data &= ahc->src_mask;
>          break;
> @@ -589,13 +838,19 @@ static void aspeed_hace_write(void *opaque,
> hwaddr addr, uint64_t data,
>          break;
>      }
>      case R_CRYPT_CMD:
> -        qemu_log_mask(LOG_UNIMP, "%s: Crypt commands not
> implemented\n",
> -                       __func__);
> -        if (ahc->raise_crypt_interrupt_workaround) {
> -            s->regs[R_STATUS] |= CRYPT_IRQ;
> -            if (data & CRYPT_IRQ_EN) {
> -                qemu_irq_raise(s->irq);
> -            }
> +        /*
> +         * The AST2700 crypto engine needs 64-bit DMA and AES-GCM,
> which are
> +         * added later; until then it keeps the temporary workaround of
> only
> +         * raising the completion interrupt without running the command.
> +         */
> +        if (!ahc->raise_crypt_interrupt_workaround) {
> +            do_crypt_operation(s, data);
> +        }
> +
> +        /* Hardware raises the crypt interrupt once the command finishes.
> */
> +        s->regs[R_STATUS] |= CRYPT_IRQ;
> +        if (data & CRYPT_CMD_ISR_EN) {
> +            qemu_irq_raise(s->irq);
>          }
>          break;
>      case R_HASH_SRC_HI:
> --
> 2.43.0