Re: [PATCH for-4.5 2/2] x86/hvm: Improve "Emulation failed @" error messages

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/09/14 12:39, Jan Beulich wrote:
>>>> On 26.09.14 at 12:10, <andrew.cooper3@citrix.com> wrote:
>> @@ -1449,6 +1441,37 @@ struct segment_register *hvmemul_get_seg_reg(
>>      return &hvmemul_ctxt->seg_reg[seg];
>>  }
>>  
>> +static const char *guest_x86_mode_to_str(int mode)
>> +{
>> +    switch ( mode )
>> +    {
>> +    case 0:
>> +        return "Real";
>> +    case 1:
>> +        return "v8086";
>> +    case 2:
> return "16bit";

case 2 is 32bit mode code in a 16bit segment.  Therefore, 32bit is still
the correct text when aiding decode of the instruction.

What I want to avoid is the confusing statement of "16bit mode" which is
easily confused as "Real mode" and a set of bytes which should be
decoded as 32bit instructions.

>
>> +    case 4:
>> +        return "32bit";
>> +    case 8:
>> +        return "64bit";
>> +    default:
>> +        return "Unknown";
>> +    }
>> +}
>> +
>> +void hvm_dump_emulation_state(const char *prefix,
>> +                              struct hvm_emulate_ctxt *hvmemul_ctxt)
>> +{
>> +    struct vcpu *curr = current;
>> +    const char *mode_str = guest_x86_mode_to_str(hvm_guest_x86_mode(curr));
>> +    struct segment_register *cs = hvmemul_get_seg_reg(x86_seg_cs, hvmemul_ctxt);
> Long line. And perhaps add "const"?

Ok

>
>> +
>> +    printk("%s emulation failed: %pv %s mode, %u bytes @ %04x:%lx: %*ph\n",
>> +           prefix, curr, mode_str, hvmemul_ctxt->insn_buf_bytes,
> Do you really need to print the byte count as a number when the
> new formatting will suitably limit output anyway?

I considered that, but thought that "@ xxxx:xxxx:\n" might be a little
obscure.  On the other hand, it might be ok.  I am happy dropping the
"%u bytes" if that is considered ok.

~Andrew