RE: [PATCH 06/16] vmx: nest: handling VMX instruction exits

["Dong, Eddie" <eddie.dong@intel.com>]

Keir Fraser wrote:
> On 15/09/2010 08:17, "Qing He" <qing.he@intel.com> wrote:
> 
>>> What is wrong with simply extending x86_emulate to handle these
>>> VMX-related instructions? We've dealt with emulators provided by
>>> Intel guys in the past and frankly they were full of holes.
>> 
>> That needs additional callback when handling vmcs and state change,
>> doesn't it? I'm a little worried that it's too vmx-specific to get
>> into x86_emulate, and that's why we used a separate decoder in the
>> first place (I know it's ugly, though).
> 
> A few VMX-specific callbacks would be fine. Extra callbacks are
> cheap. Just focus on making the callback interface clean and tidy.
> I'd *much* rather have VMX-specific callbacks than an extra emulator.
> 
>> And if we are to use x86_emulate, is it possible to avoid redecoding
>> the opcode, because exit reason is already there?
> 
> If vmexit reason fully decodes the instruction for you then I would

Yes, VM exit reason + VMX_INSTRUCTION_INFO includes everything we needs :)
It is mainly because VMX instruction is simple.

> agree that skipping x86_emulate could make sense. And then your
> instruction emulator would be really simple and fast --
> vmx_io_instruction() is a good example of this. If you still need to

Sure, the emulation is very simple and fast, I cab put all privilege check into one function as well.

> parse the instruction to decode ModRM and the like, then I don't see

No need to decode ModRM.

> that the partial decode from vmexit reason saves you much at all, and
> you might as well go the whole hog and do full decode. I don't see
> much saving from a hacky middle-ground. 

So how about we reuse some functions in x86 emulate like this one?

static enum x86_segment
decode_segment(uint8_t modrm_reg)
{
    switch ( modrm_reg )
    {
    case 0: return x86_seg_es;
    case 1: return x86_seg_cs;
    case 2: return x86_seg_ss;
    case 3: return x86_seg_ds;
    case 4: return x86_seg_fs;
    case 5: return x86_seg_gs;
    default: break;
    }
    return decode_segment_failed;
}

Thx, Eddie