From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation Date: Tue, 17 Jul 2012 11:27:06 -0500 Message-ID: <5005925A.7090502@freescale.com> References: <1341830087-12728-1-git-send-email-Bharat.Bhushan@freescale.com> <50044CF1.8070806@suse.de> <5004B995.8060303@freescale.com> <25F0D173-5EFA-4516-B478-F5E220E502CE@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D03DCB08D@039-SN2MPN1-023.039d.mgd.msft <50055FEC.4020602@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D03DCB328@039-SN2MPN1-023.039d.mgd.msft.net> <50057039.6070108@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D03DCB508@039-SN2MPN1-023.039d.mgd.msft.net> <5005781E.1020502@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Bhushan Bharat-R65777 , Wood Scott-B07421 , "" , "" , "" , Benjamin Herrenschmidt , Kumar Gala , Avi Kivity To: Alexander Graf Return-path: In-Reply-To: <5005781E.1020502@suse.de> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/17/2012 09:35 AM, Alexander Graf wrote: > On 07/17/2012 04:13 PM, Bhushan Bharat-R65777 wrote: >> >>> -----Original Message----- >>> From: kvm-ppc-owner@vger.kernel.org >>> [mailto:kvm-ppc-owner@vger.kernel.org] On >>> Behalf Of Alexander Graf >>> Sent: Tuesday, July 17, 2012 7:31 PM >>> To: Bhushan Bharat-R65777 >>> Cc: Wood Scott-B07421; ; ; >>> ; Benjamin Herrenschmidt; Kumar Gala; Avi >>> Kivity >>> Subject: Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation >>> >>> On 07/17/2012 03:15 PM, Bhushan Bharat-R65777 wrote: >>>>> -----Original Message----- >>>>> From: kvm-ppc-owner@vger.kernel.org >>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf >>>>> Sent: Tuesday, July 17, 2012 6:22 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: Wood Scott-B07421; ; >>>>> ; ; Benjamin >>>>> Herrenschmidt; Kumar Gala; Avi Kivity >>>>> Subject: Re: [PATCH 2/2 v2] KVM: PPC: booke: Add watchdog emulation >>>>> >>>>> On 07/17/2012 11:57 AM, Bhushan Bharat-R65777 wrote: >>>>>> Is not setting the TSR.WRS atomic here (cmpxchg() will handle this)? >>>>> Final expiration sets TCR. TSR should be ok. >>>> It is clearing some TCR bits :) >>>> >>>> Let us move the TCR clearing to userspace (please see below response >>>> ^^). Then >>> it is just setting TSR. Right? >>> >>> Then TSR is still set, right? So we don't set it, it just keeps on >>> being 1. We >>> need to trigger another expired event in that if branch now. >> I am sorry Alex but I did not get you. >> >> What I meant was that you were having concern that dec is atomic while >> watchdog final expiration (complex) is not atomic, but if we move the >> TCR.WRC clearing to userspace on final expiration then are you ok with >> current code? > > If we move the final expiration TCR.WRC clearing out of that branch, you > don't have any condition left to check if we are in the final expiration > path. I don't follow. Determining whether we're on final expiration is based only on the previous value of TSR[ENW,WIS] when the timer expires. We look at TCR to determine whether we need to actually exit to QEMU (as opposed to a final expiration with no action, which still should result in the timer being stopped), but I don't see any problematic races there as long as we don't try to update TCR from that context. > So you need to set some other bit somewhere (eventually > pending_exceptions probably) that indicates that we need to return to > user space. We should do that to simplify other parts of the code, but I'm not sure what it has to do with TCR[WRC]. -Scott