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:26, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 02.10.2019 10:59, Paul Durrant wrote:
> > On Wed, 2 Oct 2019 at 09:42, Jan Beulich <jbeulich@xxxxxxxx> 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.
> >>
> >
> > This is a small patch and it does not close the door on being able to
> > add such an interface later. I'm not saying that I dislike that
> > alternative, but it will inevitably be quite a lot more code and I'm
> > not sure it really buys anything.
> >
> >> What's worse though - you don't go all the way to the end of what your
> >> argumentation would lead to: There's no reason for Xen to veto the
> >> request then even in the shared page table case.
> >
> > Well, I address that in the commit comment.
>
> Do you? I've just read it again, without finding mention of this case.
>
> >> The only device
> >> assigned to a guest in question may be doing DMA reads only. Following
> >> your reasoning, Xen shouldn't be getting in the way then either. Once
> >> again the situation could be taken care of by informing Xen about this
> >> property of a device (assuming it can't tell all by itself).
> >
> > I am not aware of a mechansim to configure even a PCI express device
> > to only allow read TLPs and thus we must assume that any device with
> > bus mastering enabled may attempt to issue a write TLP. Thus I think
> > it is reasonable for Xen to veto logdirty in the case of shared EPT
> > because a side effect of Xen's behaviour may have detrimental affect
> > on device functionality, and cause bus errors to be reported.
>
> I don't follow you here: There's no need to configure a device this
> way. If it is claimed (e.g. by an admin or its manufacturer) to only
> ever issue DMA reads, then that's all we need. Any erroneous or
> malicious write (other than perhaps the ones to trigger MSI) would
> potentially result in misbehavior of the guest, but not the host (I
> don't see why you mention "bus errors" - it would be IOMMU faults,
> with associated aborts reported back to the device). And it's only
> the latter you say you're concerned about when it comes to allow Xen
> to veto something.

ISTR that for at least some systems IOMMU faults are reported as bus
errors in the f/w logs. Such faults (deliberately caused during
development of IOMMU code) have actually led to one of our sysadmins
declaring my test rig to be faulty. This is why I'm keen to avoid such
faults.

>
> > I guess
> > it would be reasonable to check all assigned devices' BME bit and only
> > veto if any are set though. I would prefer that be an incremental
> > patch though.
>
> Sure - this wouldn't really belong here.
>

Ok, good.

  Paul

> Jan

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