Re: [PATCH v1 5/8] hw/misc/aspeed_sbc: Support the ECDSA verify command

On 8/18/26 11:22, Jamin Lin wrote:
> The AST10x0 secure boot controller register block also hosts an ECDSA
> engine. Emulate its secp384r1 "verify" command: on a trigger write to
> the command register, read the public key, signature and SHA-384 digest
> that the guest staged in the SEC SRAM and defer the verification to the
> crypto akcipher backend, reporting the result through the status
> register.
> 
> The engine reads its operands from the SEC SRAM through a dedicated
> address space over the SoC memory, using a 'sram' link and a 'sram-base'
> property.
> 
> The absolute address is sram_base + the SRAM-relative offset.
> 
> Signed-off-by: Jamin Lin <[email protected]>
> ---
>   hw/misc/aspeed_sbc.c         | 154 +++++++++++++++++++++++++++++++++++
>   hw/misc/trace-events         |   2 +
>   include/hw/misc/aspeed_sbc.h |   5 ++
>   3 files changed, 161 insertions(+)
> 
> diff --git a/hw/misc/aspeed_sbc.c b/hw/misc/aspeed_sbc.c
> index 1dfcf14e5b..724f4cb06c 100644
> --- a/hw/misc/aspeed_sbc.c
> +++ b/hw/misc/aspeed_sbc.c
> @@ -10,11 +10,13 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
> +#include "qemu/cutils.h"
>   #include "qemu/error-report.h"
>   #include "hw/core/qdev-properties.h"
>   #include "hw/misc/aspeed_sbc.h"
>   #include "qapi/error.h"
>   #include "migration/vmstate.h"
> +#include "crypto/akcipher.h"
>   #include "trace.h"
>   
>   #define R_PROT          (0x000 / 4)
> @@ -24,8 +26,22 @@
>   #define R_CAMP1         (0x020 / 4)
>   #define R_CAMP2         (0x024 / 4)
>   #define R_QSR           (0x040 / 4)
> +#define R_ECDSA_CMD     (0x0bc / 4)
> +
> +/*
> + * SEC SRAM layout for a secp384r1 ECDSA verify operation. All operands are
> + * 48-byte big-endian integers.
> + */
> +#define ECDSA_SRAM_QX   0x2080
> +#define ECDSA_SRAM_QY   0x20c0
> +#define ECDSA_SRAM_R    0x21c0
> +#define ECDSA_SRAM_S    0x2200
> +#define ECDSA_SRAM_M    0x2240
> +#define ECDSA_P384_COORD_LEN    48
>   
>   /* R_STATUS */
> +#define ECDSA_VERIFY_PASS       BIT(21)
> +#define ECDSA_VERIFY_DONE       BIT(20)
>   #define ABR_EN                  BIT(14) /* Mirrors SCU510[11] */
>   #define ABR_IMAGE_SOURCE        BIT(13)
>   #define SPI_ABR_IMAGE_SOURCE    BIT(12)
> @@ -42,6 +58,9 @@
>   #define OTP_MEM_IDLE            BIT(1)
>   #define OTP_COMPARE_STATUS      BIT(0)
>   
> +/* R_ECDSA_CMD */
> +#define ECDSA_CMD_TRIGGER BIT(1)
> +
>   /* QSR */
>   #define QSR_RSA_MASK           (0x3 << 12)
>   #define QSR_HASH_MASK          (0x3 << 10)
> @@ -220,10 +239,121 @@ static void aspeed_sbc_handle_command(void *opaque, uint32_t cmd)
>       s->regs[R_STATUS] |= (OTP_MEM_IDLE | OTP_IDLE);
>   }
>   
> +static void sbc_ecdsa_hexdump(const char *desc, const char *buf, size_t size)
> +{
> +    g_autoptr(GString) str = g_string_sized_new(64);
> +    size_t len;
> +    size_t i;
> +
> +    for (i = 0; i < size; i += len) {
> +        len = MIN(16, size - i);
> +        g_string_truncate(str, 0);
> +        qemu_hexdump_line(str, buf + i, len, 1, 4);
> +        trace_aspeed_sbc_ecdsa_hexdump(desc, i, str->str);
> +    }
> +}
> +
> +/*
> + * The hardware only supports ECDSA secp384r1 (NIST P-384).
> + * The guest has already staged the public key,
> + * signature and digest in the SEC SRAM; read them out and defer the actual
> + * verification to the crypto backend.
> + *
> + * Returns true if the signature verifies. The caller is responsible for
> + * updating the status register.
> + */
> +static bool aspeed_sbc_ecdsa_verify(AspeedSBCState *s)
> +{
> +    /* Only ECDSA secp384r1 is supported by this engine. */
> +    QCryptoAkCipherOptions opts = {
> +        .alg = QCRYPTO_AK_CIPHER_ALGO_ECDSA,
> +        .u.ecdsa.curve_id = QCRYPTO_CURVE_ID_SECP384R1,
> +    };
> +    g_autoptr(QCryptoAkCipher) akcipher = NULL;
> +    uint8_t pubkey[ECDSA_P384_COORD_LEN * 2];
> +    uint8_t sig[ECDSA_P384_COORD_LEN * 2];
> +    uint8_t dgst[ECDSA_P384_COORD_LEN];
> +    Error *err = NULL;
> +
> +    if (!qcrypto_akcipher_supports(&opts)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: ECDSA secp384r1 is not supported by the crypto "
> +                      "backend\n", __func__);
> +        return false;
> +    }
> +
> +    /*
> +     * ECDSA_SRAM_* are SRAM-relative offsets, but sram_as is an AddressSpace
> +     * over the SoC memory, where the SEC SRAM is mapped at sram_base - so the
> +     * absolute address is sram_base + ECDSA_SRAM_*.
> +     */
> +    if (address_space_read(&s->sram_as, s->sram_base + ECDSA_SRAM_QX,
> +                           MEMTXATTRS_UNSPECIFIED, pubkey,
> +                           ECDSA_P384_COORD_LEN) != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to read ECDSA Qx from SEC SRAM\n", __func__);
> +        return false;
> +    }
> +    if (address_space_read(&s->sram_as, s->sram_base + ECDSA_SRAM_QY,
> +                           MEMTXATTRS_UNSPECIFIED,
> +                           pubkey + ECDSA_P384_COORD_LEN,
> +                           ECDSA_P384_COORD_LEN) != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to read ECDSA Qy from SEC SRAM\n", __func__);
> +        return false;
> +    }
> +    if (address_space_read(&s->sram_as, s->sram_base + ECDSA_SRAM_R,
> +                           MEMTXATTRS_UNSPECIFIED, sig,
> +                           ECDSA_P384_COORD_LEN) != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to read ECDSA r from SEC SRAM\n", __func__);
> +        return false;
> +    }
> +    if (address_space_read(&s->sram_as, s->sram_base + ECDSA_SRAM_S,
> +                           MEMTXATTRS_UNSPECIFIED, sig + ECDSA_P384_COORD_LEN,
> +                           ECDSA_P384_COORD_LEN) != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to read ECDSA s from SEC SRAM\n", __func__);
> +        return false;
> +    }
> +    if (address_space_read(&s->sram_as, s->sram_base + ECDSA_SRAM_M,
> +                           MEMTXATTRS_UNSPECIFIED, dgst,
> +                           ECDSA_P384_COORD_LEN) != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to read ECDSA digest from SEC SRAM\n",
> +                      __func__);
> +        return false;
> +    }
> +
> +    if (trace_event_get_state_backends(TRACE_ASPEED_SBC_ECDSA_HEXDUMP)) {
> +        sbc_ecdsa_hexdump("pubkey", (char *)pubkey, sizeof(pubkey));
> +        sbc_ecdsa_hexdump("signature", (char *)sig, sizeof(sig));
> +        sbc_ecdsa_hexdump("digest", (char *)dgst, sizeof(dgst));
> +    }
> +
> +    akcipher = qcrypto_akcipher_new(&opts, QCRYPTO_AK_CIPHER_KEY_TYPE_PUBLIC,
> +                                    pubkey, sizeof(pubkey), &err);
> +    if (!akcipher) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: %s\n", __func__,
> +                      error_get_pretty(err));
> +        error_free(err);
> +        return false;
> +    }
> +
> +    if (qcrypto_akcipher_verify(akcipher, sig, sizeof(sig),
> +                                dgst, sizeof(dgst), &err) != 0) {
> +        error_free(err);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static void aspeed_sbc_write(void *opaque, hwaddr addr, uint64_t data,
>                                 unsigned int size)
>   {
>       AspeedSBCState *s = ASPEED_SBC(opaque);
> +    AspeedSBCClass *sc = ASPEED_SBC_GET_CLASS(s);
>   
>       addr >>= 2;
>   
> @@ -244,6 +374,18 @@ static void aspeed_sbc_write(void *opaque, hwaddr addr, uint64_t data,
>       case R_CMD:
>           aspeed_sbc_handle_command(opaque, data);
>           return;
> +    case R_ECDSA_CMD:
> +        if (sc->has_ecdsa && data == ECDSA_CMD_TRIGGER) {
> +            s->regs[R_STATUS] &= ~(ECDSA_VERIFY_DONE | ECDSA_VERIFY_PASS);
> +            if (aspeed_sbc_ecdsa_verify(s)) {
> +                s->regs[R_STATUS] |= ECDSA_VERIFY_PASS;
> +            }
> +            s->regs[R_STATUS] |= ECDSA_VERIFY_DONE;
> +            trace_aspeed_sbc_ecdsa_verify(
> +                (s->regs[R_STATUS] & ECDSA_VERIFY_PASS) ? "pass" : "fail");
> +        }
> +        s->regs[addr] = data;
> +        return;
>       default:
>           break;
>       }
> @@ -306,6 +448,14 @@ static void aspeed_sbc_realize(DeviceState *dev, Error **errp)
>           }
>       }
>   
> +    if (sc->has_ecdsa) {
> +        if (!s->sram) {
> +            error_setg(errp, TYPE_ASPEED_SBC ": 'sram' link not set");
> +            return;
> +        }
> +        address_space_init(&s->sram_as, s->sram, TYPE_ASPEED_SBC ".sram");
> +    }
> +
>       memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_sbc_ops, s,
>               TYPE_ASPEED_SBC, 0x1000);
>   
> @@ -325,6 +475,9 @@ static const VMStateDescription vmstate_aspeed_sbc = {
>   static const Property aspeed_sbc_properties[] = {
>       DEFINE_PROP_BOOL("emmc-abr", AspeedSBCState, emmc_abr, 0),
>       DEFINE_PROP_UINT32("signing-settings", AspeedSBCState, signing_settings, 0),
> +    DEFINE_PROP_LINK("sram", AspeedSBCState, sram,
> +                     TYPE_MEMORY_REGION, MemoryRegion *),
> +    DEFINE_PROP_UINT64("sram-base", AspeedSBCState, sram_base, 0),
>   };
>   
>   static void aspeed_sbc_class_init(ObjectClass *klass, const void *data)
> @@ -355,6 +508,7 @@ static void aspeed_ast10x0_sbc_class_init(ObjectClass *klass, const void *data)
>   
>       dc->desc = "AST10X0 Secure Boot Controller";
>       sc->has_otp = true;
> +    sc->has_ecdsa = true;
>   }
>   
>   static const TypeInfo aspeed_sbc_types[] = {
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index c9a868b3ef..1cd2ef8acc 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -95,6 +95,8 @@ aspeed_sbc_ignore_cmd(uint32_t cmd) "Ignoring command 0x%" PRIx32
>   aspeed_sbc_handle_cmd(uint32_t cmd, uint32_t addr, bool ret) "Handling command 0x%" PRIx32 " for OTP addr 0x%" PRIx32 " Result: %d"
>   aspeed_sbc_otp_read(uint32_t addr, uint32_t value) "OTP Memory read: addr 0x%" PRIx32 " value 0x%" PRIx32
>   aspeed_sbc_otp_prog(uint32_t addr, uint32_t value) "OTP Memory write: addr 0x%" PRIx32 " value 0x%" PRIx32
> +aspeed_sbc_ecdsa_verify(const char *result) "ECDSA verify done: %s"
> +aspeed_sbc_ecdsa_hexdump(const char *desc, uint32_t offset, const char *s) "%s: 0x%08x: %s"
>   
>   # aspeed_scu.c
>   aspeed_scu_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64 " of size %u: 0x%" PRIx32
> diff --git a/include/hw/misc/aspeed_sbc.h b/include/hw/misc/aspeed_sbc.h
> index 07c7c22a86..9b04a10aed 100644
> --- a/include/hw/misc/aspeed_sbc.h
> +++ b/include/hw/misc/aspeed_sbc.h

Please add to your .git/config :

   [diff]
	  orderFile = /path/to/qemu/scripts/git.orderfile



> @@ -40,12 +40,17 @@ struct AspeedSBCState {
>       uint32_t regs[ASPEED_SBC_NR_REGS];
>   
>       AspeedOTPState otp;
> +
> +    MemoryRegion *sram;
> +    AddressSpace sram_as;
> +    uint64_t sram_base;

See comment on

https://lore.kernel.org/qemu-devel/[email protected]/#t

Thanks,

C.


>   };
>   
>   struct AspeedSBCClass {
>       SysBusDeviceClass parent_class;
>   
>       bool has_otp;
> +    bool has_ecdsa;
>   };
>   
>   #endif /* ASPEED_SBC_H */