Re: [PATCH 5/5] just realized that it's broken

[Keir Fraser <keir.fraser@eu.citrix.com>]

On 04/02/2009 08:27, "Jan Beulich" <jbeulich@novell.com> wrote:

> The issue mentioned above that I think current code has even without
> that patch (though I may easily have missed some aspect here) is that
> there is no enforcement of an immediate TLB flush when a writeable
> page's type count drops to zero - instead the flush is deferred until the
> end of the current operation or batch. With the removal of the use of
> the per-domain lock in these code paths it is no longer valid to defer
> the flush this much - it must be carried out before the page in question
> gets unlocked.

Oh, good point. We don't need the synchronous flush in free_page_type() any
more as we do not rely on correctness of the linear map any more (we use it,
but then verify sufficient correctness via get_page()/lock_page(), so now a
guest can only shoot itself in the foot).

I'll revert back to the lazy scheme we used to use. Which of course means
cpumask-in-the-page will not be safe. I will kill it off by other means.

For invalidate_shadow_ldt(), the LDT mappings are per-VCPU so only the local
CPU needs flushing. The reason for flush_tlb_mask() is I think a
conservative attempt at flushing the correct TLB if we are not the VCPU in
question. I would think it is better to use v->vcpu_dirty_cpumask, which
maybe didn't exist when i_s_l() was last looked at. I shall take a look.

 -- Keir