On 04.06.2020 12:50, Paul Durrant wrote:
>> From: Jan Beulich <jbeul...@suse.com>
>> Sent: 04 June 2020 11:34
>>
>> 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.
Even with sync-ed IOMMU mappings dirtying happening by I/O won't be
noticed, and hence the purpose of global log-dirty is undermined.
Jan
