[U-Boot] [PATCH 0/5 v4] Add ACE HW support for SHA 256

This patch set adds hardware acceleration for SHA 256 with the help of ACE.
Changes since v1: - Patch-1: Fixed few nits. - Patch-2: Removed not required config. - Patch-3: Added sha256 to hash command instead of new sha256 command.
Changes since v2: - Patch-1: - Added falling back to software sha256 in case length exceeds buffer limit. - Reduced one tab at lines 533, 559 and 571 in the patch. - Removed space after a cast at line 506 in the patch. - Removed blank line at line 561 in the patch. - Removed space before semicolon at line 576 in the patch. - Patch-2: - Added "SHA1" in the comment for config. - Patch-3: - Added new nodes for SHA1 and SHA256 in struct hash_algo for the case when ACE is enabled. - Added new declaration for function pointer hash_func_ws with different return type. - Patch-4: - New patch to enable config for hash command.
Changes since v3: - Patch-1: - Removed buffer limit since there are 2 regs for address hash_msg_size_high and low. That means buffer length could go upto 2^64 bits which is practically - Removed falling back to software sha256 because there is no buffer limit. - Removed "/ 4" to sha1 and sha256 lengths and added increment to 4 in for loop at line 573. - Timed out still kept to be 100 ms since this is enough for hardware to switch status to idle from busy. In case it couldn't that means h/w is faulty. - Patch-2: - Added "Acked-by: Simon Glass sjg@chromium.org". - Patch-3: - New patch. - Patch-4: - Changed command names to lower case in algo struct. - Added generic ace_sha config. - Patch-5: Added acked-by Simon Glass - Added new generic config for ace_sha to enable ace support in hash.c.
Akshay Saraswat (5): Exynos: Add hardware accelerated SHA 256 Exynos: config: Enable ACE HW for SHA 256 for Exynos gen: Change return type to int for hash_func_ws function pointer gen: Add ACE acceleration to hash Exynos: config: Enable hash command
Makefile | 1 + arch/arm/include/asm/arch-exynos/cpu.h | 4 + common/hash.c | 14 ++ drivers/crypto/Makefile | 47 +++++ drivers/crypto/ace_sfr.h | 310 +++++++++++++++++++++++++++++++++ drivers/crypto/ace_sha.c | 122 +++++++++++++ include/ace_sha.h | 42 +++++ include/configs/exynos5250-dt.h | 5 + include/hash.h | 2 +- include/sha1.h | 2 +- include/sha256.h | 2 +- lib/sha1.c | 4 +- lib/sha256.c | 4 +- 13 files changed, 554 insertions(+), 5 deletions(-) create mode 100644 drivers/crypto/Makefile create mode 100644 drivers/crypto/ace_sfr.h create mode 100644 drivers/crypto/ace_sha.c create mode 100644 include/ace_sha.h

SHA-256 and SHA-1 accelerated using ACE hardware.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com --- Changes since v1: - Moved code to drivers/crypto. - Fixed few other nits.
Changes since v2: - Added falling back to software sha256 in case length exceeds buffer limit. - Reduced one tab at lines 533, 559 and 571 in this patch. - Removed space after a cast at line 506 in this patch. - Removed blank line at line 561 in this patch. - Removed space before semicolon at line 576 in this patch.
Changes since v3: - Removed buffer limit since there are 2 regs for address hash_msg_size_high and low. That means buffer length could go upto 2^64 bits which is practically - Removed falling back to software sha256 because there is no buffer limit. - Removed "/ 4" to sha1 and sha256 lengths and added increment to 4 in for loop at line 573. - Timed out still kept to be 100 ms since this is enough for hardware to switch status to idle from busy. In case it couldn't that means h/w is faulty.
Makefile | 1 + arch/arm/include/asm/arch-exynos/cpu.h | 4 + drivers/crypto/Makefile | 47 +++++ drivers/crypto/ace_sfr.h | 310 +++++++++++++++++++++++++++++++++ drivers/crypto/ace_sha.c | 122 +++++++++++++ include/ace_sha.h | 42 +++++ 6 files changed, 526 insertions(+) create mode 100644 drivers/crypto/Makefile create mode 100644 drivers/crypto/ace_sfr.h create mode 100644 drivers/crypto/ace_sha.c create mode 100644 include/ace_sha.h
diff --git a/Makefile b/Makefile index fc18dd4..4c41130 100644 --- a/Makefile +++ b/Makefile @@ -272,6 +272,7 @@ LIBS-y += disk/libdisk.o LIBS-y += drivers/bios_emulator/libatibiosemu.o LIBS-y += drivers/block/libblock.o LIBS-$(CONFIG_BOOTCOUNT_LIMIT) += drivers/bootcount/libbootcount.o +LIBS-y += drivers/crypto/libcrypto.o LIBS-y += drivers/dma/libdma.o LIBS-y += drivers/fpga/libfpga.o LIBS-y += drivers/gpio/libgpio.o diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h index eb34422..2a20558 100644 --- a/arch/arm/include/asm/arch-exynos/cpu.h +++ b/arch/arm/include/asm/arch-exynos/cpu.h @@ -62,6 +62,7 @@ #define EXYNOS4_GPIO_PART4_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4_DP_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4_SPI_ISP_BASE DEVICE_NOT_AVAILABLE +#define EXYNOS4_ACE_SFR_BASE DEVICE_NOT_AVAILABLE
/* EXYNOS4X12 */ #define EXYNOS4X12_GPIO_PART3_BASE 0x03860000 @@ -95,6 +96,7 @@ #define EXYNOS4X12_I2S_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4X12_SPI_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4X12_SPI_ISP_BASE DEVICE_NOT_AVAILABLE +#define EXYNOS4X12_ACE_SFR_BASE DEVICE_NOT_AVAILABLE
/* EXYNOS5 Common*/ #define EXYNOS5_I2C_SPACING 0x10000 @@ -106,6 +108,7 @@ #define EXYNOS5_SWRESET 0x10040400 #define EXYNOS5_SYSREG_BASE 0x10050000 #define EXYNOS5_WATCHDOG_BASE 0x101D0000 +#define EXYNOS5_ACE_SFR_BASE 0x10830000 #define EXYNOS5_DMC_PHY0_BASE 0x10C00000 #define EXYNOS5_DMC_PHY1_BASE 0x10C10000 #define EXYNOS5_GPIO_PART3_BASE 0x10D10000 @@ -205,6 +208,7 @@ static inline unsigned int samsung_get_base_##device(void) \
SAMSUNG_BASE(adc, ADC_BASE) SAMSUNG_BASE(clock, CLOCK_BASE) +SAMSUNG_BASE(ace_sfr, ACE_SFR_BASE) SAMSUNG_BASE(dp, DP_BASE) SAMSUNG_BASE(sysreg, SYSREG_BASE) SAMSUNG_BASE(fimd, FIMD_BASE) diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile new file mode 100644 index 0000000..2c54793 --- /dev/null +++ b/drivers/crypto/Makefile @@ -0,0 +1,47 @@ +# +# Copyright (c) 2013 Samsung Electronics Co., Ltd. +# http://www.samsung.com +# +# See file CREDITS for list of people who contributed to this +# project. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307 USA +# + +include $(TOPDIR)/config.mk + +LIB := $(obj)libcrypto.o + +COBJS-$(CONFIG_EXYNOS_ACE_SHA) += ace_sha.o + +COBJS := $(COBJS-y) +SRCS := $(COBJS:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS)) + +all: $(LIB) + +$(LIB): $(obj).depend $(OBJS) + $(call cmd_link_o_target, $(OBJS)) + + +######################################################################### + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +######################################################################## diff --git a/drivers/crypto/ace_sfr.h b/drivers/crypto/ace_sfr.h new file mode 100644 index 0000000..8f1c893 --- /dev/null +++ b/drivers/crypto/ace_sfr.h @@ -0,0 +1,310 @@ +/* + * Header file for Advanced Crypto Engine - SFR definitions + * + * Copyright (c) 2012 Samsung Electronics + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef __ACE_SFR_H +#define __ACE_SFR_H + +struct exynos_ace_sfr { + unsigned int fc_intstat; /* base + 0 */ + unsigned int fc_intenset; + unsigned int fc_intenclr; + unsigned int fc_intpend; + unsigned int fc_fifostat; + unsigned int fc_fifoctrl; + unsigned int fc_global; + unsigned int res1; + unsigned int fc_brdmas; + unsigned int fc_brdmal; + unsigned int fc_brdmac; + unsigned int res2; + unsigned int fc_btdmas; + unsigned int fc_btdmal; + unsigned int fc_btdmac; + unsigned int res3; + unsigned int fc_hrdmas; + unsigned int fc_hrdmal; + unsigned int fc_hrdmac; + unsigned int res4; + unsigned int fc_pkdmas; + unsigned int fc_pkdmal; + unsigned int fc_pkdmac; + unsigned int fc_pkdmao; + unsigned char res5[0x1a0]; + + unsigned int aes_control; /* base + 0x200 */ + unsigned int aes_status; + unsigned char res6[0x8]; + unsigned int aes_in[4]; + unsigned int aes_out[4]; + unsigned int aes_iv[4]; + unsigned int aes_cnt[4]; + unsigned char res7[0x30]; + unsigned int aes_key[8]; + unsigned char res8[0x60]; + + unsigned int tdes_control; /* base + 0x300 */ + unsigned int tdes_status; + unsigned char res9[0x8]; + unsigned int tdes_key[6]; + unsigned int tdes_iv[2]; + unsigned int tdes_in[2]; + unsigned int tdes_out[2]; + unsigned char res10[0xc0]; + + unsigned int hash_control; /* base + 0x400 */ + unsigned int hash_control2; + unsigned int hash_fifo_mode; + unsigned int hash_byteswap; + unsigned int hash_status; + unsigned char res11[0xc]; + unsigned int hash_msgsize_low; + unsigned int hash_msgsize_high; + unsigned int hash_prelen_low; + unsigned int hash_prelen_high; + unsigned int hash_in[16]; + unsigned int hash_key_in[16]; + unsigned int hash_iv[8]; + unsigned char res12[0x30]; + unsigned int hash_result[8]; + unsigned char res13[0x20]; + unsigned int hash_seed[8]; + unsigned int hash_prng[8]; + unsigned char res14[0x180]; + + unsigned int pka_sfr[5]; /* base + 0x700 */ +}; + +/* ACE_FC_INT */ +#define ACE_FC_PKDMA (1 << 0) +#define ACE_FC_HRDMA (1 << 1) +#define ACE_FC_BTDMA (1 << 2) +#define ACE_FC_BRDMA (1 << 3) +#define ACE_FC_PRNG_ERROR (1 << 4) +#define ACE_FC_MSG_DONE (1 << 5) +#define ACE_FC_PRNG_DONE (1 << 6) +#define ACE_FC_PARTIAL_DONE (1 << 7) + +/* ACE_FC_FIFOSTAT */ +#define ACE_FC_PKFIFO_EMPTY (1 << 0) +#define ACE_FC_PKFIFO_FULL (1 << 1) +#define ACE_FC_HRFIFO_EMPTY (1 << 2) +#define ACE_FC_HRFIFO_FULL (1 << 3) +#define ACE_FC_BTFIFO_EMPTY (1 << 4) +#define ACE_FC_BTFIFO_FULL (1 << 5) +#define ACE_FC_BRFIFO_EMPTY (1 << 6) +#define ACE_FC_BRFIFO_FULL (1 << 7) + +/* ACE_FC_FIFOCTRL */ +#define ACE_FC_SELHASH_MASK (3 << 0) +#define ACE_FC_SELHASH_EXOUT (0 << 0) /* independent source */ +#define ACE_FC_SELHASH_BCIN (1 << 0) /* blk cipher input */ +#define ACE_FC_SELHASH_BCOUT (2 << 0) /* blk cipher output */ +#define ACE_FC_SELBC_MASK (1 << 2) +#define ACE_FC_SELBC_AES (0 << 2) /* AES */ +#define ACE_FC_SELBC_DES (1 << 2) /* DES */ + +/* ACE_FC_GLOBAL */ +#define ACE_FC_SSS_RESET (1 << 0) +#define ACE_FC_DMA_RESET (1 << 1) +#define ACE_FC_AES_RESET (1 << 2) +#define ACE_FC_DES_RESET (1 << 3) +#define ACE_FC_HASH_RESET (1 << 4) +#define ACE_FC_AXI_ENDIAN_MASK (3 << 6) +#define ACE_FC_AXI_ENDIAN_LE (0 << 6) +#define ACE_FC_AXI_ENDIAN_BIBE (1 << 6) +#define ACE_FC_AXI_ENDIAN_WIBE (2 << 6) + +/* Feed control - BRDMA control */ +#define ACE_FC_BRDMACFLUSH_OFF (0 << 0) +#define ACE_FC_BRDMACFLUSH_ON (1 << 0) +#define ACE_FC_BRDMACSWAP_ON (1 << 1) +#define ACE_FC_BRDMACARPROT_MASK (0x7 << 2) +#define ACE_FC_BRDMACARPROT_OFS (2) +#define ACE_FC_BRDMACARCACHE_MASK (0xf << 5) +#define ACE_FC_BRDMACARCACHE_OFS (5) + +/* Feed control - BTDMA control */ +#define ACE_FC_BTDMACFLUSH_OFF (0 << 0) +#define ACE_FC_BTDMACFLUSH_ON (1 << 0) +#define ACE_FC_BTDMACSWAP_ON (1 << 1) +#define ACE_FC_BTDMACAWPROT_MASK (0x7 << 2) +#define ACE_FC_BTDMACAWPROT_OFS (2) +#define ACE_FC_BTDMACAWCACHE_MASK (0xf << 5) +#define ACE_FC_BTDMACAWCACHE_OFS (5) + +/* Feed control - HRDMA control */ +#define ACE_FC_HRDMACFLUSH_OFF (0 << 0) +#define ACE_FC_HRDMACFLUSH_ON (1 << 0) +#define ACE_FC_HRDMACSWAP_ON (1 << 1) +#define ACE_FC_HRDMACARPROT_MASK (0x7 << 2) +#define ACE_FC_HRDMACARPROT_OFS (2) +#define ACE_FC_HRDMACARCACHE_MASK (0xf << 5) +#define ACE_FC_HRDMACARCACHE_OFS (5) + +/* Feed control - PKDMA control */ +#define ACE_FC_PKDMACBYTESWAP_ON (1 << 3) +#define ACE_FC_PKDMACDESEND_ON (1 << 2) +#define ACE_FC_PKDMACTRANSMIT_ON (1 << 1) +#define ACE_FC_PKDMACFLUSH_ON (1 << 0) + +/* Feed control - PKDMA offset */ +#define ACE_FC_SRAMOFFSET_MASK (0xfff) + +/* AES control */ +#define ACE_AES_MODE_MASK (1 << 0) +#define ACE_AES_MODE_ENC (0 << 0) +#define ACE_AES_MODE_DEC (1 << 0) +#define ACE_AES_OPERMODE_MASK (3 << 1) +#define ACE_AES_OPERMODE_ECB (0 << 1) +#define ACE_AES_OPERMODE_CBC (1 << 1) +#define ACE_AES_OPERMODE_CTR (2 << 1) +#define ACE_AES_FIFO_MASK (1 << 3) +#define ACE_AES_FIFO_OFF (0 << 3) /* CPU mode */ +#define ACE_AES_FIFO_ON (1 << 3) /* FIFO mode */ +#define ACE_AES_KEYSIZE_MASK (3 << 4) +#define ACE_AES_KEYSIZE_128 (0 << 4) +#define ACE_AES_KEYSIZE_192 (1 << 4) +#define ACE_AES_KEYSIZE_256 (2 << 4) +#define ACE_AES_KEYCNGMODE_MASK (1 << 6) +#define ACE_AES_KEYCNGMODE_OFF (0 << 6) +#define ACE_AES_KEYCNGMODE_ON (1 << 6) +#define ACE_AES_SWAP_MASK (0x1f << 7) +#define ACE_AES_SWAPKEY_OFF (0 << 7) +#define ACE_AES_SWAPKEY_ON (1 << 7) +#define ACE_AES_SWAPCNT_OFF (0 << 8) +#define ACE_AES_SWAPCNT_ON (1 << 8) +#define ACE_AES_SWAPIV_OFF (0 << 9) +#define ACE_AES_SWAPIV_ON (1 << 9) +#define ACE_AES_SWAPDO_OFF (0 << 10) +#define ACE_AES_SWAPDO_ON (1 << 10) +#define ACE_AES_SWAPDI_OFF (0 << 11) +#define ACE_AES_SWAPDI_ON (1 << 11) +#define ACE_AES_COUNTERSIZE_MASK (3 << 12) +#define ACE_AES_COUNTERSIZE_128 (0 << 12) +#define ACE_AES_COUNTERSIZE_64 (1 << 12) +#define ACE_AES_COUNTERSIZE_32 (2 << 12) +#define ACE_AES_COUNTERSIZE_16 (3 << 12) + +/* AES status */ +#define ACE_AES_OUTRDY_MASK (1 << 0) +#define ACE_AES_OUTRDY_OFF (0 << 0) +#define ACE_AES_OUTRDY_ON (1 << 0) +#define ACE_AES_INRDY_MASK (1 << 1) +#define ACE_AES_INRDY_OFF (0 << 1) +#define ACE_AES_INRDY_ON (1 << 1) +#define ACE_AES_BUSY_MASK (1 << 2) +#define ACE_AES_BUSY_OFF (0 << 2) +#define ACE_AES_BUSY_ON (1 << 2) + +/* TDES control */ +#define ACE_TDES_MODE_MASK (1 << 0) +#define ACE_TDES_MODE_ENC (0 << 0) +#define ACE_TDES_MODE_DEC (1 << 0) +#define ACE_TDES_OPERMODE_MASK (1 << 1) +#define ACE_TDES_OPERMODE_ECB (0 << 1) +#define ACE_TDES_OPERMODE_CBC (1 << 1) +#define ACE_TDES_SEL_MASK (3 << 3) +#define ACE_TDES_SEL_DES (0 << 3) +#define ACE_TDES_SEL_TDESEDE (1 << 3) /* TDES EDE mode */ +#define ACE_TDES_SEL_TDESEEE (3 << 3) /* TDES EEE mode */ +#define ACE_TDES_FIFO_MASK (1 << 5) +#define ACE_TDES_FIFO_OFF (0 << 5) /* CPU mode */ +#define ACE_TDES_FIFO_ON (1 << 5) /* FIFO mode */ +#define ACE_TDES_SWAP_MASK (0xf << 6) +#define ACE_TDES_SWAPKEY_OFF (0 << 6) +#define ACE_TDES_SWAPKEY_ON (1 << 6) +#define ACE_TDES_SWAPIV_OFF (0 << 7) +#define ACE_TDES_SWAPIV_ON (1 << 7) +#define ACE_TDES_SWAPDO_OFF (0 << 8) +#define ACE_TDES_SWAPDO_ON (1 << 8) +#define ACE_TDES_SWAPDI_OFF (0 << 9) +#define ACE_TDES_SWAPDI_ON (1 << 9) + +/* TDES status */ +#define ACE_TDES_OUTRDY_MASK (1 << 0) +#define ACE_TDES_OUTRDY_OFF (0 << 0) +#define ACE_TDES_OUTRDY_ON (1 << 0) +#define ACE_TDES_INRDY_MASK (1 << 1) +#define ACE_TDES_INRDY_OFF (0 << 1) +#define ACE_TDES_INRDY_ON (1 << 1) +#define ACE_TDES_BUSY_MASK (1 << 2) +#define ACE_TDES_BUSY_OFF (0 << 2) +#define ACE_TDES_BUSY_ON (1 << 2) + +/* Hash control */ +#define ACE_HASH_ENGSEL_MASK (0xf << 0) +#define ACE_HASH_ENGSEL_SHA1HASH (0x0 << 0) +#define ACE_HASH_ENGSEL_SHA1HMAC (0x1 << 0) +#define ACE_HASH_ENGSEL_SHA1HMACIN (0x1 << 0) +#define ACE_HASH_ENGSEL_SHA1HMACOUT (0x9 << 0) +#define ACE_HASH_ENGSEL_MD5HASH (0x2 << 0) +#define ACE_HASH_ENGSEL_MD5HMAC (0x3 << 0) +#define ACE_HASH_ENGSEL_MD5HMACIN (0x3 << 0) +#define ACE_HASH_ENGSEL_MD5HMACOUT (0xb << 0) +#define ACE_HASH_ENGSEL_SHA256HASH (0x4 << 0) +#define ACE_HASH_ENGSEL_SHA256HMAC (0x5 << 0) +#define ACE_HASH_ENGSEL_PRNG (0x8 << 0) +#define ACE_HASH_STARTBIT_ON (1 << 4) +#define ACE_HASH_USERIV_EN (1 << 5) + +/* Hash control 2 */ +#define ACE_HASH_PAUSE_ON (1 << 0) + +/* Hash control - FIFO mode */ +#define ACE_HASH_FIFO_MASK (1 << 0) +#define ACE_HASH_FIFO_OFF (0 << 0) +#define ACE_HASH_FIFO_ON (1 << 0) + +/* Hash control - byte swap */ +#define ACE_HASH_SWAP_MASK (0xf << 0) +#define ACE_HASH_SWAPKEY_OFF (0 << 0) +#define ACE_HASH_SWAPKEY_ON (1 << 0) +#define ACE_HASH_SWAPIV_OFF (0 << 1) +#define ACE_HASH_SWAPIV_ON (1 << 1) +#define ACE_HASH_SWAPDO_OFF (0 << 2) +#define ACE_HASH_SWAPDO_ON (1 << 2) +#define ACE_HASH_SWAPDI_OFF (0 << 3) +#define ACE_HASH_SWAPDI_ON (1 << 3) + +/* Hash status */ +#define ACE_HASH_BUFRDY_MASK (1 << 0) +#define ACE_HASH_BUFRDY_OFF (0 << 0) +#define ACE_HASH_BUFRDY_ON (1 << 0) +#define ACE_HASH_SEEDSETTING_MASK (1 << 1) +#define ACE_HASH_SEEDSETTING_OFF (0 << 1) +#define ACE_HASH_SEEDSETTING_ON (1 << 1) +#define ACE_HASH_PRNGBUSY_MASK (1 << 2) +#define ACE_HASH_PRNGBUSY_OFF (0 << 2) +#define ACE_HASH_PRNGBUSY_ON (1 << 2) +#define ACE_HASH_PARTIALDONE_MASK (1 << 4) +#define ACE_HASH_PARTIALDONE_OFF (0 << 4) +#define ACE_HASH_PARTIALDONE_ON (1 << 4) +#define ACE_HASH_PRNGDONE_MASK (1 << 5) +#define ACE_HASH_PRNGDONE_OFF (0 << 5) +#define ACE_HASH_PRNGDONE_ON (1 << 5) +#define ACE_HASH_MSGDONE_MASK (1 << 6) +#define ACE_HASH_MSGDONE_OFF (0 << 6) +#define ACE_HASH_MSGDONE_ON (1 << 6) +#define ACE_HASH_PRNGERROR_MASK (1 << 7) +#define ACE_HASH_PRNGERROR_OFF (0 << 7) +#define ACE_HASH_PRNGERROR_ON (1 << 7) + +#endif diff --git a/drivers/crypto/ace_sha.c b/drivers/crypto/ace_sha.c new file mode 100644 index 0000000..d33e405 --- /dev/null +++ b/drivers/crypto/ace_sha.c @@ -0,0 +1,122 @@ +/* + * Advanced Crypto Engine - SHA Firmware + * Copyright (c) 2012 Samsung Electronics + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ +#include <common.h> +#include <sha256.h> +#include <sha1.h> +#include <ace_sha.h> +#include <asm/errno.h> +#include "ace_sfr.h" + +/* + * Timeout limit in case h/w fails. + * operation should not spen more time than this. + */ +#define TIMEOUT_MS 100 + +/* SHA1 value for the message of zero length */ +static const unsigned char sha1_digest_emptymsg[SHA1_SUM_LEN] = { + 0xDA, 0x39, 0xA3, 0xEE, 0x5E, 0x6B, 0x4B, 0x0D, + 0x32, 0x55, 0xBF, 0xFF, 0x95, 0x60, 0x18, 0x90, + 0xAF, 0xD8, 0x07, 0x09}; + +/* SHA256 value for the message of zero length */ +static const unsigned char sha256_digest_emptymsg[SHA256_SUM_LEN] = { + 0xE3, 0xB0, 0xC4, 0x42, 0x98, 0xFC, 0x1C, 0x14, + 0x9A, 0xFB, 0xF4, 0xC8, 0x99, 0x6F, 0xB9, 0x24, + 0x27, 0xAE, 0x41, 0xE4, 0x64, 0x9B, 0x93, 0x4C, + 0xA4, 0x95, 0x99, 0x1B, 0x78, 0x52, 0xB8, 0x55}; + +int ace_sha_hash_digest(const unsigned char *pbuf, unsigned int buf_len, + unsigned char *pout, unsigned int hash_type) +{ + unsigned int i, reg, len; + unsigned int *pdigest; + ulong start; + struct exynos_ace_sfr *ace_sha_reg = + (struct exynos_ace_sfr *)samsung_get_base_ace_sfr(); + + if (buf_len == 0) { + /* ACE H/W cannot compute hash value for empty string */ + if (hash_type == ACE_SHA_TYPE_SHA1) + memcpy(pout, sha1_digest_emptymsg, SHA1_SUM_LEN); + else + memcpy(pout, sha256_digest_emptymsg, SHA256_SUM_LEN); + return 0; + } + + /* Flush HRDMA */ + writel(ACE_FC_HRDMACFLUSH_ON, &ace_sha_reg->fc_hrdmac); + writel(ACE_FC_HRDMACFLUSH_OFF, &ace_sha_reg->fc_hrdmac); + + /* Set byte swap of data in */ + writel(ACE_HASH_SWAPDI_ON | ACE_HASH_SWAPDO_ON | ACE_HASH_SWAPIV_ON, + &ace_sha_reg->hash_byteswap); + + /* Select Hash input mux as external source */ + reg = readl(&ace_sha_reg->fc_fifoctrl); + reg = (reg & ~ACE_FC_SELHASH_MASK) | ACE_FC_SELHASH_EXOUT; + writel(reg, &ace_sha_reg->fc_fifoctrl); + + /* Set Hash as SHA1 or SHA256 and start Hash engine */ + reg = (hash_type == ACE_SHA_TYPE_SHA1) ? + ACE_HASH_ENGSEL_SHA1HASH : ACE_HASH_ENGSEL_SHA256HASH; + reg |= ACE_HASH_STARTBIT_ON; + writel(reg, &ace_sha_reg->hash_control); + + /* Enable FIFO mode */ + writel(ACE_HASH_FIFO_ON, &ace_sha_reg->hash_fifo_mode); + + /* Set message length */ + writel(buf_len, &ace_sha_reg->hash_msgsize_low); + writel(0, &ace_sha_reg->hash_msgsize_high); + + /* Set HRDMA */ + writel((unsigned int)pbuf, &ace_sha_reg->fc_hrdmas); + writel(buf_len, &ace_sha_reg->fc_hrdmal); + + start = get_timer(0); + + while ((readl(&ace_sha_reg->hash_status) & ACE_HASH_MSGDONE_MASK) == + ACE_HASH_MSGDONE_OFF) { + if (get_timer(start) > TIMEOUT_MS) { + debug("%s: Timeout waiting for ACE\n", __func__); + return -ETIMEDOUT; + } + } + + /* Clear MSG_DONE bit */ + writel(ACE_HASH_MSGDONE_ON, &ace_sha_reg->hash_status); + + /* Read hash result */ + pdigest = (unsigned int *)pout; + len = (hash_type == ACE_SHA_TYPE_SHA1) ? SHA1_SUM_LEN : SHA256_SUM_LEN; + + for (i = 0; i < len; i += 4) { + pdigest[i] = readl(&ace_sha_reg->hash_result[i]); + pdigest[i + 1] = readl(&ace_sha_reg->hash_result[i + 1]); + pdigest[i + 2] = readl(&ace_sha_reg->hash_result[i + 2]); + pdigest[i + 3] = readl(&ace_sha_reg->hash_result[i + 3]); + } + + /* Clear HRDMA pending bit */ + writel(ACE_FC_HRDMA, &ace_sha_reg->fc_intpend); + + return 0; +} diff --git a/include/ace_sha.h b/include/ace_sha.h new file mode 100644 index 0000000..e005636 --- /dev/null +++ b/include/ace_sha.h @@ -0,0 +1,42 @@ +/* + * Header file for SHA firmware + * + * Copyright (c) 2012 Samsung Electronics + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ +#ifndef __ACE_FW_SHA1_H +#define __ACE_FW_SHA1_H + +#define ACE_SHA_TYPE_SHA1 1 +#define ACE_SHA_TYPE_SHA256 2 + +/** + * Computes hash value of input pbuf using ACE + * + * @param in_addr A pointer to the input buffer + * @param bufleni Byte length of input buffer + * @param out_addr A pointer to the output buffer. When complete + * 32 bytes are copied to pout[0]...pout[31]. Thus, a user + * should allocate at least 32 bytes at pOut in advance. + * @param hash_type SHA1 or SHA256 + * + * @return 0 on Success, -1 on Failure (Timeout) + */ +int ace_sha_hash_digest(const uchar *in_addr, uint buflen, + uchar *out_addr, uint hash_type); + +#endif

Hi Akshay,
On Tue, Mar 5, 2013 at 5:19 AM, Akshay Saraswat akshay.s@samsung.com wrote:
SHA-256 and SHA-1 accelerated using ACE hardware.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com
Just a nit below, probably a misunderstanding and my fault. Once that is fixed:
Acked-by: Simon Glass sjg@chromium.org
Changes since v1: - Moved code to drivers/crypto. - Fixed few other nits.
Changes since v2: - Added falling back to software sha256 in case length exceeds buffer limit. - Reduced one tab at lines 533, 559 and 571 in this patch. - Removed space after a cast at line 506 in this patch. - Removed blank line at line 561 in this patch. - Removed space before semicolon at line 576 in this patch.
Changes since v3: - Removed buffer limit since there are 2 regs for address hash_msg_size_high and low. That means buffer length could go upto 2^64 bits which is practically - Removed falling back to software sha256 because there is no buffer limit. - Removed "/ 4" to sha1 and sha256 lengths and added increment to 4 in for loop at line 573. - Timed out still kept to be 100 ms since this is enough for hardware to switch status to idle from busy. In case it couldn't that means h/w is faulty.
Makefile | 1 + arch/arm/include/asm/arch-exynos/cpu.h | 4 + drivers/crypto/Makefile | 47 +++++ drivers/crypto/ace_sfr.h | 310 +++++++++++++++++++++++++++++++++ drivers/crypto/ace_sha.c | 122 +++++++++++++ include/ace_sha.h | 42 +++++ 6 files changed, 526 insertions(+) create mode 100644 drivers/crypto/Makefile create mode 100644 drivers/crypto/ace_sfr.h create mode 100644 drivers/crypto/ace_sha.c create mode 100644 include/ace_sha.h
diff --git a/Makefile b/Makefile index fc18dd4..4c41130 100644 --- a/Makefile +++ b/Makefile @@ -272,6 +272,7 @@ LIBS-y += disk/libdisk.o LIBS-y += drivers/bios_emulator/libatibiosemu.o LIBS-y += drivers/block/libblock.o LIBS-$(CONFIG_BOOTCOUNT_LIMIT) += drivers/bootcount/libbootcount.o +LIBS-y += drivers/crypto/libcrypto.o LIBS-y += drivers/dma/libdma.o LIBS-y += drivers/fpga/libfpga.o LIBS-y += drivers/gpio/libgpio.o diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h index eb34422..2a20558 100644 --- a/arch/arm/include/asm/arch-exynos/cpu.h +++ b/arch/arm/include/asm/arch-exynos/cpu.h @@ -62,6 +62,7 @@ #define EXYNOS4_GPIO_PART4_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4_DP_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4_SPI_ISP_BASE DEVICE_NOT_AVAILABLE +#define EXYNOS4_ACE_SFR_BASE DEVICE_NOT_AVAILABLE
/* EXYNOS4X12 */ #define EXYNOS4X12_GPIO_PART3_BASE 0x03860000 @@ -95,6 +96,7 @@ #define EXYNOS4X12_I2S_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4X12_SPI_BASE DEVICE_NOT_AVAILABLE #define EXYNOS4X12_SPI_ISP_BASE DEVICE_NOT_AVAILABLE +#define EXYNOS4X12_ACE_SFR_BASE DEVICE_NOT_AVAILABLE
/* EXYNOS5 Common*/ #define EXYNOS5_I2C_SPACING 0x10000 @@ -106,6 +108,7 @@ #define EXYNOS5_SWRESET 0x10040400 #define EXYNOS5_SYSREG_BASE 0x10050000 #define EXYNOS5_WATCHDOG_BASE 0x101D0000 +#define EXYNOS5_ACE_SFR_BASE 0x10830000 #define EXYNOS5_DMC_PHY0_BASE 0x10C00000 #define EXYNOS5_DMC_PHY1_BASE 0x10C10000 #define EXYNOS5_GPIO_PART3_BASE 0x10D10000 @@ -205,6 +208,7 @@ static inline unsigned int samsung_get_base_##device(void) \
SAMSUNG_BASE(adc, ADC_BASE) SAMSUNG_BASE(clock, CLOCK_BASE) +SAMSUNG_BASE(ace_sfr, ACE_SFR_BASE) SAMSUNG_BASE(dp, DP_BASE) SAMSUNG_BASE(sysreg, SYSREG_BASE) SAMSUNG_BASE(fimd, FIMD_BASE) diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile new file mode 100644 index 0000000..2c54793 --- /dev/null +++ b/drivers/crypto/Makefile @@ -0,0 +1,47 @@ +# +# Copyright (c) 2013 Samsung Electronics Co., Ltd. +# http://www.samsung.com +# +# See file CREDITS for list of people who contributed to this +# project. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307 USA +#
+include $(TOPDIR)/config.mk
+LIB := $(obj)libcrypto.o
+COBJS-$(CONFIG_EXYNOS_ACE_SHA) += ace_sha.o
+COBJS := $(COBJS-y) +SRCS := $(COBJS:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS))
+all: $(LIB)
+$(LIB): $(obj).depend $(OBJS)
$(call cmd_link_o_target, $(OBJS))
+#########################################################################
+# defines $(obj).depend target +include $(SRCTREE)/rules.mk
+sinclude $(obj).depend
+######################################################################## diff --git a/drivers/crypto/ace_sfr.h b/drivers/crypto/ace_sfr.h new file mode 100644 index 0000000..8f1c893 --- /dev/null +++ b/drivers/crypto/ace_sfr.h @@ -0,0 +1,310 @@ +/*
- Header file for Advanced Crypto Engine - SFR definitions
- Copyright (c) 2012 Samsung Electronics
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
+#ifndef __ACE_SFR_H +#define __ACE_SFR_H
+struct exynos_ace_sfr {
unsigned int fc_intstat; /* base + 0 */
unsigned int fc_intenset;
unsigned int fc_intenclr;
unsigned int fc_intpend;
unsigned int fc_fifostat;
unsigned int fc_fifoctrl;
unsigned int fc_global;
unsigned int res1;
unsigned int fc_brdmas;
unsigned int fc_brdmal;
unsigned int fc_brdmac;
unsigned int res2;
unsigned int fc_btdmas;
unsigned int fc_btdmal;
unsigned int fc_btdmac;
unsigned int res3;
unsigned int fc_hrdmas;
unsigned int fc_hrdmal;
unsigned int fc_hrdmac;
unsigned int res4;
unsigned int fc_pkdmas;
unsigned int fc_pkdmal;
unsigned int fc_pkdmac;
unsigned int fc_pkdmao;
unsigned char res5[0x1a0];
unsigned int aes_control; /* base + 0x200 */
unsigned int aes_status;
unsigned char res6[0x8];
unsigned int aes_in[4];
unsigned int aes_out[4];
unsigned int aes_iv[4];
unsigned int aes_cnt[4];
unsigned char res7[0x30];
unsigned int aes_key[8];
unsigned char res8[0x60];
unsigned int tdes_control; /* base + 0x300 */
unsigned int tdes_status;
unsigned char res9[0x8];
unsigned int tdes_key[6];
unsigned int tdes_iv[2];
unsigned int tdes_in[2];
unsigned int tdes_out[2];
unsigned char res10[0xc0];
unsigned int hash_control; /* base + 0x400 */
unsigned int hash_control2;
unsigned int hash_fifo_mode;
unsigned int hash_byteswap;
unsigned int hash_status;
unsigned char res11[0xc];
unsigned int hash_msgsize_low;
unsigned int hash_msgsize_high;
unsigned int hash_prelen_low;
unsigned int hash_prelen_high;
unsigned int hash_in[16];
unsigned int hash_key_in[16];
unsigned int hash_iv[8];
unsigned char res12[0x30];
unsigned int hash_result[8];
unsigned char res13[0x20];
unsigned int hash_seed[8];
unsigned int hash_prng[8];
unsigned char res14[0x180];
unsigned int pka_sfr[5]; /* base + 0x700 */
+};
+/* ACE_FC_INT */ +#define ACE_FC_PKDMA (1 << 0) +#define ACE_FC_HRDMA (1 << 1) +#define ACE_FC_BTDMA (1 << 2) +#define ACE_FC_BRDMA (1 << 3) +#define ACE_FC_PRNG_ERROR (1 << 4) +#define ACE_FC_MSG_DONE (1 << 5) +#define ACE_FC_PRNG_DONE (1 << 6) +#define ACE_FC_PARTIAL_DONE (1 << 7)
+/* ACE_FC_FIFOSTAT */ +#define ACE_FC_PKFIFO_EMPTY (1 << 0) +#define ACE_FC_PKFIFO_FULL (1 << 1) +#define ACE_FC_HRFIFO_EMPTY (1 << 2) +#define ACE_FC_HRFIFO_FULL (1 << 3) +#define ACE_FC_BTFIFO_EMPTY (1 << 4) +#define ACE_FC_BTFIFO_FULL (1 << 5) +#define ACE_FC_BRFIFO_EMPTY (1 << 6) +#define ACE_FC_BRFIFO_FULL (1 << 7)
+/* ACE_FC_FIFOCTRL */ +#define ACE_FC_SELHASH_MASK (3 << 0) +#define ACE_FC_SELHASH_EXOUT (0 << 0) /* independent source */ +#define ACE_FC_SELHASH_BCIN (1 << 0) /* blk cipher input */ +#define ACE_FC_SELHASH_BCOUT (2 << 0) /* blk cipher output */ +#define ACE_FC_SELBC_MASK (1 << 2) +#define ACE_FC_SELBC_AES (0 << 2) /* AES */ +#define ACE_FC_SELBC_DES (1 << 2) /* DES */
+/* ACE_FC_GLOBAL */ +#define ACE_FC_SSS_RESET (1 << 0) +#define ACE_FC_DMA_RESET (1 << 1) +#define ACE_FC_AES_RESET (1 << 2) +#define ACE_FC_DES_RESET (1 << 3) +#define ACE_FC_HASH_RESET (1 << 4) +#define ACE_FC_AXI_ENDIAN_MASK (3 << 6) +#define ACE_FC_AXI_ENDIAN_LE (0 << 6) +#define ACE_FC_AXI_ENDIAN_BIBE (1 << 6) +#define ACE_FC_AXI_ENDIAN_WIBE (2 << 6)
+/* Feed control - BRDMA control */ +#define ACE_FC_BRDMACFLUSH_OFF (0 << 0) +#define ACE_FC_BRDMACFLUSH_ON (1 << 0) +#define ACE_FC_BRDMACSWAP_ON (1 << 1) +#define ACE_FC_BRDMACARPROT_MASK (0x7 << 2) +#define ACE_FC_BRDMACARPROT_OFS (2) +#define ACE_FC_BRDMACARCACHE_MASK (0xf << 5) +#define ACE_FC_BRDMACARCACHE_OFS (5)
+/* Feed control - BTDMA control */ +#define ACE_FC_BTDMACFLUSH_OFF (0 << 0) +#define ACE_FC_BTDMACFLUSH_ON (1 << 0) +#define ACE_FC_BTDMACSWAP_ON (1 << 1) +#define ACE_FC_BTDMACAWPROT_MASK (0x7 << 2) +#define ACE_FC_BTDMACAWPROT_OFS (2) +#define ACE_FC_BTDMACAWCACHE_MASK (0xf << 5) +#define ACE_FC_BTDMACAWCACHE_OFS (5)
+/* Feed control - HRDMA control */ +#define ACE_FC_HRDMACFLUSH_OFF (0 << 0) +#define ACE_FC_HRDMACFLUSH_ON (1 << 0) +#define ACE_FC_HRDMACSWAP_ON (1 << 1) +#define ACE_FC_HRDMACARPROT_MASK (0x7 << 2) +#define ACE_FC_HRDMACARPROT_OFS (2) +#define ACE_FC_HRDMACARCACHE_MASK (0xf << 5) +#define ACE_FC_HRDMACARCACHE_OFS (5)
+/* Feed control - PKDMA control */ +#define ACE_FC_PKDMACBYTESWAP_ON (1 << 3) +#define ACE_FC_PKDMACDESEND_ON (1 << 2) +#define ACE_FC_PKDMACTRANSMIT_ON (1 << 1) +#define ACE_FC_PKDMACFLUSH_ON (1 << 0)
+/* Feed control - PKDMA offset */ +#define ACE_FC_SRAMOFFSET_MASK (0xfff)
+/* AES control */ +#define ACE_AES_MODE_MASK (1 << 0) +#define ACE_AES_MODE_ENC (0 << 0) +#define ACE_AES_MODE_DEC (1 << 0) +#define ACE_AES_OPERMODE_MASK (3 << 1) +#define ACE_AES_OPERMODE_ECB (0 << 1) +#define ACE_AES_OPERMODE_CBC (1 << 1) +#define ACE_AES_OPERMODE_CTR (2 << 1) +#define ACE_AES_FIFO_MASK (1 << 3) +#define ACE_AES_FIFO_OFF (0 << 3) /* CPU mode */ +#define ACE_AES_FIFO_ON (1 << 3) /* FIFO mode */ +#define ACE_AES_KEYSIZE_MASK (3 << 4) +#define ACE_AES_KEYSIZE_128 (0 << 4) +#define ACE_AES_KEYSIZE_192 (1 << 4) +#define ACE_AES_KEYSIZE_256 (2 << 4) +#define ACE_AES_KEYCNGMODE_MASK (1 << 6) +#define ACE_AES_KEYCNGMODE_OFF (0 << 6) +#define ACE_AES_KEYCNGMODE_ON (1 << 6) +#define ACE_AES_SWAP_MASK (0x1f << 7) +#define ACE_AES_SWAPKEY_OFF (0 << 7) +#define ACE_AES_SWAPKEY_ON (1 << 7) +#define ACE_AES_SWAPCNT_OFF (0 << 8) +#define ACE_AES_SWAPCNT_ON (1 << 8) +#define ACE_AES_SWAPIV_OFF (0 << 9) +#define ACE_AES_SWAPIV_ON (1 << 9) +#define ACE_AES_SWAPDO_OFF (0 << 10) +#define ACE_AES_SWAPDO_ON (1 << 10) +#define ACE_AES_SWAPDI_OFF (0 << 11) +#define ACE_AES_SWAPDI_ON (1 << 11) +#define ACE_AES_COUNTERSIZE_MASK (3 << 12) +#define ACE_AES_COUNTERSIZE_128 (0 << 12) +#define ACE_AES_COUNTERSIZE_64 (1 << 12) +#define ACE_AES_COUNTERSIZE_32 (2 << 12) +#define ACE_AES_COUNTERSIZE_16 (3 << 12)
+/* AES status */ +#define ACE_AES_OUTRDY_MASK (1 << 0) +#define ACE_AES_OUTRDY_OFF (0 << 0) +#define ACE_AES_OUTRDY_ON (1 << 0) +#define ACE_AES_INRDY_MASK (1 << 1) +#define ACE_AES_INRDY_OFF (0 << 1) +#define ACE_AES_INRDY_ON (1 << 1) +#define ACE_AES_BUSY_MASK (1 << 2) +#define ACE_AES_BUSY_OFF (0 << 2) +#define ACE_AES_BUSY_ON (1 << 2)
+/* TDES control */ +#define ACE_TDES_MODE_MASK (1 << 0) +#define ACE_TDES_MODE_ENC (0 << 0) +#define ACE_TDES_MODE_DEC (1 << 0) +#define ACE_TDES_OPERMODE_MASK (1 << 1) +#define ACE_TDES_OPERMODE_ECB (0 << 1) +#define ACE_TDES_OPERMODE_CBC (1 << 1) +#define ACE_TDES_SEL_MASK (3 << 3) +#define ACE_TDES_SEL_DES (0 << 3) +#define ACE_TDES_SEL_TDESEDE (1 << 3) /* TDES EDE mode */ +#define ACE_TDES_SEL_TDESEEE (3 << 3) /* TDES EEE mode */ +#define ACE_TDES_FIFO_MASK (1 << 5) +#define ACE_TDES_FIFO_OFF (0 << 5) /* CPU mode */ +#define ACE_TDES_FIFO_ON (1 << 5) /* FIFO mode */ +#define ACE_TDES_SWAP_MASK (0xf << 6) +#define ACE_TDES_SWAPKEY_OFF (0 << 6) +#define ACE_TDES_SWAPKEY_ON (1 << 6) +#define ACE_TDES_SWAPIV_OFF (0 << 7) +#define ACE_TDES_SWAPIV_ON (1 << 7) +#define ACE_TDES_SWAPDO_OFF (0 << 8) +#define ACE_TDES_SWAPDO_ON (1 << 8) +#define ACE_TDES_SWAPDI_OFF (0 << 9) +#define ACE_TDES_SWAPDI_ON (1 << 9)
+/* TDES status */ +#define ACE_TDES_OUTRDY_MASK (1 << 0) +#define ACE_TDES_OUTRDY_OFF (0 << 0) +#define ACE_TDES_OUTRDY_ON (1 << 0) +#define ACE_TDES_INRDY_MASK (1 << 1) +#define ACE_TDES_INRDY_OFF (0 << 1) +#define ACE_TDES_INRDY_ON (1 << 1) +#define ACE_TDES_BUSY_MASK (1 << 2) +#define ACE_TDES_BUSY_OFF (0 << 2) +#define ACE_TDES_BUSY_ON (1 << 2)
+/* Hash control */ +#define ACE_HASH_ENGSEL_MASK (0xf << 0) +#define ACE_HASH_ENGSEL_SHA1HASH (0x0 << 0) +#define ACE_HASH_ENGSEL_SHA1HMAC (0x1 << 0) +#define ACE_HASH_ENGSEL_SHA1HMACIN (0x1 << 0) +#define ACE_HASH_ENGSEL_SHA1HMACOUT (0x9 << 0) +#define ACE_HASH_ENGSEL_MD5HASH (0x2 << 0) +#define ACE_HASH_ENGSEL_MD5HMAC (0x3 << 0) +#define ACE_HASH_ENGSEL_MD5HMACIN (0x3 << 0) +#define ACE_HASH_ENGSEL_MD5HMACOUT (0xb << 0) +#define ACE_HASH_ENGSEL_SHA256HASH (0x4 << 0) +#define ACE_HASH_ENGSEL_SHA256HMAC (0x5 << 0) +#define ACE_HASH_ENGSEL_PRNG (0x8 << 0) +#define ACE_HASH_STARTBIT_ON (1 << 4) +#define ACE_HASH_USERIV_EN (1 << 5)
+/* Hash control 2 */ +#define ACE_HASH_PAUSE_ON (1 << 0)
+/* Hash control - FIFO mode */ +#define ACE_HASH_FIFO_MASK (1 << 0) +#define ACE_HASH_FIFO_OFF (0 << 0) +#define ACE_HASH_FIFO_ON (1 << 0)
+/* Hash control - byte swap */ +#define ACE_HASH_SWAP_MASK (0xf << 0) +#define ACE_HASH_SWAPKEY_OFF (0 << 0) +#define ACE_HASH_SWAPKEY_ON (1 << 0) +#define ACE_HASH_SWAPIV_OFF (0 << 1) +#define ACE_HASH_SWAPIV_ON (1 << 1) +#define ACE_HASH_SWAPDO_OFF (0 << 2) +#define ACE_HASH_SWAPDO_ON (1 << 2) +#define ACE_HASH_SWAPDI_OFF (0 << 3) +#define ACE_HASH_SWAPDI_ON (1 << 3)
+/* Hash status */ +#define ACE_HASH_BUFRDY_MASK (1 << 0) +#define ACE_HASH_BUFRDY_OFF (0 << 0) +#define ACE_HASH_BUFRDY_ON (1 << 0) +#define ACE_HASH_SEEDSETTING_MASK (1 << 1) +#define ACE_HASH_SEEDSETTING_OFF (0 << 1) +#define ACE_HASH_SEEDSETTING_ON (1 << 1) +#define ACE_HASH_PRNGBUSY_MASK (1 << 2) +#define ACE_HASH_PRNGBUSY_OFF (0 << 2) +#define ACE_HASH_PRNGBUSY_ON (1 << 2) +#define ACE_HASH_PARTIALDONE_MASK (1 << 4) +#define ACE_HASH_PARTIALDONE_OFF (0 << 4) +#define ACE_HASH_PARTIALDONE_ON (1 << 4) +#define ACE_HASH_PRNGDONE_MASK (1 << 5) +#define ACE_HASH_PRNGDONE_OFF (0 << 5) +#define ACE_HASH_PRNGDONE_ON (1 << 5) +#define ACE_HASH_MSGDONE_MASK (1 << 6) +#define ACE_HASH_MSGDONE_OFF (0 << 6) +#define ACE_HASH_MSGDONE_ON (1 << 6) +#define ACE_HASH_PRNGERROR_MASK (1 << 7) +#define ACE_HASH_PRNGERROR_OFF (0 << 7) +#define ACE_HASH_PRNGERROR_ON (1 << 7)
+#endif diff --git a/drivers/crypto/ace_sha.c b/drivers/crypto/ace_sha.c new file mode 100644 index 0000000..d33e405 --- /dev/null +++ b/drivers/crypto/ace_sha.c @@ -0,0 +1,122 @@ +/*
- Advanced Crypto Engine - SHA Firmware
- Copyright (c) 2012 Samsung Electronics
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
+#include <common.h> +#include <sha256.h> +#include <sha1.h> +#include <ace_sha.h> +#include <asm/errno.h> +#include "ace_sfr.h"
+/*
- Timeout limit in case h/w fails.
- operation should not spen more time than this.
- */
+#define TIMEOUT_MS 100
+/* SHA1 value for the message of zero length */ +static const unsigned char sha1_digest_emptymsg[SHA1_SUM_LEN] = {
0xDA, 0x39, 0xA3, 0xEE, 0x5E, 0x6B, 0x4B, 0x0D,
0x32, 0x55, 0xBF, 0xFF, 0x95, 0x60, 0x18, 0x90,
0xAF, 0xD8, 0x07, 0x09};
+/* SHA256 value for the message of zero length */ +static const unsigned char sha256_digest_emptymsg[SHA256_SUM_LEN] = {
0xE3, 0xB0, 0xC4, 0x42, 0x98, 0xFC, 0x1C, 0x14,
0x9A, 0xFB, 0xF4, 0xC8, 0x99, 0x6F, 0xB9, 0x24,
0x27, 0xAE, 0x41, 0xE4, 0x64, 0x9B, 0x93, 0x4C,
0xA4, 0x95, 0x99, 0x1B, 0x78, 0x52, 0xB8, 0x55};
+int ace_sha_hash_digest(const unsigned char *pbuf, unsigned int buf_len,
unsigned char *pout, unsigned int hash_type)
+{
unsigned int i, reg, len;
unsigned int *pdigest;
ulong start;
struct exynos_ace_sfr *ace_sha_reg =
(struct exynos_ace_sfr *)samsung_get_base_ace_sfr();
if (buf_len == 0) {
/* ACE H/W cannot compute hash value for empty string */
if (hash_type == ACE_SHA_TYPE_SHA1)
memcpy(pout, sha1_digest_emptymsg, SHA1_SUM_LEN);
else
memcpy(pout, sha256_digest_emptymsg, SHA256_SUM_LEN);
return 0;
}
/* Flush HRDMA */
writel(ACE_FC_HRDMACFLUSH_ON, &ace_sha_reg->fc_hrdmac);
writel(ACE_FC_HRDMACFLUSH_OFF, &ace_sha_reg->fc_hrdmac);
/* Set byte swap of data in */
writel(ACE_HASH_SWAPDI_ON | ACE_HASH_SWAPDO_ON | ACE_HASH_SWAPIV_ON,
&ace_sha_reg->hash_byteswap);
/* Select Hash input mux as external source */
reg = readl(&ace_sha_reg->fc_fifoctrl);
reg = (reg & ~ACE_FC_SELHASH_MASK) | ACE_FC_SELHASH_EXOUT;
writel(reg, &ace_sha_reg->fc_fifoctrl);
/* Set Hash as SHA1 or SHA256 and start Hash engine */
reg = (hash_type == ACE_SHA_TYPE_SHA1) ?
ACE_HASH_ENGSEL_SHA1HASH : ACE_HASH_ENGSEL_SHA256HASH;
reg |= ACE_HASH_STARTBIT_ON;
writel(reg, &ace_sha_reg->hash_control);
/* Enable FIFO mode */
writel(ACE_HASH_FIFO_ON, &ace_sha_reg->hash_fifo_mode);
/* Set message length */
writel(buf_len, &ace_sha_reg->hash_msgsize_low);
writel(0, &ace_sha_reg->hash_msgsize_high);
/* Set HRDMA */
writel((unsigned int)pbuf, &ace_sha_reg->fc_hrdmas);
writel(buf_len, &ace_sha_reg->fc_hrdmal);
start = get_timer(0);
while ((readl(&ace_sha_reg->hash_status) & ACE_HASH_MSGDONE_MASK) ==
ACE_HASH_MSGDONE_OFF) {
if (get_timer(start) > TIMEOUT_MS) {
debug("%s: Timeout waiting for ACE\n", __func__);
return -ETIMEDOUT;
}
}
/* Clear MSG_DONE bit */
writel(ACE_HASH_MSGDONE_ON, &ace_sha_reg->hash_status);
/* Read hash result */
pdigest = (unsigned int *)pout;
len = (hash_type == ACE_SHA_TYPE_SHA1) ? SHA1_SUM_LEN : SHA256_SUM_LEN;
for (i = 0; i < len; i += 4) {
pdigest[i] = readl(&ace_sha_reg->hash_result[i]);
pdigest[i + 1] = readl(&ace_sha_reg->hash_result[i + 1]);
pdigest[i + 2] = readl(&ace_sha_reg->hash_result[i + 2]);
pdigest[i + 3] = readl(&ace_sha_reg->hash_result[i + 3]);
}
Suggest:
for (i = 0; i < len / 4; i++)
pdigest[i] = readl(&ace_sha_reg->hash_result[i]);
/* Clear HRDMA pending bit */
writel(ACE_FC_HRDMA, &ace_sha_reg->fc_intpend);
return 0;
+} diff --git a/include/ace_sha.h b/include/ace_sha.h new file mode 100644 index 0000000..e005636 --- /dev/null +++ b/include/ace_sha.h @@ -0,0 +1,42 @@ +/*
- Header file for SHA firmware
- Copyright (c) 2012 Samsung Electronics
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
+#ifndef __ACE_FW_SHA1_H +#define __ACE_FW_SHA1_H
+#define ACE_SHA_TYPE_SHA1 1 +#define ACE_SHA_TYPE_SHA256 2
+/**
- Computes hash value of input pbuf using ACE
- @param in_addr A pointer to the input buffer
- @param bufleni Byte length of input buffer
- @param out_addr A pointer to the output buffer. When complete
32 bytes are copied to pout[0]...pout[31]. Thus, a user
should allocate at least 32 bytes at pOut in advance.
- @param hash_type SHA1 or SHA256
- @return 0 on Success, -1 on Failure (Timeout)
- */
+int ace_sha_hash_digest(const uchar *in_addr, uint buflen,
uchar *out_addr, uint hash_type);
+#endif
1.8.0
Regards, Simon

This enables SHA 256 for exynos.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com Acked-by: Simon Glass sjg@chromium.org --- Changes since v1: - Removed not required config.
Changes sice v2: - Added "SHA1" in the comment for config.
Changes sice v3: - Added "Acked-by: Simon Glass sjg@chromium.org".
include/configs/exynos5250-dt.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index cabd2f2..0b43984 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -45,6 +45,9 @@ /* Keep L2 Cache Disabled */ #define CONFIG_SYS_DCACHE_OFF
+/* Enable ACE acceleration for SHA1 and SHA256 */ +#define CONFIG_EXYNOS_ACE_SHA + #define CONFIG_SYS_SDRAM_BASE 0x40000000 #define CONFIG_SYS_TEXT_BASE 0x43E00000

This patch changes return type for hash_func_ws function pointer to int because hash.c is the host for all the crypto algorithm requests. Ace h/w acceleration as of now and many more to be included in future may require a type to convey the result ("encoding success or failure") to the caller.
Signed-off-by: Akshay Saraswat akshay.s@samsung.com --- Changes since v3: - New patch.
include/hash.h | 2 +- include/sha1.h | 2 +- include/sha256.h | 2 +- lib/sha1.c | 4 +++- lib/sha256.c | 4 +++- 5 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/hash.h b/include/hash.h index 34ba558..3d7200e 100644 --- a/include/hash.h +++ b/include/hash.h @@ -40,7 +40,7 @@ struct hash_algo { * @output: Checksum result (length depends on algorithm) * @chunk_sz: Trigger watchdog after processing this many bytes */ - void (*hash_func_ws)(const unsigned char *input, unsigned int ilen, + int (*hash_func_ws)(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz); int chunk_size; /* Watchdog chunk size */ }; diff --git a/include/sha1.h b/include/sha1.h index da09dab..3f45b3c 100644 --- a/include/sha1.h +++ b/include/sha1.h @@ -88,7 +88,7 @@ void sha1_csum(const unsigned char *input, unsigned int ilen, * \param output SHA-1 checksum result * \param chunk_sz watchdog triggering period (in bytes of input processed) */ -void sha1_csum_wd(const unsigned char *input, unsigned int ilen, +int sha1_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz);
/** diff --git a/include/sha256.h b/include/sha256.h index beadab3..07b80c8 100644 --- a/include/sha256.h +++ b/include/sha256.h @@ -16,7 +16,7 @@ void sha256_starts(sha256_context * ctx); void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length); void sha256_finish(sha256_context * ctx, uint8_t digest[SHA256_SUM_LEN]);
-void sha256_csum_wd(const unsigned char *input, unsigned int ilen, +int sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz);
#endif /* _SHA256_H */ diff --git a/lib/sha1.c b/lib/sha1.c index a121224..f272c48 100644 --- a/lib/sha1.c +++ b/lib/sha1.c @@ -320,7 +320,7 @@ void sha1_csum(const unsigned char *input, unsigned int ilen, * Output = SHA-1( input buffer ). Trigger the watchdog every 'chunk_sz' * bytes of input processed. */ -void sha1_csum_wd(const unsigned char *input, unsigned int ilen, +int sha1_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha1_context ctx; @@ -347,6 +347,8 @@ void sha1_csum_wd(const unsigned char *input, unsigned int ilen, #endif
sha1_finish (&ctx, output); + + return 0; }
/* diff --git a/lib/sha256.c b/lib/sha256.c index ab2db48..ea16d03 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -265,7 +265,7 @@ void sha256_finish(sha256_context * ctx, uint8_t digest[32]) * Output = SHA-256( input buffer ). Trigger the watchdog every 'chunk_sz' * bytes of input processed. */ -void sha256_csum_wd(const unsigned char *input, unsigned int ilen, +int sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha256_context ctx; @@ -292,4 +292,6 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, #endif
sha256_finish(&ctx, output); + + return 0; }

Hi Akshay,
On Tue, Mar 5, 2013 at 5:19 AM, Akshay Saraswat akshay.s@samsung.com wrote:
This patch changes return type for hash_func_ws function pointer to int because hash.c is the host for all the crypto algorithm requests. Ace h/w acceleration as of now and many more to be included in future may require a type to convey the result ("encoding success or failure") to the caller.
Signed-off-by: Akshay Saraswat akshay.s@samsung.com
This looks OK except that unluckily mainline has just changed so that crc32 has been brought in also. Can you please update your patch to take account of this?
Regards, Simon
Changes since v3: - New patch.
include/hash.h | 2 +- include/sha1.h | 2 +- include/sha256.h | 2 +- lib/sha1.c | 4 +++- lib/sha256.c | 4 +++- 5 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/hash.h b/include/hash.h index 34ba558..3d7200e 100644 --- a/include/hash.h +++ b/include/hash.h @@ -40,7 +40,7 @@ struct hash_algo { * @output: Checksum result (length depends on algorithm) * @chunk_sz: Trigger watchdog after processing this many bytes */
void (*hash_func_ws)(const unsigned char *input, unsigned int ilen,
int (*hash_func_ws)(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz); int chunk_size; /* Watchdog chunk size */
}; diff --git a/include/sha1.h b/include/sha1.h index da09dab..3f45b3c 100644 --- a/include/sha1.h +++ b/include/sha1.h @@ -88,7 +88,7 @@ void sha1_csum(const unsigned char *input, unsigned int ilen,
- \param output SHA-1 checksum result
- \param chunk_sz watchdog triggering period (in bytes of input processed)
*/ -void sha1_csum_wd(const unsigned char *input, unsigned int ilen, +int sha1_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz);
/** diff --git a/include/sha256.h b/include/sha256.h index beadab3..07b80c8 100644 --- a/include/sha256.h +++ b/include/sha256.h @@ -16,7 +16,7 @@ void sha256_starts(sha256_context * ctx); void sha256_update(sha256_context *ctx, const uint8_t *input, uint32_t length); void sha256_finish(sha256_context * ctx, uint8_t digest[SHA256_SUM_LEN]);
-void sha256_csum_wd(const unsigned char *input, unsigned int ilen, +int sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz);
#endif /* _SHA256_H */ diff --git a/lib/sha1.c b/lib/sha1.c index a121224..f272c48 100644 --- a/lib/sha1.c +++ b/lib/sha1.c @@ -320,7 +320,7 @@ void sha1_csum(const unsigned char *input, unsigned int ilen,
- Output = SHA-1( input buffer ). Trigger the watchdog every 'chunk_sz'
- bytes of input processed.
*/ -void sha1_csum_wd(const unsigned char *input, unsigned int ilen, +int sha1_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha1_context ctx; @@ -347,6 +347,8 @@ void sha1_csum_wd(const unsigned char *input, unsigned int ilen, #endif
sha1_finish (&ctx, output);
return 0;
}
/* diff --git a/lib/sha256.c b/lib/sha256.c index ab2db48..ea16d03 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -265,7 +265,7 @@ void sha256_finish(sha256_context * ctx, uint8_t digest[32])
- Output = SHA-256( input buffer ). Trigger the watchdog every 'chunk_sz'
- bytes of input processed.
*/ -void sha256_csum_wd(const unsigned char *input, unsigned int ilen, +int sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz) { sha256_context ctx; @@ -292,4 +292,6 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, #endif
sha256_finish(&ctx, output);
return 0;
}
1.8.0

ACE H/W acceleration support is added to hash which can be used to test SHA 256 hash algorithm.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com --- Changes since v1: - Added sha256 support to "hash" command instead of new sha256 command.
Changes sice v2: - Added new nodes for SHA1 and SHA256 in struct hash_algo for the case when ACE is enabled. - Added new declaration for function pointer hash_func_ws with different return type.
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
common/hash.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/common/hash.c b/common/hash.c index e3a6e43..10da26d 100644 --- a/common/hash.c +++ b/common/hash.c @@ -28,12 +28,26 @@ #include <hash.h> #include <sha1.h> #include <sha256.h> +#include <ace_sha.h>
/* * These are the hash algorithms we support. Chips which support accelerated * crypto could perhaps add named version of these algorithms here. */ static struct hash_algo hash_algo[] = { +#ifdef CONFIG_ACE_SHA + { + "sha1", + SHA1_SUM_LEN, + ace_sha_hash_digest, + ACE_SHA_TYPE_SHA1, + }, { + "sha256", + SHA256_SUM_LEN, + ace_sha_hash_digest, + ACE_SHA_TYPE_SHA256, + }, +#endif #ifdef CONFIG_SHA1 { "SHA1",

On Tue, 5 Mar 2013 08:19:59 -0500 Akshay Saraswat akshay.s@samsung.com wrote:
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
patches 1,2,4,5 all contain this "tested with" text, yet obviously were not. It also indicates that a data buffer that's > 8MB was not tested?
I also asked about speed relative to software running on the core and didn't get a response. Software should be faster up to a certain buffer size: what is that threshold?
Changes sice v3:
- Changed command names to lower case in algo struct.
- Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
Kim

Hi Kim,
On Tue, Mar 5, 2013 at 2:43 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 08:19:59 -0500 Akshay Saraswat akshay.s@samsung.com wrote:
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
patches 1,2,4,5 all contain this "tested with" text, yet obviously were not. It also indicates that a data buffer that's > 8MB was not tested?
Would be useful to test a larger transfer.
I also asked about speed relative to software running on the core and didn't get a response. Software should be faster up to a certain buffer size: what is that threshold?
You can fairly easily do that by temporarily modifying your patch to use "acesha1" for the name, enable CONFIG_CMD_TIME, then you can compare the two with:
time hash sha1 <addr> <size> time hash acesha1 <addr> <size>
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be CONFIG_EXYNOS_ACE_SHA? Will the ACE unit not appear on any other SOC?
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
Regards, Simon
Kim
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
Hi Kim,
On Tue, Mar 5, 2013 at 2:43 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 08:19:59 -0500 Akshay Saraswat akshay.s@samsung.com wrote:
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
patches 1,2,4,5 all contain this "tested with" text, yet obviously were not. It also indicates that a data buffer that's > 8MB was not tested?
Would be useful to test a larger transfer.
esp. since it's now advertised.
I also asked about speed relative to software running on the core and didn't get a response. Software should be faster up to a certain buffer size: what is that threshold?
You can fairly easily do that by temporarily modifying your patch to use "acesha1" for the name, enable CONFIG_CMD_TIME, then you can compare the two with:
time hash sha1 <addr> <size> time hash acesha1 <addr> <size>
sure, but I don't have an ACE - that's why I asked.
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
CONFIG_EXYNOS_ACE_SHA? Will the ACE unit not appear on any other SOC?
my point is other SoCs can use the same entry in the array - there's nothing h/w-vendor or model-specific about it.
Something like CONFIG_HW_SHA{1,256} ought to do it.
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
the same priority that any h/w accelerated device would get - higher than that of software crypto.
Another question for Akshay wrt the timeout (since I never got a reply re: documentation): how can the h/w fault?
Kim
Regards, Simon
Kim
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
oh, and please stop full-quoting - I'm tired of looking at my scrollbar go by with no inline content.
Thanks,
Kim

Hi Kim,
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
[snip for Kim]
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be extended later when someone else has one.
CONFIG_EXYNOS_ACE_SHA? Will the ACE unit not appear on any other SOC?
my point is other SoCs can use the same entry in the array - there's nothing h/w-vendor or model-specific about it.
OK, know you of such an SOC?
Something like CONFIG_HW_SHA{1,256} ought to do it.
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
the same priority that any h/w accelerated device would get - higher than that of software crypto.
Another question for Akshay wrt the timeout (since I never got a reply re: documentation): how can the h/w fault?
Kim
oh, and please stop full-quoting - I'm tired of looking at my scrollbar go by with no inline content.
I will try harder. You could look at trying another mailer :-)
Thanks,
Kim
Regards, Simon

On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
[snip for Kim]
and others too, I hope.
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
extended later when someone else has one.
maybe, but we can try to avoid people copying existing code patterns, i.e. polluting common code when adding crypto routines for their h/w which are basically the same function declarations but with different names.
CONFIG_EXYNOS_ACE_SHA? Will the ACE unit not appear on any other SOC?
my point is other SoCs can use the same entry in the array - there's nothing h/w-vendor or model-specific about it.
OK, know you of such an SOC?
I'm not familiar with Samsung crypto products, and I can't become familiar either, judging by the state of their public documentation (if a company doesn't make their crypto unit documentation public, that only has to mean something's insecure - and not just through obscurity :).
A large majority of Freescale's PowerQUICC & QorIQ chips have crypto units. A lot of them have crypto as an option, so discovery has to be done at runtime (but we can add that later).
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Something like CONFIG_HW_SHA{1,256} ought to do it.
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
the same priority that any h/w accelerated device would get - higher than that of software crypto.
Another question for Akshay wrt the timeout (since I never got a reply re: documentation): how can the h/w fault?
Kim
oh, and please stop full-quoting - I'm tired of looking at my scrollbar go by with no inline content.
I will try harder. You could look at trying another mailer :-)
Thanks, I appreciate it. Gmail? just more clicking, no?
Kim

Hi Kim,
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
[snip for Kim]
and others too, I hope.
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
extended later when someone else has one.
maybe, but we can try to avoid people copying existing code patterns, i.e. polluting common code when adding crypto routines for their h/w which are basically the same function declarations but with different names.
Are you referring to adding code in into the hash algorithm table in hash.c? I specifically designed it so that people could add their algorithms in there. Once we have device model in place then I'm sure we can do better, but for now that's the U-Boot way.
CONFIG_EXYNOS_ACE_SHA? Will the ACE unit not appear on any other SOC?
my point is other SoCs can use the same entry in the array - there's nothing h/w-vendor or model-specific about it.
OK, know you of such an SOC?
I'm not familiar with Samsung crypto products, and I can't become familiar either, judging by the state of their public documentation (if a company doesn't make their crypto unit documentation public, that only has to mean something's insecure - and not just through obscurity :).
A large majority of Freescale's PowerQUICC & QorIQ chips have crypto units. A lot of them have crypto as an option, so discovery has to be done at runtime (but we can add that later).
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
Something like CONFIG_HW_SHA{1,256} ought to do it.
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
the same priority that any h/w accelerated device would get - higher than that of software crypto.
Another question for Akshay wrt the timeout (since I never got a reply re: documentation): how can the h/w fault?
[...]
Regards, Simon
Kim

On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
> Changes sice v3: > - Changed command names to lower case in algo struct. > - Added generic ace_sha config.
I wouldn't call "ace" a generic name - crypto units other than ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
extended later when someone else has one.
maybe, but we can try to avoid people copying existing code patterns, i.e. polluting common code when adding crypto routines for their h/w which are basically the same function declarations but with different names.
Are you referring to adding code in into the hash algorithm table in hash.c? I specifically designed it so that people could add their
yes.
algorithms in there. Once we have device model in place then I'm sure we can do better, but for now that's the U-Boot way.
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
common/hash.c and include/ace_sha.h should have all occurrences of 'ace' and 'ACE' replaced with something like 'hw' or 'accel'...including the ace_sha.h filename itself.
Since it's probably common enough, the 0-length handler could move into the common code, and be enabled iff the hw-accelerated config is set.
Also, can we try to leave the existing void return type for the hash functions for the rest of u-boot. What do you think about changing common/hash.c to do the s/w fallback if h/w failed instead of in the driver(s)?
Something like CONFIG_HW_SHA{1,256} ought to do it.
But I don't think crypto units other than ACE will use the code in this patch - it is intended to implement ACE support, and put it ahead of software support in terms of priority.
the same priority that any h/w accelerated device would get - higher than that of software crypto.
Another question for Akshay wrt the timeout (since I never got a reply re: documentation): how can the h/w fault?
[...]
Regards, Simon
Kim
you're doing it again :)
Kim

Hi Kim,
On Thu, Mar 7, 2013 at 4:25 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote:
>> Changes sice v3: >> - Changed command names to lower case in algo struct. >> - Added generic ace_sha config. > > I wouldn't call "ace" a generic name - crypto units other than > ACE should be able to use this code.
I don't really understand this comment. A new CONFIG has been added, and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
That is in the design of the hash.c module. It is intended to permit insertion of different algorithm implementations. We could perhaps introduce a hash_register() function to deal with this, but that's the way it is at present.
extended later when someone else has one.
maybe, but we can try to avoid people copying existing code patterns, i.e. polluting common code when adding crypto routines for their h/w which are basically the same function declarations but with different names.
Are you referring to adding code in into the hash algorithm table in hash.c? I specifically designed it so that people could add their
yes.
algorithms in there. Once we have device model in place then I'm sure we can do better, but for now that's the U-Boot way.
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
Fair enough, and that might be a good idea, but that is an issue for the hash infrastructure. It would be good to get a second hardware accelerator upstream so we can judge where to take this.
If you have one in a Freescale SOC can I please request that you send some patches upstream and we can then evaluate how best to arrange the code.
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
common/hash.c and include/ace_sha.h should have all occurrences of 'ace' and 'ACE' replaced with something like 'hw' or 'accel'...including the ace_sha.h filename itself.
Since it's probably common enough, the 0-length handler could move into the common code, and be enabled iff the hw-accelerated config is set.
To be honest I think we are getting ahead of ourselves. We are dealing here with a patch to enable hardware acceleration in one SOC, with a few lines of changes to common code, changing it in a way that fits nicely with how hash.c was designed.
Also, can we try to leave the existing void return type for the hash functions for the rest of u-boot. What do you think about changing common/hash.c to do the s/w fallback if h/w failed instead of in the driver(s)?
Yes I was thinking about that - probably something for a follow-on patch though. I have just finished getting crc32 in, so will add it to the queue to think about. Patches welcome.
Regards, Simon
[...]

On Thu, 7 Mar 2013 17:05:15 -0800 Simon Glass sjg@chromium.org wrote:
On Thu, Mar 7, 2013 at 4:25 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 17:51:00 -0800 Simon Glass sjg@chromium.org wrote: > >> Changes sice v3: > >> - Changed command names to lower case in algo struct. > >> - Added generic ace_sha config. > > > > I wouldn't call "ace" a generic name - crypto units other than > > ACE should be able to use this code. > > I don't really understand this comment. A new CONFIG has been added, > and this is specific to that unit. Are you suggesting that it be
right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
That is in the design of the hash.c module. It is intended to permit insertion of different algorithm implementations. We could perhaps introduce a hash_register() function to deal with this, but that's the way it is at present.
ok, well this is the first time a new alternate algorithm implementation is being posted, and it's bringing up a flaw in the design - no vendor-specific stuff in common areas.
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
Fair enough, and that might be a good idea, but that is an issue for the hash infrastructure. It would be good to get a second hardware accelerator upstream so we can judge where to take this.
well right now as it stands the 2nd h/w accelerator would have to duplicate hash entry point function signatures, just changing the ACE in the name to that of its h/w, and then spin on a tough decision: what priority does my h/w have vs. Samsung's ACE??
If you have one in a Freescale SOC can I please request that you send some patches upstream and we can then evaluate how best to arrange the code.
they'll come, eventually I hope. Other than the driver internals, the only thing different from the ACE functional capacity provided in this patchseries for the analogous Freescale parts is that the hash infrastructure would need to be adapted for runtime detection of the crypto unit's existence.
But let's not use this as an excuse to let things slide, please.
this is new infrastructure, right? It's common to make mistakes without seeing the entire picture, and this patchset represents the next piece.
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
common/hash.c and include/ace_sha.h should have all occurrences of 'ace' and 'ACE' replaced with something like 'hw' or 'accel'...including the ace_sha.h filename itself.
Since it's probably common enough, the 0-length handler could move into the common code, and be enabled iff the hw-accelerated config is set.
To be honest I think we are getting ahead of ourselves. We are dealing here with a patch to enable hardware acceleration in one SOC, with a few lines of changes to common code, changing it in a way that fits nicely with how hash.c was designed.
sorry, I disagree: This is a brand new driver class that's being added, and the design enforces adding h/w vendor specific symbols in common code. This tells me the design is broken. Net doesn't do this - why should crypto get away with it? Plus, as I explain above, it's not that hard to fix (well, for now at least): just change the common ace parts to have a generic name.
Kim

Hi Kim,
On Thu, Mar 7, 2013 at 6:18 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Thu, 7 Mar 2013 17:05:15 -0800 Simon Glass sjg@chromium.org wrote:
On Thu, Mar 7, 2013 at 4:25 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote: > On Tue, 5 Mar 2013 17:51:00 -0800 > Simon Glass sjg@chromium.org wrote: >> >> Changes sice v3: >> >> - Changed command names to lower case in algo struct. >> >> - Added generic ace_sha config. >> > >> > I wouldn't call "ace" a generic name - crypto units other than >> > ACE should be able to use this code. >> >> I don't really understand this comment. A new CONFIG has been added, >> and this is specific to that unit. Are you suggesting that it be > > right, and that's the problem - it needn't be specific to that unit.
Really? I think here we have a patch for an ACE unit, and currently the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
That is in the design of the hash.c module. It is intended to permit insertion of different algorithm implementations. We could perhaps introduce a hash_register() function to deal with this, but that's the way it is at present.
ok, well this is the first time a new alternate algorithm implementation is being posted, and it's bringing up a flaw in the design - no vendor-specific stuff in common areas.
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
Fair enough, and that might be a good idea, but that is an issue for the hash infrastructure. It would be good to get a second hardware accelerator upstream so we can judge where to take this.
well right now as it stands the 2nd h/w accelerator would have to duplicate hash entry point function signatures, just changing the ACE in the name to that of its h/w, and then spin on a tough decision: what priority does my h/w have vs. Samsung's ACE??
I thought you said that only one h/w accelerator would be used on a board?
If you have one in a Freescale SOC can I please request that you send some patches upstream and we can then evaluate how best to arrange the code.
they'll come, eventually I hope. Other than the driver internals, the only thing different from the ACE functional capacity provided in this patchseries for the analogous Freescale parts is that the hash infrastructure would need to be adapted for runtime detection of the crypto unit's existence.
But let's not use this as an excuse to let things slide, please.
this is new infrastructure, right? It's common to make mistakes without seeing the entire picture, and this patchset represents the next piece.
I would prefer to invent new infrastructure when we have two users, not one, otherwise we run the risk of over-engineering. I already hit a code size snag with crc32 integration. This is just the first user and there is not yet a clear picture of what other users will want. If you are saying that Freescale will need things to be done a particular way, please post the patches and we can take a look at something that fits both SOCs.
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
common/hash.c and include/ace_sha.h should have all occurrences of 'ace' and 'ACE' replaced with something like 'hw' or 'accel'...including the ace_sha.h filename itself.
Since it's probably common enough, the 0-length handler could move into the common code, and be enabled iff the hw-accelerated config is set.
To be honest I think we are getting ahead of ourselves. We are dealing here with a patch to enable hardware acceleration in one SOC, with a few lines of changes to common code, changing it in a way that fits nicely with how hash.c was designed.
sorry, I disagree: This is a brand new driver class that's being added, and the design enforces adding h/w vendor specific symbols in common code. This tells me the design is broken. Net doesn't do this - why should crypto get away with it? Plus, as I explain above, it's not that hard to fix (well, for now at least): just change the common ace parts to have a generic name.
I'm willing to come up with patches to move to a hash_register() type implementation which will allow us to address this shortcoming - but people will need to understand that it will increase code size a little more. I was intending to wait for device model, but I don't mind doing something in the interim. Then I will happily adjust this driver to use that new mechanism. In the meantime, it would help if you could post some Freescale patches for us to compare / review.
Regards. Simon

On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
Hi Kim,
On Thu, Mar 7, 2013 at 6:18 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Thu, 7 Mar 2013 17:05:15 -0800 Simon Glass sjg@chromium.org wrote:
On Thu, Mar 7, 2013 at 4:25 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 5 Mar 2013 22:22:09 -0800 Simon Glass sjg@chromium.org wrote:
> On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips kim.phillips@freescale.com wrote: > > On Tue, 5 Mar 2013 17:51:00 -0800 > > Simon Glass sjg@chromium.org wrote: > >> >> Changes sice v3: > >> >> - Changed command names to lower case in algo struct. > >> >> - Added generic ace_sha config. > >> > > >> > I wouldn't call "ace" a generic name - crypto units other than > >> > ACE should be able to use this code. > >> > >> I don't really understand this comment. A new CONFIG has been added, > >> and this is specific to that unit. Are you suggesting that it be > > > > right, and that's the problem - it needn't be specific to that unit. > > Really? I think here we have a patch for an ACE unit, and currently > the only implementation is in an Exynos chip. It can easily be
so make the ace_sha.o build depend on whichever level of chip config applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No need for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs since some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
That is in the design of the hash.c module. It is intended to permit insertion of different algorithm implementations. We could perhaps introduce a hash_register() function to deal with this, but that's the way it is at present.
ok, well this is the first time a new alternate algorithm implementation is being posted, and it's bringing up a flaw in the design - no vendor-specific stuff in common areas.
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
Fair enough, and that might be a good idea, but that is an issue for the hash infrastructure. It would be good to get a second hardware accelerator upstream so we can judge where to take this.
well right now as it stands the 2nd h/w accelerator would have to duplicate hash entry point function signatures, just changing the ACE in the name to that of its h/w, and then spin on a tough decision: what priority does my h/w have vs. Samsung's ACE??
I thought you said that only one h/w accelerator would be used on a board?
yes, I was trying to be funny, but as usual, it didn't work.
If you have one in a Freescale SOC can I please request that you send some patches upstream and we can then evaluate how best to arrange the code.
they'll come, eventually I hope. Other than the driver internals, the only thing different from the ACE functional capacity provided in this patchseries for the analogous Freescale parts is that the hash infrastructure would need to be adapted for runtime detection of the crypto unit's existence.
But let's not use this as an excuse to let things slide, please.
this is new infrastructure, right? It's common to make mistakes without seeing the entire picture, and this patchset represents the next piece.
I would prefer to invent new infrastructure when we have two users, not one, otherwise we run the risk of over-engineering. I already hit a code size snag with crc32 integration. This is just the first user and there is not yet a clear picture of what other users will want. If you are saying that Freescale will need things to be done a particular way, please post the patches and we can take a look at something that fits both SOCs.
I'm saying that - at least with the current patchseries as-is - other crypto vendors would have to touch common code.
Freescale would need runtime device detection, but that's completely orthogonal to this patchseries.
The primary objective here is to not add h/w vendor dependencies in common areas when they can easily be avoided.
Please can you point specifically to the lines of code you are wanting changed?
common/hash.c and include/ace_sha.h should have all occurrences of 'ace' and 'ACE' replaced with something like 'hw' or 'accel'...including the ace_sha.h filename itself.
Since it's probably common enough, the 0-length handler could move into the common code, and be enabled iff the hw-accelerated config is set.
To be honest I think we are getting ahead of ourselves. We are dealing here with a patch to enable hardware acceleration in one SOC, with a few lines of changes to common code, changing it in a way that fits nicely with how hash.c was designed.
sorry, I disagree: This is a brand new driver class that's being added, and the design enforces adding h/w vendor specific symbols in common code. This tells me the design is broken. Net doesn't do this - why should crypto get away with it? Plus, as I explain above, it's not that hard to fix (well, for now at least): just change the common ace parts to have a generic name.
I'm willing to come up with patches to move to a hash_register() type implementation which will allow us to address this shortcoming - but people will need to understand that it will increase code size a little more. I was intending to wait for device model, but I don't mind doing something in the interim. Then I will happily adjust this driver to use that new mechanism. In the meantime, it would help if you could post some Freescale patches for us to compare / review.
see above.
Kim

Hi Kim,
On Mon, Mar 11, 2013 at 5:44 PM, Kim Phillips kim.phillips@freescale.comwrote:
On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
Hi Kim,
On Thu, Mar 7, 2013 at 6:18 PM, Kim Phillips kim.phillips@freescale.com
wrote:
On Thu, 7 Mar 2013 17:05:15 -0800 Simon Glass sjg@chromium.org wrote:
On Thu, Mar 7, 2013 at 4:25 PM, Kim Phillips <
kim.phillips@freescale.com> wrote:
On Wed, 6 Mar 2013 18:08:21 -0800 Simon Glass sjg@chromium.org wrote:
On Wed, Mar 6, 2013 at 5:22 PM, Kim Phillips <
kim.phillips@freescale.com> wrote:
> On Tue, 5 Mar 2013 22:22:09 -0800 > Simon Glass sjg@chromium.org wrote: > >> On Tue, Mar 5, 2013 at 9:04 PM, Kim Phillips <
kim.phillips@freescale.com> wrote:
>> > On Tue, 5 Mar 2013 17:51:00 -0800 >> > Simon Glass sjg@chromium.org wrote: >> >> >> Changes sice v3: >> >> >> - Changed command names to lower case in algo
struct.
>> >> >> - Added generic ace_sha config. >> >> > >> >> > I wouldn't call "ace" a generic name - crypto units other
than
>> >> > ACE should be able to use this code. >> >> >> >> I don't really understand this comment. A new CONFIG has
been added,
>> >> and this is specific to that unit. Are you suggesting that
it be
>> > >> > right, and that's the problem - it needn't be specific to
that unit.
>> >> Really? I think here we have a patch for an ACE unit, and
currently
>> the only implementation is in an Exynos chip. It can easily be > > so make the ace_sha.o build depend on whichever level of chip
config
> applies - CONFIG_S5P, CONFIG_EXYNOS5, or CONFIG_SMDK5250. No
need
> for a new define specifically for ACE, right?
No, the ACE may appear in multiple chips, and anyway we may want to enable it or disable it. Drivers tend to have their own configs
since
some boards want to use (for example) USB, crypto, mmc, and some don't.
ok, if you really need the ACE define, restrict it to platform code and the driver, but not common code.
That is in the design of the hash.c module. It is intended to permit insertion of different algorithm implementations. We could perhaps introduce a hash_register() function to deal with this, but that's the way it is at present.
ok, well this is the first time a new alternate algorithm implementation is being posted, and it's bringing up a flaw in the design - no vendor-specific stuff in common areas.
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
Fine with me. The effect is the same - this is just a rename. Should not be done in the ace.h file though, only in the naming of the functions called from hash.c, right?
the problem is there are only two algorithms for all - the accelerated, and the non-. Presumably we get the acceleration for free, so we always will prefer to use the accelerated version, and if it doesn't work, then the core s/w implementation. The common/hash.c infrastructure currently doesn't support that: it assumes the existence of multiple heterogeneous crypto units will exist on a single u-boot instance, each used depending on its priority, which is not the case.
Fair enough, and that might be a good idea, but that is an issue for the hash infrastructure. It would be good to get a second hardware accelerator upstream so we can judge where to take this.
well right now as it stands the 2nd h/w accelerator would have to duplicate hash entry point function signatures, just changing the ACE in the name to that of its h/w, and then spin on a tough decision: what priority does my h/w have vs. Samsung's ACE??
I thought you said that only one h/w accelerator would be used on a
board?
yes, I was trying to be funny, but as usual, it didn't work.
OK, sorry I missed it.
If you have one in a Freescale SOC can I please request that you send some patches upstream and we can then evaluate how best to arrange the code.
they'll come, eventually I hope. Other than the driver internals, the only thing different from the ACE functional capacity provided in this patchseries for the analogous Freescale parts is that the hash infrastructure would need to be adapted for runtime detection of the crypto unit's existence.
But let's not use this as an excuse to let things slide, please.
this is new infrastructure, right? It's common to make mistakes without seeing the entire picture, and this patchset represents the next piece.
I would prefer to invent new infrastructure when we have two users, not one, otherwise we run the risk of over-engineering. I already hit a code size snag with crc32 integration. This is just the first user and there is not yet a clear picture of what other users will want. If you are saying that Freescale will need things to be done a particular way, please post the patches and we can take a look at something that fits both SOCs.
I'm saying that - at least with the current patchseries as-is - other crypto vendors would have to touch common code.
Yes.
Freescale would need runtime device detection, but that's completely orthogonal to this patchseries.
You can use device tree - CONFIG_OF_CONTROL - in fact ACE could move to that also.
[snip]
Regards, Simon

On Mon, 11 Mar 2013 17:53:37 -0700 Simon Glass sjg@chromium.org wrote:
On Mon, Mar 11, 2013 at 5:44 PM, Kim Phillips kim.phillips@freescale.comwrote:
On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
Fine with me. The effect is the same - this is just a rename. Should not be done in the ace.h file though, only in the naming of the functions called from hash.c, right?
the ace_sha_hash_digest() declaration should be removed from ace_sha.h (it's only needed by the driver, which is ok without it being there). ACE_SHA_TYPE_SHA* definitions belong in the driver too - they're ACE h/w-specific. Rename the filename ace_sha.h to hw_sha.h, and all remaining ACE references contained in it.
If the 'hw_' nomenclature is undesired, other possibilities are 'accel_', 'hw_accel_', 'alt_'...
Kim

Hi Kim,
On Tue, Mar 12, 2013 at 12:32 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Mon, 11 Mar 2013 17:53:37 -0700 Simon Glass sjg@chromium.org wrote:
On Mon, Mar 11, 2013 at 5:44 PM, Kim Phillips kim.phillips@freescale.comwrote:
On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
Fine with me. The effect is the same - this is just a rename. Should not be done in the ace.h file though, only in the naming of the functions called from hash.c, right?
the ace_sha_hash_digest() declaration should be removed from ace_sha.h (it's only needed by the driver, which is ok without it being there). ACE_SHA_TYPE_SHA* definitions belong in the driver too - they're ACE h/w-specific. Rename the filename ace_sha.h to hw_sha.h, and all remaining ACE references contained in it.
Maybe I misunderstand - are you saying that that all the ACE symbols in the driver should be renamed to hw? That doesn't make a lot of sense to me - why is this needed?
If the 'hw_' nomenclature is undesired, other possibilities are 'accel_', 'hw_accel_', 'alt_'...
Kim
Regards, Simon

On Tue, 12 Mar 2013 16:40:38 -0700 Simon Glass sjg@chromium.org wrote:
Hi Kim,
On Tue, Mar 12, 2013 at 12:32 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Mon, 11 Mar 2013 17:53:37 -0700 Simon Glass sjg@chromium.org wrote:
On Mon, Mar 11, 2013 at 5:44 PM, Kim Phillips kim.phillips@freescale.comwrote:
On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
Fine with me. The effect is the same - this is just a rename. Should not be done in the ace.h file though, only in the naming of the functions called from hash.c, right?
the ace_sha_hash_digest() declaration should be removed from ace_sha.h (it's only needed by the driver, which is ok without it being there). ACE_SHA_TYPE_SHA* definitions belong in the driver too - they're ACE h/w-specific. Rename the filename ace_sha.h to hw_sha.h, and all remaining ACE references contained in it.
Maybe I misunderstand - are you saying that that all the ACE symbols in the driver should be renamed to hw? That doesn't make a lot of sense to me - why is this needed?
no, only the entry points to the driver should be renamed - they are currently called ace_sha{1,256}(). The include/ace_sha.h file as it is currently can be made completely h/w agnostic.
Kim

Hi Kim,
On Tue, Mar 12, 2013 at 5:03 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Tue, 12 Mar 2013 16:40:38 -0700 Simon Glass sjg@chromium.org wrote:
Hi Kim,
On Tue, Mar 12, 2013 at 12:32 PM, Kim Phillips kim.phillips@freescale.com wrote:
On Mon, 11 Mar 2013 17:53:37 -0700 Simon Glass sjg@chromium.org wrote:
On Mon, Mar 11, 2013 at 5:44 PM, Kim Phillips kim.phillips@freescale.comwrote:
On Thu, 7 Mar 2013 19:11:16 -0800 Simon Glass sjg@chromium.org wrote:
OK so let's look at adding the hash_register() idea. But not in this series. That should come later in a revision of the hash.c infrastructure, since it needs to adjust sha1, sha255 and crc32.
I don't understand: why not s/ace/hw/g in common/ and include/ on this patchseries, then move straight to the device model at some later point? It's a compromise, but it works fine for the time being - other vendors can add their hash support without having to touch common code, code size is not affected, etc.
Fine with me. The effect is the same - this is just a rename. Should not be done in the ace.h file though, only in the naming of the functions called from hash.c, right?
the ace_sha_hash_digest() declaration should be removed from ace_sha.h (it's only needed by the driver, which is ok without it being there). ACE_SHA_TYPE_SHA* definitions belong in the driver too - they're ACE h/w-specific. Rename the filename ace_sha.h to hw_sha.h, and all remaining ACE references contained in it.
Maybe I misunderstand - are you saying that that all the ACE symbols in the driver should be renamed to hw? That doesn't make a lot of sense to me - why is this needed?
no, only the entry points to the driver should be renamed - they are currently called ace_sha{1,256}(). The include/ace_sha.h file as it is currently can be made completely h/w agnostic.
OK, that sounds better.
Regards, Simon
Kim

Hi Akshay,
On Tue, Mar 5, 2013 at 5:19 AM, Akshay Saraswat akshay.s@samsung.com wrote:
ACE H/W acceleration support is added to hash which can be used to test SHA 256 hash algorithm.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com
Changes since v1: - Added sha256 support to "hash" command instead of new sha256 command.
Changes sice v2: - Added new nodes for SHA1 and SHA256 in struct hash_algo for the case when ACE is enabled. - Added new declaration for function pointer hash_func_ws with different return type.
Changes sice v3: - Changed command names to lower case in algo struct. - Added generic ace_sha config.
common/hash.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/common/hash.c b/common/hash.c index e3a6e43..10da26d 100644 --- a/common/hash.c +++ b/common/hash.c @@ -28,12 +28,26 @@ #include <hash.h> #include <sha1.h> #include <sha256.h> +#include <ace_sha.h>
/*
- These are the hash algorithms we support. Chips which support accelerated
- crypto could perhaps add named version of these algorithms here.
*/ static struct hash_algo hash_algo[] = { +#ifdef CONFIG_ACE_SHA
{
"sha1",
SHA1_SUM_LEN,
ace_sha_hash_digest,
ACE_SHA_TYPE_SHA1,
This should be CHUNKSZ_SHA1 I think. You can't reuse this field for anything you want :-)
}, {
"sha256",
SHA256_SUM_LEN,
ace_sha_hash_digest,
ACE_SHA_TYPE_SHA256,
Similar here.
},
+#endif #ifdef CONFIG_SHA1 { "SHA1", -- 1.8.0
Regards, Simon

This enables hash command.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com --- Changes since v2: - New patch to enable config for hash command.
Changes since v3: - Added new generic config for ace_sha to enable ace support in hash.c.
include/configs/exynos5250-dt.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 0b43984..7e1acbd 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -47,6 +47,7 @@
/* Enable ACE acceleration for SHA1 and SHA256 */ #define CONFIG_EXYNOS_ACE_SHA +#define CONFIG_ACE_SHA
#define CONFIG_SYS_SDRAM_BASE 0x40000000 #define CONFIG_SYS_TEXT_BASE 0x43E00000 @@ -116,6 +117,7 @@ #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT #define CONFIG_CMD_NET +#define CONFIG_CMD_HASH
#define CONFIG_BOOTDELAY 3 #define CONFIG_ZERO_BOOTDELAY_CHECK

On Tue, Mar 5, 2013 at 5:20 AM, Akshay Saraswat akshay.s@samsung.com wrote:
This enables hash command.
Tested with command "hash sha256 0x40008000 0x2B 0x40009000". Used mm and md to write a standard string to memory location 0x40008000 and ran the above command to verify the output.
Signed-off-by: ARUN MANKUZHI arun.m@samsung.com Signed-off-by: Akshay Saraswat akshay.s@samsung.com
Acked-by: Simon Glass sjg@chromium.org
Changes since v2: - New patch to enable config for hash command.
Changes since v3: - Added new generic config for ace_sha to enable ace support in hash.c.
include/configs/exynos5250-dt.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h index 0b43984..7e1acbd 100644 --- a/include/configs/exynos5250-dt.h +++ b/include/configs/exynos5250-dt.h @@ -47,6 +47,7 @@
/* Enable ACE acceleration for SHA1 and SHA256 */ #define CONFIG_EXYNOS_ACE_SHA +#define CONFIG_ACE_SHA
#define CONFIG_SYS_SDRAM_BASE 0x40000000 #define CONFIG_SYS_TEXT_BASE 0x43E00000 @@ -116,6 +117,7 @@ #define CONFIG_CMD_EXT2 #define CONFIG_CMD_FAT #define CONFIG_CMD_NET +#define CONFIG_CMD_HASH
#define CONFIG_BOOTDELAY 3
#define CONFIG_ZERO_BOOTDELAY_CHECK
1.8.0
participants (3)
-
Akshay Saraswat
-
Kim Phillips
-
Simon Glass