[Qemu-devel] [PATCH 0/3] Update capstone module

[Qemu-devel] [PATCH 0/3] Update capstone module

[Richard Henderson]

There has recently been some good progress in the upstream
capstone repository, syncing the instruction sets from the
(further) upstream llvm.

In particular, there are Power9 and System z13 instructions.
Both of which were missing from our current snapshot and from
our (ancient) binutils opcodes snapshots.


r~


Richard Henderson (3):
  capstone: Adjust include of capstone.h
  capstone: Update to master
  capstone: Enable disassembly for s390x

 Makefile                 |  1 +
 include/disas/capstone.h |  4 ++++
 disas.c                  | 40 ++++++++++++++++++++++++++++++++++++++++
 target/s390x/cpu.c       |  4 ++++
 capstone                 |  2 +-
 configure                |  7 +++++++
 6 files changed, 57 insertions(+), 1 deletion(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Richard Henderson]

Since v4.0, capstone.h has moved to <capstone/capstone.h>.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/capstone.h | 4 ++++
 configure                | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/include/disas/capstone.h b/include/disas/capstone.h
index e29068dd97..90631d84a9 100644
--- a/include/disas/capstone.h
+++ b/include/disas/capstone.h
@@ -3,7 +3,11 @@
 
 #ifdef CONFIG_CAPSTONE
 
+#ifdef CONFIG_CAPSTONE_CAPSTONE_H
+#include <capstone/capstone.h>
+#else
 #include <capstone.h>
+#endif
 
 #else
 
diff --git a/configure b/configure
index d2fc346302..eec7f061c3 100755
--- a/configure
+++ b/configure
@@ -5021,6 +5021,9 @@ case "$capstone" in
   system)
     QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
     LIBS="$($pkg_config --libs capstone) $LIBS"
+    if check_include capstone/capstone.h; then
+      capstone_capstone_h=yes
+    fi
     ;;
 
   no)
@@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
 fi
 if test "$capstone" != "no" ; then
   echo "CONFIG_CAPSTONE=y" >> $config_host_mak
+  if test "$capstone_capstone_h" != "no" ; then
+    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
+  fi
 fi
 if test "$debug_mutex" = "yes" ; then
   echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
-- 
2.17.1

[Qemu-devel] [PATCH 2/3] capstone: Update to master

[Richard Henderson]

Update to fbb20ea83c5a.  Choose this over the 4.0.1 tag because
master now includes the s390x z13 vector opcodes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 capstone  | 2 +-
 configure | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/capstone b/capstone
index 22ead3e0bf..fbb20ea83c 160000
--- a/capstone
+++ b/capstone
@@ -1 +1 @@
-Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
+Subproject commit fbb20ea83c5af4f29b40c17fbadb1f71b0a08fae
diff --git a/configure b/configure
index eec7f061c3..a2e4636a34 100755
--- a/configure
+++ b/configure
@@ -5016,6 +5016,7 @@ case "$capstone" in
       LIBCAPSTONE=libcapstone.a
     fi
     LIBS="-L\$(BUILD_DIR)/capstone -lcapstone $LIBS"
+    capstone_capstone_h=yes
     ;;
 
   system)
-- 
2.17.1

[Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

Enable s390x, aka SYSZ, in the git submodule build.
Set the capstone parameters for both s390x host and guest.
Install a skipdata hook to keep capstone in sync with the
instruction stream for unknown opcodes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 Makefile           |  1 +
 disas.c            | 40 ++++++++++++++++++++++++++++++++++++++++
 target/s390x/cpu.c |  4 ++++
 3 files changed, 45 insertions(+)

diff --git a/Makefile b/Makefile
index 155f066a20..a37e872825 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
 CAP_CFLAGS += -DCAPSTONE_HAS_ARM
 CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
 CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
+CAP_CFLAGS += -DCAPSTONE_HAS_SYSZ
 CAP_CFLAGS += -DCAPSTONE_HAS_X86
 
 subdir-capstone: .git-submodule-status
diff --git a/disas.c b/disas.c
index 41ad0102e2..c1ecd2d769 100644
--- a/disas.c
+++ b/disas.c
@@ -179,6 +179,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
    to share this across calls and across host vs target disassembly.  */
 static __thread cs_insn *cap_insn;
 
+/*
+ * The capstone library always skips 2 bytes for S390X.
+ * This is less than ideal, since we can tell from the first two bits
+ * the size of the insn and thus stay in sync with the insn stream.
+ */
+static size_t CAPSTONE_API
+cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size,
+                      size_t offset, void *user_data)
+{
+    size_t ilen;
+
+    /* See get_ilen() in target/s390x/internal.h.  */
+    switch (code[offset] >> 6) {
+    case 0:
+        ilen = 2;
+        break;
+    case 1:
+    case 2:
+        ilen = 4;
+        break;
+    default:
+        ilen = 6;
+        break;
+    }
+
+    return ilen;
+}
+
+static const cs_opt_skipdata cap_skipdata_s390x = {
+    .mnemonic = ".byte",
+    .callback = cap_skipdata_s390x_cb
+};
+
 /* Initialize the Capstone library.  */
 /* ??? It would be nice to cache this.  We would need one handle for the
    host and one for the target.  For most targets we can reset specific
@@ -209,6 +242,10 @@ static cs_err cap_disas_start(disassemble_info *info, csh *handle)
 
     /* "Disassemble" unknown insns as ".byte W,X,Y,Z".  */
     cs_option(*handle, CS_OPT_SKIPDATA, CS_OPT_ON);
+    if (info->cap_arch == CS_ARCH_SYSZ) {
+        cs_option(*handle, CS_OPT_SKIPDATA_SETUP,
+                  (uintptr_t)&cap_skipdata_s390x);
+    }
 
     /* Allocate temp space for cs_disasm_iter.  */
     if (cap_insn == NULL) {
@@ -551,6 +588,9 @@ void disas(FILE *out, void *code, unsigned long size)
     print_insn = print_insn_m68k;
 #elif defined(__s390__)
     print_insn = print_insn_s390;
+    s.info.cap_arch = CS_ARCH_SYSZ;
+    s.info.cap_insn_unit = 2;
+    s.info.cap_insn_split = 6;
 #elif defined(__hppa__)
     print_insn = print_insn_hppa;
 #endif
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b1df63d82c..553571d86b 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -41,6 +41,7 @@
 #include "sysemu/sysemu.h"
 #endif
 #include "fpu/softfloat.h"
+#include "disas/capstone.h"
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
@@ -175,6 +176,9 @@ static void s390_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     info->mach = bfd_mach_s390_64;
     info->print_insn = print_insn_s390;
+    info->cap_arch = CS_ARCH_SYSZ;
+    info->cap_insn_unit = 2;
+    info->cap_insn_split = 6;
 }
 
 static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>
>  #ifdef CONFIG_CAPSTONE
>
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>
>  #else
>
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak


--
Alex Bennée

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[David Hildenbrand]

On 23.05.19 04:42, Richard Henderson wrote:
> Enable s390x, aka SYSZ, in the git submodule build.
> Set the capstone parameters for both s390x host and guest.
> Install a skipdata hook to keep capstone in sync with the
> instruction stream for unknown opcodes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  Makefile           |  1 +
>  disas.c            | 40 ++++++++++++++++++++++++++++++++++++++++
>  target/s390x/cpu.c |  4 ++++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 155f066a20..a37e872825 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -477,6 +477,7 @@ CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
>  CAP_CFLAGS += -DCAPSTONE_HAS_ARM
>  CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
>  CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
> +CAP_CFLAGS += -DCAPSTONE_HAS_SYSZ
>  CAP_CFLAGS += -DCAPSTONE_HAS_X86
>  
>  subdir-capstone: .git-submodule-status
> diff --git a/disas.c b/disas.c
> index 41ad0102e2..c1ecd2d769 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -179,6 +179,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>     to share this across calls and across host vs target disassembly.  */
>  static __thread cs_insn *cap_insn;
>  
> +/*
> + * The capstone library always skips 2 bytes for S390X.
> + * This is less than ideal, since we can tell from the first two bits
> + * the size of the insn and thus stay in sync with the insn stream.
> + */
> +static size_t CAPSTONE_API
> +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size,
> +                      size_t offset, void *user_data)
> +{
> +    size_t ilen;
> +
> +    /* See get_ilen() in target/s390x/internal.h.  */
> +    switch (code[offset] >> 6) {
> +    case 0:
> +        ilen = 2;
> +        break;
> +    case 1:
> +    case 2:
> +        ilen = 4;
> +        break;
> +    default:
> +        ilen = 6;
> +        break;
> +    }
> +
> +    return ilen;

return (code[offset] >> 6) << 1; ?



-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/3] capstone: Update to master

[David Hildenbrand]

On 23.05.19 04:42, Richard Henderson wrote:
> Update to fbb20ea83c5a.  Choose this over the 4.0.1 tag because
> master now includes the s390x z13 vector opcodes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  capstone  | 2 +-
>  configure | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/capstone b/capstone
> index 22ead3e0bf..fbb20ea83c 160000
> --- a/capstone
> +++ b/capstone
> @@ -1 +1 @@
> -Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
> +Subproject commit fbb20ea83c5af4f29b40c17fbadb1f71b0a08fae
> diff --git a/configure b/configure
> index eec7f061c3..a2e4636a34 100755
> --- a/configure
> +++ b/configure
> @@ -5016,6 +5016,7 @@ case "$capstone" in
>        LIBCAPSTONE=libcapstone.a
>      fi
>      LIBS="-L\$(BUILD_DIR)/capstone -lcapstone $LIBS"
> +    capstone_capstone_h=yes
>      ;;
>  
>    system)
> 

Acked-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[David Hildenbrand]

On 23.05.19 04:42, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Philippe Mathieu-Daudé]

On 5/23/19 4:42 AM, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Daniel P. Berrangé]

On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.

NB this was a regression bug in capstone pkg-config file which has been
fixed upstream

   https://github.com/aquynh/capstone/pull/1276

In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
other distros should do the same

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 2/3] capstone: Update to master

[Philippe Mathieu-Daudé]

On 5/23/19 4:42 AM, Richard Henderson wrote:
> Update to fbb20ea83c5a.  Choose this over the 4.0.1 tag because
> master now includes the s390x z13 vector opcodes.

Cool! I was watching few fixed issues on the capstone repo, which are
now fixed in QEMU too :)

Various riscv/arm64 improvement too.

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  capstone  | 2 +-
>  configure | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/capstone b/capstone
> index 22ead3e0bf..fbb20ea83c 160000
> --- a/capstone
> +++ b/capstone
> @@ -1 +1 @@
> -Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
> +Subproject commit fbb20ea83c5af4f29b40c17fbadb1f71b0a08fae
> diff --git a/configure b/configure
> index eec7f061c3..a2e4636a34 100755
> --- a/configure
> +++ b/configure
> @@ -5016,6 +5016,7 @@ case "$capstone" in
>        LIBCAPSTONE=libcapstone.a
>      fi
>      LIBS="-L\$(BUILD_DIR)/capstone -lcapstone $LIBS"
> +    capstone_capstone_h=yes
>      ;;
>  
>    system)
> 

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Philippe Mathieu-Daudé]

On 5/23/19 1:07 PM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

Are you suggesting to not include this patch? It is less invasive to
distributions that package QEMU but don't package libcapstone.

> 
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/disas/capstone.h | 4 ++++
>>  configure                | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
>> index e29068dd97..90631d84a9 100644
>> --- a/include/disas/capstone.h
>> +++ b/include/disas/capstone.h
>> @@ -3,7 +3,11 @@
>>  
>>  #ifdef CONFIG_CAPSTONE
>>  
>> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
>> +#include <capstone/capstone.h>
>> +#else
>>  #include <capstone.h>
>> +#endif
>>  
>>  #else
>>  
>> diff --git a/configure b/configure
>> index d2fc346302..eec7f061c3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5021,6 +5021,9 @@ case "$capstone" in
>>    system)
>>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>>      LIBS="$($pkg_config --libs capstone) $LIBS"
>> +    if check_include capstone/capstone.h; then
>> +      capstone_capstone_h=yes
>> +    fi
>>      ;;
>>  
>>    no)
>> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>>  fi
>>  if test "$capstone" != "no" ; then
>>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
>> +  if test "$capstone_capstone_h" != "no" ; then
>> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
>> +  fi
>>  fi
>>  if test "$debug_mutex" = "yes" ; then
>>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 
> Regards,
> Daniel
> 

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Daniel P. Berrangé]

On Thu, May 23, 2019 at 01:17:40PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/23/19 1:07 PM, Daniel P. Berrangé wrote:
> > On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
> >> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> > 
> > NB this was a regression bug in capstone pkg-config file which has been
> > fixed upstream
> > 
> >    https://github.com/aquynh/capstone/pull/1276
> > 
> > In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> > other distros should do the same
> 
> Are you suggesting to not include this patch? It is less invasive to
> distributions that package QEMU but don't package libcapstone.

If building against a bundled capstone, we ought to be able to get
the right -I flag to not need this.  So its just a question of how
how much we care about distros who've not fixed the regression.

> 
> > 
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>  include/disas/capstone.h | 4 ++++
> >>  configure                | 6 ++++++
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> >> index e29068dd97..90631d84a9 100644
> >> --- a/include/disas/capstone.h
> >> +++ b/include/disas/capstone.h
> >> @@ -3,7 +3,11 @@
> >>  
> >>  #ifdef CONFIG_CAPSTONE
> >>  
> >> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> >> +#include <capstone/capstone.h>
> >> +#else
> >>  #include <capstone.h>
> >> +#endif
> >>  
> >>  #else
> >>  
> >> diff --git a/configure b/configure
> >> index d2fc346302..eec7f061c3 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -5021,6 +5021,9 @@ case "$capstone" in
> >>    system)
> >>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
> >>      LIBS="$($pkg_config --libs capstone) $LIBS"
> >> +    if check_include capstone/capstone.h; then
> >> +      capstone_capstone_h=yes
> >> +    fi
> >>      ;;
> >>  
> >>    no)
> >> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
> >>  fi
> >>  if test "$capstone" != "no" ; then
> >>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> >> +  if test "$capstone_capstone_h" != "no" ; then
> >> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> >> +  fi
> >>  fi
> >>  if test "$debug_mutex" = "yes" ; then
> >>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> > 
> > Regards,
> > Daniel
> > 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
>
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
>
>    https://github.com/aquynh/capstone/pull/1276
>
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

But I think it's worth keeping the workaround in the likely event that
it takes a while for that to filter through.

>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/disas/capstone.h | 4 ++++
>>  configure                | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
>> index e29068dd97..90631d84a9 100644
>> --- a/include/disas/capstone.h
>> +++ b/include/disas/capstone.h
>> @@ -3,7 +3,11 @@
>>
>>  #ifdef CONFIG_CAPSTONE
>>
>> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
>> +#include <capstone/capstone.h>
>> +#else
>>  #include <capstone.h>
>> +#endif
>>
>>  #else
>>
>> diff --git a/configure b/configure
>> index d2fc346302..eec7f061c3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5021,6 +5021,9 @@ case "$capstone" in
>>    system)
>>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>>      LIBS="$($pkg_config --libs capstone) $LIBS"
>> +    if check_include capstone/capstone.h; then
>> +      capstone_capstone_h=yes
>> +    fi
>>      ;;
>>
>>    no)
>> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>>  fi
>>  if test "$capstone" != "no" ; then
>>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
>> +  if test "$capstone_capstone_h" != "no" ; then
>> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
>> +  fi
>>  fi
>>  if test "$debug_mutex" = "yes" ; then
>>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
>
> Regards,
> Daniel


--
Alex Bennée

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

On 5/23/19 4:27 AM, David Hildenbrand wrote:
> On 23.05.19 04:42, Richard Henderson wrote:
>> Enable s390x, aka SYSZ, in the git submodule build.
>> Set the capstone parameters for both s390x host and guest.
>> Install a skipdata hook to keep capstone in sync with the
>> instruction stream for unknown opcodes.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  Makefile           |  1 +
>>  disas.c            | 40 ++++++++++++++++++++++++++++++++++++++++
>>  target/s390x/cpu.c |  4 ++++
>>  3 files changed, 45 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index 155f066a20..a37e872825 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -477,6 +477,7 @@ CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
>>  CAP_CFLAGS += -DCAPSTONE_HAS_ARM
>>  CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
>>  CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC
>> +CAP_CFLAGS += -DCAPSTONE_HAS_SYSZ
>>  CAP_CFLAGS += -DCAPSTONE_HAS_X86
>>  
>>  subdir-capstone: .git-submodule-status
>> diff --git a/disas.c b/disas.c
>> index 41ad0102e2..c1ecd2d769 100644
>> --- a/disas.c
>> +++ b/disas.c
>> @@ -179,6 +179,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info)
>>     to share this across calls and across host vs target disassembly.  */
>>  static __thread cs_insn *cap_insn;
>>  
>> +/*
>> + * The capstone library always skips 2 bytes for S390X.
>> + * This is less than ideal, since we can tell from the first two bits
>> + * the size of the insn and thus stay in sync with the insn stream.
>> + */
>> +static size_t CAPSTONE_API
>> +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size,
>> +                      size_t offset, void *user_data)
>> +{
>> +    size_t ilen;
>> +
>> +    /* See get_ilen() in target/s390x/internal.h.  */
>> +    switch (code[offset] >> 6) {
>> +    case 0:
>> +        ilen = 2;
>> +        break;
>> +    case 1:
>> +    case 2:
>> +        ilen = 4;
>> +        break;
>> +    default:
>> +        ilen = 6;
>> +        break;
>> +    }
>> +
>> +    return ilen;
> 
> return (code[offset] >> 6) << 1; ?

Doesn't work for 1.  Anyway, with the comment I wanted to match get_ilen() exactly.


r~

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Richard Henderson]

On 5/23/19 7:07 AM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

Heh.  And here's me thinking this was an intentional change.


r~

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

[Richard Henderson]

On 5/23/19 7:07 AM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same
> 

It seems this fix is present in the tagged 4.0 release.

This would have only been present if a distro packaged snapshots.
At least one may have done so, based on

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00329.html

but there's no further information to go on.

I've checked Fedora 30 and Debian Buster, which are the only two I could
immediately identify that shipped 4.0.1, as opposed to some 3.x version.  Both
have the pkg-config bug fixed.

Since I cannot test any fixup path, I'm going to drop this patch entirely.


r~

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

On 5/22/19 10:42 PM, Richard Henderson wrote:
> Enable s390x, aka SYSZ, in the git submodule build.
> Set the capstone parameters for both s390x host and guest.
> Install a skipdata hook to keep capstone in sync with the
> instruction stream for unknown opcodes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  Makefile           |  1 +
>  disas.c            | 40 ++++++++++++++++++++++++++++++++++++++++
>  target/s390x/cpu.c |  4 ++++
>  3 files changed, 45 insertions(+)

I'm going to put this patch on hold for now, as I'm trying to get this fixed
upstream.  If that fails, I'll re-introduce it here.


r~

Re: [Qemu-devel] [PATCH 3/3] capstone: Enable disassembly for s390x

[Richard Henderson]

On 5/23/19 3:26 PM, Richard Henderson wrote:
> On 5/22/19 10:42 PM, Richard Henderson wrote:
>> Enable s390x, aka SYSZ, in the git submodule build.
>> Set the capstone parameters for both s390x host and guest.
>> Install a skipdata hook to keep capstone in sync with the
>> instruction stream for unknown opcodes.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  Makefile           |  1 +
>>  disas.c            | 40 ++++++++++++++++++++++++++++++++++++++++
>>  target/s390x/cpu.c |  4 ++++
>>  3 files changed, 45 insertions(+)
> 
> I'm going to put this patch on hold for now, as I'm trying to get this fixed
> upstream.  If that fails, I'll re-introduce it here.

Bah.  What I meant is the skipdata hook portion of the patch.  I'll split out
the bare "enable s390" portion for v2.


r~