Re: [PATCH 3/5] x86/hvm: RFC - PROBABLY BROKEN - Defer all debugging/monitor actions to {svm,vmx}_inject_event()

[Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>]

On 8/22/23 01:34, Tamas K Lengyel wrote:
> On Mon, Aug 21, 2023 at 5:57 PM Jinoh Kang <jinoh.kang.kr@xxxxxxxxx> wrote:
>> This is RFC because it probably breaks introspection, as injection replies
>> from the introspection engine will (probably, but I haven't confirmed) 
>> trigger
>> new monitor events.
>>
>> First of all, monitoring emulated debug events is a change in behaviour,
>> although IMO it is more of a bugfix than a new feature.  Also, similar 
>> changes
>> will happen to other monitored events as we try to unify the
>> intercept/emulation paths.
>>
>> As for the recursive triggering of monitor events, I was considering 
>> extending
>> the monitor infrastructure to have a "in monitor reply" boolean which causes
>> hvm_monitor_debug() to ignore the recursive request.
>>
>> Does this plan sound ok, or have I missed something more subtle with monitor
>> handling?
> 
> Sorry but from a monitor perspective this is a no-go. AFAIU injection
> will only happen once the vCPU gets rescheduled after the exit, which
> may take a long time, introducing potentially significant delay.

AFAIU hvm_monitor_debug() call still happens *before* VMEntry when the softirq
processing and actual injection takes place.  vmx_inject_event() is responsible
for queueing an event to the inject slot, not processing what is already
injected.

> The monitor agent itself may need to inject events or alter what gets
> injected, so now it may need to undo the injection of the event from
> where it was called from - if that's even possible.

The monitor still gets the chance to drop the injection; 'rc > 0' from
hvm_monitor_debug() still results in the injection request simply being
ignored.

> I also find the
> change in behavior whereby we would be getting events for emulated
> events on the monitor ring equiavelly problematic, making it very hard
> to reason about the state of the VM on the monitor side. If that could
> be made optional that a monitor user can subscribe to that would be
> fine, but doing so unconditionally is also no-go.

We could use an additional field to 'struct x86_event' to indicate the
source of the injected exception--be it VMExit, emulator, or
monitor-triggered events.  I'd like to hear about Andrew's opinion on
this first, though.

> 
> Tamas

-- 
Sincerely,
Jinoh Kang