Re: [PATCH v4 for-4.14] x86/monitor: revert default behavior when monitoring register write events

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Jun 9, 2020 at 3:48 AM Paul Durrant <xadimgnik@xxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: Jan Beulich <jbeulich@xxxxxxxx>
> > Sent: 09 June 2020 10:37
> > To: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
> > <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>;
> > Roger Pau Monné <roger.pau@xxxxxxxxxx>; George Dunlap 
> > <george.dunlap@xxxxxxxxxx>; Ian Jackson
> > <ian.jackson@xxxxxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano 
> > Stabellini
> > <sstabellini@xxxxxxxxxx>; Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>; Petre 
> > Pircalabu
> > <ppircalabu@xxxxxxxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>
> > Subject: Re: [PATCH v4 for-4.14] x86/monitor: revert default behavior when 
> > monitoring register write
> > events
> >
> > On 03.06.2020 14:52, Tamas K Lengyel wrote:
> > > For the last couple years we have received numerous reports from users of
> > > monitor vm_events of spurious guest crashes when using events. In 
> > > particular,
> > > it has observed that the problem occurs when vm_events are being 
> > > disabled. The
> > > nature of the guest crash varied widely and has only occured 
> > > occasionally. This
> > > made debugging the issue particularly hard. We had discussions about this 
> > > issue
> > > even here on the xen-devel mailinglist with no luck figuring it out.
> > >
> > > The bug has now been identified as a race-condition between register event
> > > handling and disabling the monitor vm_event interface. The default 
> > > behavior
> > > regarding emulation of register write events is changed so that they get
> > > postponed until the corresponding vm_event handler decides whether to 
> > > allow such
> > > write to take place. Unfortunately this can only be implemented by 
> > > performing the
> > > deny/allow step when the vCPU gets scheduled.
> > >
> > > Due to that postponed emulation of the event if the user decides to pause 
> > > the
> > > VM in the vm_event handler and then disable events, the entire emulation 
> > > step
> > > is skipped the next time the vCPU is resumed. Even if the user doesn't 
> > > pause
> > > during the vm_event handling but exits immediately and disables vm_event, 
> > > the
> > > situation becomes racey as disabling vm_event may succeed before the 
> > > guest's
> > > vCPUs get scheduled with the pending emulation task. This has been 
> > > particularly
> > > the case with VMS that have several vCPUs as after the VM is unpaused it 
> > > may
> > > actually take a long time before all vCPUs get scheduled.
> > >
> > > In this patch we are reverting the default behavior to always perform 
> > > emulation
> > > of register write events when the event occurs. To postpone them can be 
> > > turned
> > > on as an option. In that case the user of the interface still has to take 
> > > care
> > > of only disabling the interface when its safe as it remains buggy.
> > >
> > > Fixes: 96760e2fba10 ('vm_event: deny register writes if refused by 
> > > vm_event
> > > reply').
> > >
> > > Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> >
> > Applicable parts
> > Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> >
>
> Release-acked-by: Paul Durrant <paul@xxxxxxx>

Thanks guys!

Tamas