Re: [Xen-devel] [PATCH 4/4] x86emul: correct FPU code/data pointers and opcode handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.03.17 at 11:56, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 13/03/17 11:05, Jan Beulich wrote:
>> @@ -1027,13 +1076,48 @@ do {
>>  static void put_fpu(
>>      struct fpu_insn_ctxt *fic,
>>      bool failed_late,
>> +    const struct x86_emulate_state *state,
>>      struct x86_emulate_ctxt *ctxt,
>>      const struct x86_emulate_ops *ops)
>>  {
>>      if ( unlikely(failed_late) && fic->type == X86EMUL_FPU_fpu )
>> -        ops->put_fpu(ctxt, X86EMUL_FPU_fpu);
>> +        ops->put_fpu(ctxt, X86EMUL_FPU_fpu, NULL);
>> +    else if ( unlikely(fic->type == X86EMUL_FPU_fpu) && !state->fpu_ctrl )
>> +    {
>> +        struct x86_emul_fpu_aux aux = {
>> +            .ip = ctxt->regs->r(ip),
>> +            .cs = ctxt->regs->cs,
>> +            .op = ((ctxt->opcode & 7) << 8) | state->modrm,
>> +        };
>> +        struct segment_register sreg;
>> +
>> +        if ( ops->read_segment &&
>> +             ops->read_segment(x86_seg_cs, &sreg, ctxt) == X86EMUL_OKAY )
> 
> Why are the read_segment hooks optional here?
> 
> In the case that we are hitting FPU instructions for emulation, we can
> reasonably require read/write_segment hooks.
> 
> In particular, regs->%sreg are only valid for PV guests.

That's the precise reason I made their availability optional here:
A caller caring only about PV guests has no need to set these,
as long as for what might be emulated only selector values are
of interest. And it is clear that we can't distinguish guest types
here.

>> @@ -4231,8 +4268,10 @@ x86_emulate(
>>                  dst.bytes = 4;
>>                  break;
>>              case 4: /* fldenv - TODO */
>> +                state->fpu_ctrl = true;
> 
> Arguably, state->fpu_ctrl is a decode property rather than an emulation
> property.  It is the kind of information which we might plausibly want
> an x86_insn_*() accessor for.

If we had a consumer for this, I'd agree, but adding an accessor
without user and moving the changes here to the decode phase
(increasing code churn) seems counterproductive to me.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel