| Message ID | 1393320755-23906-3-git-send-email-will.newton@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 25 February 2014 09:32, Will Newton <will.newton@linaro.org> wrote: > Add support for AArch32 CRC32 and CRC32C instructions added in ARMv8. > The CRC32-C implementation used is the built-in qemu implementation > and The CRC-32 implementation is from zlib. This requires adding zlib > to LIBS to ensure it is linked for the linux-user binary. > > Signed-off-by: Will Newton <will.newton@linaro.org> > --- > configure | 2 +- > target-arm/helper.c | 39 +++++++++++++++++++++++++++++++++++++++ > target-arm/helper.h | 3 +++ > target-arm/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 91 insertions(+), 1 deletion(-) > > Changes in v3: > - Use extract32 to get register fields from instruction > > diff --git a/configure b/configure > index 4648117..822842c 100755 > --- a/configure > +++ b/configure > @@ -1550,7 +1550,7 @@ EOF > "Make sure to have the zlib libs and headers installed." > fi > fi > -libs_softmmu="$libs_softmmu -lz" > +LIBS="$LIBS -lz" > > ########################################## > # libseccomp check > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 5ae08c9..294cfaf 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -5,6 +5,8 @@ > #include "sysemu/arch_init.h" > #include "sysemu/sysemu.h" > #include "qemu/bitops.h" > +#include "qemu/crc32c.h" > +#include <zlib.h> /* For crc32 */ > > #ifndef CONFIG_USER_ONLY > static inline int get_phys_addr(CPUARMState *env, uint32_t address, > @@ -4468,3 +4470,40 @@ int arm_rmode_to_sf(int rmode) > } > return rmode; > } > + > +static void crc_init_buffer(uint8_t *buf, uint32_t val, uint32_t bytes) > +{ > + memset(buf, 0, 4); > + > + if (bytes == 1) { > + buf[0] = val & 0xff; > + } else if (bytes == 2) { > + buf[0] = val & 0xff; > + buf[1] = (val >> 8) & 0xff; > + } else { > + buf[0] = val & 0xff; > + buf[1] = (val >> 8) & 0xff; > + buf[2] = (val >> 16) & 0xff; > + buf[3] = (val >> 24) & 0xff; > + } > +} > + > +uint32_t HELPER(crc32)(uint32_t acc, uint32_t val, uint32_t bytes) > +{ > + uint8_t buf[4]; > + > + crc_init_buffer(buf, val, bytes); > + > + /* zlib crc32 converts the accumulator and output to one's complement. */ > + return crc32(acc ^ 0xffffffff, buf, bytes) ^ 0xffffffff; > +} > + > +uint32_t HELPER(crc32c)(uint32_t acc, uint32_t val, uint32_t bytes) > +{ > + uint8_t buf[4]; > + > + crc_init_buffer(buf, val, bytes); > + > + /* Linux crc32c converts the output to one's complement. */ > + return crc32c(acc, buf, bytes) ^ 0xffffffff; > +} > diff --git a/target-arm/helper.h b/target-arm/helper.h > index 951e6ad..fb92e53 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -494,6 +494,9 @@ DEF_HELPER_3(neon_qzip32, void, env, i32, i32) > DEF_HELPER_4(crypto_aese, void, env, i32, i32, i32) > DEF_HELPER_4(crypto_aesmc, void, env, i32, i32, i32) > > +DEF_HELPER_3(crc32, i32, i32, i32, i32) > +DEF_HELPER_3(crc32c, i32, i32, i32, i32) These helpers have no side effects and don't access TCG globals, so we can set the flags appropriately: DEF_HELPER_FLAGS_3(crc32, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) and similarly for crc32c. > + > #ifdef TARGET_AARCH64 > #include "helper-a64.h" > #endif > diff --git a/target-arm/translate.c b/target-arm/translate.c > index 782aab8..8e87869 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -7541,6 +7541,32 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) > store_reg(s, 14, tmp2); > gen_bx(s, tmp); > break; > + case 0x4: > + { > + /* crc32/crc32c */ > + uint32_t c = extract32(insn, 9, 1); > + > + /* size == 64 is UNPREDICTABLE but handle as UNDEFINED. */ > + if (op1 == 0x3) { > + goto illegal_op; > + } I think it would also be nice to UNDEF if the SBZ bits 11, 10 and 8 are not all zero. (This is a CONSTRAINED UNPREDICTABLE case and "instruction operates as if the bit had the /should-be/ value" is a valid implementation choice, but as a quality-of-implementation issue I think choosing to UNDEF is better.) > + > + rn = extract32(insn, 16, 4); > + rd = extract32(insn, 12, 4); > + > + tmp = load_reg(s, rn); > + tmp2 = load_reg(s, rm); > + tmp3 = tcg_const_i32(1 << op1); > + if (c) { > + gen_helper_crc32c(tmp, tmp, tmp2, tmp3); > + } else { > + gen_helper_crc32(tmp, tmp, tmp2, tmp3); > + } > + tcg_temp_free_i32(tmp2); > + tcg_temp_free_i32(tmp3); > + store_reg(s, rd, tmp); > + break; This looks good, but it's enabling the CRC instructions for all CPUs, and we only want to do that for CPUs which actually implement them. (They're optional even for v8 CPUs). You need to add a new ARM_FEATURE_CRC flag to the arm_features enum in cpu.h, set it in the cpu.c arm_any_initfn(), and test for it here. > + } > case 0x5: /* saturating add/subtract */ > ARCH(5TE); > rd = (insn >> 12) & 0xf; > @@ -9125,6 +9151,28 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw > case 0x18: /* clz */ > gen_helper_clz(tmp, tmp); > break; > + case 0x20: > + case 0x21: > + case 0x22: > + case 0x28: > + case 0x29: > + case 0x2a: > + { > + /* crc32/crc32c */ > + uint32_t sz = op & 0x3; > + uint32_t c = op & 0x8; > + > + tmp2 = load_reg(s, rm); > + tmp3 = tcg_const_i32(1 << sz); > + if (c) { > + gen_helper_crc32c(tmp, tmp, tmp2, tmp3); > + } else { > + gen_helper_crc32(tmp, tmp, tmp2, tmp3); > + } > + tcg_temp_free_i32(tmp2); > + tcg_temp_free_i32(tmp3); > + break; > + } Similarly this needs to check ARM_FEATURE_CRC. Patch looks good otherwise and passes my tests. thanks -- PMM
diff --git a/configure b/configure index 4648117..822842c 100755 --- a/configure +++ b/configure @@ -1550,7 +1550,7 @@ EOF "Make sure to have the zlib libs and headers installed." fi fi -libs_softmmu="$libs_softmmu -lz" +LIBS="$LIBS -lz" ########################################## # libseccomp check diff --git a/target-arm/helper.c b/target-arm/helper.c index 5ae08c9..294cfaf 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5,6 +5,8 @@ #include "sysemu/arch_init.h" #include "sysemu/sysemu.h" #include "qemu/bitops.h" +#include "qemu/crc32c.h" +#include <zlib.h> /* For crc32 */ #ifndef CONFIG_USER_ONLY static inline int get_phys_addr(CPUARMState *env, uint32_t address, @@ -4468,3 +4470,40 @@ int arm_rmode_to_sf(int rmode) } return rmode; } + +static void crc_init_buffer(uint8_t *buf, uint32_t val, uint32_t bytes) +{ + memset(buf, 0, 4); + + if (bytes == 1) { + buf[0] = val & 0xff; + } else if (bytes == 2) { + buf[0] = val & 0xff; + buf[1] = (val >> 8) & 0xff; + } else { + buf[0] = val & 0xff; + buf[1] = (val >> 8) & 0xff; + buf[2] = (val >> 16) & 0xff; + buf[3] = (val >> 24) & 0xff; + } +} + +uint32_t HELPER(crc32)(uint32_t acc, uint32_t val, uint32_t bytes) +{ + uint8_t buf[4]; + + crc_init_buffer(buf, val, bytes); + + /* zlib crc32 converts the accumulator and output to one's complement. */ + return crc32(acc ^ 0xffffffff, buf, bytes) ^ 0xffffffff; +} + +uint32_t HELPER(crc32c)(uint32_t acc, uint32_t val, uint32_t bytes) +{ + uint8_t buf[4]; + + crc_init_buffer(buf, val, bytes); + + /* Linux crc32c converts the output to one's complement. */ + return crc32c(acc, buf, bytes) ^ 0xffffffff; +} diff --git a/target-arm/helper.h b/target-arm/helper.h index 951e6ad..fb92e53 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -494,6 +494,9 @@ DEF_HELPER_3(neon_qzip32, void, env, i32, i32) DEF_HELPER_4(crypto_aese, void, env, i32, i32, i32) DEF_HELPER_4(crypto_aesmc, void, env, i32, i32, i32) +DEF_HELPER_3(crc32, i32, i32, i32, i32) +DEF_HELPER_3(crc32c, i32, i32, i32, i32) + #ifdef TARGET_AARCH64 #include "helper-a64.h" #endif diff --git a/target-arm/translate.c b/target-arm/translate.c index 782aab8..8e87869 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -7541,6 +7541,32 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s) store_reg(s, 14, tmp2); gen_bx(s, tmp); break; + case 0x4: + { + /* crc32/crc32c */ + uint32_t c = extract32(insn, 9, 1); + + /* size == 64 is UNPREDICTABLE but handle as UNDEFINED. */ + if (op1 == 0x3) { + goto illegal_op; + } + + rn = extract32(insn, 16, 4); + rd = extract32(insn, 12, 4); + + tmp = load_reg(s, rn); + tmp2 = load_reg(s, rm); + tmp3 = tcg_const_i32(1 << op1); + if (c) { + gen_helper_crc32c(tmp, tmp, tmp2, tmp3); + } else { + gen_helper_crc32(tmp, tmp, tmp2, tmp3); + } + tcg_temp_free_i32(tmp2); + tcg_temp_free_i32(tmp3); + store_reg(s, rd, tmp); + break; + } case 0x5: /* saturating add/subtract */ ARCH(5TE); rd = (insn >> 12) & 0xf; @@ -9125,6 +9151,28 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw case 0x18: /* clz */ gen_helper_clz(tmp, tmp); break; + case 0x20: + case 0x21: + case 0x22: + case 0x28: + case 0x29: + case 0x2a: + { + /* crc32/crc32c */ + uint32_t sz = op & 0x3; + uint32_t c = op & 0x8; + + tmp2 = load_reg(s, rm); + tmp3 = tcg_const_i32(1 << sz); + if (c) { + gen_helper_crc32c(tmp, tmp, tmp2, tmp3); + } else { + gen_helper_crc32(tmp, tmp, tmp2, tmp3); + } + tcg_temp_free_i32(tmp2); + tcg_temp_free_i32(tmp3); + break; + } default: goto illegal_op; }
Add support for AArch32 CRC32 and CRC32C instructions added in ARMv8. The CRC32-C implementation used is the built-in qemu implementation and The CRC-32 implementation is from zlib. This requires adding zlib to LIBS to ensure it is linked for the linux-user binary. Signed-off-by: Will Newton <will.newton@linaro.org> --- configure | 2 +- target-arm/helper.c | 39 +++++++++++++++++++++++++++++++++++++++ target-arm/helper.h | 3 +++ target-arm/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 1 deletion(-) Changes in v3: - Use extract32 to get register fields from instruction