From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: KVM: Fix the invlpg instruction emulation on AMD64 Date: Wed, 17 Oct 2007 16:37:31 +0200 Message-ID: <47161E2B.1000006@qumranet.com> References: <20071015190823.GA11333@hall.aurel32.net> <47148403.6010603@qumranet.com> <47148867.9070600@aurel32.net> <47148E82.9090103@qumranet.com> <20071017135021.GA32185@hall.aurel32.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Aurelien Jarno Return-path: In-Reply-To: <20071017135021.GA32185-OqXK5JiLQY5aJl8KAwiEcA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org 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 > > 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. Also, we need to initialize the fields of src in this case. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ------------------------------------------------------------------------- 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/