Re: [Xen-devel] [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 07/25/2017 08:40 PM, Razvan Cojocaru wrote:
> On 07/18/2017 01:20 PM, Razvan Cojocaru wrote:
>> On 07/18/2017 01:09 PM, Andrew Cooper wrote:
>>> On 18/07/17 10:37, Petre Pircalabu wrote:
>>>> If case of a vm_event with the emulate_flags set, if the instruction
>>>> cannot be emulated, the monitor should be notified instead of directly
>>>> injecting a hw exception.
>>>> This behavior can be used to re-execute an instruction not supported by
>>>> the emulator using the real processor (e.g. altp2m) instead of just
>>>> crashing.
>>>>
>>>> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
>>>
>>> There are many situations which end up failing an emulation with
>>> UNHANDLEABLE.
>>>
>>> What exact scenario are you looking to catch?  Is it just instructions
>>> which aren't implemented in the emulator?
>>
>> Instructions that are not implemented in the emulator are our main use
>> case for this, yes. In which case, we'd like a chance to be able to
>> single-step them (using altp2m), so that the guest will continue to run
>> even with an incomplete emulator.
>>
>> We don't care about instructions that would have failed to run in both
>> scenarios (emulated or single-stepped). I'm not sure if there are other
>> cases in which an instruction, although supported by the emulator, would
>> fail emulation but pass single-stepping.
> 
> Is there further action required on our part the get this patch
> commited? FWIW, while not ideal, from our perspective trying to
> single-step an instruction that was UNHANDLEABLE when attempting to
> emulate it is acceptable (even if UNHANDLEABLE doesn't mean that the
> instruction is not supported by the emulator). At worst it won't run
> when single-stepped either, and that'll be that.
Since this seems to be blocked as-is, I propose transforming this patch
into a series, with one patch adding a new return code specifically for
unsupported instructions (X86_EMUL_UNIMPLEMENTED or
X86_EMUL_UNSUPPORTED?), and this patch sending the vm_event out only for
that. (In which case the event's name should probably change as well to
reflect the name of the new error code.)

Thoughts?


Thanks,
Razvan

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