[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

>>> On 26.09.14 at 14:04, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 26/09/14 12:39, Jan Beulich wrote:
>>>>> On 26.09.14 at 12:10, <andrew.cooper3@xxxxxxxxxx> 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.

It's specifically not: Operand and address size (and respective
prefixes) have different meaning. You really don't care about
the mode the CPU as a whole is in, but the kind of instructions
it executes.

> 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.

But instructions in a 16-bit segment should be decoded as 16-bit
instructions, not 32-bit ones. Yes, OS/2 and 16-bit Windows are
long gone, but this hasn't changed.

>>> +    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.

Just make it "%04x:%08lx -> %*ph"? (Intentionally not using %lx
as you did - I'd really dislike seeing addresses like 0000:12, while I'd
be much less concerned for digit counts between 8 and 16 to vary.)


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.