From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Fri, 02 Sep 2011 21:28:22 +0000 Subject: Re: [PATCH 1/5] KVM: PPC: booke: Fix int_pending calculation for Message-Id: <4E614A76.90302@freescale.com> List-Id: References: <20110826233139.GA30607@schlenkerla.am.freescale.net> In-Reply-To: <20110826233139.GA30607@schlenkerla.am.freescale.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kvm-ppc@vger.kernel.org On 09/02/2011 02:25 PM, Alexander Graf wrote: > > On 02.09.2011, at 20:17, Scott Wood wrote: > >> On 09/02/2011 08:53 AM, Alexander Graf wrote: >>> On 08/27/2011 01:31 AM, Scott Wood wrote: >>>> int_pending was only being lowered if a bit in pending_exceptions >>>> was cleared during exception delivery -- but for interrupts, we clear >>>> it during IACK/TSR emulation. This caused paravirt for enabling >>>> MSR[EE] to be ineffective. >>> >>> But that means that int_pending can still be 1 even though there is none >>> pending as we don't get the call to deliver_interrupts when it gets >>> lowered. Please create a common function to remove a bit from >>> pending_exceptions and do the check there. >> >> I can do that if you want, but kvmppc_core_deliver_interrupts() should >> always get called before we return to the guest. Dequeues that are >> asynchronous to a guest exit should be very rare, and would be cured on >> the first subsequent guest exit. > > Yes, but this means we have yet another subtile indirect assumption. > The more we have those, the more people who work on the code need to > know about the code to actually work on it. So the easier we can keep > the scheme, the better IMHO. > > So yes, please replace clear_bit(pending, ...) with > clear_bit_pending(...) or so which then would also do the required > magic to set the shared page values. Then we need to deal with races, if another exception is queued after we check pending_exceptions but before we clear int_pending, whereas the guest exit path will always be invoked if we queue an extirq or timer. What if we rename it from deliver_interrupts() to prepare_to_enter(), to make it more obvious? We also will want to check for MSR[WE] before returning to the guest, rather than immediately when MSR is set, and this would be a place to put that. -Scott