From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 5/5] just realized that it's broken Date: Wed, 04 Feb 2009 08:49:39 +0000 Message-ID: References: <49895F7C.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <49895F7C.76E4.0078.0@novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 04/02/2009 08:27, "Jan Beulich" 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