Hello Troy, On Wed, 12 Jan 2022 at 08:10, Troy Lee <troy_...@aspeedtech.com> wrote: > > Accumulative mode will supply a initial state and append padding bit at > the end of hash stream. However, the crypto library will padding those > bit automatically, so ripped it off from iov array. > > The aspeed ast2600 acculumative mode is described in datasheet > ast2600v10.pdf section 25.6.4: > 1. Allocationg and initiating accumulative hash digest write buffer > with initial state. > * Since QEMU crypto/hash api doesn't provide the API to set initial > state of hash library, and the initial state is already setted by > crypto library (gcrypt/glib/...), so skip this step. > 2. Calculating accumulative hash digest. > (a) When receiving the last accumulative data, software need to add > padding message at the end of the accumulative data. Padding > message described in specific of MD5, SHA-1, SHA224, SHA256, > SHA512, SHA512/224, SHA512/256. > * Since the crypto library (gcrypt/glib) already pad the > padding message internally. > * This patch is to remove the padding message which fed byguest > machine driver.
I tested the latest aspeed SDK u-boot, loaded form mmc (with our mmc model that lives in Cedric's tree) and qemu crashed: #0 0x00007fe867d44932 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0 #1 0x0000557aba2b6e22 in qcrypto_glib_hash_bytesv (alg=<optimized out>, iov=0x7fe8662ee0b0, niov=1, result=0x7fe8662ee0a8, resultlen=0x7fe8662ee0a0, errp=0x0) at ../crypto/hash-glib.c:68 #2 0x0000557ab9f549ea in do_hash_operation (s=s@entry=0x7fe866e1b3b0, algo=5, sg_mode=sg_mode@entry=true, acc_mode=acc_mode@entry=true) at ../hw/misc/aspeed_hace.c:161 #3 0x0000557ab9f54dd1 in aspeed_hace_write (opaque=<optimized out>, addr=12, data=262504, size=<optimized out>) at ../hw/misc/aspeed_hace.c:260 WIthout your patch applied the HACE operation fails, as we do not have support for accumulative mode, but we do not crash. > > Changes in v2: > - Coding style > - Add accumulative mode description in comment > > Signed-off-by: Troy Lee <troy_...@aspeedtech.com> > --- > hw/misc/aspeed_hace.c | 43 ++++++++++++++++++++++++++++------- > include/hw/misc/aspeed_hace.h | 1 + > 2 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c > index 10f00e65f4..0710f44621 100644 > --- a/hw/misc/aspeed_hace.c > +++ b/hw/misc/aspeed_hace.c > @@ -11,6 +11,7 @@ > #include "qemu/osdep.h" > #include "qemu/log.h" > #include "qemu/error-report.h" > +#include "qemu/bswap.h" > #include "hw/misc/aspeed_hace.h" > #include "qapi/error.h" > #include "migration/vmstate.h" > @@ -27,6 +28,7 @@ > > #define R_HASH_SRC (0x20 / 4) > #define R_HASH_DEST (0x24 / 4) > +#define R_HASH_KEY_BUFF (0x28 / 4) > #define R_HASH_SRC_LEN (0x2c / 4) > > #define R_HASH_CMD (0x30 / 4) > @@ -94,7 +96,8 @@ static int hash_algo_lookup(uint32_t reg) > return -1; > } > > -static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode) > +static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode, > + bool acc_mode) > { > struct iovec iov[ASPEED_HACE_MAX_SG]; > g_autofree uint8_t *digest_buf; > @@ -103,6 +106,7 @@ static void do_hash_operation(AspeedHACEState *s, int > algo, bool sg_mode) > > if (sg_mode) { > uint32_t len = 0; > + uint32_t total_len = 0; > > for (i = 0; !(len & SG_LIST_LEN_LAST); i++) { > uint32_t addr, src; > @@ -123,10 +127,26 @@ static void do_hash_operation(AspeedHACEState *s, int > algo, bool sg_mode) > MEMTXATTRS_UNSPECIFIED, NULL); > addr &= SG_LIST_ADDR_MASK; > > - iov[i].iov_len = len & SG_LIST_LEN_MASK; > - plen = iov[i].iov_len; > + plen = len & SG_LIST_LEN_MASK; > iov[i].iov_base = address_space_map(&s->dram_as, addr, &plen, > false, > MEMTXATTRS_UNSPECIFIED); > + > + if (acc_mode) { > + total_len += plen; > + > + if (len & SG_LIST_LEN_LAST) { > + /* > + * In the padding message, the last 64/128 bit represents > + * the total length of bitstream in big endian. > + * SHA-224, SHA-256 are 64 bit > + * SHA-384, SHA-512, SHA-512/224, SHA-512/256 are 128 bit > + * However, we would not process such a huge bit stream. > + */ > + plen -= total_len - (ldq_be_p(iov[i].iov_base + plen - > 8) / 8); > + } > + } > + > + iov[i].iov_len = plen; > } > } else { > hwaddr len = s->regs[R_HASH_SRC_LEN]; > @@ -210,6 +230,9 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, > uint64_t data, > case R_HASH_DEST: > data &= ahc->dest_mask; > break; > + case R_HASH_KEY_BUFF: > + data &= ahc->key_mask; > + break; > case R_HASH_SRC_LEN: > data &= 0x0FFFFFFF; > break; > @@ -229,12 +252,13 @@ static void aspeed_hace_write(void *opaque, hwaddr > addr, uint64_t data, > } > algo = hash_algo_lookup(data); > if (algo < 0) { > - qemu_log_mask(LOG_GUEST_ERROR, > - "%s: Invalid hash algorithm selection 0x%"PRIx64"\n", > - __func__, data & ahc->hash_mask); > - break; > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: Invalid hash algorithm selection 0x%"PRIx64"\n", > + __func__, data & ahc->hash_mask); > + break; > } > - do_hash_operation(s, algo, data & HASH_SG_EN); > + do_hash_operation(s, algo, data & HASH_SG_EN, > + ((data & HASH_HMAC_MASK) == HASH_DIGEST_ACCUM)); > > if (data & HASH_IRQ_EN) { > qemu_irq_raise(s->irq); > @@ -333,6 +357,7 @@ static void aspeed_ast2400_hace_class_init(ObjectClass > *klass, void *data) > > ahc->src_mask = 0x0FFFFFFF; > ahc->dest_mask = 0x0FFFFFF8; > + ahc->key_mask = 0x0FFFFFC0; > ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */ > } > > @@ -351,6 +376,7 @@ static void aspeed_ast2500_hace_class_init(ObjectClass > *klass, void *data) > > ahc->src_mask = 0x3fffffff; > ahc->dest_mask = 0x3ffffff8; > + ahc->key_mask = 0x3FFFFFC0; > ahc->hash_mask = 0x000003ff; /* No SG or SHA512 modes */ > } > > @@ -369,6 +395,7 @@ static void aspeed_ast2600_hace_class_init(ObjectClass > *klass, void *data) > > ahc->src_mask = 0x7FFFFFFF; > ahc->dest_mask = 0x7FFFFFF8; > + ahc->key_mask = 0x7FFFFFF8; > ahc->hash_mask = 0x00147FFF; > } > > diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h > index 94d5ada95f..2242945eb4 100644 > --- a/include/hw/misc/aspeed_hace.h > +++ b/include/hw/misc/aspeed_hace.h > @@ -37,6 +37,7 @@ struct AspeedHACEClass { > > uint32_t src_mask; > uint32_t dest_mask; > + uint32_t key_mask; > uint32_t hash_mask; > }; > > -- > 2.25.1 >