[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3 2/3] x86/mm: allocate logdirty_ranges for altp2ms

On 10/30/18 6:22 PM, Jan Beulich wrote:
>>>> On 29.10.18 at 13:40, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> This patch is a pre-requisite for the one fixing VGA logdirty
>> freezes when using altp2m. It only concerns itself with the
>> ranges allocation / deallocation / initialization part.
> But while looking (briefly only for now) over patch 3 I couldn't
> see any sync-ing of the log-dirty ranges there either. Doesn't
> this need doing either there or here, if you go the copy route?

The syncing is done in patch 3, in p2m_change_entry_type_global(),
p2m_memory_type_changed(), p2m_change_type_range(), where now all active
p2ms (hostp2m + altp2ms) receive the same changes.

On allocating the logdirty_ranges for a new altp2m, the hostp2m (which
is sync with all others) is copied over, and then whatever logdirty
ranges the hypervisor is using for reading should be fine.

That was the thinking at least, am I missing something?

>> @@ -2271,6 +2297,7 @@ void p2m_flush_altp2m(struct domain *d)
>>      {
>>          p2m_flush_table(d->arch.altp2m_p2m[i]);
>>          /* Uninit and reinit ept to force TLB shootdown */
>> +        p2m_free_logdirty(d->arch.altp2m_p2m[i]);
>>          ept_p2m_uninit(d->arch.altp2m_p2m[i]);
>>          ept_p2m_init(d->arch.altp2m_p2m[i]);
>>          d->arch.altp2m_eptp[i] = mfn_x(INVALID_MFN);
>> @@ -2341,6 +2385,7 @@ int p2m_destroy_altp2m_by_id(struct domain *d, 
>> unsigned int idx)
>>          {
>>              p2m_flush_table(d->arch.altp2m_p2m[idx]);
>>              /* Uninit and reinit ept to force TLB shootdown */
>> +            p2m_free_logdirty(d->arch.altp2m_p2m[idx]);
>>              ept_p2m_uninit(d->arch.altp2m_p2m[idx]);
>>              ept_p2m_init(d->arch.altp2m_p2m[idx]);
>>              d->arch.altp2m_eptp[idx] = mfn_x(INVALID_MFN);
>> @@ -2471,6 +2516,7 @@ static void p2m_reset_altp2m(struct p2m_domain *p2m)
>>  {
>>      p2m_flush_table(p2m);
>>      /* Uninit and reinit ept to force TLB shootdown */
>> +    p2m_free_logdirty(p2m);
>>      ept_p2m_uninit(p2m);
>>      ept_p2m_init(p2m);
>>      p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
> For one these look all pretty similar, so I wonder why there's
> no helper function. But that's not something you need to change.
> Yet why are you freeing the log-dirty ranges here? These aren't
> full cleanup paths afaict.

After the first iteration Andrew has suggested that I should free in


at which point I did so for V2, where you've suggested that that
function should be static and live in p2m.c. Which led to V3 where
basically I've plopped this function right before ept_p2m_uninit(p2m)
wherever it was called to match the V2 behaviour.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.