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@xxxxxxxxxxxxxxx >>> [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On >>> Behalf Of Alexander Graf >>> Sent: Tuesday, July 17, 2012 7:31 PM >>> To: Bhushan Bharat-R65777 >>> Cc: Wood Scott-B07421; <kvm-ppc@xxxxxxxxxxxxxxx>; <kvm@xxxxxxxxxxxxxxx>; >>> <bharatb.yadav@xxxxxxxxx>; 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@xxxxxxxxxxxxxxx >>>>> [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On Behalf Of Alexander Graf >>>>> Sent: Tuesday, July 17, 2012 6:22 PM >>>>> To: Bhushan Bharat-R65777 >>>>> Cc: Wood Scott-B07421; <kvm-ppc@xxxxxxxxxxxxxxx>; >>>>> <kvm@xxxxxxxxxxxxxxx>; <bharatb.yadav@xxxxxxxxx>; 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html