From: Christoph Egger Subject: Re: [PATCH 02/14] Nested Virtualization: localevent Date: Fri, 6 Aug 2010 11:17:33 +0200 Message-ID: <201008061117.33675.Christoph.Egger@amd.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On Thursday 05 August 2010 18:19:14 Keir Fraser wrote: > I seem to remember we discussed the reason for this a bit some time ago. Yes, I remember, too. > It looked to me like you were calling a function that makes sense only on a > running guest (and a locally currently running guest at that) after the > guest was dead, during cleanup/teardown. If I'm remembering correctly then > the fix would be to not do that then. ;-) I am not sure if I got you. I try to repeat you with my own words: This localevent patch on its own is pointless. It makes only sense when a guest (the level 1 guest) runs its own guest, the level 2 guest. When the level 2 guest was dead, during cleanup/teardown. I retry to explain: This localevent patch on its own is pointless, I agree with you in this case. The patch is needed when the level 1 guest was running a level 2 guest and get destroyed. During the destroy process Xen may still want to inject (pending) interrupts/events. For this reason, nestedhvm_vcpu_destroy() (added in patch 5/14) does a nestedsvm_vcpu_stgi() to prevent the interrupts/events from being blocked by hvm_interrupt_blocked() (see patch 9/14) and level 1 guest remaining in a zombie state. nestedsvm_vcpu_stgi() calls local_event_delivery_enable() (only on AMD now). At the time when nestedsvm_vcpu_stgi() is called from nestedhvm_vcpu_destroy(), "current" may be an invalid pointer while the vcpu pointer is valid. When local_event_delivery_enable() accesses "current" then Xen crashes. That is why local_event_delivery_enable() needs the vcpu argument. Christoph > -- Keir > > On 05/08/2010 16:46, "Keir Fraser" wrote: > > The functions are called local_event_delivery* because they implicitly > > act on current. They don't need to take a vcpu parameter. If you find you > > need a vcpu parameter then you are using them, or one of their callers, > > incorrectly. > > > > -- Keir > > > > On 05/08/2010 16:00, "Christoph Egger" wrote: > >> Signed-off-by: Christoph Egger > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632