From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PART1 RFC v3 12/12] svm: Manage vcpu load/unload when enable
 AVIC
Date: Thu, 31 Mar 2016 16:19:08 +0200
Message-ID: <20160331141908.GA2171@potion.brq.redhat.com>
References: <1458281388-14452-1-git-send-email-Suravee.Suthikulpanit@amd.com>
 <1458281388-14452-13-git-send-email-Suravee.Suthikulpanit@amd.com>
 <20160318214450.GB2332@potion.brq.redhat.com>
 <56FCE556.80306@amd.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: pbonzini@redhat.com, joro@8bytes.org, bp@alien8.de,
	gleb@kernel.org, alex.williamson@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, wei@redhat.com,
	sherry.hurwitz@amd.com
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:60068 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756233AbcCaOTN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 31 Mar 2016 10:19:13 -0400
Content-Disposition: inline
In-Reply-To: <56FCE556.80306@amd.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2016-03-31 15:52+0700, Suravee Suthikulpanit:
> On 03/19/2016 04:44 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote:
>>2016-03-18 01:09-0500, Suravee Suthikulpanit:
>>>+	} else {
>>>+		new_entry =3D READ_ONCE(*entry);
>>>+		/**
>>>+		 * This handles the case when vcpu is scheduled out
>>>+		 * and has not yet not called blocking. We save the
>>>+		 * AVIC running flag so that we can restore later.
>>>+		 */
>>
>>is_running must be disabled in between ...blocking and ...unblocking,
>>because we don't want to miss interrupts and block forever.
>>I somehow don't get it from the comment. :)
>=20
> Not sure if I understand your concern.  However, the is_running bit
> setting/clearing should be handled in the avic_set_running below. Thi=
s part
> only handles othe case when the is_running bit still set when calling
> vcpu_put (and later on loading some other vcpus). This way, when we a=
re
> re-loading this vcpu, we can restore the is_running bit accordingly.

I think that the comment is misleading.  The saved is_running flag only
matters after svm_vcpu_blocking, yet the comment says that it handles
the irrelevant case before.

Another minor bug is that was_running isn't initialized to 1, so we nee=
d
to halt before is_running gets set.
It would be clearer to toggle a flag in svm_vcpu_(un)blocking and set
is_running =3D !is_blocking.  Doing so will also be immeasurably faster=
,
because avic_vcpu_load is called far more than svm_vcpu_(un)blocking.