Re: [PATCH v2 25/30] KVM: x86 emulator: fix in/out emulation.

[Avi Kivity <avi@redhat.com>]

On 03/14/2010 07:35 PM, Gleb Natapov wrote:
> On Sun, Mar 14, 2010 at 06:54:11PM +0200, Avi Kivity wrote:
>    
>> On 03/14/2010 06:21 PM, Gleb Natapov wrote:
>>      
>>> in/out emulation is broken now. The breakage is different depending
>>> on where IO device resides. If it is in userspace emulator reports
>>> emulation failure since it incorrectly interprets kvm_emulate_pio()
>>> return value. If IO device is in the kernel emulation of 'in' will do
>>> nothing since kvm_emulate_pio() stores result directly into vcpu
>>> registers, so emulator will overwrite result of emulation during
>>> commit of shadowed register.
>>>
>>>
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> index 8f5e4c8..344e17b 100644
>>> --- a/arch/x86/kvm/emulate.c
>>> +++ b/arch/x86/kvm/emulate.c
>>> @@ -210,13 +210,13 @@ static u32 opcode_table[256] = {
>>>   	0, 0, 0, 0, 0, 0, 0, 0,
>>>   	/* 0xE0 - 0xE7 */
>>>   	0, 0, 0, 0,
>>> -	ByteOp | SrcImmUByte, SrcImmUByte,
>>> -	ByteOp | SrcImmUByte, SrcImmUByte,
>>> +	ByteOp | SrcImmUByte | DstAcc, SrcImmUByte | DstAcc,
>>> +	ByteOp | SrcImmUByte | DstAcc, SrcImmUByte | DstAcc,
>>>        
>> REX prefix shouldn't expand DstAcc to 64 bits here.  Might cause
>> problems further down in the pipeline.+
>>      
> Is REX prefix allowed with this opcodes?
>
> If yes:
>
> if (c->dst.bytes == 8)
> 	c->dst.bytes = 4;
>
> inside IN/OUT emulation will fix this.
>    

I don't know, but I guess REX is allowed and ignored.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.