From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Vivier Subject: Re: [PATCH 4/5] vmx.c uses x86_decode_prefix() instead of get_io_count(). Date: Thu, 02 Aug 2007 18:46:13 +0200 Message-ID: <46B20A55.10807@bull.net> References: <46B04CCA.2010503@bull.net> <46B04DD6.7010702@bull.net> <46B04EB9.5010103@bull.net> <46B04F56.60607@bull.net> <46B0501C.6060409@bull.net> <46B19A7B.2030109@qumranet.com> <46B1A51C.2040104@bull.net> <46B1A6B8.7020404@qumranet.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0539729247==" Cc: kvm-devel To: Avi Kivity Return-path: In-Reply-To: <46B1A6B8.7020404-atKUWr5tajBWk0Htik3J/w@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 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --===============0539729247== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigDBD0F260A1389BD0880BFB29" This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigDBD0F260A1389BD0880BFB29 Content-Type: multipart/mixed; boundary="------------000005010905070805070608" This is a multi-part message in MIME format. --------------000005010905070805070608 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Avi Kivity wrote: > Laurent Vivier wrote: >>> >>> How about just calliing emulate_instruction() from here (just for the= >>> string case)? That will eliminate all the setup code. >>> =20 >> >> But this setup is in emulate_instruction() so it will be executed anyw= ay. >> >> =20 >=20 > I'm not worried about run-time overhead, but about the amount of code.= >=20 >>> x86_emulate_memop() will need to be extended to decode ins/outs, but >>> that's fairly easy. >>> =20 >> >> 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...) >> >> =20 >=20 > Yes. Note that x86_emulate_memop() will eventually call kvm_setup_pio(= ) > to complete the emulation. Just to have some comments, have a look to my little dirty patch (see att= achment) Laurent --=20 ------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org -------------- "Software is hard" - Donald Knuth --------------000005010905070805070608 Content-Type: text/plain; name="emulate_ins_outs" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="emulate_ins_outs" Index: kvm/drivers/kvm/kvm_main.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- kvm.orig/drivers/kvm/kvm_main.c 2007-08-02 18:32:19.000000000 +0200 +++ kvm/drivers/kvm/kvm_main.c 2007-08-02 18:32:22.000000000 +0200 @@ -1222,6 +1222,8 @@ =20 vcpu->mmio_is_write =3D 0; r =3D x86_emulate_memop(&emulate_ctxt, &emulate_ops); + if (vcpu->pio.count) + return EMULATE_DO_MMIO; =20 if ((r || vcpu->mmio_is_write) && run) { run->exit_reason =3D KVM_EXIT_MMIO; Index: kvm/drivers/kvm/svm.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- kvm.orig/drivers/kvm/svm.c 2007-08-02 18:32:19.000000000 +0200 +++ kvm/drivers/kvm/svm.c 2007-08-02 18:35:03.000000000 +0200 @@ -98,20 +98,6 @@ return svm_features & feat; } =20 -static unsigned get_addr_size(struct vcpu_svm *svm) -{ - struct vmcb_save_area *sa =3D &svm->vmcb->save; - u16 cs_attrib; - - if (!(sa->cr0 & X86_CR0_PE) || (sa->rflags & X86_EFLAGS_VM)) - return 2; - - cs_attrib =3D sa->cs.attrib; - - return (cs_attrib & SVM_SELECTOR_L_MASK) ? 8 : - (cs_attrib & SVM_SELECTOR_DB_MASK) ? 4 : 2; -} - static inline u8 pop_irq(struct kvm_vcpu *vcpu) { int word_index =3D __ffs(vcpu->irq_summary); @@ -995,147 +981,32 @@ return 0; } =20 -static int io_get_override(struct vcpu_svm *svm, - struct vmcb_seg **seg, - int *addr_override) -{ - u8 inst[MAX_INST_SIZE]; - unsigned ins_length; - gva_t rip; - int i; - - rip =3D svm->vmcb->save.rip; - ins_length =3D svm->next_rip - rip; - rip +=3D svm->vmcb->save.cs.base; - - if (ins_length > MAX_INST_SIZE) - printk(KERN_DEBUG - "%s: inst length err, cs base 0x%llx rip 0x%llx " - "next rip 0x%llx ins_length %u\n", - __FUNCTION__, - svm->vmcb->save.cs.base, - svm->vmcb->save.rip, - svm->vmcb->control.exit_info_2, - ins_length); - - if (emulator_read_std(rip, inst, ins_length, &svm->vcpu) - !=3D X86EMUL_CONTINUE) - /* #PF */ - return 0; - - *addr_override =3D 0; - *seg =3D NULL; - for (i =3D 0; i < ins_length; i++) - switch (inst[i]) { - case 0xf0: - case 0xf2: - case 0xf3: - case 0x66: - continue; - case 0x67: - *addr_override =3D 1; - continue; - case 0x2e: - *seg =3D &svm->vmcb->save.cs; - continue; - case 0x36: - *seg =3D &svm->vmcb->save.ss; - continue; - case 0x3e: - *seg =3D &svm->vmcb->save.ds; - continue; - case 0x26: - *seg =3D &svm->vmcb->save.es; - continue; - case 0x64: - *seg =3D &svm->vmcb->save.fs; - continue; - case 0x65: - *seg =3D &svm->vmcb->save.gs; - continue; - default: - return 1; - } - printk(KERN_DEBUG "%s: unexpected\n", __FUNCTION__); - return 0; -} - -static unsigned long io_address(struct vcpu_svm *svm, int ins, gva_t *ad= dress) -{ - unsigned long addr_mask; - unsigned long *reg; - struct vmcb_seg *seg; - int addr_override; - struct vmcb_save_area *save_area =3D &svm->vmcb->save; - u16 cs_attrib =3D save_area->cs.attrib; - unsigned addr_size =3D get_addr_size(svm); - - if (!io_get_override(svm, &seg, &addr_override)) - return 0; - - if (addr_override) - addr_size =3D (addr_size =3D=3D 2) ? 4: (addr_size >> 1); - - if (ins) { - reg =3D &svm->vcpu.regs[VCPU_REGS_RDI]; - seg =3D &svm->vmcb->save.es; - } else { - reg =3D &svm->vcpu.regs[VCPU_REGS_RSI]; - seg =3D (seg) ? seg : &svm->vmcb->save.ds; - } - - addr_mask =3D ~0ULL >> (64 - (addr_size * 8)); - - if ((cs_attrib & SVM_SELECTOR_L_MASK) && - !(svm->vmcb->save.rflags & X86_EFLAGS_VM)) { - *address =3D (*reg & addr_mask); - return addr_mask; - } - - if (!(seg->attrib & SVM_SELECTOR_P_SHIFT)) { - svm_inject_gp(&svm->vcpu, 0); - return 0; - } - - *address =3D (*reg & addr_mask) + seg->base; - return addr_mask; -} - static int io_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run= ) { u32 io_info =3D svm->vmcb->control.exit_info_1; //address size bug? int size, down, in, string, rep; unsigned port; - unsigned long count; - gva_t address =3D 0; =20 ++svm->vcpu.stat.io_exits; =20 svm->next_rip =3D svm->vmcb->control.exit_info_2; =20 + string =3D (io_info & SVM_IOIO_STR_MASK) !=3D 0; + + if (string) { + if (emulate_instruction(&svm->vcpu, kvm_run, 0, 0) =3D=3D EMULATE_DO_M= MIO) + return 0; + return 1; + } + in =3D (io_info & SVM_IOIO_TYPE_MASK) !=3D 0; port =3D io_info >> 16; size =3D (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT; - string =3D (io_info & SVM_IOIO_STR_MASK) !=3D 0; rep =3D (io_info & SVM_IOIO_REP_MASK) !=3D 0; - count =3D 1; down =3D (svm->vmcb->save.rflags & X86_EFLAGS_DF) !=3D 0; =20 - if (string) { - unsigned addr_mask; - - addr_mask =3D io_address(svm, in, &address); - if (!addr_mask) { - printk(KERN_DEBUG "%s: get io address failed\n", - __FUNCTION__); - return 1; - } - - if (rep) - count =3D svm->vcpu.regs[VCPU_REGS_RCX] & addr_mask; - } - return kvm_setup_pio(&svm->vcpu, kvm_run, in, size, count, string, - down, address, rep, port); + return kvm_setup_pio(&svm->vcpu, kvm_run, in, size, 1, 0, + down, 0, rep, port); } =20 static int nop_on_interception(struct vcpu_svm *svm, struct kvm_run *kvm= _run) Index: kvm/drivers/kvm/vmx.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- kvm.orig/drivers/kvm/vmx.c 2007-08-02 18:32:19.000000000 +0200 +++ kvm/drivers/kvm/vmx.c 2007-08-02 18:34:39.000000000 +0200 @@ -1761,82 +1761,30 @@ return 0; } =20 -static int get_io_count(struct kvm_vcpu *vcpu, unsigned long *count) -{ - u64 inst; - gva_t rip; - int countr_size; - int i; - - if ((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_VM)) { - countr_size =3D 2; - } else { - u32 cs_ar =3D vmcs_read32(GUEST_CS_AR_BYTES); - - countr_size =3D (cs_ar & AR_L_MASK) ? 8: - (cs_ar & AR_DB_MASK) ? 4: 2; - } - - rip =3D vmcs_readl(GUEST_RIP); - if (countr_size !=3D 8) - rip +=3D vmcs_readl(GUEST_CS_BASE); - - if (emulator_read_std(rip, &inst, sizeof(inst), vcpu) !=3D - X86EMUL_CONTINUE) - return 0; - - for (i =3D 0; i < sizeof(inst); i++) { - switch (((u8*)&inst)[i]) { - case 0xf0: - case 0xf2: - case 0xf3: - case 0x2e: - case 0x36: - case 0x3e: - case 0x26: - case 0x64: - case 0x65: - case 0x66: - break; - case 0x67: - countr_size =3D (countr_size =3D=3D 2) ? 4: (countr_size >> 1); - default: - goto done; - } - } - return 0; -done: - countr_size *=3D 8; - *count =3D vcpu->regs[VCPU_REGS_RCX] & (~0ULL >> (64 - countr_size)); - //printk("cx: %lx\n", vcpu->regs[VCPU_REGS_RCX]); - return 1; -} - static int handle_io(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) { u64 exit_qualification; int size, down, in, string, rep; unsigned port; - unsigned long count; - gva_t address; =20 ++vcpu->stat.io_exits; exit_qualification =3D vmcs_read64(EXIT_QUALIFICATION); - in =3D (exit_qualification & 8) !=3D 0; - size =3D (exit_qualification & 7) + 1; string =3D (exit_qualification & 16) !=3D 0; + + if (string) { + if (emulate_instruction(vcpu, kvm_run, 0, 0) =3D=3D EMULATE_DO_MMIO) + return 0; + return 1; + } + + size =3D (exit_qualification & 7) + 1; + in =3D (exit_qualification & 8) !=3D 0; down =3D (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_DF) !=3D 0; - count =3D 1; rep =3D (exit_qualification & 32) !=3D 0; port =3D exit_qualification >> 16; - address =3D 0; - if (string) { - if (rep && !get_io_count(vcpu, &count)) - return 1; - address =3D vmcs_readl(GUEST_LINEAR_ADDRESS); - } - return kvm_setup_pio(vcpu, kvm_run, in, size, count, string, down, - address, rep, port); + + return kvm_setup_pio(vcpu, kvm_run, in, size, 1, 0, down, + 0, rep, port); } =20 static void Index: kvm/drivers/kvm/x86_emulate.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- kvm.orig/drivers/kvm/x86_emulate.c 2007-08-02 18:32:19.000000000 +020= 0 +++ kvm/drivers/kvm/x86_emulate.c 2007-08-02 18:34:06.000000000 +0200 @@ -103,9 +103,12 @@ /* 0x58 - 0x5F */ ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, - /* 0x60 - 0x6F */ + /* 0x60 - 0x6B */ 0, 0, 0, DstReg | SrcMem32 | ModRM | Mov /* movsxd (x86/64) */ , - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + /* 0x6C - 0x6F */ + ByteOp | ImplicitOps, ImplicitOps, /* insb, insw/insd */ + ByteOp | ImplicitOps, ImplicitOps, /* outsb, outsw/outsd */ /* 0x70 - 0x7F */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 0x80 - 0x87 */ @@ -428,10 +431,11 @@ }) =20 /* Access/update address held in a register, based on addressing mode. *= / +#define address_mask(reg) \ + ((ad_bytes =3D=3D sizeof(unsigned long)) ? \ + (reg) : ((reg) & ((1UL << (ad_bytes << 3)) - 1))) #define register_address(base, reg) = \ - ((base) + ((ad_bytes =3D=3D sizeof(unsigned long)) ? (reg) : \ - ((reg) & ((1UL << (ad_bytes << 3)) - 1)))) - + ((base) + address_mask(reg)) #define register_address_increment(reg, inc) = \ do { \ /* signed type ensures sign extension to long */ \ @@ -1139,6 +1143,39 @@ register_address_increment(_regs[VCPU_REGS_RDI], (_eflags & EFLG_DF) ? -dst.bytes : dst.bytes); break; + case 0x6c: /* insb */ + case 0x6d: /* insw/insd */ + if (kvm_setup_pio(ctxt->vcpu, NULL, + 1, /* in */ + (d & ByteOp) ? 1 : op_bytes, /* size */ + rep_prefix ? + address_mask(_regs[VCPU_REGS_RCX]) + 1: 1, /* count */ + 1, /* strings */ + (_eflags & EFLG_DF), /* down */ + register_address(ctxt->es_base, + _regs[VCPU_REGS_RDI]), /* address */ + rep_prefix, + _regs[VCPU_REGS_RDX] /* port */ + ) =3D=3D 0) + return -1; + return 0; + case 0x6e: /* outsb */ + case 0x6f: /* outsw/outsd */ + if (kvm_setup_pio(ctxt->vcpu, NULL, + 0, /* in */ + (d & ByteOp) ? 1 : op_bytes, /* size */ + rep_prefix ? + address_mask(_regs[VCPU_REGS_RCX]) + 1: 1, /* count */ + 1, /* strings */ + (_eflags & EFLG_DF), /* down */ + register_address(override_base ? + *override_base : ctxt->ds_base, + _regs[VCPU_REGS_RSI]), /* address */ + rep_prefix, + _regs[VCPU_REGS_RDX] /* port */ + ) =3D=3D 0) + return -1; + return 0; case 0xa6 ... 0xa7: /* cmps */ DPRINTF("Urk! I don't handle CMPS.\n"); goto cannot_emulate; --------------000005010905070805070608-- --------------enigDBD0F260A1389BD0880BFB29 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.7 (GNU/Linux) iD8DBQFGsgpZ9Kffa9pFVzwRAsFMAKDU9pJAd/9WCQSs+nxBris24j0B+wCeJpM7 1HO+I1sdjIErxUjrOY3d2oM= =p1nR -----END PGP SIGNATURE----- --------------enigDBD0F260A1389BD0880BFB29-- --===============0539729247== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------- 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/ --===============0539729247== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kvm-devel mailing list kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/kvm-devel --===============0539729247==--