* [PATCH v3 0/3] Update capstone module
@ 2019-10-15 17:51 Richard Henderson
2019-10-15 17:51 ` [PATCH v3 1/3] capstone: Update to master Richard Henderson
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Richard Henderson @ 2019-10-15 17:51 UTC (permalink / raw)
To: qemu-devel
Tested vs centos7, fedora30, and bionic (with and without
system capstone installed).
Changes for v3:
* Work around the various include directory nonsense.
* Re-add the s390 skipdata callback, as a separate patch.
Changes for v2:
* Drop the installed directory change. This does force a
different include change when building from git.
* Drop the s390 skipdata callback for now.
r~
Richard Henderson (3):
capstone: Update to master
capstone: Enable disassembly for s390x
capstone: Fix s390x skipdata
Makefile | 2 ++
disas.c | 40 ++++++++++++++++++++++++++++++++++++++++
target/s390x/cpu.c | 4 ++++
capstone | 2 +-
configure | 2 +-
5 files changed, 48 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] capstone: Update to master
2019-10-15 17:51 [PATCH v3 0/3] Update capstone module Richard Henderson
@ 2019-10-15 17:51 ` Richard Henderson
2019-10-15 18:50 ` Thomas Huth
2019-10-15 17:51 ` [PATCH v3 2/3] capstone: Enable disassembly for s390x Richard Henderson
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2019-10-15 17:51 UTC (permalink / raw)
To: qemu-devel
Update to 418d36d695e0. Choose this over the 4.0.1 tag because
master now includes the s390x z13 vector opcodes.
Acked-by: David Hildenbrand <david@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Makefile | 1 +
capstone | 2 +-
configure | 2 +-
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 30f0abfb42..8ce48e0342 100644
--- a/Makefile
+++ b/Makefile
@@ -498,6 +498,7 @@ dtc/%: .git-submodule-status
# Remove all the extra -Warning flags that QEMU uses that Capstone doesn't;
# no need to annoy QEMU developers with such things.
CAP_CFLAGS = $(patsubst -W%,,$(CFLAGS) $(QEMU_CFLAGS))
+CAP_CFLAGS += -I$(SRC_PATH)/capstone/include
CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM
CAP_CFLAGS += -DCAPSTONE_HAS_ARM
CAP_CFLAGS += -DCAPSTONE_HAS_ARM64
diff --git a/capstone b/capstone
index 22ead3e0bf..418d36d695 160000
--- a/capstone
+++ b/capstone
@@ -1 +1 @@
-Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf
+Subproject commit 418d36d695e075955674ace5a1191b495da50f84
diff --git a/configure b/configure
index 08ca4bcb46..f4f1860065 100755
--- a/configure
+++ b/configure
@@ -5008,7 +5008,7 @@ case "$capstone" in
git_submodules="${git_submodules} capstone"
fi
mkdir -p capstone
- QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include"
+ QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include/capstone"
if test "$mingw32" = "yes"; then
LIBCAPSTONE=capstone.lib
else
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] capstone: Enable disassembly for s390x
2019-10-15 17:51 [PATCH v3 0/3] Update capstone module Richard Henderson
2019-10-15 17:51 ` [PATCH v3 1/3] capstone: Update to master Richard Henderson
@ 2019-10-15 17:51 ` Richard Henderson
2019-10-15 17:51 ` [PATCH v3 3/3] capstone: Add s390x skipdata callback Richard Henderson
2020-01-03 7:16 ` [PATCH v3 0/3] Update capstone module Philippe Mathieu-Daudé
3 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2019-10-15 17:51 UTC (permalink / raw)
To: qemu-devel
Enable s390x, aka SYSZ, in the git submodule build.
Set the capstone parameters for both s390x host and guest.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Makefile | 1 +
disas.c | 3 +++
target/s390x/cpu.c | 4 ++++
3 files changed, 8 insertions(+)
diff --git a/Makefile b/Makefile
index 8ce48e0342..97e34be162 100644
--- a/Makefile
+++ b/Makefile
@@ -503,6 +503,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
.PHONY: capstone/all
diff --git a/disas.c b/disas.c
index 3e2bfa572b..51c71534a3 100644
--- a/disas.c
+++ b/disas.c
@@ -550,6 +550,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 3abe7e80fd..44f40f1f8c 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -43,6 +43,7 @@
#include "sysemu/tcg.h"
#endif
#include "fpu/softfloat-helpers.h"
+#include "disas/capstone.h"
#define CR0_RESET 0xE0UL
#define CR14_RESET 0xC2000000UL;
@@ -180,6 +181,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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] capstone: Add s390x skipdata callback
2019-10-15 17:51 [PATCH v3 0/3] Update capstone module Richard Henderson
2019-10-15 17:51 ` [PATCH v3 1/3] capstone: Update to master Richard Henderson
2019-10-15 17:51 ` [PATCH v3 2/3] capstone: Enable disassembly for s390x Richard Henderson
@ 2019-10-15 17:51 ` Richard Henderson
2019-10-15 18:46 ` Thomas Huth
2020-01-03 7:16 ` [PATCH v3 0/3] Update capstone module Philippe Mathieu-Daudé
3 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2019-10-15 17:51 UTC (permalink / raw)
To: qemu-devel
Capstone assumes any unknown instruction is 2 bytes.
Instead, use the ilen field in the first two bits of
the instruction to stay in sync with the insn stream.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
disas.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/disas.c b/disas.c
index 51c71534a3..2a000cbeb0 100644
--- a/disas.c
+++ b/disas.c
@@ -178,6 +178,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
@@ -208,6 +241,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) {
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] capstone: Add s390x skipdata callback
2019-10-15 17:51 ` [PATCH v3 3/3] capstone: Add s390x skipdata callback Richard Henderson
@ 2019-10-15 18:46 ` Thomas Huth
2019-10-15 19:22 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2019-10-15 18:46 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 15/10/2019 19.51, Richard Henderson wrote:
> Capstone assumes any unknown instruction is 2 bytes.
> Instead, use the ilen field in the first two bits of
> the instruction to stay in sync with the insn stream.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> disas.c | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/disas.c b/disas.c
> index 51c71534a3..2a000cbeb0 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -178,6 +178,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;
> +}
The kernel has also a nice function to calculate this:
static inline int insn_length(unsigned char code)
{
return ((((int) code + 64) >> 7) + 1) << 1;
}
... but the switch-case is likely easier to read, so anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] capstone: Update to master
2019-10-15 17:51 ` [PATCH v3 1/3] capstone: Update to master Richard Henderson
@ 2019-10-15 18:50 ` Thomas Huth
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Huth @ 2019-10-15 18:50 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 15/10/2019 19.51, Richard Henderson wrote:
> Update to 418d36d695e0. Choose this over the 4.0.1 tag because
> master now includes the s390x z13 vector opcodes.
In case you respin, please mention that this (hopefully) also fixes
https://bugs.launchpad.net/qemu/+bug/1826175
Thanks,
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] capstone: Add s390x skipdata callback
2019-10-15 18:46 ` Thomas Huth
@ 2019-10-15 19:22 ` Richard Henderson
0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2019-10-15 19:22 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
On 10/15/19 11:46 AM, Thomas Huth wrote:
>> +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;
>> +}
>
> The kernel has also a nice function to calculate this:
>
> static inline int insn_length(unsigned char code)
> {
> return ((((int) code + 64) >> 7) + 1) << 1;
> }
>
> ... but the switch-case is likely easier to read, so anyway:
Clever.
I don't mind swapping to the kernel version, so long as we convert the
target/s390x/internal.h function as well.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] Update capstone module
2019-10-15 17:51 [PATCH v3 0/3] Update capstone module Richard Henderson
` (2 preceding siblings ...)
2019-10-15 17:51 ` [PATCH v3 3/3] capstone: Add s390x skipdata callback Richard Henderson
@ 2020-01-03 7:16 ` Philippe Mathieu-Daudé
2020-01-03 21:48 ` Richard Henderson
3 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-03 7:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel, qemu-s390x, David Hildenbrand
Hi Richard,
On 10/15/19 7:51 PM, Richard Henderson wrote:
> Tested vs centos7, fedora30, and bionic (with and without
> system capstone installed).
Change noted while testing:
* Raw byte dumped as hexa
before: no
after: dumped by 16-bit
OK
* Address format
before: "0x" TARGET_FMT_lx (16 chars)
after: "0x%08" PRIx64 (8 chars)
Shorten this might be OK because we now also dump
the raw bytes previous to the mnemonic/arguments
-0x0000000000010014: mvi 163,1
-0x0000000000010018: slr %r0,%r0
-0x000000000001001a: lhi %r1,2
+0x00010014: 9201 00a3 mvi 0xa3, 1
+0x00010018: 1f00 slr %r0, %r0
+0x0001001a: a718 0002 lhi %r1, 2
* Number argument format
before: decimal
after: hexa
-0x00010014: mvi 163,1
+0x00010014: mvi 0xa3, 1
OK
* (Priviledged) Instruction missing
-0x0001001e: sigp %r1,%r0,18
+0x0001001e: .byte 0xae, 0x10, 0x00, 0x12
-0x00010066: lmh %r0,%r15,0(%r13)
+0x00010066: .byte 0xeb, 0x0f, 0xd0, 0x00, 0x00, 0x96
-0x0001006c: sam64
+0x0001006c: .byte 0x01, 0x0e
-0x00010088: lctlg %c0,%c15,512
+0x00010088: .byte 0xeb, 0x0f, 0x02, 0x00, 0x00, 0x2f
-0x0001008e: stcke 808
+0x0001008e: .byte 0xb2, 0x78, 0x03, 0x28
-0x00010098: spt 80(%r13)
+0x00010098: .byte 0xb2, 0x08, 0xd0, 0x50
-0x000149b6: stfl 0
+0x000149b6: .byte 0xb2, 0xb1, 0x00, 0x00
-0x000149da: stfle 0(%r1)
+0x000149da: .byte 0xb2, 0xb0, 0x10, 0x00
-0x00011a34: icm %r5,3,0(%r1)
+0x00011a34: .byte 0xbf, 0x53, 0x10, 0x00
-0x0010e8f6: lpswe 160(%r15)
+0x0010e8f6: .byte 0xb2, 0xb2, 0xf0, 0xa0
Is it possible to fallback to the older disassembler on a
per-instruction basis if Capstone doesn't know about an instruction?
> Changes for v3:
> * Work around the various include directory nonsense.
> * Re-add the s390 skipdata callback, as a separate patch.
>
> Changes for v2:
> * Drop the installed directory change. This does force a
> different include change when building from git.
> * Drop the s390 skipdata callback for now.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] Update capstone module
2020-01-03 7:16 ` [PATCH v3 0/3] Update capstone module Philippe Mathieu-Daudé
@ 2020-01-03 21:48 ` Richard Henderson
0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2020-01-03 21:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, qemu-s390x, David Hildenbrand
On 1/3/20 6:16 PM, Philippe Mathieu-Daudé wrote:
> -0x0010e8f6: lpswe 160(%r15)
> +0x0010e8f6: .byte 0xb2, 0xb2, 0xf0, 0xa0
>
> Is it possible to fallback to the older disassembler on a per-instruction basis
> if Capstone doesn't know about an instruction?
Not as written. But I suppose we could rearrange both dump loops to allow such
a thing.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-01-03 21:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-15 17:51 [PATCH v3 0/3] Update capstone module Richard Henderson
2019-10-15 17:51 ` [PATCH v3 1/3] capstone: Update to master Richard Henderson
2019-10-15 18:50 ` Thomas Huth
2019-10-15 17:51 ` [PATCH v3 2/3] capstone: Enable disassembly for s390x Richard Henderson
2019-10-15 17:51 ` [PATCH v3 3/3] capstone: Add s390x skipdata callback Richard Henderson
2019-10-15 18:46 ` Thomas Huth
2019-10-15 19:22 ` Richard Henderson
2020-01-03 7:16 ` [PATCH v3 0/3] Update capstone module Philippe Mathieu-Daudé
2020-01-03 21:48 ` Richard Henderson
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.