RE: [PATCH 3/3] x86/monitor: Add new monitor event to catch all vmexits

["Lengyel, Tamas" <tamas.lengyel@xxxxxxxxx>]

> -----Original Message-----
> From: Tian, Kevin <kevin.tian@xxxxxxxxx>
> Sent: Thursday, May 19, 2022 8:35 PM
> To: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Lengyel, Tamas <tamas.lengyel@xxxxxxxxx>; Wei Liu <wl@xxxxxxx>;
> Anthony PERARD <anthony.perard@xxxxxxxxxx>; Gross, Jurgen
> <jgross@xxxxxxxx>; Cooper, Andrew <andrew.cooper3@xxxxxxxxxx>;
> George Dunlap <george.dunlap@xxxxxxxxxx>; Beulich, Jan
> <JBeulich@xxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>; Petre
> Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>; Pau Monné, Roger
> <roger.pau@xxxxxxxxxx>; Nakajima, Jun <jun.nakajima@xxxxxxxxx>
> Subject: RE: [PATCH 3/3] x86/monitor: Add new monitor event to catch all
> vmexits
> 
> > From: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > Sent: Wednesday, May 18, 2022 11:02 PM
> >
> > On Thu, May 12, 2022 at 9:47 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > wrote:
> > >
> > > On Wed, May 4, 2022 at 9:12 AM Tamas K Lengyel
> <tamas@xxxxxxxxxxxxx>
> > wrote:
> > > >
> > > > On Wed, Apr 27, 2022 at 11:51 AM Tamas K Lengyel
> > > > <tamas.lengyel@xxxxxxxxx> wrote:
> > > > >
> > > > > Add monitor event that hooks the vmexit handler allowing for
> > > > > both sync
> > and
> > > > > async monitoring of events. With async monitoring an event is
> > > > > placed
> > on the
> > > > > monitor ring for each exit and the rest of the vmexit handler
> > > > > resumes
> > normally.
> > > > > If there are additional monitor events configured those will
> > > > > also place
> > their
> > > > > respective events on the monitor ring.
> > > > >
> > > > > With the sync version an event is placed on the monitor ring but
> > > > > the
> > handler
> > > > > does not get resumed, thus the sync version is only useful when
> > > > > the VM
> > is not
> > > > > expected to resume normally after the vmexit. Our use-case is
> > > > > primarily
> > with
> > > > > the sync version with VM forks where the fork gets reset after
> > > > > sync
> > vmexit
> > > > > event, thus the rest of the vmexit handler can be safely
> > > > > skipped. This is very useful when we want to avoid Xen crashing
> > > > > the VM under any
> > circumstance,
> > > > > for example during fuzzing. Collecting all vmexit information
> > > > > regardless
> > of
> > > > > the root cause makes it easier to reason about the state of the
> > > > > VM on
> > the
> > > > > monitor side, hence we opt to receive all events, even for
> > > > > external
> > interrupt
> > > > > and NMI exits and let the monitor agent decide how to proceed.
> > > > >
> > > > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
> > > > > ---
> > > > > v5: wrap vmexit fields in arch.vmx structures in the public
> > > > > vm_event ABI
> > > >
> > > > Patch ping. Could a toolstack maintainer please take a look at this?
> > > > The hypervisor side already has a Reviewed-by.
> > >
> > > Patch ping.
> >
> > Patch ping.
> >
> 
> I guess what you really missed is an ack from toostack maintainer, but
> anyway:
> 
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>

Thanks, the review is still appreciated!
Tamas