Re: KVM: Fix the invlpg instruction emulation on AMD64

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Aurelien Jarno wrote:
> Avi Kivity a écrit :
>   
>> Aurelien Jarno wrote:
>>     
>>> The patch below removes the check for c->modrm_reg == 7 to detect the 
>>> invlpg instruction, as it was the case before before commit 
>>> aa38840d3d2e0a804e628077df8d8879b496d741. This fixes the boot of FreeBSD
>>> on an AMD64 CPU.
>>>
>>> It also moves the assignation of c->src.bytes after the test as it is
>>> not needed for the invlpg instruction.
>>>
>>> Signed-off-by: Aurelien Jarno <aurelien-rXXEIb44qovR7s880joybQ@public.gmane.org>
>>>
>>> diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
>>> index fa33fcd..01aa952 100644
>>> --- a/drivers/kvm/x86_emulate.c
>>> +++ b/drivers/kvm/x86_emulate.c
>>> @@ -824,12 +824,10 @@ modrm_done:
>>>  		c->src.bytes = 4;
>>>  		goto srcmem_common;
>>>  	case SrcMem:
>>> -		c->src.bytes = (c->d & ByteOp) ? 1 :
>>> -							   c->op_bytes;
>>>  		/* Don't fetch the address for invlpg: it could be unmapped. */
>>> -		if (c->twobyte && c->b == 0x01
>>> -				    && c->modrm_reg == 7)
>>> +		if (c->twobyte && c->b == 0x01)
>>>  			break;
>>> +		c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
>>>
>>>   
>>>       
>> I don't understand why this helps.  All of the other instructions in 
>> this group either have modrm_mod == 3 or do require evaluation of the 
>>     
>                          ^^^^^^^^^
> The test actually concerns modrm_reg and not modrm_mod. Maybe it is wrong?
>
>   

I meant, the instructions all require modrm_mod == 3 which means they 
reference registers, and not memory.


>> source.  invlpg is the only one that doesn't.
>>     
>
> I have marked the invlpg instruction the same way as it is done in
> kvm-37 to know what happens. I get either modrm_reg = 4 or = 6 when the
> invlpg instruction is executed, but never = 7.
>
>   

Then it isn't the invlpg instruction at all.  Rather smsw (modrm_reg == 
4) or lmsw ( == 6).

I'm confused.

(looks)

Okay.  What we have here is total breakage when emulating an instruction 
that uses a mod r/m encoding that actually refers to a register 
(modrm_mod == 3).  In x86_decode_insn() we set src.type as OP_MEM, and 
in x86_emulate_insn() we happily fetch it even though it's a register, 
generating a fault.

It usually doesn't bite us because these instructions are directly executed.

The fix is probably to switch to OP_REG if SrcMem and ModRM and 
modrm_mod == 3 (similarly for DstMem).

-- 
error compiling committee.c: too many arguments to function


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