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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 01 October 2019 09:46
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
> <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Juergen 
> Gross <jgross@xxxxxxxx>; Wei
> Liu <wl@xxxxxxx>
> Subject: Re: [Xen-devel] [PATCH-for-4.13] x86/mm: don't needlessly veto 
> migration
> 
> On 01.10.2019 10:28, 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).
> 
> But that's taking into account only half of the reason of the
> exclusion. The other half is that assigned devices may cause pages
> to be dirtied behind the back of the log-dirty logic.

But that's no reason to veto logdirty. Some devices have drivers (in dom0) 
which can extract DMA dirtying information and set dirty tracking information 
appropriately.

  Paul

> Therefore ...
> 
> > --- a/xen/arch/x86/mm/paging.c
> > +++ b/xen/arch/x86/mm/paging.c
> > @@ -209,15 +209,15 @@ static int paging_free_log_dirty_bitmap(struct domain 
> > *d, int rc)
> >      return rc;
> >  }
> >
> > -int paging_log_dirty_enable(struct domain *d, bool_t log_global)
> > +int paging_log_dirty_enable(struct domain *d, bool log_global)
> >  {
> >      int ret;
> >
> > -    if ( is_iommu_enabled(d) && log_global )
> > +    if ( has_arch_pdevs(d) && iommu_use_hap_pt(d) && log_global )
> 
> ... the iommu_use_hap_pt(d) needs to be dropped again, I think.
> 
> > --- a/xen/drivers/passthrough/pci.c
> > +++ b/xen/drivers/passthrough/pci.c
> > @@ -1486,11 +1486,15 @@ static int assign_device(struct domain *d, u16 seg, 
> > u8 bus, u8 devfn, u32
> flag)
> >      if ( !is_iommu_enabled(d) )
> >          return 0;
> >
> > -    /* Prevent device assign if mem paging or mem sharing have been
> > -     * enabled for this domain */
> > +    /*
> > +     * Prevent device assign if mem paging or mem sharing have been
> > +     * enabled for this domain, or logdirty is enabled and the P2M is
> > +     * shared with the IOMMU.
> > +     */
> >      if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
> >                    vm_event_check_ring(d->vm_event_paging) ||
> > -                  p2m_get_hostp2m(d)->global_logdirty) )
> > +                  (p2m_get_hostp2m(d)->global_logdirty &&
> > +                   iommu_use_hap_pt(d))) )
> 
> This change wants dropping altogether then.
> 
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel