Re: [PATCH RFC V7 1/5] xen: Emulate with no writes

["Jan Beulich" <JBeulich@suse.com>]

>>> On 26.08.14 at 16:01, <rcojocaru@bitdefender.com> wrote:
> On 08/26/2014 04:56 PM, Jan Beulich wrote:
>>>>> On 13.08.14 at 17:28, <rcojocaru@bitdefender.com> wrote:
>>> +void hvm_emulate_one_full(bool_t nowrite, unsigned int trapnr,
>>> +    unsigned int errcode)
>>> +{
>>> +    struct hvm_emulate_ctxt ctx = {{ 0 }};
>>> +    int rc;
>>> +
>>> +    hvm_emulate_prepare(&ctx, guest_cpu_user_regs());
>>> +
>>> +    if ( nowrite )
>>> +        rc = hvm_emulate_one_no_write(&ctx);
>>> +    else
>>> +        rc = hvm_emulate_one(&ctx);
>>> +
>>> +    switch ( rc )
>>> +    {
>>> +    case X86EMUL_UNHANDLEABLE:
>>> +        gdprintk(XENLOG_DEBUG, "Emulation failed @ %04x:%lx: "
>>> +               "%02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
>>> +               hvmemul_get_seg_reg(x86_seg_cs, &ctx)->sel,
>>> +               ctx.insn_buf_eip,
>>> +               ctx.insn_buf[0], ctx.insn_buf[1],
>>> +               ctx.insn_buf[2], ctx.insn_buf[3],
>>> +               ctx.insn_buf[4], ctx.insn_buf[5],
>>> +               ctx.insn_buf[6], ctx.insn_buf[7],
>>> +               ctx.insn_buf[8], ctx.insn_buf[9]);
>>> +        hvm_inject_hw_exception(trapnr, errcode);
>>> +        break;
>>> +    case X86EMUL_EXCEPTION:
>>> +        if ( ctx.exn_pending )
>>> +            hvm_inject_hw_exception(ctx.exn_vector, ctx.exn_error_code);
>>> +        break;
>> 
>> Shouldn't you act on X86EMUL_RETRY here? Or at least not fall through
>> to the writeback below?
> 
> Thanks for the review, I did initially loop around hvm_emulate_one()
> until rc != X86EMUL_RETRY, but I've been told that that might block
> against time calibration rendezvous points.

In any event it strikes me as odd that you ignore that state
altogether rather than propagating it back up, so that someone
in suitable position to do the retry can invoke it.

Jan