Re: [Xen-devel] Ping: [PATCH] x86/mm: drop bogus assertion

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 02:31 -0700 on 06 Dec (1512527481), Jan Beulich wrote:
> >>> On 30.11.17 at 10:10, <JBeulich@xxxxxxxx> wrote:
> > i.e. the guest specified a runstate area address which has a non-present
> > mapping in the page tables [EC=0002 CR2=ffff88003d405220], but that's
> > not something the hypervisor needs to be concerned about.) Release
> > builds work fine, which is a first indication that the assertion isn't
> > really needed.

Yep, this assertion should just go away, so:

Reviewed-by: Tim Deegan <tim@xxxxxxx>

> > What's worse though - there appears to be a timing window where the
> > guest runs in shadow mode, but not in log-dirty mode, and that is what
> > triggers the assertion (the same could, afaict, be achieved by test-
> > enabling shadow mode on a PV guest). This is because turing off log-
> > dirty mode is being performed in two steps: First the log-dirty bit gets
> > cleared (paging_log_dirty_disable() [having paused the domain] ->
> > sh_disable_log_dirty() -> shadow_one_bit_disable()), followed by
> > unpausing the domain and only then clearing shadow mode (via
> > shadow_test_disable(), which pauses the domain a second time).
> > 
> > Hence besides removing the ASSERT() here (or optionally replacing it by
> > explicit translate and refcounts mode checks, but this seems rather
> > pointless now that the three are tied together) I wonder whether either
> > shadow_one_bit_disable() should turn off shadow mode if no other bit
> > besides PG_SH_enable remains set (just like shadow_one_bit_enable()
> > enables it if not already set), or the domain pausing scope should be
> > extended so that both steps occur without the domain getting a chance to
> > run in between.

I'd be fine with either of those.  It would avoid unexpected surprises
in the relatively untested pv-shadow-without-logdirty paths.  

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel