Menu
▾
▴
kvm-devel
|
From: Nitin A K. <nit...@in...> - 2007-09-18 22:14:24
|
Hi Avi, Some emulation case statements have gone to wrong place in the upstream tree. This patch fixes that. This time I have created the patch using the git-format-patch command as per your suggestion. Please apply. -- Thanks & Regards, Nitin Open Source Technology Center, Intel Corporation ----------------------------------------------------------------- The mind is like a parachute; it works much better when it's open |
|
From: Avi K. <av...@qu...> - 2007-09-19 12:12:44
|
Nitin A Kamble wrote: > Hi Avi, > Some emulation case statements have gone to wrong place in the > upstream tree. This patch fixes that. Applied, thanks. > This time I have created the patch > using the git-format-patch command as per your suggestion. > Much easier to apply. But now my mailer doesn't quote it... -- error compiling committee.c: too many arguments to function |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:39:29
|
Nitin A Kamble wrote:
> Hi Avi,
> Attached is the patch to implement instructions:
> inc reg
> opcode: 0x40 - 0x47
> Please apply.
>
> @@ -1390,6 +1393,22 @@ pop_instruction:
> _eip = ctxt->vcpu->rip;
> }
> switch (b) {
> + case 0x40 ... 0x47: /* inc reg */
> + dst.ptr = (unsigned long *)&_regs[b & 0x7];
> + dst.val = *dst.ptr;
> + switch (op_bytes) {
>
Too much indentation here?
> + case 2:
> + *(u16 *)dst.ptr = (u16)dst.val + 1;
> + break;
> + case 4:
> + *dst.ptr = (u32)dst.val + 1;
> + break; /* 64b: zero-ext */
> + case 8:
> + *dst.ptr = dst.val + 1;
> + break;
> + }
> + no_wb = 1; /* Disable writeback. */
> + break;
> case 0xa4 ... 0xa5: /* movs */
> dst.type = OP_MEM;
> dst.bytes = (d & ByteOp) ? 1 : op_bytes;
>
Why are we disabling writeback instead of using the regular writeback
mechanism? Shouldn't just setting dst.val be sufficient?
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
|
|
From: Avi K. <av...@qu...> - 2007-09-15 07:41:37
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to implement instruction: > jump absolute > opcode: 0xff /4 > Applied, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:45:17
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to implement emulation of instruction: > popf > opcode: 0x9d > > Applied, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:43:45
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to initialize src.val & dst.val. Without this, > certain instructions are getting affected in their emulation. > Applied, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:48:30
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to implement instruction: > mov rl/rh/r, imm > opcodes: 0xb0 - 0xbf > > case 0xb0 ... 0xb3: /* mov rl, imm8 */ > + dst.ptr = (unsigned long *)&_regs[VCPU_REGS_RAX + (b & 0x3)]; > + dst.val = src.val; > + dst.type = OP_REG; > + dst.bytes = 1; > + break; > + case 0xb4 ... 0xb7: /* mov rh, imm8 */ > + dst.ptr = ((void *)&_regs[VCPU_REGS_RAX + (b & 0x3)] + 1); > + dst.val = src.val; > + dst.type = OP_REG; > + dst.bytes = 1; > + break; > + case 0xb8 ... 0xbf: /* mov r, imm */ > + dst.ptr = (unsigned long *)&_regs[VCPU_REGS_RAX + (b & 0x7)]; > + dst.val = src.val; > + dst.type = OP_REG; > + break; Can't the decoder select the dst reg? Looks like duplicate work here. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:36:05
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to implement instruction: > lea > opcode: 0x8d > > Applied, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:33:35
|
Nitin A Kamble wrote: > Hi Avi, > This patch corrects the emulation of the instruction "or" for opcodes > 0xc & 0cd. > Please Apply. > > + case 0x0c: /* or al imm8 */ > + dst.type = OP_REG; > + dst.ptr = &_regs[VCPU_REGS_RAX]; > + dst.val = *(u8 *)dst.ptr; > + dst.bytes = 1; > + dst.orig_val = dst.val; > + goto or; > + case 0x0d: /* or ax imm16, or eax imm32 */ > + dst.type = OP_REG; > + dst.bytes = op_bytes; > + dst.ptr = &_regs[VCPU_REGS_RAX]; > + if (op_bytes == 2) > + dst.val = *(u16 *)dst.ptr; > + else > + dst.val = *(u32 *)dst.ptr; > + dst.orig_val = dst.val; > + goto or; Instead of repeating this code for all instructions that use the accumulator implicitly, we should define a bit in the decoder flags (like DstAcc) so that all the code is consolidated in the decoder. This applies to 'sub imm' and 'cmp correction', and probably others as well. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-15 07:29:53
|
Nitin A Kamble wrote: > Hi Avi, > Attached is the patch to implement instruction: > jump conditional relative (like: jnz jo etc) > opcode : 0x0f80 - 0x0f8f > Please apply. > > Applied this and jump conditional short, thanks. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
|
From: Avi K. <av...@qu...> - 2007-09-14 17:08:45
|
Nitin A Kamble wrote:
> Hi Avi,
> Attached is the patch to initialize src.val & dst.val. Without this,
> certain instructions are getting affected in their emulation.
>
> Please apply.
>
>
This seems like it is papering over other bugs. Some instructions use
src.val or dst.val without having decoded the src or dst operand.
Which instructions are these? Can we fix them instead?
> Intialize src.val & dst.val, to fix bugs in certain instruction emulations.
>
> Signed-off-by: Nitin A Kamble <nit...@in...>
>
> diff --git a/drivers/kvm/x86_emulate.c b/drivers/kvm/x86_emulate.c
> index c2540c3..90ee392 100644
> --- a/drivers/kvm/x86_emulate.c
> +++ b/drivers/kvm/x86_emulate.c
> @@ -832,6 +832,7 @@ done_prefixes:
> srcmem_common:
> src.type = OP_MEM;
> src.ptr = (unsigned long *)cr2;
> + src.val = 0;
> if ((rc = ops->read_emulated((unsigned long)src.ptr,
> &src.val, src.bytes, ctxt->vcpu)) != 0)
> goto done;
> @@ -896,6 +897,7 @@ done_prefixes:
> dst.type = OP_MEM;
> dst.ptr = (unsigned long *)cr2;
> dst.bytes = (d & ByteOp) ? 1 : op_bytes;
> + dst.val = 0;
> if (d & BitOp) {
> unsigned long mask = ~(dst.bytes * 8 - 1);
>
>
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
|
|
From: Nitin A K. <nit...@in...> - 2007-09-14 17:34:03
|
On Fri, 2007-09-14 at 10:08 -0700, Avi Kivity wrote: > This seems like it is papering over other bugs. Some instructions use > src.val or dst.val without having decoded the src or dst operand. >=20 > Which instructions are these? Can we fix them instead? Instructions using 8bit operands such as al, ah are affected. Especially utilizing signed operands. By not using this initialization these operands are getting wrong value from remaining stale bits. --=20 Thanks & Regards, Nitin Open Source Technology Center, Intel Corporation ----------------------------------------------------------------- The mind is like a parachute; it works much better when it's open |
|
From: Avi K. <av...@qu...> - 2007-09-14 17:42:18
|
Nitin A Kamble wrote: > On Fri, 2007-09-14 at 10:08 -0700, Avi Kivity wrote: > >> This seems like it is papering over other bugs. Some instructions use >> src.val or dst.val without having decoded the src or dst operand. >> >> Which instructions are these? Can we fix them instead? >> > > Instructions using 8bit operands such as al, ah are affected. > Especially utilizing signed operands. By not using this initialization > these operands are getting wrong value from remaining stale bits. > > I see. SrcMem decode does ->read_emulated() into src.val, leaving stale bits. I agree your patch is the best way to fix it. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X