Re: KVM: x86: use proper port value when checking io instruction permission (v3)

2011-05-30 Thread Marcelo Tosatti
On Sun, May 29, 2011 at 11:34:43AM +0300, Avi Kivity wrote:
> On 05/26/2011 02:56 PM, Marcelo Tosatti wrote:
> >Commit fa4491a6b667304 moved the permission check for io instructions
> >to the ->check_perm callback. It failed to copy the port value from RDX
> >register for string and "in,out ax,dx" instructions.
> >
> >Fix it by reading RDX register at decode stage when appropriate.
> >
> >Fixes FC8.32 installation.
> >
> >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >index 3bc6b7a..fc3d2d9 100644
> >--- a/arch/x86/kvm/emulate.c
> >+++ b/arch/x86/kvm/emulate.c
> >@@ -47,7 +47,7 @@
> >  #define DstDI   (5<<1) /* Destination is in ES:(E)DI */
> >  #define DstMem64(6<<1) /* 64bit memory operand */
> >  #define DstImmUByte (7<<1) /* 8-bit unsigned immediate operand */
> >-#define DstMask (7<<1)
> >+#define DstMask ((7<<1) | (1<<18))
> 
> Ouch, why??
> 
> My comment about bits 19-20 was only about the shifting of of Prot
> etc. from 21+ to 22+.

Oh, OK, please check v4.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: KVM: x86: use proper port value when checking io instruction permission (v3)

2011-05-29 Thread Avi Kivity

On 05/26/2011 02:56 PM, Marcelo Tosatti wrote:

Commit fa4491a6b667304 moved the permission check for io instructions
to the ->check_perm callback. It failed to copy the port value from RDX
register for string and "in,out ax,dx" instructions.

Fix it by reading RDX register at decode stage when appropriate.

Fixes FC8.32 installation.

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 3bc6b7a..fc3d2d9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -47,7 +47,7 @@
  #define DstDI   (5<<1)  /* Destination is in ES:(E)DI */
  #define DstMem64(6<<1)  /* 64bit memory operand */
  #define DstImmUByte (7<<1)  /* 8-bit unsigned immediate operand */
-#define DstMask (7<<1)
+#define DstMask ((7<<1) | (1<<18))


Ouch, why??

My comment about bits 19-20 was only about the shifting of of Prot etc. 
from 21+ to 22+.


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


KVM: x86: use proper port value when checking io instruction permission (v3)

2011-05-26 Thread Marcelo Tosatti

Commit fa4491a6b667304 moved the permission check for io instructions
to the ->check_perm callback. It failed to copy the port value from RDX
register for string and "in,out ax,dx" instructions.

Fix it by reading RDX register at decode stage when appropriate.

Fixes FC8.32 installation.

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 3bc6b7a..fc3d2d9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -47,7 +47,7 @@
 #define DstDI   (5<<1) /* Destination is in ES:(E)DI */
 #define DstMem64(6<<1) /* 64bit memory operand */
 #define DstImmUByte (7<<1) /* 8-bit unsigned immediate operand */
-#define DstMask (7<<1)
+#define DstMask ((7<<1) | (1<<18))
 /* Source operand type. */
 #define SrcNone (0<<4) /* No source operand. */
 #define SrcReg  (1<<4) /* Register operand. */
@@ -64,7 +64,7 @@
 #define SrcMemFAddr (0xc<<4)   /* Source is far address in memory */
 #define SrcAcc  (0xd<<4)   /* Source Accumulator */
 #define SrcImmU16   (0xe<<4)/* Immediate operand, unsigned, 16 bits */
-#define SrcMask (0xf<<4)
+#define SrcMask ((0xf<<4) | (1<<19))
 /* Generic ModRM decode. */
 #define ModRM   (1<<8)
 /* Destination is only written; never read. */
@@ -79,6 +79,8 @@
 #define Prefix  (3<<14) /* Instruction varies with 66/f2/f3 prefix */
 #define RMExt   (4<<14) /* Opcode extension in ModRM r/m if mod == 3 */
 #define Sse (1<<17) /* SSE Vector instruction */
+#define DstDX   (1<<18)/* Destination is in DX register */
+#define SrcDX   (1<<19)/* Source is in DX register */
 /* Misc flags */
 #define Prot(1<<21) /* instruction generates #UD if not in prot-mode */
 #define VendorSpecific (1<<22) /* Vendor specific instruction */
@@ -3124,8 +3126,8 @@ static struct opcode opcode_table[256] = {
I(DstReg | SrcMem | ModRM | Src2Imm, em_imul_3op),
I(SrcImmByte | Mov | Stack, em_push),
I(DstReg | SrcMem | ModRM | Src2ImmByte, em_imul_3op),
-   D2bvIP(DstDI | Mov | String, ins, check_perm_in), /* insb, insw/insd */
-   D2bvIP(SrcSI | ImplicitOps | String, outs, check_perm_out), /* outsb, 
outsw/outsd */
+   D2bvIP(DstDI | SrcDX | Mov | String, ins, check_perm_in), /* insb, 
insw/insd */
+   D2bvIP(SrcSI | DstDX | String, outs, check_perm_out), /* outsb, 
outsw/outsd */
/* 0x70 - 0x7F */
X16(D(SrcImmByte)),
/* 0x80 - 0x87 */
@@ -3182,8 +3184,8 @@ static struct opcode opcode_table[256] = {
/* 0xE8 - 0xEF */
D(SrcImm | Stack), D(SrcImm | ImplicitOps),
D(SrcImmFAddr | No64), D(SrcImmByte | ImplicitOps),
-   D2bvIP(SrcNone | DstAcc, in,  check_perm_in),
-   D2bvIP(SrcAcc | ImplicitOps, out, check_perm_out),
+   D2bvIP(SrcDX | DstAcc, in,  check_perm_in),
+   D2bvIP(SrcAcc | DstDX, out, check_perm_out),
/* 0xF0 - 0xF7 */
N, DI(ImplicitOps, icebp), N, N,
DI(ImplicitOps | Priv, hlt), D(ImplicitOps),
@@ -3580,6 +3582,12 @@ done_prefixes:
memop.bytes = c->op_bytes + 2;
goto srcmem_common;
break;
+   case SrcDX:
+   c->src.type = OP_REG;
+   c->src.bytes = 2;
+   c->src.addr.reg = &c->regs[VCPU_REGS_RDX];
+   fetch_register_operand(&c->src);
+   break;
}
 
if (rc != X86EMUL_CONTINUE)
@@ -3649,6 +3657,12 @@ done_prefixes:
c->dst.addr.mem.seg = VCPU_SREG_ES;
c->dst.val = 0;
break;
+   case DstDX:
+   c->dst.type = OP_REG;
+   c->dst.bytes = 2;
+   c->dst.addr.reg = &c->regs[VCPU_REGS_RDX];
+   fetch_register_operand(&c->dst);
+   break;
case ImplicitOps:
/* Special instructions do their own operand decoding. */
default:
@@ -3993,7 +4007,6 @@ special_insn:
break;
case 0xec: /* in al,dx */
case 0xed: /* in (e/r)ax,dx */
-   c->src.val = c->regs[VCPU_REGS_RDX];
do_io_in:
if (!pio_in_emulated(ctxt, c->dst.bytes, c->src.val,
 &c->dst.val))
@@ -4001,7 +4014,6 @@ special_insn:
break;
case 0xee: /* out dx,al */
case 0xef: /* out dx,(e/r)ax */
-   c->dst.val = c->regs[VCPU_REGS_RDX];
do_io_out:
ops->pio_out_emulated(ctxt, c->src.bytes, c->dst.val,
  &c->src.val, 1);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html