On 07/17/2012 06:27 PM, Scott Wood wrote:
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.
Yeah, what I was trying to say is that I would like to keep the TSR
state unmodified for the final expiry case. If user space decides to not
act upon it, it should be able to call into VCPU_RUN and have the same
behavior as if the watchdog never expired.
So there needs to be a check whether we return to user space based on
TCR, yes. But we shouldn't modify TSR or TCR in that branch. We only
trigger an event saying "return to user space with a watchdog expired exit".
Then user space can decide whether to act on it. If it decides to ignore
it, we don't have an oddball TSR.WRS state lingering around that'd need
clearing.
That way we can put a single if (watchdog_enabled) on the qemu bubble-up
event injection. If it's not enabled, it'd be as if TCR.WRC is 0 and the
watchdog expired without action. If it's enabled, user space can decide
to fire it up again if it deems it necessary.
Alex
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-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
