Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count().

[Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2844 bytes --]

Avi Kivity wrote:
> Laurent Vivier wrote:
>> vmx.c uses x86_decode_prefix() instead of get_io_count().
>>
>>    static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>>  {
>>      u64 exit_qualification;
>> @@ -1831,8 +1780,32 @@
>>      port = exit_qualification >> 16;
>>      address = 0;
>>      if (string) {
>> -        if (rep && !get_io_count(vcpu, &count))
>> +        int mode;
>> +        u64 inst;
>> +        gva_t rip;
>> +        struct x86_prefix prefix;
>> +        u32 cs_ar = vmcs_read32(GUEST_CS_AR_BYTES);
>> +        unsigned long addr_mask;
>> +
>> +        mode = (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM) ?
>> +                X86EMUL_MODE_REAL : (cs_ar & AR_L_MASK)
>> +                ? X86EMUL_MODE_PROT64 : (cs_ar & AR_DB_MASK)
>> +                ? X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16;
>> +
>> +        rip = vmcs_readl(GUEST_RIP);
>> +        if (mode != X86EMUL_MODE_PROT64)
>> +            rip += vmcs_readl(GUEST_CS_BASE);
>> +        if (emulator_read_std(rip, &inst, sizeof(inst), vcpu)
>> +                    != X86EMUL_CONTINUE)
>> +            return 1;
>> +
>> +        if (x86_decode_prefix(mode, (u8*)&inst, &prefix) == -1)
>>              return 1;
>> +
>> +        addr_mask = (~0ULL >> (64 - (prefix.ad_bytes <<3)));
>> +        if (rep)
>> +            count = vcpu->regs[VCPU_REGS_RCX] & addr_mask;
>> +
>>          address = vmcs_readl(GUEST_LINEAR_ADDRESS);
>>      }
>>      return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down,
> 
> How about just calliing emulate_instruction() from here (just for the
> string case)?  That will eliminate all the setup code.

But this setup is in emulate_instruction() so it will be executed anyway.

> x86_emulate_memop() will need to be extended to decode ins/outs, but
> that's fairly easy.

X86_decode_prefix() is a subset of instruction decoding part of
x86_emulate_memop(), kvm_setup_pio() can be seen as a subset of instruction
emulating part of x86_emulate_memop(). So I think in term of performance it is
better to do like that, but I agree by doing:

if (string)
	return emulate_instruction(vcpu, kvm_run, 0, 0);
else
	return kvm_setup_pio(vcpu, kvm_run, in, size, port);

it is more more ... more simple.

If you prefer simplicity, I can do like that ?
(but I know you prefer simplicity...)

BTW, I think PATCH 1,2 and 3 should be applied anyway because they allow to
introduce the separation between instruction decoding and instruction emulation
requested by the TODO "Split the emulator into two functions: one to decode into
the emulation context, and the other to actually execute the instruction."

Laurent
-- 
------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  --------------
          "Software is hard" - Donald Knuth


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel