Re: [PATCH] Don't track all memory when enabling log dirty to track vram

[George Dunlap <george.dunlap@eu.citrix.com>]

On 02/17/2014 10:18 AM, Jan Beulich wrote:
>>>> On 13.02.14 at 17:20, Tim Deegan <tim@xen.org> wrote:
>> At 15:55 +0000 on 13 Feb (1392303343), Jan Beulich wrote:
>>>>>> On 13.02.14 at 16:46, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>> On 02/12/2014 12:53 AM, Zhang, Yang Z wrote:
>>>>> George Dunlap wrote on 2014-02-11:
>>>>>> I think I got a bit distracted with the "A isn't really so bad" thing.
>>>>>> Actually, if the overhead of not sharing tables isn't very high, then
>>>>>> B isn't such a bad option.  In fact, B is what I expected Yang to
>>>>>> submit when he originally described the problem.
>>>>> Actually, the first solution came to my mind is B. Then I realized that
>> even
>>>> chose B, we still cannot track the memory updating from DMA(even with A/D
>>>> bit, it still a problem). Also, considering the current usage case of log
>>>> dirty in Xen(only vram tracking has problem), I though A is better.:
>>>> Hypervisor only need to track the vram change. If a malicious guest try to
>>>> DMA to vram range, it only crashed himself (This should be reasonable).
>>>>>> I was going to say, from a release perspective, B is probably the
>>>>>> safest option for now.  But on the other hand, if we've been testing
>>>>>> sharing all this time, maybe switching back over to non-sharing whole-hog has
>>>> the higher risk?
>>>>> Another problem with B is that current VT-d large paging supporting relies
>> on
>>>> the sharing EPT and VT-d page table. This means if we choose B, then we need
>>>> to re-enable VT-d large page. This would be a huge performance impaction for
>>>> Xen 4.4 on using VT-d solution.
>>>>
>>>> OK -- if that's the case, then it definitely tips the balance back to
>>>> A.  Unless Tim or Jan disagrees, can one of you two check it in?
>>>>
>>>> Don't rush your judgement; but it would be nice to have this in before
>>>> RC4, which would mean checking it in today preferrably, or early
>>>> tomorrow at the latest.
>>> That would be Tim then, as he would have to approve of it anyway.
>> Done.
> Actually I'm afraid there are two problems with this patch:
>
> For one, is enabling "global" log dirty mode still going to work
> after VRAM-only mode already got enabled? I ask because the
> paging_mode_log_dirty() check which paging_log_dirty_enable()
> does first thing suggests otherwise to me (i.e. the now
> conditional setting of all p2m entries to p2m_ram_logdirty would
> seem to never get executed). IOW I would think that we're now
> lacking a control operation allowing the transition from dirty VRAM
> tracking mode to full log dirty mode.

Hrm, will so far playing with this I've been unable to get a localhost 
migrate to fail with the vncviewer attached.  Which seems a bit strange...

  -George