Re: [Xen-devel] [PATCH v8 1/2] x86emul: New return code for unimplemented instruction

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.08.17 at 19:06, <ppircalabu@xxxxxxxxxxxxxxx> wrote:

Please don't top-post. It makes it quite hard to see ...

> The main use-case for the new return code is to have a clear distinction 
> between an instruction not implemented by the emulator (e.g. ?fldenv or 
> fnstenv) and the failure to emulate .
> 
> 
> - hvm_process_io_incercept returns X86EMUL_UNHANDLEABLE if one of the 
> hvm_io_ops (read/write) failed or one of the hvm_copy_to(_from)_guest_phys 
> returned an error code which is not handled in their correspondent switch 
> statement. In either cases this is not caused by an unimplemented 
> instruction.
> 
> - hvm_do_io / hvm_do_io_buffer call hvm_process_io_incercept which cannot 
> return unimplemented.
> 
> - Thank-you very much for pointing out the invoke_stub issue. I have added a 
> new label "unimplemented_insn" and updated the patch.

... which of the replies above correspond to which of my earlier
replies.

Jan

> I will re-send a new patchset with the changes and a more detailed 
> description of the places where the new return value was not handled.
> 
> 
> Many thanks,
> 
> Petre
> 
> 
> ________________________________
> From: Jan Beulich <JBeulich@xxxxxxxx>
> Sent: Tuesday, August 22, 2017 11:09 AM
> To: Petre Ovidiu PIRCALABU
> Cc: rcojocaru@xxxxxxxxxxxxxxx; andrew.cooper3@xxxxxxxxxx; 
> paul.durrant@xxxxxxxxxx; wei.liu2@xxxxxxxxxx; George.Dunlap@xxxxxxxxxxxxx; 
> ian.jackson@xxxxxxxxxxxxx; jun.nakajima@xxxxxxxxx; kevin.tian@xxxxxxxxx; 
> sstabellini@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx; 
> tamas@xxxxxxxxxxxxx; tim@xxxxxxx 
> Subject: Re: [PATCH v8 1/2] x86emul: New return code for unimplemented 
> instruction
> 
>>>> On 08.08.17 at 20:06, <ppircalabu@xxxxxxxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
> 
> What about the use in a switch() statement in hvmemul_do_io()
> in this file? And the use in hvmemul_do_io_buffer()?
> 
>> @@ -2044,6 +2044,8 @@ int hvm_emulate_one_mmio(unsigned long mfn, unsigned 
> long gla)
>>      switch ( rc )
>>      {
>>      case X86EMUL_UNHANDLEABLE:
>> +        /* fall-through */
>> +    case X86EMUL_UNIMPLEMENTED:
> 
> The fall-through comment is pointless in such a case.
> 
> hvm/intercept.c has a use in hvm_process_io_intercept() which
> looks like it needs dealing with too. And there are more. Any
> places you perhaps leave alone intentionally should be reasoned
> about in the description.
> 
>> @@ -7717,7 +7717,7 @@ x86_emulate(
>>
>>      default:
>>      cannot_emulate:
>> -        rc = X86EMUL_UNHANDLEABLE;
>> +        rc = X86EMUL_UNIMPLEMENTED;
> 
> There's at least one goto to the label here which can't stay as is
> (in invoke_stub()). Did you really audit them all?
> 
> Jan
> 
> 
> ________________________
> This email was scanned by Bitdefender




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