> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 04 June 2020 11:34
> To: p...@xen.org
> Cc: 'Igor Druzhinin' <igor.druzhi...@citrix.com>;
> xen-devel@lists.xenproject.org;
> andrew.coop...@citrix.com; w...@xen.org; roger....@citrix.com;
> george.dun...@citrix.com
> Subject: Re: [PATCH for-4.14 v3] x86/svm: do not try to handle recalc NPT
> faults immediately
>
> On 04.06.2020 09:49, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Igor Druzhinin <igor.druzhi...@citrix.com>
> >> Sent: 03 June 2020 23:42
> >> To: xen-devel@lists.xenproject.org
> >> Cc: jbeul...@suse.com; andrew.coop...@citrix.com; w...@xen.org;
> >> roger....@citrix.com;
> >> george.dun...@citrix.com; p...@xen.org; Igor Druzhinin
> >> <igor.druzhi...@citrix.com>
> >> Subject: [PATCH for-4.14 v3] x86/svm: do not try to handle recalc NPT
> >> faults immediately
> >>
> >> A recalculation NPT fault doesn't always require additional handling
> >> in hvm_hap_nested_page_fault(), moreover in general case if there is no
> >> explicit handling done there - the fault is wrongly considered fatal.
> >>
> >> This covers a specific case of migration with vGPU assigned which
> >> uses direct MMIO mappings made by XEN_DOMCTL_memory_mapping hypercall:
> >> at a moment log-dirty is enabled globally, recalculation is requested
> >> for the whole guest memory including those mapped MMIO regions
> >
> > I still think it is odd to put this in the commit comment since, as I
> > said before, Xen ensures that this situation cannot happen at
> > the moment.
>
> Aiui Igor had replaced reference to passed-through devices by reference
> to mere handing of an MMIO range to a guest. Are you saying we suppress
> log-dirty enabling in this case as well? I didn't think we do:
No, but the comment says "migration with vGPU *assigned*" (my emphasis), which
surely means has_arch_pdevs() will be true.
>
> if ( has_arch_pdevs(d) && log_global )
> {
> /*
> * Refuse to turn on global log-dirty mode
> * if the domain is sharing the P2M with the IOMMU.
> */
> return -EINVAL;
> }
>
> Seeing this code I wonder about the non-sharing case: If what the
> comment says was true, the condition would need to change, but I
> think it's the comment which is wrong, and we don't want global
> log-dirty as long as an IOMMU is in use at all for a domain.
I think is the comment that is correct, not the condition. It is only when
using shared EPT that enabling logdirty is clearly an unsafe thing to do. Using
sync-ed IOMMU mappings should be ok.
Paul
