All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions
@ 2014-02-25  9:32 Will Newton
  2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 1/2] include/qemu/crc32c.h: Rename include guards to match filename Will Newton
  2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
  0 siblings, 2 replies; 4+ messages in thread
From: Will Newton @ 2014-02-25  9:32 UTC (permalink / raw)
  To: qemu-devel

This series adds support for the AArch32 CRC32 instructions added in
ARMv8.

Will Newton (2):
  include/qemu/crc32c.h: Rename include guards to match filename
  target-arm: Add support for AArch32 ARMv8 CRC32 instructions

 configure              |  2 +-
 include/qemu/crc32c.h  |  4 ++--
 target-arm/helper.c    | 39 +++++++++++++++++++++++++++++++++++++++
 target-arm/helper.h    |  3 +++
 target-arm/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+), 3 deletions(-)

-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v3 1/2] include/qemu/crc32c.h: Rename include guards to match filename
  2014-02-25  9:32 [Qemu-devel] [PATCH v3 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
@ 2014-02-25  9:32 ` Will Newton
  2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
  1 sibling, 0 replies; 4+ messages in thread
From: Will Newton @ 2014-02-25  9:32 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Will Newton <will.newton@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/crc32c.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Changes in v3:
 - None

diff --git a/include/qemu/crc32c.h b/include/qemu/crc32c.h
index 56d1c3b..dafb6a1 100644
--- a/include/qemu/crc32c.h
+++ b/include/qemu/crc32c.h
@@ -25,8 +25,8 @@
  *
  */
 
-#ifndef QEMU_CRC32_H
-#define QEMU_CRC32_H
+#ifndef QEMU_CRC32C_H
+#define QEMU_CRC32C_H
 
 #include "qemu-common.h"
 
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions
  2014-02-25  9:32 [Qemu-devel] [PATCH v3 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
  2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 1/2] include/qemu/crc32c.h: Rename include guards to match filename Will Newton
@ 2014-02-25  9:32 ` Will Newton
  2014-02-25 19:49   ` Peter Maydell
  1 sibling, 1 reply; 4+ messages in thread
From: Will Newton @ 2014-02-25  9:32 UTC (permalink / raw)
  To: qemu-devel

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)
+
 #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;
                 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions
  2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
@ 2014-02-25 19:49   ` Peter Maydell
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2014-02-25 19:49 UTC (permalink / raw)
  To: Will Newton; +Cc: QEMU Developers

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-02-25 19:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-25  9:32 [Qemu-devel] [PATCH v3 0/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 1/2] include/qemu/crc32c.h: Rename include guards to match filename Will Newton
2014-02-25  9:32 ` [Qemu-devel] [PATCH v3 2/2] target-arm: Add support for AArch32 ARMv8 CRC32 instructions Will Newton
2014-02-25 19:49   ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.