On Mon, Aug 06, 2012 at 11:50:20AM +0300, Avi Kivity wrote:
> On 07/30/2012 05:38 PM, Gleb Natapov wrote:
> > Optimize "rep ins" by allowing emulator to write back more than one
> > datum at a time. Introduce new operand type OP_MEM_STR which tells
> > writeback() that dst contains pointer to an array that should be written
> > back as opposite to just one data element.
> > 
> >     }
> >  
> > -   memcpy(dest, rc->data + rc->pos, size);
> > -   rc->pos += size;
> > +   if (ctxt->rep_prefix && !(ctxt->eflags & EFLG_DF)) {
> > +           ctxt->dst.data = rc->data + rc->pos;
> > +           ctxt->dst.type = OP_MEM_STR;
> > +           ctxt->dst.count = (rc->end - rc->pos) / size;
> > +           rc->pos = rc->end;
> 
> Should take into account the segment limit.
> 
It does. During write back. pio_in_emulated() should linearize() address
before calculating page boundary, but this is (minor) bug unrelated to the patch
series.

> > +   } else {
> > +           memcpy(dest, rc->data + rc->pos, size);
> > +           rc->pos += size;
> > +   }
> >     return 1;
> >  }
> >  
> > @@ -1500,6 +1507,14 @@ static int writeback(struct x86_emulate_ctxt *ctxt)
> >             if (rc != X86EMUL_CONTINUE)
> >                     return rc;
> >             break;
> > +   case OP_MEM_STR:
> > +           rc = segmented_write(ctxt,
> > +                           ctxt->dst.addr.mem,
> > +                           ctxt->dst.data,
> > +                           ctxt->dst.bytes * ctxt->dst.count);
> > +           if (rc != X86EMUL_CONTINUE)
> > +                   return rc;
> > +           break;
> >     case OP_XMM:
> >             write_sse_reg(ctxt, &ctxt->dst.vec_val, ctxt->dst.addr.xmm);
> >             break;
> > @@ -2732,7 +2747,7 @@ int emulator_task_switch(struct x86_emulate_ctxt 
> > *ctxt,
> >  static void string_addr_inc(struct x86_emulate_ctxt *ctxt, int reg,
> >             struct operand *op)
> >  {
> > -   int df = (ctxt->eflags & EFLG_DF) ? -1 : 1;
> > +   int df = (ctxt->eflags & EFLG_DF) ? -op->count : op->count;
> >  
> >     register_address_increment(ctxt, &ctxt->regs[reg], df * op->bytes);
> >     op->addr.mem.ea = register_address(ctxt, ctxt->regs[reg]);
> > @@ -3672,7 +3687,7 @@ 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),
> > -   I2bvIP(DstDI | SrcDX | Mov | String, em_in, ins, check_perm_in), /* 
> > insb, insw/insd */
> > +   I2bvIP(DstDI | SrcDX | Mov | String | Unaligned, em_in, ins, 
> > check_perm_in), /* insb, insw/insd */
> 
> Eww.
This brings us back to the question what alignment check is doing in
linearize :)

> 
> >     I2bvIP(SrcSI | DstDX | String, em_out, outs, check_perm_out), /* outsb, 
> > outsw/outsd */
> >     /* 0x70 - 0x7F */
> >     X16(D(SrcImmByte)),
> > @@ -3930,6 +3945,7 @@ static int decode_operand(struct x86_emulate_ctxt 
> > *ctxt, struct operand *op,
> >                     register_address(ctxt, ctxt->regs[VCPU_REGS_RDI]);
> >             op->addr.mem.seg = VCPU_SREG_ES;
> >             op->val = 0;
> > +           op->count = 1;
> >             break;
> >     case OpDX:
> >             op->type = OP_REG;
> > @@ -3973,6 +3989,7 @@ static int decode_operand(struct x86_emulate_ctxt 
> > *ctxt, struct operand *op,
> >                     register_address(ctxt, ctxt->regs[VCPU_REGS_RSI]);
> >             op->addr.mem.seg = seg_override(ctxt);
> >             op->val = 0;
> > +           op->count = 1;
> >             break;
> >     case OpImmFAddr:
> >             op->type = OP_IMM;
> > @@ -4513,8 +4530,14 @@ writeback:
> >             string_addr_inc(ctxt, VCPU_REGS_RDI, &ctxt->dst);
> >  
> >     if (ctxt->rep_prefix && (ctxt->d & String)) {
> > +           unsigned int count;
> >             struct read_cache *r = &ctxt->io_read;
> > -           register_address_increment(ctxt, &ctxt->regs[VCPU_REGS_RCX], 
> > -1);
> > +           if ((ctxt->d & SrcMask) == SrcSI)
> > +                   count = ctxt->src.count;
> > +           else
> > +                   count = ctxt->dst.count;
> 
> Does this work correctly for 'rep movs' and friends?
> 
(src|dst).count is initialized to 1 during decode, so anything that does
not touch "count" behaves exactly like before.

--
                        Gleb.
--
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

Reply via email to