> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 02 May 2019 14:57 > To: Andrew Cooper <andrew.coop...@citrix.com> > Cc: Paul Durrant <paul.durr...@citrix.com>; Roger Pau Monne > <roger....@citrix.com>; Wei Liu > <wei.l...@citrix.com>; George Dunlap <george.dun...@citrix.com>; xen-devel > <xen- > de...@lists.xenproject.org> > Subject: Re: [PATCH] x86/HVM: p2m_ram_ro is incompatible with device > pass-through > > >>> On 02.05.19 at 15:42, <andrew.coop...@citrix.com> wrote: > > On 02/05/2019 14:16, Jan Beulich wrote: > >>>>> On 02.05.19 at 14:59, <andrew.coop...@citrix.com> wrote: > >>> On 02/05/2019 13:29, Jan Beulich wrote: > >>>> --- a/xen/drivers/passthrough/pci.c > >>>> +++ b/xen/drivers/passthrough/pci.c > >>>> @@ -1450,17 +1450,36 @@ static int assign_device(struct domain * > >>>> if ( !iommu_enabled || !hd->platform_ops ) > >>>> return 0; > >>>> > >>>> - /* Prevent device assign if mem paging or mem sharing have been > >>>> - * enabled for this domain */ > >>>> - if ( unlikely(d->arch.hvm.mem_sharing_enabled || > >>>> - vm_event_check_ring(d->vm_event_paging) || > >>>> + domain_lock(d); > >>>> + > >>>> + /* > >>>> + * Prevent device assignment if any of > >>>> + * - mem paging > >>>> + * - mem sharing > >>>> + * - the p2m_ram_ro type > >>>> + * - global log-dirty mode > >>> XenServer has working live migration with GPUs, which this change would > >>> regress. > >>> > >>> Behind the scenes, we combine Xen's logdirty bitmap with one provided by > >>> the GPU, to ensure that all dirty updates are tracked. > >> But this says nothing for the patch here. > > > > Yes it does. > > Well, okay, then the wording of your reply plus me just adding > a comment for a pre-existing check has mislead me. > > > There is nothing inherent about global log-dirty mode which is > > incompatible with passthrough when the IOMMU pagetables are not shared > > with EPT. > > > >> As long as it doesn't work in the staging tree, it should be prevented. > > > > ... but it does work. > > Note how (as said above) the patch does _not_ add any > ->global_logdirty check, it merely adds a comment enumerating > everything that's getting checked. I'm afraid I don't see how > adding a comment to state how things are can regress anything. > > The only check the patch adds is that of the new > p2m_ram_ro_used flag. >
Actually, since global_logdirty is somewhat transient should we not also have a check to prevent p2m_ram_logdirty from being set for a domain with assigned devices and shared EPT? Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
