Re: [Xen-devel] [PATCH-for-4.13 v2] x86/mm: don't needlessly veto migration

[Paul Durrant <pdurrant@xxxxxxxxx>]

On Wed, 2 Oct 2019 at 10:42, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 02.10.2019 11:10, Andrew Cooper wrote:
> > On 02/10/2019 09:40, Jan Beulich wrote:
> >> On 01.10.2019 17:11, Paul Durrant wrote:
> >>> Now that xl.cfg has an option to explicitly enable IOMMU mappings for a
> >>> domain, migration may be needlessly vetoed due to the check of
> >>> is_iommu_enabled() in paging_log_dirty_enable().
> >>> There is actually no need to prevent logdirty from being enabled unless
> >>> devices are assigned to a domain and that domain is sharing HAP mappings
> >>> with the IOMMU (in which case disabling write permissions in the P2M may
> >>> cause DMA faults). It is quite possible that some assigned devices may
> >>> provide information about which pages may have been dirtied by DMA via
> >>> an API exported by their managing emulator. Thus Xen's logdirty map is 
> >>> only
> >>> one source of information that may be available to the toolstack when
> >>> performing a migration and hence it is the toolstack that is best placed
> >>> to decide under what circumstances it can be performed, not the 
> >>> hypervisor.
> >> While I'm happy about the extended description, it's still written in
> >> a way suggesting that this is the only possible way of viewing things.
> >> As expressed by George and me, putting the hypervisor in a position to
> >> be able to judge is at least an alternative worth considering.
> >
> > No, for exactly the same reason as I'm purging the disable_migrate flag.
> >
> > This is totally backwards thinking, because the check is in the wrong place.
> >
> > There really are cases where the toolstack, *and only* the toolstack is
> > in a position to determine migration safety.
>
> Then, as already said to Paul, the remaining vetoing in Xen is still too
> much. It then should wholesale trust the tool stack. But no, I don't think
> that's a viable model - when multiple parties are involved in some
> operation, it is quite common that all of them have to say "yes" in order
> for it to actually succeed. (It's not like anyone would have suggested
> for the tool stack to blindly trust Xen's judgement.)
>
> >  When it comes to
> > disable_migrate, the area under argument is the ITSC flag, which *is*
> > safe to offer on migrate for viridian guests which are known to use
> > reference_tsc, or if the destination hardware supports tsc scaling.
> > (Hilariously, nothing, not even the toolstack, prohibits migration based
> > on Xen's no-migrate flag, because its a write-only field which can't be
> > retrieved by the tools.)
> >
> > The two options are:
> >
> > 1) New hypercall,
> > DOMCTL_the_toolstack_knows_wtf_its_doing_so_let_the_doimain_migrate,
> > which disables the vetos,
>
> Nobody has ever suggested something like this. If (see above) the tool
> stack was to be blindly trusted on all matters, then such an operation
> wouldn't be needed in the first place.
>
> > or
> >
> > 2) Delete the erroneous vetos, and trust that the toolstack knows what
> > it is doing, and will only initiate a migrate in safe situations.
> >
> > Option 2 has the safety checks perfomed at the level which is actually
> > capable of calculating the results correctly.
>
> Then why don't we trust the tool stack to avoid assigning a device to
> a mem-paging or mem-sharing enabled guest? Surely the tool stack has
> means to know whether one of these is in use? And I'm not convinced
> there's a risk to the host if such was done in error; it's merely
> known that it wouldn't end well for the involved guest(s).

I haven't looked but I'd expect sharing or paging out a page to
involve clearing write perms or invalidating a P2M entry, in which
case the main danger with sharing (as with logdirty) comes from
sharing the P2M with the IOMMU and hence taking faults. Far paging
though, I don't know whether there is any logic to clear entries from
the IOMMU mappings when a page is invalidated. If not then that would
pose a significant danger to system integrity.

  Paul

>
> 3) Have the tool stack (assuming we consider emulators part of it)
> report the necessary (device) properties such that Xen is in a position
> to judge correctly itself.
>
> Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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