On Mon, Apr 22, 2013 at 11:52:03AM +0200, Borislav Petkov wrote: > On Mon, Apr 22, 2013 at 12:42:46PM +0300, Gleb Natapov wrote: > > > Btw, I wanted to ask: when kvm commits the results, does it look at > > > ctxt->op_bytes to know exactly how many bytes to write to the guest? > > > Because if it does, we can save ourselves the trouble here. > > > > > > Or does it simply write both the full sizeof(unsigned long) bytes of > > > ->src.val and ->dst.val to the guest? > > > > > No, it does this in case of register operand: > > > > static void write_register_operand(struct operand *op) > > { > > /* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */ > > switch (op->bytes) { > > case 1: > > *(u8 *)op->addr.reg = (u8)op->val; > > break; > > case 2: > > *(u16 *)op->addr.reg = (u16)op->val; > > break; > > case 4: > > *op->addr.reg = (u32)op->val; > > break; /* 64b: zero-extend */ > > case 8: > > *op->addr.reg = op->val; > > break; > > } > > } > > Ok, and for OP_MEM it does look at ctxt->dst.bytes in writeback(), > AFAICT. And I see other emulated instructions like POPF, for example, do > this: > > ctxt->dst.bytes = ctxt->op_bytes; > > Which means, we can drop all the bullshit in em_movbe and even destroy > some of the bytes in dst.val but only write out the correct ones. Which > means, a simpler code and a lot less jumping through hoops. > > Would that be the more accepted practice? > For most instructions the decoder already sets op->bytes to correct value, given that all flags a correctly specified in opcode table. Explicit op->bytes setting should be done only if it cannot be expressed by opcode flags. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html