Avi Kivity a écrit :
> Aurelien Jarno wrote:
>> On Tue, Oct 16, 2007 at 12:12:18PM +0200, Avi Kivity wrote:
>>
>>> Aurelien Jarno wrote:
>>>
>>>> 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).
>>>
>>>
>> Ok, I have tried to implement that for SrcMem, and it works in my case.
>> I am able to boot FreeBSD, and I am seeing no regression for Linux or
>> Hurd VMs. Does the patch below looks ok to you?
>>
>>
>>
>> KVM: Fix the invlpg instruction emulation on AMD64
>>
>> The patch below correctly detects the invlpg instruction, and switch
>> src.type to OP_REG. This fixes the boot of FreeBSD on an AMD64 CPU, it has
>> been broken since commit aa38840d3d2e0a804e628077df8d8879b496d741.
>>
>> 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 <[EMAIL PROTECTED]>
>>
>> diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
>> index e974ace..06e183b 100644
>> --- a/drivers/kvm/x86_emulate.c
>> +++ b/drivers/kvm/x86_emulate.c
>> @@ -825,12 +825,12 @@ 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->d & ModRM) && c->modrm_mod == 3) {
>> + c->src.type = OP_REG;
>> break;
>> + }
>> + c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
>> srcmem_common:
>> c->src.type = OP_MEM;
>> break;
>>
>
> Why did you remove the check for invlpg? It need not be executed with
> modrm_mod == 3 (in fact invlpg with modrm_mod == 3 is useless), so it
> won't be caught by this.
Because I really thing this check is buggy. On my tests, modrm_reg is
always different than 7 for the invlpg instruction, so the test is
always false.
> Also, we need to initialize the fields of src in this case.
>
Correct.
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' [EMAIL PROTECTED] | [EMAIL PROTECTED]
`- people.debian.org/~aurel32 | www.aurel32.net
-------------------------------------------------------------------------
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/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
