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

[Gleb Natapov <gleb@redhat.com>]

On Thu, May 26, 2011 at 09:31:50AM +0300, Avi Kivity wrote:
> On 05/25/2011 09:18 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.
> >
> >+#define Sse         (1<<18)     /* SSE Vector instruction */
> 
> 19/20 are still available, no need to go 64-bit just yet.
> 
> >  /* Misc flags */
> >-#define Prot        (1<<21) /* instruction generates #UD if not in prot-mode */
> >
> >+	case SrcDX:
> >+		c->src.type = OP_REG;
> >+		c->src.bytes = c->op_bytes;
> 
> Needs to be 2.  Otherwise we'll see extra bits from edx, or lose
> bits from dx if it's a 1-byte instruction.
> 
But those extra bits will be dropped by check_perm_in() anyway. Can
c->op_bytes ever be 1?

> >+		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 = c->op_bytes;
> 
> 2 again.
> 
> >+		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:
> 
> We also need to unify Src/Dst decode eventually.
> 
> -- 
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.

--
			Gleb.