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 <address@hidden>
>> ---
>>  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~